New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 908988 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug


Sign in to add a comment

Switch from Proguard to R8

Project Member Reported by smaier@chromium.org, Nov 27

Issue description

This bug is tracking the actual transition from R8 to Proguard.

Investigation bug, which is where prior work for this was tracked, can be found here https://bugs.chromium.org/p/chromium/issues/detail?id=872904

 
Blocking: 887975
Description: Show this description
Checking method_count.py output for R8 we have:
classes.dex size 7346232
*RESULT Monochrome.apk_classes.dex_methods: total= 62089 methods
*RESULT Monochrome.apk_classes.dex_types: total= 12712 types
*RESULT Monochrome.apk_classes.dex_fields: total= 31519 fields
*RESULT Monochrome.apk_classes.dex_strings: total= 34780 strings
*RESULT MonochromePublic.apk_classes.dex_methods: total= 57371 methods
*RESULT MonochromePublic.apk_classes.dex_types: total= 11371 types
*RESULT MonochromePublic.apk_classes.dex_fields: total= 28779 fields
*RESULT MonochromePublic.apk_classes.dex_strings: total= 31968 strings


This compares to the current state with Proguard:
Monochrome.apk classes.dex size: 7253372
*RESULT Monochrome.apk_classes.dex_methods: total= 63336 methods
*RESULT Monochrome.apk_classes.dex_types: total= 12844 types
*RESULT Monochrome.apk_classes.dex_fields: total= 33128 fields
*RESULT Monochrome.apk_classes.dex_strings: total= 36106 strings
*RESULT MonochromePublic.apk_classes.dex_methods: total= 65529 methods
*RESULT MonochromePublic.apk_classes.dex_types: total= 12285 types
*RESULT MonochromePublic.apk_classes.dex_fields: total= 33211 fields
*RESULT MonochromePublic.apk_classes.dex_strings: total= 33606 strings
*RESULT MonochromePublic.apk_classes2.dex_methods: total= 5779 methods
*RESULT MonochromePublic.apk_classes2.dex_types: total= 1077 types
*RESULT MonochromePublic.apk_classes2.dex_fields: total= 3154 fields
*RESULT MonochromePublic.apk_classes2.dex_strings: total= 3368 strings


So, as it stands, it is a 90 KiB size regression, but actually a pretty good win over Proguard in all other areas. I guess it must be aggressively inlining?
Cc: ricow@chromium.org r8-team@google.com
We will take a look at the sizes.
QQ: these numbers, are they based on the compressed size and are the proguard numbers based on using D8 as the dexer or DX?
Answering myself here, since I reproed this. This is uncompressed sizes, and they dexer is D8 for the proguarded output.

Also, for the MonoChromePublic apk we are considerably smaller, the difference is pretty strange
For monochrome public, we're currently using a several-year old version of proguard, which doesn't do a good job at optimizing lambdas, or synthetic accessors.
These numbers are uncompressed, and the dexer is D8 in the proguard case, and R8 in the R8 case.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 30

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

commit b6ceab81b62b803fced2028ac73c4da3318eba57
Author: agrieve <agrieve@chromium.org>
Date: Fri Nov 30 01:08:14 2018

Revert "Android: Using R8 instead of ProGuard for public targets"

This reverts commit 8dc97871b1493acdfc19e3a85b8ab65064acb98a.

Reason for revert: Broke compile for android-rel
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8928477879975314896/+/steps/compile/0/stdout

Original change's description:
> Android: Using R8 instead of ProGuard for public targets
> 
> Bug: 908988
> Change-Id: Iaaee8124c56945a8769e6d4ff2a0859ef39c1aea
> Reviewed-on: https://chromium-review.googlesource.com/c/1355424
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612439}

TBR=agrieve@chromium.org,smaier@chromium.org

Change-Id: I6cdf8fd95b254abe733426f9e38aa1446661f9e8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 908988
Reviewed-on: https://chromium-review.googlesource.com/c/1356229
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612483}
[modify] https://crrev.com/b6ceab81b62b803fced2028ac73c4da3318eba57/build/config/android/config.gni
[modify] https://crrev.com/b6ceab81b62b803fced2028ac73c4da3318eba57/build/config/android/internal_rules.gni
[modify] https://crrev.com/b6ceab81b62b803fced2028ac73c4da3318eba57/build/config/android/rules.gni
[modify] https://crrev.com/b6ceab81b62b803fced2028ac73c4da3318eba57/chrome/android/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 30

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

commit 79d17cde236c3ccf66bfdad2a6da6ee44b0e8f8d
Author: Sam Maier <smaier@chromium.org>
Date: Fri Nov 30 22:28:43 2018

Android: rolling R8 1.3.40 -> 1.4.13-dev

Fixes the issues that broke the previous 1.4 roll:
https://issuetracker.google.com/issues/120130435

