Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add isSettingsAppServiceRunningInForeground to check the settings' service existence better #715

Merged
merged 10 commits into from
Jan 9, 2024

Conversation

KazuCocoa
Copy link
Member

@KazuCocoa KazuCocoa commented Jan 8, 2024

It looks like a command below launches io.appium.settings process, which can be found by [debug] [ADB] Running '/Users/kazu/Library/Android/sdk/platform-tools/adb -P 5037 -s D0AA002182JC0202126 shell pgrep -f \(\[\[:blank:\]\]\|\^\)io\.appium\.settings\(\[\[:blank:\]\]\|\$\)', but the app was not started in the background properly (no the app in the notification). It causes io.appium.settings process did not start properly situation, thus for example driver.location to get the location/set location causes an error because the io.appium.settings was not started properly with this.startApp.

We must start the settings app forcefully once with this.startApp to start the settings app process properly.

[debug] [ADB] Running '/Users/kazu/Library/Android/sdk/platform-tools/adb -P 5037 -s D0AA002182JC0202126 shell cmd notification allow_listener io.appium.settings/.NLService'
  await this.startApp({
    pkg: SETTINGS_HELPER_ID,
    activity: SETTINGS_HELPER_MAIN_ACTIVITY,
    action: 'android.intent.action.MAIN',
    category: 'android.intent.category.LAUNCHER',
    stopApp: false,
    waitForLaunch: false,
  });

I also found -g option could launch the io.appium.settings app process on Android 11. I saw the behavior was not always, but actually occurred. Then, await this.processExists(SETTINGS_HELPER_ID) skipped the io.appium.settings process with this.startApp while uia2 driver must have started the settings app with this.startApp to make the get location etc work properly

appium/appium-android-driver#895 is this PR's usage

@KazuCocoa
Copy link
Member Author

I figured out the root cause a bit more. io.appium.settings/.NLService service was running while the app process was stopped by settings' "force stop," for example. We should have io.appium.settings/.ForegroundService as the active foreground apps with adb shell dumpsys activity services instead of this.processExists for settings app since io.appium.settings/.NLService could be running but the io.appium.settings/.ForegroundService was not running. It cause the settings app no functions but the process existed.

...
Active foreground apps - user 0:
  #0: io.appium.settings
    mNumActive=1 mAppOnTop=false mShownWhileTop=true mShownWhileScreenOn=true
    mStartTime=-7m24s274ms mStartVisibleTime=-7m24s274ms

@mykola-mokhnach
Copy link
Contributor

I figured out the root cause a bit more. io.appium.settings/.NLService service was running while the app process was stopped by settings' "force stop," for example. We should have io.appium.settings/.ForegroundService as the active foreground apps with adb shell dumpsys activity services instead of this.processExists for settings app since io.appium.settings/.NLService could be running but the io.appium.settings/.ForegroundService was not running. It cause the settings app no functions but the process existed.

...
Active foreground apps - user 0:
  #0: io.appium.settings
    mNumActive=1 mAppOnTop=false mShownWhileTop=true mShownWhileScreenOn=true
    mStartTime=-7m24s274ms mStartVisibleTime=-7m24s274ms

that's a great find.
Lets then add a separate method to verify if settings app and corresponding services are running into appium-adb and then call it instead of processExists in the android driver

@mykola-mokhnach
Copy link
Contributor

mykola-mokhnach commented Jan 8, 2024

I also think it would make sense to move more javascript code into io.appium.settings package. We should not keep this logic in appium-adb and have it more abstract

@mykola-mokhnach
Copy link
Contributor

I also think it would make sense to move more javascript code into io.appium.settings package. We should not keep this logic in appium-adb and have it more abstract

Will probably do it later myself

@KazuCocoa
Copy link
Member Author

KazuCocoa commented Jan 8, 2024

Note for this. It looks like the simplest way was if isForeground=true exists with the adb shell dumpsys activity services io.appium.settings result. The adb shell dumpsys activity services io.appium.settings prints not the foreground only but at least if the package includes a foreground process, it looks like having isForeground=true, thus maybe isForeground=true existing check should be sufficient for this purpose.

