Merging under test java into instrumentation test java.

This enables us to run all ProGuard uninhibited by tests, since Proguard will
now consider both the tests and the tested code while performing its
optimizations. This is opposed to running ProGuard on the tested java,
keeping stuff around for the tests, which would then use the tested code as
a library.

This turns on optimizations and removes test code from the shipped apk, saving
us ~550kb in .dex size, and ~70kb memory per process.

BUG=620323,625704,626710

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

BUILD.gn

@@ -412,16 +412,12 @@ group("both_gn_and_gyp") {
         "//android_webview/tools/system_webview_shell",
         "//chrome/android:chrome_junit_tests",
         "//chrome/android:chrome_public_apk",
+        "//chrome/android:chrome_public_test_apk",
+        "//chrome/android:chrome_sync_shell_test_apk",
         "//chrome/test/chromedriver/test/webview_shell:chromedriver_webview_shell_apk",
         "//content/shell/android:content_shell_test_apk",
         "//third_party/custom_tabs_client:custom_tabs_client_example_apk",
       ]
-      if (!enable_all_proguard_optimizations) {
-        deps += [
-          "//chrome/android:chrome_public_test_apk",
-          "//chrome/android:chrome_sync_shell_test_apk",
-        ]
-      }
     }
 
     if (target_cpu != "x64") {

base/android/base_proguard_config.flags

@@ -1,8 +1,9 @@
 # Keep line number information, useful for stack traces.
 -keepattributes SourceFile,LineNumberTable
 
-# Keep all runtime visible annotations
--keepattributes RuntimeVisibleAnnotations
+# Keep all annotation related attributes that can affect runtime
+-keepattributes RuntimeVisible*Annotations
+-keepattributes AnnotationDefault
 
 # Keep the annotations, because if we don't, the ProGuard rules that use them
 # will not be respected. These classes then show up in our final dex, which we

build/android/gyp/util/proguard_util.py

@@ -105,18 +105,6 @@ def build(self):
     if self._tested_apk_info_path:
       tested_apk_info = build_utils.ReadJson(self._tested_apk_info_path)
       self._configs += tested_apk_info['configs']
-      self._injars = [
-          p for p in self._injars if not p in tested_apk_info['inputs']]
-      if not self._libraries:
-        self._libraries = []
-      self._libraries += tested_apk_info['inputs']
-      self._mapping = tested_apk_info['mapping']
-      cmd += [
-        '-dontobfuscate',
-        '-dontoptimize',
-        '-dontshrink',
-        '-dontskipnonpubliclibraryclassmembers',
-      ]
 
     if self._mapping:
       cmd += [

build/android/gyp/write_build_config.py

@@ -521,14 +521,33 @@ def main(argv):
     javac_classpath = [c['jar_path'] for c in direct_library_deps]
     java_full_classpath = [c['jar_path'] for c in all_library_deps]
 
-  # An instrumentation test apk should exclude the dex files that are in the apk
-  # under test.
+  # The java code for an instrumentation test apk is assembled differently for
+  # ProGuard vs. non-ProGuard.
+  #
+  # Without ProGuard: Each library's jar is dexed separately and then combined
+  # into a single classes.dex. A test apk will include all dex files not already
+  # present in the apk-under-test. At runtime all test code lives in the test
+  # apk, and the program code lives in the apk-under-test.
+  #
+  # With ProGuard: Each library's .jar file is fed into ProGuard, which outputs
+  # a single .jar, which is then dexed into a classes.dex. A test apk includes
+  # all jar files from the program and the tests because having them separate
+  # doesn't work with ProGuard's whole-program optimizations. Although the
+  # apk-under-test still has all of its code in its classes.dex, none of it is
+  # used at runtime because the copy of it within the test apk takes precidence.
   if options.type == 'android_apk' and options.tested_apk_config:
     tested_apk_config = GetDepConfig(options.tested_apk_config)
 
     expected_tested_package = tested_apk_config['package_name']
     AndroidManifest(options.android_manifest).CheckInstrumentation(
         expected_tested_package)
+    if options.proguard_enabled:
+      # Add all tested classes to the test's classpath to ensure that the test's
+      # java code is a superset of the tested apk's java code
+      java_full_classpath += [
+          jar for jar in tested_apk_config['java']['full_classpath']
+          if jar not in java_full_classpath]
+
     if tested_apk_config['proguard_enabled']:
       assert options.proguard_enabled, ('proguard must be enabled for '
           'instrumentation apks if it\'s enabled for the tested apk.')
@@ -564,7 +583,7 @@ def main(argv):
     config['javac']['classpath'] = javac_classpath
     config['javac']['interface_classpath'] = [
         _AsInterfaceJar(p) for p in javac_classpath]
-    config['java'] = {
+    deps_info['java'] = {
       'full_classpath': java_full_classpath
     }
 

build/config/android/config.gni

@@ -118,13 +118,6 @@ if (is_android) {
     # Required for Android M+ due to SELinux policies (stronger sandboxing).
     disable_incremental_isolated_processes = false
 
-    # Enables all ProGuard optimizations. These optimizations must not be
-    # enabled for instrumentation tests, since they cause code required by the
-    # tests to be removed.
-    # TODO(smaier): when buildbots get updated to set this flag, change the
-    # default to is_official_build
-    enable_all_proguard_optimizations = false
-
     # Speed up incremental compiles by compiling only changed files.
     enable_incremental_javac = false
 
@@ -136,11 +129,6 @@ if (is_android) {
     use_order_profiling = false
   }
 
-  # Ensuring we never have a situation where we are asking to have debug java
-  # on alongside all ProGuard optimizations turned on, as these are mutually
-  # exclusive.
-  assert(!(is_java_debug && enable_all_proguard_optimizations))
-
   # Neither of these should ever be used for release builds since they are
   # somewhat experimental and dx --incremental is known to not produce
   # byte-for-byte identical output.

build/config/android/internal_rules.gni

@@ -867,7 +867,7 @@ if (enable_java_templates) {
         rebase_path(depfile, root_build_dir),
         "--output",
         rebase_path(java_script, root_build_dir),
-        "--classpath=@FileArg($_rebased_build_config:java:full_classpath)",
+        "--classpath=@FileArg($_rebased_build_config:deps_info:java:full_classpath)",
         "--jar-path",
         rebase_path(_jar_path, root_build_dir),
         "--main-class",

build/config/android/rules.gni

@@ -2290,6 +2290,15 @@ if (enable_java_templates) {
         data_deps += invoker.additional_apks
       }
 
+      if (defined(invoker.proguard_enabled) && invoker.proguard_enabled) {
+        # When ProGuard is on, we use ProGuard to combine the under test java
+        # code and the test java code. This is to allow us to apply all ProGuard
+        # optimizations that we ship with, but not have them break tests. The
+        # apk under test will still have the same resources, assets, and
+        # manifest, all of which are the ones used in the tests.
+        proguard_configs = [ "//testing/android/proguard_for_test.flags" ]
+      }
+
       create_dist_ijar = true
       if (defined(invoker.run_findbugs_override)) {
         # Only allow findbugs when there are java files.

chrome/android/BUILD.gn

@@ -571,27 +571,25 @@ jinja_template("chrome_sync_shell_test_apk_manifest") {
   variables = chrome_sync_shell_jinja_variables
 }
 
-if (!enable_all_proguard_optimizations) {
-  # GYP: //chrome/android/chrome_apk.gyp:chrome_public_test_apk
-  instrumentation_test_apk("chrome_public_test_apk") {
-    apk_name = "ChromePublicTest"
-    apk_under_test = ":chrome_public_apk"
-    android_manifest = chrome_public_test_apk_manifest
-    android_manifest_dep = ":chrome_public_test_apk_manifest"
+# GYP: //chrome/android/chrome_apk.gyp:chrome_public_test_apk
+instrumentation_test_apk("chrome_public_test_apk") {
+  apk_name = "ChromePublicTest"
+  apk_under_test = ":chrome_public_apk"
+  android_manifest = chrome_public_test_apk_manifest
+  android_manifest_dep = ":chrome_public_test_apk_manifest"
 
-    deps = [
-      ":chrome_test_java",
-      "//chrome/android/webapk/libs/runtime_library:runtime_library_javatests",
-      "//chrome/android/webapk/shell_apk:shell_apk_javatests",
-    ]
-    additional_apks = [
-      "//chrome/android/webapk/shell_apk/javatests/dex_optimizer:dex_optimizer_apk",
-      "//chrome/test/android/chrome_public_test_support:chrome_public_test_support_apk",
-      "//net/android:net_test_support_apk",
-    ]
-    isolate_file = "../chrome_public_test_apk.isolate"
-    proguard_enabled = !is_java_debug
-  }
+  deps = [
+    ":chrome_test_java",
+    "//chrome/android/webapk/libs/runtime_library:runtime_library_javatests",
+    "//chrome/android/webapk/shell_apk:shell_apk_javatests",
+  ]
+  additional_apks = [
+    "//chrome/android/webapk/shell_apk/javatests/dex_optimizer:dex_optimizer_apk",
+    "//chrome/test/android/chrome_public_test_support:chrome_public_test_support_apk",
+    "//net/android:net_test_support_apk",
+  ]
+  isolate_file = "../chrome_public_test_apk.isolate"
+  proguard_enabled = !is_java_debug
 }
 
 android_library("chrome_sync_shell_test_apk_java") {
@@ -618,16 +616,14 @@ android_library("chrome_sync_shell_test_apk_java") {
   ]
 }
 
-if (!enable_all_proguard_optimizations) {
-  # GYP: //chrome/android/chrome_apk.gyp:chrome_sync_shell_test_apk
-  instrumentation_test_apk("chrome_sync_shell_test_apk") {
-    apk_name = "ChromeSyncShellTest"
-    apk_under_test = ":chrome_sync_shell_apk"
-    android_manifest = chrome_sync_shell_test_apk_manifest
-    android_manifest_dep = ":chrome_sync_shell_test_apk_manifest"
-    deps = [
-      ":chrome_sync_shell_test_apk_java",
-    ]
-    proguard_enabled = !is_java_debug
-  }
+# GYP: //chrome/android/chrome_apk.gyp:chrome_sync_shell_test_apk
+instrumentation_test_apk("chrome_sync_shell_test_apk") {
+  apk_name = "ChromeSyncShellTest"
+  apk_under_test = ":chrome_sync_shell_apk"
+  android_manifest = chrome_sync_shell_test_apk_manifest
+  android_manifest_dep = ":chrome_sync_shell_test_apk_manifest"
+  deps = [
+    ":chrome_sync_shell_test_apk_java",
+  ]
+  proguard_enabled = !is_java_debug
 }

chrome/android/chrome_public_apk_tmpl.gni

@@ -53,9 +53,6 @@ template("chrome_public_apk_tmpl") {
                            "//chrome/android/java/proguard.flags",
                            "//base/android/base_proguard_config.flags",
                          ]
-      if (!enable_all_proguard_optimizations) {
-        proguard_configs += [ "//testing/android/proguard_for_test.flags" ]
-      }
     }
 
     if (!defined(use_chromium_linker)) {

chrome/android/webapk/libs/runtime_library/javatests/src/org/chromium/webapk/lib/runtime_library/WebApkServiceImplTest.java

@@ -12,8 +12,8 @@
 import android.content.pm.PackageManager;
 import android.os.IBinder;
 import android.test.InstrumentationTestCase;
+import android.test.suitebuilder.annotation.SmallTest;
 
-import org.chromium.base.test.util.DisabledTest;
 import org.chromium.chrome.test.webapk.TestWebApkServiceImplWrapper;
 import org.chromium.content.browser.test.util.CallbackHelper;
 
@@ -62,11 +62,8 @@ public void setUp() {
 
     /**
      * Test that an application which is not allowed to use the WebAPK service actually cannot.
-     *
-     * @SmallTest
-     * crbug.com/634390
      */
-    @DisabledTest
+    @SmallTest
     public void testApiFailsIfNoPermission() throws Exception {
         IWebApkApi api = bindService(mContext, mTargetUid + 1, SMALL_ICON_ID);
         try {
@@ -79,11 +76,8 @@ public void testApiFailsIfNoPermission() throws Exception {
 
     /**
      * Test that an application which is allowed to use the WebAPK service actually can.
-     *
-     * @SmallTest
-     * crbug.com/634390
      */
-    @DisabledTest
+    @SmallTest
     public void testApiWorksIfHasPermission() throws Exception {
         IWebApkApi api = bindService(mContext, mTargetUid, SMALL_ICON_ID);
         try {

testing/android/proguard_for_test.flags

@@ -2,104 +2,27 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-# This file is for proguard flags which will be applied to apks that are being
-# built to be instrumentation tested. The flags in this file should only affect
-# how the instrumentation tests interact with the .apk. Do not put any flags in
-# this file which might affect the correctness of the .apk.
+# This file is for proguard flags which are applied to the combined test and
+# tested code. Do not put any flags in this file which might affect the
+# correctness of the .apk we are testing, since it will apply to that .apk as
+# well.
 
-# This line prevents optmization from being applied to either packages. We rely
-# on optimizations being off to prevent inlining functions our instrumentation
-# tests use.
-# TODO(smaier): when buildbot scripts have been updated to not include this
-# config as part of its release build, switch these lines to:
-# -dontoptimize
--keepnames,allowobfuscation class com.google.android.apps.chrome.**,org.chromium.** {
-  *;
-}
-
-# Keeping @VisibleForTesting and its annotated classes + methods
--keep @interface org.chromium.base.VisibleForTesting
--keep @org.chromium.base.VisibleForTesting class **
--keepclasseswithmembers class * {
-  @org.chromium.base.VisibleForTesting <methods>;
-}
-
-# TODO(aurimas): figure out why we need to keep these classes.
--keep class org.chromium.base.test.** {
-  *;
-}
-
-# Everything below this is kept because they are referenced by the test APK.
--keep class android.support.v7.mediarouter.R* {
-  *;
-}
-
--keep class android.support.v7.media.MediaRouteProvider** {
-  *;
-}
-
--keep class android.support.v4.app.FragmentManager** {
-  *;
-}
-
--keep class android.support.v4.app.DialogFragment** {
-  *;
-}
-
--keep class android.support.v7.app.NotificationCompat** {
-  *;
-}
-
--keep class android.support.v7.app.AlertDialog** {
-  *;
-}
-
--keep class com.google.android.gms.cast.CastMediaControlIntent* {
-  *;
-}
-
--keepnames class com.google.android.gms.gcm.** {
-  *;
-}
-
--keepclassmembers class com.google.android.gms.gcm.TaskParams {
-  public <init>(java.lang.String);
-}
-
--keepnames class jp.tomorrowkey.android.gifplayer.** {
-  public *;
-}
-
-# Used in tests.
--keep class android.support.v4.view.ViewCompat {
-  public static int getLayoutDirection(android.view.View);
-}
-
-# flingViewport is used by Android WebView and a Chrome test.
--keepclassmembers class org.chromium.content.browser.ContentViewCore {
-  public void flingViewport(long, int, int);
-}
-
-# Needed to compile ChromeTest.apk
--keep class android.support.customtabs.** {
+# We want all tests to stick around.
+-keep class * extends junit.framework.TestCase {
   *;
 }
 
 # TODO(yfriedman): Remove when crbug.com/488192 is fixed.
 -dontwarn org.apache.http.conn.scheme.LayeredSocketFactory
 
-# Needed to run ChromeTest.apk
--keepnames class com.google.android.gms.common.GoogleApiAvailability {
-  *;
-}
+# We have some "library class WebView depends on program class SslCertificate"
+# warnings, and they don't affect us.
+-dontwarn android.webkit.WebView*
 
-# Needed for chrome_sync_shell_test_apk. Note - these do no affect chrome_apk's
-# size.
--keep class org.chromium.components.sync.protocol.* { *; }
-
-# These resources are referenced in tests, but not in the real application.
--keepclassmembers class org.chromium.chrome.R$id {
-  int webapp_splash_space;
-  int mr_chooser_list;
-  int find_toolbar;
+# We don't want BasicHttpParams or AbstractHttpParams to get renamed since that
+# then makes our calls to them use the implementation that we find in our .dex
+# file, which is broken. We need to rely on these calls resolving to the
+# system's implementation. See crbug.com/488192#c36.
+-keep class org.apache.** {
+  *;
 }