Bug: 908988
Change-Id: Ibe4f16e2b12ed569762ace16b1d573e9328b1cf4
Reviewed-on: https://chromium-review.googlesource.com/c/1357511
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612821}
[modify] https://crrev.com/79d17cde236c3ccf66bfdad2a6da6ee44b0e8f8d/DEPS
[modify] https://crrev.com/79d17cde236c3ccf66bfdad2a6da6ee44b0e8f8d/third_party/r8/README.chromium

Looks like the r8-enabling change is hitting some kitkat multidex verification failures:
r8-team@ - any idea what might be wrong with the multidex setup?

Review: https://chromium-review.googlesource.com/c/chromium/src/+/1357306
Logcat: https://luci-logdog.appspot.com/logs/chromium/android/swarming/logcats/418f48fecb11da11/+/logcat_logcat_org.chromium.chrome.browser.banners.AppBannerManagerTest.testBitmapFetchersCanOverlapWithoutCrashing_20181203T235127-UTC_06b2684c003bfab5

Logs:
12-03 23:51:27.589 16570 16570 I MultiDex: VM with version 1.6.0 does not have multidex support
12-03 23:51:27.589 16570 16570 I MultiDex: install
12-03 23:51:27.599 16570 16570 I MultiDex: MultiDexExtractor.load(MultiDexExtractor.java)
12-03 23:51:27.599 16570 16570 I MultiDex: loading existing secondary dex files
12-03 23:51:27.599 16570 16570 I MultiDex: load found 1 secondary dex files
12-03 23:51:27.599 16570 16570 I MultiDex: install done
...
12-03 23:51:27.679 16570 16585 W dalvikvm: Class resolved by unexpected DEX: Lorg/mockito/Mock;(0x425ddb48):0x75d38000 ref [Lorg/mockito/Answers;] Lorg/mockito/Answers;(0x425ddb48):0x76e0d000
12-03 23:51:27.679 16570 16585 W dalvikvm: (Lorg/mockito/Mock; had used a different Lorg/mockito/Answers; during pre-verification)
...
12-03 23:51:28.779 16570 16585 I TestRunner: java.lang.IllegalAccessError: Class ref in pre-verified class resolved to unexpected implementation
12-03 23:51:28.779 16570 16585 I TestRunner: 	at java.lang.reflect.Method.getDefaultValue(Method.java)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at java.lang.reflect.Method.getDefaultValue(Method.java:353)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at libcore.reflect.AnnotationFactory.getElementsDescription(AnnotationFactory.java:75)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at libcore.reflect.AnnotationFactory.<init>(AnnotationFactory.java:112)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at libcore.reflect.AnnotationFactory.createAnnotation(AnnotationFactory.java:94)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at java.lang.reflect.Field.getDeclaredAnnotations(Field.java)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at java.lang.reflect.Field.getDeclaredAnnotations(Field.java:201)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at java.lang.reflect.AccessibleObject.getAnnotations(AccessibleObject.java:138)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.model.FrameworkField.getAnnotations(FrameworkField.java:31)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.model.TestClass.addToAnnotationLists(TestClass.java:84)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.model.TestClass.scanAnnotatedMembers(TestClass.java:71)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.model.TestClass.<init>(TestClass.java:57)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.ParentRunner.createTestClass(ParentRunner.java:88)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.ParentRunner.<init>(ParentRunner.java:83)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.BlockJUnit4ClassRunner.<init>(BlockJUnit4ClassRunner.java:65)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at android.support.test.internal.runner.junit4.AndroidJUnit4ClassRunner.<init>(AndroidJUnit4ClassRunner.java:37)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.chromium.base.test.BaseJUnit4ClassRunner.<init>(BaseJUnit4ClassRunner.java:87)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.chromium.content_public.browser.test.ContentJUnit4ClassRunner.<init>(ContentJUnit4ClassRunner.java:31)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.chromium.chrome.test.ChromeJUnit4ClassRunner.<init>(ChromeJUnit4ClassRunner.java:44)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at java.lang.reflect.Constructor.constructNative(Constructor.java)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.internal.builders.AnnotatedBuilder.buildRunner(AnnotatedBuilder.java:104)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.internal.builders.AnnotatedBuilder.runnerForClass(AnnotatedBuilder.java:86)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at android.support.test.internal.runner.junit4.AndroidAnnotatedBuilder.runnerForClass(AndroidAnnotatedBuilder.java:77)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.internal.builders.AllDefaultPossibilitiesBuilder.runnerForClass(AllDefaultPossibilitiesBuilder.java:26)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runner.Computer.getRunner(Computer.java:40)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runner.Computer$1.runnerForClass(Computer.java:31)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.model.RunnerBuilder.runners(RunnerBuilder.java:101)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.model.RunnerBuilder.runners(RunnerBuilder.java:87)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runners.Suite.<init>(Suite.java:81)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.junit.runner.Computer.getSuite(Computer.java:28)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at android.support.test.internal.runner.TestRequestBuilder.classes(TestRequestBuilder.java:789)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at android.support.test.internal.runner.TestRequestBuilder.build(TestRequestBuilder.java:753)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at android.support.test.runner.AndroidJUnitRunner.buildRequest(AndroidJUnitRunner.java:354)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at android.support.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:260)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at org.chromium.base.test.BaseChromiumAndroidJUnitRunner.onStart(BaseChromiumAndroidJUnitRunner.java:128)
12-03 23:51:28.779 16570 16585 I TestRunner: 	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1701)




Cc: sgjesse@google.com ager@google.com
I will try to repro this locally so that we can take a look. I lifted this from the bots:
I lifted this from the bots, hope this the correct way:
gn gen out/Release --args='dcheck_always_on=true ffmpeg_branding="Chrome" is_component_build=false is_debug=false proprietary_codecs=true strip_absolute_paths_from_debug_symbols=true strip_debug_info=true symbol_level=1 target_os="android" use_goma=true'

+ager@ who did some maindex fixes recently (but as far as I know, that should not effect our normal command line builds)
Andrew: is there an easy way for us to repro this? it seems that the testing is installing several apks? which may be the issue here, if the class is either different in some of the apks (or the one that art quickened is different than what we see a runtime)
Simple building and installing chrome_public_test_apk does not trigger the issue
I was able to repro, I just had to build chrome_public_test_apk (autoninja -C out/Release chrome_public_test_apk), then run it on a KitKat (API level 19) device. The Android Studio emulators worked fine for me, as well as a real device.

To run, just use this command:
out/Release/bin/run_chrome_public_test_apk -f "*testBannerFallsBackToShortNameWhenNameNotPresent"

With this, I get the stacktrace:
C   37.853s Main  [FAILURE] org.chromium.chrome.browser.banners.AppBannerManagerTest#testBannerFallsBackToShortNameWhenNameNotPresent:
C   37.853s Main  java.lang.IllegalAccessError: Class ref in pre-verified class resolved to unexpected implementation
C   37.853s Main  	at java.lang.reflect.Method.getDefaultValue(Method.java)
C   37.853s Main  	at java.lang.reflect.Method.getDefaultValue(Method.java:353)
C   37.853s Main  	at libcore.reflect.AnnotationFactory.getElementsDescription(AnnotationFactory.java:75)
C   37.853s Main  	at libcore.reflect.AnnotationFactory.<init>(AnnotationFactory.java:112)
C   37.853s Main  	at libcore.reflect.AnnotationFactory.createAnnotation(AnnotationFactory.java:94)
C   37.853s Main  	at java.lang.reflect.Field.getDeclaredAnnotations(Field.java)
C   37.853s Main  	at java.lang.reflect.Field.getDeclaredAnnotations(Field.java:201)
C   37.854s Main  	at java.lang.reflect.AccessibleObject.getAnnotations(AccessibleObject.java:138)
C   37.854s Main  	at org.junit.runners.model.FrameworkField.getAnnotations(FrameworkField.java:31)
C   37.854s Main  	at org.junit.runners.model.TestClass.addToAnnotationLists(TestClass.java:84)
C   37.854s Main  	at org.junit.runners.model.TestClass.scanAnnotatedMembers(TestClass.java:71)
C   37.854s Main  	at org.junit.runners.model.TestClass.<init>(TestClass.java:57)
C   37.854s Main  	at org.junit.runners.ParentRunner.createTestClass(ParentRunner.java:88)
C   37.854s Main  	at org.junit.runners.ParentRunner.<init>(ParentRunner.java:83)
C   37.854s Main  	at org.junit.runners.BlockJUnit4ClassRunner.<init>(BlockJUnit4ClassRunner.java:65)
C   37.854s Main  	at android.support.test.internal.runner.junit4.AndroidJUnit4ClassRunner.<init>(AndroidJUnit4ClassRunner.java:37)
C   37.854s Main  	at org.chromium.base.test.BaseJUnit4ClassRunner.<init>(BaseJUnit4ClassRunner.java:87)
C   37.854s Main  	at org.chromium.content_public.browser.test.ContentJUnit4ClassRunner.<init>(ContentJUnit4ClassRunner.java:31)
C   37.854s Main  	at org.chromium.chrome.test.ChromeJUnit4ClassRunner.<init>(ChromeJUnit4ClassRunner.java:44)
C   37.854s Main  	at java.lang.reflect.Constructor.constructNative(Constructor.java)
C   37.854s Main  	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
C   37.855s Main  	at org.junit.internal.builders.AnnotatedBuilder.buildRunner(AnnotatedBuilder.java:104)
C   37.855s Main  	at org.junit.internal.builders.AnnotatedBuilder.runnerForClass(AnnotatedBuilder.java:86)
C   37.855s Main  	at android.support.test.internal.runner.junit4.AndroidAnnotatedBuilder.runnerForClass(AndroidAnnotatedBuilder.java:77)
C   37.855s Main  	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
C   37.855s Main  	at org.junit.internal.builders.AllDefaultPossibilitiesBuilder.runnerForClass(AllDefaultPossibilitiesBuilder.java:26)
C   37.855s Main  	at org.junit.runner.Computer.getRunner(Computer.java:40)
C   37.855s Main  	at org.junit.runner.Computer$1.runnerForClass(Computer.java:31)
C   37.855s Main  	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
C   37.855s Main  	at org.junit.runners.model.RunnerBuilder.runners(RunnerBuilder.java:101)
C   37.855s Main  	at org.junit.runners.model.RunnerBuilder.runners(RunnerBuilder.java:87)
C   37.855s Main  	at org.junit.runners.Suite.<init>(Suite.java:81)
C   37.855s Main  	at org.junit.runner.Computer.getSuite(Computer.java:28)
C   37.855s Main  	at android.support.test.internal.runner.TestRequestBuilder.classes(TestRequestBuilder.java:789)
C   37.856s Main  	at android.support.test.internal.runner.TestRequestBuilder.build(TestRequestBuilder.java:753)
C   37.856s Main  	at android.support.test.runner.AndroidJUnitRunner.buildRequest(AndroidJUnitRunner.java:354)
C   37.856s Main  	at android.support.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:260)
C   37.856s Main  	at org.chromium.base.test.BaseChromiumAndroidJUnitRunner.onStart(BaseChromiumAndroidJUnitRunner.java:128)
C   37.856s Main  	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1701)

Looking at the generated dex files, we have org.mockito.Mock in the main dex, while we have org.mockito.Answers in the second dex. Mock has a method with type org.mockito.Answers, so this appears to cause issues. I'm trying to investigate why Answers is not in the main dex.
It can be OK for org.mockito.Mock to be in the main dex and org.mockito.Answers not to be. The main dex holds all classes covered by the main dex list/rules *and* their direct dependencies. If org.mockito.Mock is not covered by the main dex list/rules, then it can be in the main dex if it is a direct dependency of something covered by the main dex list/rules, and then org.mockito.Answers might not need to be there as well.

Also as this looks to be a release build, then there will be classes in the main dex, which are not related to the main dex list/rules at all, as R8 will always fill classes.dex in release mode. In debug mode R8 will always create a minimal main dex file - it will only contain classes covered by the main dex list/rules and their direct dependencies.

You can use the option --main-dex-list-output to get the exact main dex list computed by R8.
The reason I'm concerned about the mockito classes is this logcat entry found before the exception:
(Lorg/mockito/Mock; had used a different Lorg/mockito/Answers; during pre-verification)

Looking at the main dex list output, here are all the org.mockito classes listed:
org/mockito/Captor.class
org/mockito/CheckReturnValue.class
org/mockito/Incubating.class
org/mockito/InjectMocks.class
org/mockito/Mock.class
org/mockito/NotExtensible.class
org/mockito/Spy.class
org/mockito/internal/creation/bytebuddy/MockMethodAdvice$ForEquals.class
org/mockito/internal/creation/bytebuddy/MockMethodAdvice$ForHashCode.class
org/mockito/internal/creation/bytebuddy/MockMethodAdvice$ForReadObject.class
org/mockito/internal/creation/bytebuddy/MockMethodAdvice$Identifier.class
org/mockito/internal/creation/bytebuddy/MockMethodAdvice.class
org/mockito/internal/creation/bytebuddy/MockMethodDispatcher.class
org/mockito/internal/creation/bytebuddy/MockMethodInterceptor$DispatcherDefaultingToRealMethod.class
org/mockito/internal/creation/bytebuddy/MockMethodInterceptor$ForEquals.class

I've tried a bunch of -whyareyoukeeping arguments to the R8 invocation to see why Mock is in the main dex list, but for some reason I can't get it to give me any output. When I do another class (ie ChromeApplication) the -whyareyoukeeping works perfectly fine. Here are the -whyareyoukeepings I've tried:

-whyareyoukeeping class org.mockito.* {
  *;  
}
-whyareyoukeeping interface org.mockito.** {
  *;  
}
-whyareyoukeeping interface org.mockito.Mock {
  *;  
}
-whyareyoukeeping class org.mockito.Mock {
  *;  
}

Those all produce no output. When I try:
-whyareyoukeeping class org.mockito.** {
  *;  
}