% adb shell dumpsys activity services  io.appium.settings
ACTIVITY MANAGER SERVICES (dumpsys activity services)
  User 0 active services:
  * ServiceRecord{f0ad90b u0 io.appium.settings/.NLService}
    intent={act=android.service.notification.NotificationListenerService cmp=io.appium.settings/.NLService}
    packageName=io.appium.settings
    processName=io.appium.settings
    permission=android.permission.BIND_NOTIFICATION_LISTENER_SERVICE
    baseDir=/data/app/~~fHuRc6u9ehtAcXvuXy-fiw==/io.appium.settings-wJRwd1HrrbVG5ZINWuHi5Q==/base.apk
    dataDir=/data/user/0/io.appium.settings
    app=ProcessRecord{1d61746 18302:io.appium.settings/u0a320}
    whitelistManager=true
    allowWhileInUsePermissionInFgs=true
    startForegroundCount=0
    recentCallingPackage=android
    createTime=-6m21s859ms startingBgTimeout=--
    lastActivity=-6m21s783ms restartTime=-6m21s783ms createdFromFg=true
    Bindings:
    * IntentBindRecord{a5d675f CREATE}:
      intent={act=android.service.notification.NotificationListenerService cmp=io.appium.settings/.NLService}
      binder=android.os.BinderProxy@1be25ac
      requested=true received=true hasBound=true doRebind=false
      * Client AppBindRecord{c78a275 ProcessRecord{3f853b1 1847:system/1000}}
        Per-process Connections:
          ConnectionRecord{fbf1188 u0 CR FGS !PRCP io.appium.settings/.NLService:@339692b}
    All Connections:
      ConnectionRecord{fbf1188 u0 CR FGS !PRCP io.appium.settings/.NLService:@339692b}

  * ServiceRecord{e7a180b u0 io.appium.settings/.ForegroundService}
    intent={act=start cmp=io.appium.settings/.ForegroundService}
    packageName=io.appium.settings
    processName=io.appium.settings
    permission=android.permission.FOREGROUND_SERVICE
    baseDir=/data/app/~~fHuRc6u9ehtAcXvuXy-fiw==/io.appium.settings-wJRwd1HrrbVG5ZINWuHi5Q==/base.apk
    dataDir=/data/user/0/io.appium.settings
    app=ProcessRecord{1d61746 18302:io.appium.settings/u0a320}
    allowWhileInUsePermissionInFgs=true
    startForegroundCount=1
    recentCallingPackage=io.appium.settings
    isForeground=true foregroundId=1 foregroundNoti=Notification(channel=main_channel shortcut=null contentView=null vibrate=null sound=null defaults=0x0 flags=0x62 color=0x00000000 vis=PRIVATE)
    createTime=-5m1s703ms startingBgTimeout=--
    lastActivity=-5m1s702ms restartTime=-5m1s702ms createdFromFg=true
    startRequested=true delayedStop=false stopIfKilled=false callStart=true lastStartId=1

  Connection bindings to services:
  * ConnectionRecord{fbf1188 u0 CR FGS !PRCP io.appium.settings/.NLService:@339692b}
    binding=AppBindRecord{c78a275 io.appium.settings/.NLService:system}
    conn=android.app.LoadedApk$ServiceDispatcher$InnerConnection@339692b flags=0x5000101

Ideally we want to get the list of Active foreground apps but maybe adb does not have the exact adb command option fo the only the list

@KazuCocoa KazuCocoa marked this pull request as draft January 8, 2024 16:45
@KazuCocoa KazuCocoa changed the title feat: add forceStartSettingsApp in requireRunningSettingsApp feat: add hasRunningSettingsAppForegroundService to the settings' service existence better Jan 8, 2024
@KazuCocoa KazuCocoa changed the title feat: add hasRunningSettingsAppForegroundService to the settings' service existence better feat: add hasRunningSettingsAppForegroundService to check the settings' service existence better Jan 8, 2024
@KazuCocoa KazuCocoa marked this pull request as ready for review January 8, 2024 19:36
@KazuCocoa
Copy link
Member Author

Added tests. Maybe this is currently what we can do to ensure if io.appium.settings package is having a foreground service as the service

@KazuCocoa
Copy link
Member Author

checked the activity services io.appium.settings on various API levels

@KazuCocoa KazuCocoa changed the title feat: add hasRunningSettingsAppForegroundService to check the settings' service existence better feat: add isSettingsAppServiceRunningInForeground to check the settings' service existence better Jan 9, 2024
@KazuCocoa KazuCocoa merged commit be0502e into master Jan 9, 2024
15 checks passed
@KazuCocoa KazuCocoa deleted the force-start-settings branch January 9, 2024 19:33
github-actions bot pushed a commit that referenced this pull request Jan 9, 2024
## [11.1.0](v11.0.9...v11.1.0) (2024-01-09)

### Features

* add isSettingsAppServiceRunningInForeground to check the settings' service existence better ([#715](#715)) ([be0502e](be0502e))
Copy link

github-actions bot commented Jan 9, 2024

🎉 This PR is included in version 11.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants