Re-add me if you need anything specific, otherwise not sure how much I can add aside from my original comment.

Looking at the APK built with the same gn args using ClassyShark, I can see the missing resource under com.google.android.gms.base.R$string and com.google.android.gms.cast.R$string in classes2.dex but not in com.google.android.gms.R$string in classes.dex.

FYI: which target you are building is not related to which gmscore is being used. If you have a clank checkout the downstream gmscore is used even for chrome public. You have to use the play_services_version=public (or something similar) GN arg to use the upstream one. 

So it's worth checking with a public gmscore 10.0, which should not be proguarded that the issue reproes. Then we can exclude our build system from the potential causes if it's the case.

Otherwise, it might be that some new directories need to be whitelisted in the downstream update_gcore.config.json

I reproduced it with ChromePublic in my Chromium checkout for #c5.
I used an old Nexus 7 with JB and GmsCore without updates uninstalled.
The same crash occurs.

Grigoriy told me that he didn't have a clank checkout, so the upstream version of GMS Core was being used. I am planning to roll that to 10.0 today, I doubt that'll resolve the issue though.

Anton, have you tried to repro this with a clank build too? Is this an upstream repo issue only?

Will try and report back as soon as it builds :)

I noticed that the upstream GMSCore uses separate .aar files while Clank's first-party uses a single google_play_services.jar and a res/ folder. Perhaps we handle these setups differently?

Clankium built with the same gn args didn't crash for me and showed the message for updating Google Play Services as expected.

I see the gms.R$sting class is in the first dex file (classes.dex) in Chrome.apk and contains all the needed strings. While the same class in ChromePublic.apk only has one string and the string needed for the update dialog is in classes2.dex in the gms.base and gms.cast packages. See screenshots.

Deleted: chrome-apk-gms-R-string.png 275 KB

	chrome-apk-gms-R-string.png 275 KB View Download

Deleted: classy-shark-chrome-public-gms-R-string.png 155 KB

	classy-shark-chrome-public-gms-R-string.png 155 KB View Download

Any updates? :)

Well, 10.0.1 upstream play services landed earlier today. I am a bit busy with other stuff today, but if someone else wants to test out whether that helped, please do :)

I just tried (making sure your change is in the git log of my checkout) and [after bypassing a couple of unrelated DCHECKs failing) I see the same failure.

Examining the ChromePublic.apk I don't see any difference with the build before the library was updated. The string is present in ...gms.base.R$string (in classes2.dex) but not in the ...gms.R$string in classes1.dex.

so, common_google_play_services_update_text is included in the play-services-base AAR, while common_google_play_services_unknown_issues is part of play-services-basement. 

Downstream we build by merging all the resources together, while upstream we rely on the AAR processing to include all the resources. Because they are using the same package, it might be possible that the R.java/class is being overwritten instead of combined with others.

ianwen@: Any idea if this might be an issue with the android_aar_prebuilt target?

Hello dgn@. I checked play-services-basement-9.8.0.aar, in R.txt it claims to have all strings, yet in the actual res/values folder, it only has common_google_play_services_unknown_issue and no other strings.

I think this is a bug in the distribution of gmscore AARs, or the strings in question are actually in some other aar packages. The AAR compiling logic in Chrome is doing what it should be doing for this matter. :)

Wait. I think I found the reason. The missing strings are actually in play-services-base-9.8.0.aar, yet they are declared in play-services-basement-9.8.0.aar.

In our current AAR inclusion logic, we check the R.txt in a AAR first, and then use it as a list to include all resources. Yet clearly gmscore is doing something wrong here.

There are two possible fixes:
1. Revert dgn's recent change about chrome-public-apk and not let it compile against AARs. And we reland the change after gmscore team fixes their releases.
2. Introduce a hack in our GN code to somehow address this issue temporarily, and later revert this hack once gmscore team fixes their problem.


dgn@ and agrieve@, wdyt?

Removing the RVG restriction so GmsCore eng could take a look.

See the reply from the GMS Core folks on an internal thread:

"...due to historical reasons, all resources are declared in a global R.txt in com.google.android.gms (in basement, as you pointed out), even if the resource itself is in a different AAR, and yes, unfortunately this makes it difficult to determine which client provides a resource, given just the basement R.txt.

Where are you referencing the AAR distribution from? If it's the maven repo distributed through the Android SDK Manager, the POM files should express the dependencies, i.e.:

c.g.a.gms.play-services-gcm:
  -> c.g.a.gms.play-services-iid
  -> c.g.a.gms.play-services-base
  -> c.g.a.gms.play-services-basement"

Do we get it from the maven repo or have access to the POM files?