It produces a ton of output, however it's all junit related (the text "mockito" doesn't even show up). Imagine this x 100:
org.junit.runners.model.Statement org.junit.runners.BlockJUnit4ClassRunner.withAfters(org.junit.runners.model.FrameworkMethod, java.lang.Object, org.junit.runners.model.Statement)
|- is reachable because is invoked from org.junit.runners.model.Statement org.junit.runners.BlockJUnit4ClassRunner.methodBlock(org.junit.runners.model.FrameworkMethod)
|- is live because is reachable from type android.support.test.internal.runner.junit4.AndroidJUnit4ClassRunner

Might be related to this:
https://android.googlesource.com/platform/dalvik/+/master/dx/src/com/android/multidex/MainDexListBuilder.java#65

Although note that when we using dx, we were passing --disable-annotation-resolution-workaround.

Maybe worth comparing the main dex class list produced via r8 and build/android/gyp/main_dex_list.py
The old way of making main dex lists (via main_dex_list.py) generates only 821 entries, while R8 produces 2220.

Mockito does NOT appear in our old main dex list, but the few classes listed above do show up.

One other interesting thing: there is no net/bytebuddy showing up in the old list, however this new one has 971 bytebuddy entries. This is >1/3rd of the entries.
Urgh, this is a bug in our main dex list tracing where we correctly get the classes annotated with an annotation with an enum value in the main dex list. However, we forget to add the enum class as well. :-\

The reason why we are not seeing this in Android Studio where we have been doing the main dex list tracing for a long time is that Android Studio adds an explicit rule to keep *all* annotation in the main dex list. Because of that rule, we trace it and add the annotation and the enum to the main dex list. Since you are using our command-line version independently of the Android Gradle Plugin, you do not have this rule and hit our bug.

We are working on a fix. sgjesse@ will get back to you with the fix once it has landed.
A fix by sgjesse landed in R8:
https://r8.googlesource.com/r8.git/+/4d36562e63958c68346cc24556342a3335ab430c
I could not get your repro to work, I get a different exception and can't get the test to run.
I validated that the enum is now in classes.dex for the chrome public test apk.
Project Member

Comment 23 by bugdroid1@chromium.org, Dec 5

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

commit 65d4114d417ff6e5b4b917bd835c4e567d3c7d49
Author: Sam Maier <smaier@chromium.org>
Date: Wed Dec 05 18:03:30 2018

Rolling R8, still on 1.4.13-dev

Bug: 908988
Change-Id: I438f48dc01437372d5928aa2a24ac97d405f1200
Reviewed-on: https://chromium-review.googlesource.com/c/1363366
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614024}
[modify] https://crrev.com/65d4114d417ff6e5b4b917bd835c4e567d3c7d49/DEPS
[modify] https://crrev.com/65d4114d417ff6e5b4b917bd835c4e567d3c7d49/third_party/r8/README.chromium

Probably roll past https://r8.googlesource.com/r8.git/+/7c6c94d8d378c39318fdf2b197ad2e0316146a61, as that has the fixes for the warnings, which could not be otherwise avoided. That is b/120490612, and the implements/extends warnings reported by mail.
Project Member

Comment 26 by bugdroid1@chromium.org, Dec 7

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

commit b83159bf21e31ef29edbeb18eddf02af766654c0
Author: Sam Maier <smaier@chromium.org>
Date: Fri Dec 07 18:24:41 2018

Rolling R8 and D8 1.4.13 -> 1.4.14

Bug: 908988
Change-Id: I170702d077cd0d3f07894afb084a1d3a20a87de1
Reviewed-on: https://chromium-review.googlesource.com/c/1367934
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614752}
[modify] https://crrev.com/b83159bf21e31ef29edbeb18eddf02af766654c0/DEPS
[modify] https://crrev.com/b83159bf21e31ef29edbeb18eddf02af766654c0/third_party/r8/README.chromium

Project Member

Comment 27 by bugdroid1@chromium.org, Dec 7

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

commit 1f794df0d8a75aa896df6dc9e3e23f359829a1bd
Author: Sam Maier <smaier@chromium.org>
Date: Fri Dec 07 21:17:53 2018

AndroidManifest added for service in invalidation tests

This causes issues with R8 as TestableInvalidationClientService is not
kept. This should be kept, even with Proguard.

Bug: 908988
Change-Id: I2bff4a0dd8a1fd0cb3a2ec57da12863970923c71
Reviewed-on: https://chromium-review.googlesource.com/c/1368275
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614815}
[modify] https://crrev.com/1f794df0d8a75aa896df6dc9e3e23f359829a1bd/components/invalidation/impl/BUILD.gn
[add] https://crrev.com/1f794df0d8a75aa896df6dc9e3e23f359829a1bd/components/invalidation/impl/android/javatests/AndroidManifest.xml

