Make test apks only dex files not in tested apk (proguard version)

Make test apks only dex files not in tested apk (proguard version)

At runtime, the classloader will look for classes in both apk's dex
files. In the standard Android build system, an instrumentation test
apk's dex file does not include the classes included in the tested apk's
dex file.

To do this, when doing obfuscation for an apk, write the list of
libraries included in the obfuscated jar and the list of proguard config
files. Then, when proguarding the test apk's code, exclude those
libraries included in the tested apk, use the configs from the tested
apk, and apply the proguard mapping (the renames from obfuscation). Also
add some extra test-specific proguard options.

Now that the test apk does not bundle its own copy of all the tested
apk's classes, some things may need to be kept in the main apk just for
tests. However, we already keep everything in org.chromium.** and
com.google.android.apps.** because of the fact that the test apk was
using its own copy of all the classes and so we couldn't depend on the
tests to actually catch us from over-optimizing with proguard.

BUG=272790
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277257

[cjhopman, 2014-06-10 17:36:43]

yfriedman: *

[Yaron, 2014-06-11 00:45:27]

https://codereview.chromium.org/321883002/diff/40001/build/android/gyp/apk_ob...
File build/android/gyp/apk_obfuscate.py (right):

https://codereview.chromium.org/321883002/diff/40001/build/android/gyp/apk_ob...
build/android/gyp/apk_obfuscate.py:102: if 'Release' in
options.configuration_name and options.proguard_enabled:
needs rebase :)

https://codereview.chromium.org/321883002/diff/40001/build/android/gyp/apk_ob...
build/android/gyp/apk_obfuscate.py:121: assert len(configs) == 1, (
nit: move to the top of the block.

https://codereview.chromium.org/321883002/diff/40001/build/android/gyp/apk_ob...
build/android/gyp/apk_obfuscate.py:133: proguard_cmd += [ '-injars',
':'.join(proguard_injars), ]
nit: no trailing "," or leading " "

https://codereview.chromium.org/321883002/diff/40001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/321883002/diff/40001/build/java_apk.gypi#newc...
build/java_apk.gypi:165: 'proguard_enabled': 'true',
How is this used?

[cjhopman, 2014-06-12 20:36:45]

https://codereview.chromium.org/321883002/diff/40001/build/android/gyp/apk_ob...
File build/android/gyp/apk_obfuscate.py (right):

https://codereview.chromium.org/321883002/diff/40001/build/android/gyp/apk_ob...
build/android/gyp/apk_obfuscate.py:102: if 'Release' in
options.configuration_name and options.proguard_enabled:
On 2014/06/11 00:45:26, Yaron wrote:
> needs rebase :)

Done.

