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

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

Committed: https://crrev.com/d36c048e72f0e936b725d4de25179a7c41599c1a
Cr-Commit-Position: refs/heads/master@{#439485}

[mlopatkin, 2016-12-14 16:22:01]

PTAL

I verified (with test application that used some of GMS libs) that Gradle plugin
generates R.java for GMS in the similar manner.

One change that I'm slightly afraid of is removal of "dontwarn
com.google.android.gms" from proguard flags. Without this suppression Proguard
does a great job to ensure that "missing field" never happen again, but there
can be false positive errors downstream. I can imagine, for example, that some
resource is removed in the downstream copy of GMS because the code that uses
this resource is never called.

Another approach that can solve the problem is to enable "include_all_resources"
on the apks. But it breaks incremental dex-ing of chrome_public_apk.jar (too
many fields, >70000, 5000 per package) and I'm not entirely sure that multidex
will allow us to build a final apk even if we solve the problem with incremental
dex-ing.
The size of proguarded (but still debug) apk slightly increases too (by 7 Kb).
So I decided not to pursue this further.

[agrieve, 2016-12-14 18:16:02]

lgtm. Looks great! Really appreciate the level of detail in the comments you
added.

In regards to removing the proguard line - as long as it doesn't break anything
upstream (public), then it should be removed. If it breaks downstream (private),
then we'll add code downstream to unbreak it.

https://codereview.chromium.org/2570313002/diff/20001/build/android/gyp/proce...
File build/android/gyp/process_resources.py (right):

https://codereview.chromium.org/2570313002/diff/20001/build/android/gyp/proce...
build/android/gyp/process_resources.py:79: '--r-text-in',
nit: please indent same as other add_option calls

https://codereview.chromium.org/2570313002/diff/20001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/2570313002/diff/20001/build/config/android/in...
build/config/android/internal_rules.gni:2625: r_text_in_path =
invoker.r_text_in_path
super nit: EIther remove this variable (by using "invoker." below), or prefix
with an _ to denote that it's a local variable and not a parameter to a template
/ target.

[mlopatkin, 2016-12-15 16:34:41]

The CQ bit was checked by mlopatkin@yandex-team.ru to run a CQ dry run

[commit-bot: I haz the power, 2016-12-15 16:34:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-15 17:34:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-15 17:34:52]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[dgn, 2016-12-15 18:01:22]

Tested both upstream and downstream, that works, thanks! non owner lgtm

[mlopatkin, 2016-12-19 13:42:27]

The CQ bit was checked by mlopatkin@yandex-team.ru to run a CQ dry run

[commit-bot: I haz the power, 2016-12-19 13:42:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-19 14:33:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-19 14:33:26]

Dry run: This issue passed the CQ dry run.

[mlopatkin, 2016-12-19 14:44:42]

agrieve, PTAL at the latest patch.

I have to add a (small) workaround to fix aapt invocation failure in
'gvr_controller_java' that has an empty R.txt and no resources - such aars are
treated as having no resources at all, no R.java is generated for them -- like
it was before the CL.

https://codereview.chromium.org/2570313002/diff/20001/build/android/gyp/proce...
File build/android/gyp/process_resources.py (right):

https://codereview.chromium.org/2570313002/diff/20001/build/android/gyp/proce...
build/android/gyp/process_resources.py:79: '--r-text-in',
On 2016/12/14 18:16:02, agrieve wrote:
> nit: please indent same as other add_option calls

Done.

https://codereview.chromium.org/2570313002/diff/20001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/2570313002/diff/20001/build/config/android/in...
build/config/android/internal_rules.gni:2625: r_text_in_path =
invoker.r_text_in_path
On 2016/12/14 18:16:02, agrieve wrote:
> super nit: EIther remove this variable (by using "invoker." below), or prefix
> with an _ to denote that it's a local variable and not a parameter to a
template
> / target.

Done.

[agrieve, 2016-12-19 15:59:24]

On 2016/12/19 14:44:42, mlopatkin wrote:
> agrieve, PTAL at the latest patch.
> 
> I have to add a (small) workaround to fix aapt invocation failure in
> 'gvr_controller_java' that has an empty R.txt and no resources - such aars are
> treated as having no resources at all, no R.java is generated for them -- like
> it was before the CL.
> 
>
https://codereview.chromium.org/2570313002/diff/20001/build/android/gyp/proce...
> File build/android/gyp/process_resources.py (right):
> 
>
https://codereview.chromium.org/2570313002/diff/20001/build/android/gyp/proce...
> build/android/gyp/process_resources.py:79: '--r-text-in',
> On 2016/12/14 18:16:02, agrieve wrote:
> > nit: please indent same as other add_option calls
> 
> Done.
> 
>
https://codereview.chromium.org/2570313002/diff/20001/build/config/android/in...
> File build/config/android/internal_rules.gni (right):
> 
>
https://codereview.chromium.org/2570313002/diff/20001/build/config/android/in...
> build/config/android/internal_rules.gni:2625: r_text_in_path =
> invoker.r_text_in_path
> On 2016/12/14 18:16:02, agrieve wrote:
> > super nit: EIther remove this variable (by using "invoker." below), or
prefix
> > with an _ to denote that it's a local variable and not a parameter to a
> template
> > / target.
> 
> Done.

lgtm

[mlopatkin, 2016-12-19 16:00:18]

The CQ bit was checked by mlopatkin@yandex-team.ru

[mlopatkin, 2016-12-19 16:00:19]

The patchset sent to the CQ was uploaded after l-g-t-m from dgn@chromium.org
Link to the patchset: https://codereview.chromium.org/2570313002/#ps60001
(title: "Added workaround for aars with empty R.txt and no resources.")

[commit-bot: I haz the power, 2016-12-19 16:00:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-19 16:04:47]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1482163218794050,
"parent_rev": "1066996e7ca2f01bfa3b5bfaf0d888cb88938234", "commit_rev":
"0afe5b584e8143d541bdc6c16fc09e6443ec7a46"}

[commit-bot: I haz the power, 2016-12-19 16:05:13]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-12-19 16:05:14]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-19 16:09:09]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/d36c048e72f0e936b725d4de25179a7c41599c1a
Cr-Commit-Position: refs/heads/master@{#439485}
==========

[commit-bot: I haz the power, 2016-12-19 16:09:12]

Patchset 4 (id:??) landed as
https://crrev.com/d36c048e72f0e936b725d4de25179a7c41599c1a
Cr-Commit-Position: refs/heads/master@{#439485}