Blockedon: 906803
Project Member

Comment 29 by bugdroid1@chromium.org, Dec 14

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

commit b90d813df13132022b06a6b51b77b03365e6f2d5
Author: Sam Maier <smaier@chromium.org>
Date: Fri Dec 14 19:24:19 2018

Used by reflection for test CCT dynamic module

Also updated used by reflection to be more explicit.

TBR=yfriedman

Bug: 908988
Change-Id: I7a4b07fd60dd99afcf07239b9723d41cab575828
Reviewed-on: https://chromium-review.googlesource.com/c/1378240
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616781}
[modify] https://crrev.com/b90d813df13132022b06a6b51b77b03365e6f2d5/base/android/proguard/chromium_code.flags
[modify] https://crrev.com/b90d813df13132022b06a6b51b77b03365e6f2d5/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModuleTestUtils.java

Blockedon: -906803
Blocking: 906803
Blocking: 916014
Blocking: 916875
Project Member

Comment 33 by bugdroid1@chromium.org, Dec 22

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

commit d6bebdaabea9105c924d8ccc85a1a43b528f4a9f
Author: Sam Maier <smaier@chromium.org>
Date: Sat Dec 22 19:16:26 2018

Android: Using @Mock instead of Mockito.mock for feed test

@Mock can be dealt with using Proguard rules, Mockito.mock can't be. R8 is
having issues because it doesn't keep these classes, but the old Proguard wasn't
smart enough to optimize these classes away.

TBR=skym

Bug: 908988
Change-Id: Iae4cc98f26cbc18a29814d380c7d65391fbf3cf2
Reviewed-on: https://chromium-review.googlesource.com/c/1388152
Reviewed-by: Sam Maier <smaier@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618759}
[modify] https://crrev.com/d6bebdaabea9105c924d8ccc85a1a43b528f4a9f/chrome/android/javatests/src/org/chromium/chrome/browser/feed/FeedSchedulerBridgeConformanceTest.java

Project Member

Comment 34 by bugdroid1@chromium.org, Jan 2

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

commit 09f15bdb07cd457c1a96b08ce54faa4ba2771dee
Author: Sam Maier <smaier@chromium.org>
Date: Wed Jan 02 20:04:06 2019

Android: Turning on R8 instead of Proguard for public targets

We are in the process of migrating to R8 to replace Proguard. This is
the first step - move all usages of Proguard in public targets to R8.

Some refactorings were natural with the few forced changes this caused.

TBR=smaier

Bug: 908988, 913554
Change-Id: I2139919598fba1643d7560dc5557d5efb9a5887c
Reviewed-on: https://chromium-review.googlesource.com/c/1357306
Reviewed-by: Sam Maier <smaier@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619471}
[modify] https://crrev.com/09f15bdb07cd457c1a96b08ce54faa4ba2771dee/build/android/gyp/proguard.py
[modify] https://crrev.com/09f15bdb07cd457c1a96b08ce54faa4ba2771dee/build/config/android/config.gni
[modify] https://crrev.com/09f15bdb07cd457c1a96b08ce54faa4ba2771dee/build/config/android/internal_rules.gni
[modify] https://crrev.com/09f15bdb07cd457c1a96b08ce54faa4ba2771dee/build/config/android/rules.gni
[modify] https://crrev.com/09f15bdb07cd457c1a96b08ce54faa4ba2771dee/chrome/android/BUILD.gn
[modify] https://crrev.com/09f15bdb07cd457c1a96b08ce54faa4ba2771dee/testing/android/proguard_for_test.flags
[modify] https://crrev.com/09f15bdb07cd457c1a96b08ce54faa4ba2771dee/third_party/accessibility_test_framework/proguard.flags

Project Member

Comment 35 by bugdroid1@chromium.org, Jan 3

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

commit e3d0df762a401d7e7bb7629e54981bab7871668b
Author: Egor Pasko <pasko@chromium.org>
Date: Thu Jan 03 14:39:24 2019

Revert "Android: Turning on R8 instead of Proguard for public targets"

This reverts commit 09f15bdb07cd457c1a96b08ce54faa4ba2771dee.

Reason for revert: http://crbug.com/918796

Original change's description:
> Android: Turning on R8 instead of Proguard for public targets
> 
> We are in the process of migrating to R8 to replace Proguard. This is
> the first step - move all usages of Proguard in public targets to R8.
> 
> Some refactorings were natural with the few forced changes this caused.
> 
> TBR=smaier
> 
> Bug: 908988, 913554
> Change-Id: I2139919598fba1643d7560dc5557d5efb9a5887c
> Reviewed-on: https://chromium-review.googlesource.com/c/1357306
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619471}

