Minimal flavor contains gms

[tzagim]

Home Assistant Android version:
2022.1.1

Description of problem:
The minimal flavor contains google mobile services (gms), in recent versions, it probably comes from somewhere else because I did not find it in the code, you can see more here

[dshokouhi]

Probably got introduced while we were working on the Wear OS app:

android/app/build.gradle.kts

Line 159 in 18453a1

implementation("com.google.android.gms:play-services-wearable:17.1.0")

[tzagim]

Hi @dshokouhi and thanks for your work, but even after your fix both the beta version and the FDROID build still have a gms class.
According to the APK Analyzer, the class is: com.google.android.gms.common.api.GoogleApiActivity.

[dshokouhi]

Are you sure you used the correct beta build? When I look at the link above it seems to still use the production version and not the beta. This change will not be in production until the app is ready and we are not sure when the next release is going to be as we have features that need testing.

https://github.com/home-assistant/android/releases/tag/beta-1936-69ccb1ef

I do not see this class being used in the project at all.

[tzagim]

Hi @dshokouhi and thanks for your reply.

See for example Analysis of the Last Beta Using Virus Total; It appears under Activities.

I think it's been sucked into APK from somewhere else I could not figure out where it came from.

[dshokouhi]

Reopening. When I do a global search in the scope I do see this file. Not really sure where its generating from though :/

Is this a new scanner being introduced? I noticed on the F-Droid page the APK listed there already has these GMS issues but we have not seen any reports of crashes due to the dependency. In fact anything that relies on a Google Service is specifically hidden from the minimal so it will not function nor should the code be touched.

[dshokouhi]

ok have an idea on how to remove it, for the quest builds we had to explicitly remove certain nodes from the merged manifest. I am going to try that here for this activity. Running a sanity test to make sure nothing is broken first.

[dshokouhi]

debug APK is ready to see if this change actually worked :) can you give it a try in the tool?

https://github.com/home-assistant/android/actions/runs/1719666250

[tzagim]

I've already checked, he's still there :( , I think it comes from one of the dependencies.

example

[tzagim]

If it makes sense, then it also appears in quest.

Although this line is there within app/src/quest/AndroidManifest.xml
<uses-permission android:name="com.google.android.gms.permission.ACTIVITY_RECOGNITION" tools:node="remove" />

This lines mean anything to you?

  <meta-data

		android:name="com.google.android.gms.version"

		android:value="(reference) @0x7f0b000b"/>

	<activity

		android:theme="(reference) @0x01030010"

		android:name="com.google.android.gms.common.api.GoogleApiActivity"

		android:exported="false"/>

From AndroidManifest.xml.txt

[dshokouhi]

Found the issue I believe. I ran the following gradle command that spits out all dependencies and where they come from

gradlew :app:dependencies

Looks like the culprit is still android wear :) which makes sense because it just recently got added in past couple of months. Makes sense as to why this is the issue because this dependency is used to communicate between watch and phone which does indeed use google servers IIRC.

androidx.wear:wear-remote-interactions:1.0.0

+--- androidx.wear:wear-remote-interactions:1.0.0
|    +--- androidx.annotation:annotation:1.1.0 -> 1.3.0
|    +--- org.jetbrains.kotlin:kotlin-stdlib:1.5.21 -> 1.6.10 (*)
|    +--- com.google.guava:listenablefuture:1.0 -> 9999.0-empty-to-avoid-conflict-with-guava
|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-guava:1.5.0 -> 1.6.0
|    |    +--- com.google.guava:guava:28.0-jre
|    |    |    +--- com.google.guava:failureaccess:1.0.1
|    |    |    +--- com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava
|    |    |    +--- com.google.code.findbugs:jsr305:3.0.2
|    |    |    +--- org.checkerframework:checker-qual:2.8.1
|    |    |    +--- com.google.errorprone:error_prone_annotations:2.3.2
|    |    |    +--- com.google.j2objc:j2objc-annotations:1.3
|    |    |    \--- org.codehaus.mojo:animal-sniffer-annotations:1.17
|    |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.0 (*)
|    |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.6.0 (*)
|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.0 -> 1.6.10 (*)
|    +--- androidx.concurrent:concurrent-futures:1.0.0 (*)
|    +--- androidx.annotation:annotation:1.2.0 -> 1.3.0
|    +--- com.google.android.gms:play-services-basement:17.0.0 -> 17.5.0
|    |    +--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
|    |    +--- androidx.core:core:1.2.0 -> 1.7.0 (*)
|    |    \--- androidx.fragment:fragment:1.0.0 -> 1.3.6 (*)
|    \--- com.google.android.gms:play-services-wearable:17.1.0
|         +--- com.google.android.gms:play-services-base:17.5.0
|         |    +--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
|         |    +--- androidx.core:core:1.2.0 -> 1.7.0 (*)
|         |    +--- androidx.fragment:fragment:1.0.0 -> 1.3.6 (*)
|         |    +--- com.google.android.gms:play-services-basement:17.5.0 (*)
|         |    \--- com.google.android.gms:play-services-tasks:17.2.0
|         |         \--- com.google.android.gms:play-services-basement:17.4.0 -> 17.5.0 (*)
|         +--- com.google.android.gms:play-services-basement:17.5.0 (*)
|         \--- com.google.android.gms:play-services-tasks:17.0.0 -> 17.2.0 (*)

The permission you mentioned has always been there but the dependency it needs is part of the full implementation otherwise we wouldve seen a complaint a year ago :)