Seems like we need to change our AAR inclusion logic.

We do get the AARs from the maven repo, but GN doesn't do anything with the POM files yet. I had to express the dependencies in GN directly. So the dependency on basement should be in there anyway.

But from what ianwen@ said in #c18, it looks like this issue is that we don't look at dependencies at all when processing the AARs.

As this is a public bug now I can chime in :) I was debugging "building chrome_public_apk never comes to "no work to do" state" and found out something that might be useful.

>In our current AAR inclusion logic, we check the R.txt in a AAR first, and then use it as a list to include all resources.

I believe it isn't true. We do check for R.txt and extract it (and this causes rebuilds because some gms aars miss it), but this file isn't used afterwards. Instead we generate a new one based on the resources in the AAR and dependent targets - in the process_resources.py. For well-formed AARs this approach will work: good R.txt contains references for resources in AAR itself and its dependencies; then code in AAR access resources via AAR's own R.java or R.java from the dependencies. But gmscore isn't well-formed - it access resources via global R.java from basement. We aren't generating proper R.java for basement, because basement misses most resources despite having them listed in its R.txt.

As a workaround we can pass extracted R.txt from AAR to process_resources.py and use it as a source of truth when generating R.java for this AAR.

mlopatkin - Thanks for digging through this! Your suggested fix sounds fine to me. Are you interested in submitting a patch for it? If not, dgn@ - feel free to assign to me to make the change.

If either of you could land that it would be great, as I'm not familiar with the GN internals :) Thanks!

I'll take a stab at it tonight

(from a right account this time).

El mar, dic 13, 2016 03:06 AM, mlopat… via monorail <
monorail+v2.718296623@chromium.org> escribió:

Looks like the issue is not limited to the aar_prebuilt target. Using android_java_prebuilt + android_resources shows the same problem.

>Looks like the issue is not limited to the aar_prebuilt target. Using android_java_prebuilt + android_resources shows the same problem.

dgn@, could you please clarify this? The original problem can arise unless you merge all the resources from aars into a single "android_resources" target that declares its package as "com.google.android.gms".

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d36c048e72f0e936b725d4de25179a7c41599c1a

commit d36c048e72f0e936b725d4de25179a7c41599c1a
Author: mlopatkin <mlopatkin@yandex-team.ru>
Date: Mon Dec 19 16:04:44 2016

Use R.txt from AAR to generate R.java for it when building APK.

R.java and R.txt files for an AAR were generated by extracting all
resources from the AAR and its dependencies and running aapt on these
resources. This approach turned out to be problematic for AARs of
GMS (Play Services).

GMS is shipped as a bunch of AARs with dependencies between them.
However the way it deals with resources is rather unusual. GMS code
references all resources via com.google.android.gms.R class that
corresponds to the "basement" AAR. This AAR has R.txt with entries for
all resources of the GMS but contains only a small subset of the
resources. All other resources are provided by other AARs: they have
different packages and no R.txt files though.

The consumer of the GMS must generate com.google.android.gms.R class
with entries for all resources provided by used GMS AARs. Entries for
resources that belong to the unused AARs may be ommited. This is what
Gradle plugin for Android does.

However AAR support in Chromium discarded R.txt from "basement" then
generated very small com.google.android.gms.R without essential entries
and this caused crashes in runtime.

This CL changes processing of the AARs resources. If R.txt is present in
AAR then aapt-generated R.txt is discared. The one from AAR is used to
generate R.java for java dependencies of the AAR target and to generate
final R.java file during APK build which is compiled and included into
the final APK.

During obfuscation Proguard checks that all entries that are referenced
in code were in fact generated. This check cannot be done during resource
processing because "basement"'s R.txt lists resources that aren't
included in Chromium and this is because code that uses these resources
isn't included either.

BUG= 670368 
R=agrieve@chromium.org,dgn@chromium.org

Review-Url: https://codereview.chromium.org/2570313002
Cr-Commit-Position: refs/heads/master@{#439485}

[modify] https://crrev.com/d36c048e72f0e936b725d4de25179a7c41599c1a/build/android/gyp/aar.py
[modify] https://crrev.com/d36c048e72f0e936b725d4de25179a7c41599c1a/build/android/gyp/process_resources.py
[modify] https://crrev.com/d36c048e72f0e936b725d4de25179a7c41599c1a/build/config/android/internal_rules.gni
[modify] https://crrev.com/d36c048e72f0e936b725d4de25179a7c41599c1a/build/config/android/rules.gni
[modify] https://crrev.com/d36c048e72f0e936b725d4de25179a7c41599c1a/chrome/android/java/proguard.flags

 Issue 687474  has been merged into this issue.