TBR=wnwen@chromium.org,agrieve@chromium.org,smaier@chromium.org

Change-Id: Ieb34a24da7c001e78d832f7300855c7c7c749add
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 908988, 913554
Reviewed-on: https://chromium-review.googlesource.com/c/1394528
Reviewed-by: Egor Pasko <pasko@chromium.org>
Commit-Queue: Egor Pasko <pasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619634}
[modify] https://crrev.com/e3d0df762a401d7e7bb7629e54981bab7871668b/build/android/gyp/proguard.py
[modify] https://crrev.com/e3d0df762a401d7e7bb7629e54981bab7871668b/build/config/android/config.gni
[modify] https://crrev.com/e3d0df762a401d7e7bb7629e54981bab7871668b/build/config/android/internal_rules.gni
[modify] https://crrev.com/e3d0df762a401d7e7bb7629e54981bab7871668b/build/config/android/rules.gni
[modify] https://crrev.com/e3d0df762a401d7e7bb7629e54981bab7871668b/chrome/android/BUILD.gn
[modify] https://crrev.com/e3d0df762a401d7e7bb7629e54981bab7871668b/testing/android/proguard_for_test.flags
[modify] https://crrev.com/e3d0df762a401d7e7bb7629e54981bab7871668b/third_party/accessibility_test_framework/proguard.flags

Project Member

Comment 36 by bugdroid1@chromium.org, Jan 3

Labels: merge-merged-3660
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a38b3ac1b4b8ea9781400d7efaef1bea94e65a82

commit a38b3ac1b4b8ea9781400d7efaef1bea94e65a82
Author: Egor Pasko <pasko@chromium.org>
Date: Thu Jan 03 14:48:01 2019

Revert "Android: Turning on R8 instead of Proguard for public targets"

This reverts commit 09f15bdb07cd457c1a96b08ce54faa4ba2771dee.

Reason for revert: http://crbug.com/918796

Original change's description:
> Android: Turning on R8 instead of Proguard for public targets
> 
> We are in the process of migrating to R8 to replace Proguard. This is
> the first step - move all usages of Proguard in public targets to R8.
> 
> Some refactorings were natural with the few forced changes this caused.
> 
> TBR=smaier
> 
> Bug: 908988, 913554
> Change-Id: I2139919598fba1643d7560dc5557d5efb9a5887c
> Reviewed-on: https://chromium-review.googlesource.com/c/1357306
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619471}

TBR=wnwen@chromium.org,agrieve@chromium.org,smaier@chromium.org