According to this report even exoplayer relies on GMS given that this impacts webview lets hold off on it and see if we get a complaint because this has also been there for a very long time.

+--- com.google.android.exoplayer:extension-cronet:2.15.1
|    +--- com.google.android.exoplayer:exoplayer-common:2.15.1 (*)
|    +--- androidx.annotation:annotation:1.2.0 -> 1.3.0
|    \--- com.google.android.gms:play-services-cronet:17.0.1
|         +--- com.google.android.gms:play-services-base:17.0.0 -> 17.5.0 (*)
|         +--- com.google.android.gms:play-services-basement:17.0.0 -> 17.5.0 (*)
|         +--- com.google.android.gms:play-services-tasks:17.0.0 -> 17.2.0 (*)
|         \--- org.chromium.net:cronet-api:72.3626.96

[dshokouhi]

ok hope this one works :) https://github.com/home-assistant/android/actions/runs/1720017632 🤞

[tzagim]

he's still there :(

Maybe because of this line?
<uses-sdk tools:overrideLibrary="androidx.wear.remote.interactions" />

in app/src/main/AndroidManifest.xml

[dshokouhi]

Doesnt seem like it, the quest manifest still picks it up (should've check that before submitting my PR) 😕 I wonder if this pre-existed all the Wear OS stuff we did. Is there anyway you can check an older build like from January 2021?

[dshokouhi]

Actually test a build before this PR: #1588 based on the description on github this might be it

https://github.com/google/ExoPlayer/tree/dev-v2/extensions/cronet

the problem is that our video player uses this and it impacts webview so this will be a rather large change and I am unfamiliar with exoplayer.

[tzagim]

You're probably right, this first appeared in version beta-695-9a1f9c0

[jpelgrom]

@dshokouhi The page you linked does mention an option for users without Play Services, couldn't we just change the existing dependency to a full version dependency, and use the embedded or fallback dependency on the minimal/quest build? Their documentation says that the usage is the same for full/embedded versions and only slightly different for fallback.

[dshokouhi]

@dshokouhi The page you linked does mention an option for users without Play Services, couldn't we just change the existing dependency to a full version dependency, and use the embedded or fallback dependency on the minimal/quest build?

When I tried that it failed to build minimal/quest due to the import missing. We may need to create a base version and split it up that way because of the import. Not 100% on that though.

[jpelgrom]

When I tried that it failed to build minimal/quest due to the import missing.

Looking at the documentation once again, it states that you need to add an additional dependency for the embedded version. You probably replaced the existing dependency (like I also assumed in the previous comment)?

For some reason Google put the CronetDataSource in the base/Play Services dependency instead of somewhere else, even though it doesn't depend on other classes from the Play Services package, so we have to remove Play Services ourselves. When replacing the implementation("com.google.android.exoplayer:extension-cronet:2.15.1") dependency with:

    "fullImplementation"("com.google.android.exoplayer:extension-cronet:2.15.1")
    "minimalImplementation"("com.google.android.exoplayer:extension-cronet:2.15.1") {
        exclude(group = "com.google.android.gms", module = "play-services-cronet")
    }
    "questImplementation"("com.google.android.exoplayer:extension-cronet:2.15.1") {
        exclude(group = "com.google.android.gms", module = "play-services-cronet")
    }
    "minimalImplementation"("org.chromium.net:cronet-embedded:95.4638.50")
    "questImplementation"("org.chromium.net:cronet-embedded:95.4638.50")

the full and minimal app builds compile for me without any changes and seem to work fine. I'll see if I can embed a HLS stream which is the only place where this dependency is used in the coming days to actually see if it still works.

[dshokouhi]

Looking at the documentation once again, it states that you need to add an additional dependency for the embedded version. You probably replaced the existing dependency (like I also assumed in the previous comment)?

yea I only moved it because the import in WebView relies on it: https://github.com/home-assistant/android/blob/master/app/src/main/java/io/homeassistant/companion/android/webview/WebViewActivity.kt#L55

I had a feeling it was a misunderstanding on my part :(

the full and minimal app builds compile for me without any changes and seem to work fine. I'll see if I can embed a HLS stream which is the only place where this dependency is used in the coming days to actually see if it still works.

if they compile without complaint thats a good sign then, I dont have a device that supports exoplayer here. I think as long as the app loads and cameras still stream it should be ok?

In this case we probably still need to move remote-interactions right?

Thanks for taking a closer look at this :)

[jpelgrom]

if they compile without complaint thats a good sign then, I dont have a device that supports exoplayer here. I think as long as the app loads and cameras still stream it should be ok?

I don't have any cameras that stream either but just did some testing. In onResume, I added a call to exoPlayHls with a random stream so exoplayer is used :) Tried both a live stream and 'recorded' stream (regular video with fixed length). For a minimal device I just used one of the emulator images without Google Play. Without changing the dependencies this crashes on the minimal app (java.lang.RuntimeException: All available Cronet providers are disabled. A provider should be enabled before it can be used., which makes sense), after changing the dependency to use embedded on minimal/quest it works fine. Using the same code to play a stream in the full flavor results in the same player behavior.

In this case we probably still need to move remote-interactions right?

Yes, that still would add a Play Services dependency as your Gradle command shows. Changing it to fullImplementation works for me, it is only used in the full version of the app right now.

---

Looking at the merged manifest in Android Studio I don't see the GoogleApiActivity any more, and it also doesn't show up in the generated APK, so I think this should fix it! I'm going to see if I can find a way to not repeat the dependency for minimal and quest, and will submit a PR shortly.