https://codereview.chromium.org/321883002/diff/40001/build/android/gyp/apk_ob...
build/android/gyp/apk_obfuscate.py:121: assert len(configs) == 1, (
On 2014/06/11 00:45:26, Yaron wrote:
> nit: move to the top of the block.

Done.

https://codereview.chromium.org/321883002/diff/40001/build/android/gyp/apk_ob...
build/android/gyp/apk_obfuscate.py:133: proguard_cmd += [ '-injars',
':'.join(proguard_injars), ]
On 2014/06/11 00:45:26, Yaron wrote:
> nit: no trailing "," or leading " "

Done.

https://codereview.chromium.org/321883002/diff/40001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/321883002/diff/40001/build/java_apk.gypi#newc...
build/java_apk.gypi:165: 'proguard_enabled': 'true',
On 2014/06/11 00:45:26, Yaron wrote:
> How is this used?
This sets proguard_enabled to 'true' in the test apk's target. It might be more
correct to set a variable like tested_apk_proguard_enabled to 'true' and then
only use that in a test_apk that depends on this instead of all apks.

[Yaron, 2014-06-12 23:23:16]

looks like a different diffbase or issue while rebasing

https://codereview.chromium.org/321883002/diff/40001/build/java_apk.gypi
File build/java_apk.gypi (right):

https://codereview.chromium.org/321883002/diff/40001/build/java_apk.gypi#newc...
build/java_apk.gypi:165: 'proguard_enabled': 'true',
On 2014/06/12 20:36:45, cjhopman wrote:
> On 2014/06/11 00:45:26, Yaron wrote:
> > How is this used?
> This sets proguard_enabled to 'true' in the test apk's target. It might be
more
> correct to set a variable like tested_apk_proguard_enabled to 'true' and then
> only use that in a test_apk that depends on this instead of all apks.

Oh, this is for the dex_action?

https://codereview.chromium.org/321883002/diff/60001/build/uiautomator_test.gypi
File build/uiautomator_test.gypi (right):

https://codereview.chromium.org/321883002/diff/60001/build/uiautomator_test.g...
build/uiautomator_test.gypi:27: 'variables': {
Rebase?

https://codereview.chromium.org/321883002/diff/60001/content/content.gyp
File content/content.gyp (left):

https://codereview.chromium.org/321883002/diff/60001/content/content.gyp#oldc...
content/content.gyp:374:
'public/android/java/src/org/chromium/content/common/SurfaceWrapper.aidl',
Unrelated?

[cjhopman, 2014-06-13 17:43:15]

On 2014/06/12 23:23:16, Yaron wrote:
> looks like a different diffbase or issue while rebasing
Yep, that was odd. Uploaded one that seems more correct.

> 
> https://codereview.chromium.org/321883002/diff/40001/build/java_apk.gypi
> File build/java_apk.gypi (right):
> 
>
https://codereview.chromium.org/321883002/diff/40001/build/java_apk.gypi#newc...
> build/java_apk.gypi:165: 'proguard_enabled': 'true',
> On 2014/06/12 20:36:45, cjhopman wrote:
> > On 2014/06/11 00:45:26, Yaron wrote:
> > > How is this used?
> > This sets proguard_enabled to 'true' in the test apk's target. It might be
> more
> > correct to set a variable like tested_apk_proguard_enabled to 'true' and
then
> > only use that in a test_apk that depends on this instead of all apks.
> 
> Oh, this is for the dex_action?
> 
>
https://codereview.chromium.org/321883002/diff/60001/build/uiautomator_test.gypi
> File build/uiautomator_test.gypi (right):
> 
>
https://codereview.chromium.org/321883002/diff/60001/build/uiautomator_test.g...
> build/uiautomator_test.gypi:27: 'variables': {
> Rebase?
> 
> https://codereview.chromium.org/321883002/diff/60001/content/content.gyp
> File content/content.gyp (left):
> 
>
https://codereview.chromium.org/321883002/diff/60001/content/content.gyp#oldc...
> content/content.gyp:374:
> 'public/android/java/src/org/chromium/content/common/SurfaceWrapper.aidl',
> Unrelated?

[Yaron, 2014-06-13 17:52:02]

lgtm

[cjhopman, 2014-06-13 18:03:46]

The CQ bit was checked by cjhopman@chromium.org

[commit-bot: I haz the power, 2014-06-13 18:05:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cjhopman@chromium.org/321883002/120001

[commit-bot: I haz the power, 2014-06-13 21:40:30]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)

[commit-bot: I haz the power, 2014-06-13 21:46:25]

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

[commit-bot: I haz the power, 2014-06-13 21:46:26]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)

[cjhopman, 2014-06-14 00:26:47]

The CQ bit was checked by cjhopman@chromium.org

[commit-bot: I haz the power, 2014-06-14 00:30:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cjhopman@chromium.org/321883002/140001

[cjhopman, 2014-06-14 01:37:24]

The CQ bit was checked by cjhopman@chromium.org

[commit-bot: I haz the power, 2014-06-14 01:42:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cjhopman@chromium.org/321883002/160001

[commit-bot: I haz the power, 2014-06-14 19:52:05]

Change committed as 277257