Change-Id: Ieb34a24da7c001e78d832f7300855c7c7c749add
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 908988, 913554
Reviewed-on: https://chromium-review.googlesource.com/c/1394528
Reviewed-by: Egor Pasko <pasko@chromium.org>
Commit-Queue: Egor Pasko <pasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#619634}(cherry picked from commit e3d0df762a401d7e7bb7629e54981bab7871668b)
Reviewed-on: https://chromium-review.googlesource.com/c/1394647
Reviewed-by: Ben Mason <benmason@chromium.org>
Cr-Commit-Position: refs/branch-heads/3660@{#3}
Cr-Branched-From: bd3d561129870953b32dfc6fef8a04604372d2ec-refs/heads/master@{#619561}
[modify] https://crrev.com/a38b3ac1b4b8ea9781400d7efaef1bea94e65a82/build/android/gyp/proguard.py
[modify] https://crrev.com/a38b3ac1b4b8ea9781400d7efaef1bea94e65a82/build/config/android/config.gni
[modify] https://crrev.com/a38b3ac1b4b8ea9781400d7efaef1bea94e65a82/build/config/android/internal_rules.gni
[modify] https://crrev.com/a38b3ac1b4b8ea9781400d7efaef1bea94e65a82/build/config/android/rules.gni
[modify] https://crrev.com/a38b3ac1b4b8ea9781400d7efaef1bea94e65a82/chrome/android/BUILD.gn
[modify] https://crrev.com/a38b3ac1b4b8ea9781400d7efaef1bea94e65a82/testing/android/proguard_for_test.flags
[modify] https://crrev.com/a38b3ac1b4b8ea9781400d7efaef1bea94e65a82/third_party/accessibility_test_framework/proguard.flags

The error from Proguard:

Proguard Exiting with the following Errors:
	- Expecting ';' before '*' in line 28 of file '../../testing/android/proguard_for_test.flags',   included from argument number 9

Is caused by this rule:

-if class * {
  @org.mockito.Mock * *;
}
-keep interface <2> {
  <methods>;
}

Changing the field type from * to ** makes Proguard parse the rule (I have not checked that it also does the right thing), so this works with Proguard:

-if class * {
  @org.mockito.Mock ** *;
}
-keep interface <2> {
  <methods>;
}

The Proguard manual say "For convenience and for backward compatibility, the class name * refers to any class, irrespective of its package.", but apparently not in this case.
Thanks Søren! I was going around implementing a way for R8-only flags, but this looks like a cleaner solution. I will test and see if it still works. We're primarily worrying about R8 working with these flags - it really doesn't matter what Proguard does with this flag as long as it doesn't crash.
Project Member

Comment 39 by bugdroid1@chromium.org, Jan 3

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

commit 36ab751f1a2b5777acff3d430fde3004c6ed1861
Author: Sam Maier <smaier@chromium.org>
Date: Thu Jan 03 18:47:59 2019

Reland "Android: Turning on R8 instead of Proguard for public targets"

This is a reland of 09f15bdb07cd457c1a96b08ce54faa4ba2771dee

Original change's description:
> Android: Turning on R8 instead of Proguard for public targets
>
> We are in the process of migrating to R8 to replace Proguard. This is
> the first step - move all usages of Proguard in public targets to R8.
>
> Some refactorings were natural with the few forced changes this caused.
>
> TBR=smaier
>
> Bug: 908988, 913554
> Change-Id: I2139919598fba1643d7560dc5557d5efb9a5887c
> Reviewed-on: https://chromium-review.googlesource.com/c/1357306
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619471}

TBR=proguard.flags(smaier)

Bug: 908988, 913554
Change-Id: I93690ad2a6838025221b7fab4b0bee1b06c920c9
Reviewed-on: https://chromium-review.googlesource.com/c/1394387
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619702}
[modify] https://crrev.com/36ab751f1a2b5777acff3d430fde3004c6ed1861/build/android/gyp/proguard.py
[modify] https://crrev.com/36ab751f1a2b5777acff3d430fde3004c6ed1861/build/config/android/config.gni
[modify] https://crrev.com/36ab751f1a2b5777acff3d430fde3004c6ed1861/build/config/android/internal_rules.gni
[modify] https://crrev.com/36ab751f1a2b5777acff3d430fde3004c6ed1861/build/config/android/rules.gni
[modify] https://crrev.com/36ab751f1a2b5777acff3d430fde3004c6ed1861/chrome/android/BUILD.gn
[modify] https://crrev.com/36ab751f1a2b5777acff3d430fde3004c6ed1861/testing/android/proguard_for_test.flags
[modify] https://crrev.com/36ab751f1a2b5777acff3d430fde3004c6ed1861/third_party/accessibility_test_framework/proguard.flags

Stats from a nightly today of Chrome.apk with use_r8=true:

###### With downstream proguard;
 Length   Method    Size  Cmpr    Date    Time   CRC-32   Name
--------  ------  ------- ---- ---------- ----- --------  ----
 6794912  Defl:N  3180116  53% 2001-01-01 00:00 04d9aebf  classes.dex

*RESULT Chrome.apk_classes.dex_methods: total= 60728 methods
*RESULT Chrome.apk_classes.dex_types: total= 12186 types
*RESULT Chrome.apk_classes.dex_fields: total= 29106 fields
*RESULT Chrome.apk_classes.dex_strings: total= 32957 strings

###### With r8:
 Length   Method    Size  Cmpr    Date    Time   CRC-32   Name
--------  ------  ------- ---- ---------- ----- --------  ----
 6968612  Defl:N  3371920  52% 2001-01-01 00:00 bcaef788  classes.dex

*RESULT Chrome.apk_classes.dex_methods: total= 59316 methods
*RESULT Chrome.apk_classes.dex_types: total= 12005 types
*RESULT Chrome.apk_classes.dex_fields: total= 28212 fields
*RESULT Chrome.apk_classes.dex_strings: total= 32838 strings

Saves 1400 methods but grows in size by ~190kb compressed.
Note: this much size increase might be a showstopper (or at least a show-slow-downer). Are there any flags we can use to tune this (e.g. inline less)?
Project Member

Comment 42 by bugdroid1@chromium.org, Jan 11

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

commit b3500e8f287bde8708d205d9638af14189ebc562
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Jan 11 04:12:37 2019

Fix gn gen errors with use_r8=true & //clank exists

Bug: 908988
Change-Id: Ie2b5148e3bf9fb6deca72f3ea537fcfb8e55340d
Reviewed-on: https://chromium-review.googlesource.com/c/1405717
Reviewed-by: Sam Maier <smaier@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621903}
[modify] https://crrev.com/b3500e8f287bde8708d205d9638af14189ebc562/build/config/android/internal_rules.gni

What target do I build to exactly repro #40? Is it just monochrome?
You will need a downstream checkout. Then, build using the following gn args:

is_java_debug = false
use_r8 = <true/false>

The target built in #40 is chrome_apk, but you could also do monochrome_apk and it will likely be similar results.

Sign in to add a comment