Fix android open link verify

[tokou]

Proposed changes

TL;DR

I fixed a whole lot of stuff that was wrong with the autoVerify. It should now work as expected.

Note

I cherry picked the commit from #1998 locally as I seemed to stumble on that same issue in other places as I tested

Long version

• I started investigating this Android exception while opening a deeplink - java.lang.NullPointerException: appName must not be null #1956 issue. It was pretty straightforward to reproduce and fix.
  • The origin is due to the fact that the reading of the app name to tap on the right cell of the Chooser is done through apkMeta.name
  • This value which contains the value defined in <application android:label="name"> in the Manifest. Which obviously is not mandatory, not guaranteed to be there, and even better, not what the chooser shows as value
  • The fix was simply to check the nullability of the value and throw if it's nullable (because it's a String! from Java-world)
  • However, while doing this, I noticed the code related to autoVerify and I opened another can of worms...
• autoVerify is supposed to do 2 things. Select the right app from the Chooser and validate some of the onboarding screens from Chrome. Both had issues in the way they were implemented
  • Chrome onboarding
    • Depending on the version of Android and Chrome that you have, there are different variations of up to 4 onboarding screens
    • I just fixed the function to handle all of those
  • Selection of the wanted app
    • Before API 31 (Android 12) you always get a Chooser if your app has declared an IntentFilter with ACTION_VIEW on the domain
      • I fixed the code that selects the right app from the Chooser
      • I kept the selection of "Just once" over "Always" even though I've hesitated to switch it 🤔
      • This is still not perfect as we ultimately would need to get the result of something like
        intent.resolveActivity(packageManager) in order to get the labels associated with the right Activities
    • Starting from API 31, there's no more Chooser but there's the whole "Open by default" settings menu
      • I added handling for all of that automatically when autoVerify is selected
      • I did not reverse the setting after the link is opened. I think it makes more sense like that I think. Maybe we should align "Just once" in the Chooser?

Testing

This is interesting to test. I'm not sure how we can automate that (or if we should). It would make a lot of integration tests. How were the various drivers tested?

What I did

• First I had a very simple test flow

appId: dev.mobile.maestro.app
---
- openLink:
    link: 'https://maestro.mobile.dev'
    autoVerify: true
    browser: false
- openLink:
    link: 'https://maestro.mobile.dev'
    autoVerify: true
    browser: true

• I created a new module with a very barebones app dev.mobile.maestro.app that I used to test the null name

    <application>
        <activity
            android:name=".MainActivity"
            android:exported="true"
            >
            <intent-filter>
                <category android:name="android.intent.category.LAUNCHER"/>
                <action android:name="android.intent.action.MAIN"/>
            </intent-filter>
            <intent-filter>
                <action android:name="android.intent.action.VIEW" />
                <category android:name="android.intent.category.DEFAULT" />
                <category android:name="android.intent.category.BROWSABLE" />
                <data android:scheme="https" />
                <data android:host="maestro.mobile.dev" />
            </intent-filter>
        </activity>
    </application>

class MainActivity : Activity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(TextView(this).apply { text = "${applicationInfo.name}" })
    }
}

• I created a whole bunch of brand new emulators (API 24, 25, 26, 28, 32, 33, 34, 35)
• And then I ran the flow over and over on all the emulators until everything ran as expected
• In order to get the different Chrome screens, you have to Force stop and Clear data for it and they are a bit random

Chrome

Accept	Sign in	Sync	Notifications

Chooser

New	App	Browser

Open by default

Before	After

Issues fixed

Fixes #1956 (Definitely fixed)
Fixes #1397 (Fixed in spirit but not in letter. I think it's just better to use autoVerify for that purpose and not add another option)

[bartekpacia]

Whoah, this is awesome. Thanks!

I created a new module with a very barebones app dev.mobile.maestro.app that I used to test the null name

Could you share this code? We could add this to our e2e testing suite:

• add app binary link to manifest
• add tests to workspaces/<your_app_name>

Alternatively, the relevant intent filter stuff could be added to our demo_app. Wdyt?

I created a whole bunch of brand new emulators (API 24, 25, 26, 28, 32, 33, 34, 35)

This is a action item for us to run on all supported API levels in CI. Currently we run only on API 28.

[bartekpacia]

Nice! Thanks for making it shorter.

[bartekpacia]

Why this change? I'm curious to learn why /sdcard is used as fallback

[tokou]

• I encountered a permission denied when pulling the APK on some of the API levels
• This is because /data is not accessible to pull from without the device being rooted in those cases
• However running shell cp and copying the files over is possible
• And /sdcard is accessible to pull from

See this SO answer for example

If we add multi-API-level testing on the CI, we should add simple tests for AndroidAppFiles as this is the kind of things that breaks with API changes.

[bartekpacia]

This topic is much more complex that I though. Thank you very much for sharing your experience and contributing.

If you think that the documentation for openLink could be expanded, we would greatly appreciate that as well.

[ubuntudroid]

Why was this reverted? 🤔 Looks like a super useful bunch of changes and I was looking forward to finally get rid of the custom steps we were doing in our tests. Were there breaking issues?

[amanjeetsingh150]

Hey folks, just an FYI on this PR. This created a regression in openlink command with auto verify flag on cloud. While I think this PR is really useful and can be reworked and fixed. I'll do a root cause on what exactly caused issue and we can take in rest of the changes and fix rest. I'll do a follow up here this week