Creating new gn arg: enable_all_proguard_optimizations.

build/config/android/rules.gni

Index: build/config/android/rules.gni
diff --git a/build/config/android/rules.gni b/build/config/android/rules.gni
index 381016414c43ff23d9eaaefea394b6e7802e436d..b93b5a1ea8eebad870718a036e9bdf1206b6432f 100644
--- a/build/config/android/rules.gni
+++ b/build/config/android/rules.gni
@@ -1426,8 +1426,9 @@ if (enable_java_templates) {
   #   shared_libraries: List shared_library targets to bundle. If these
   #     libraries depend on other shared_library targets, those dependencies will
   #     also be included in the apk (e.g. for is_component_build).
-  #   native_lib_placeholders: List of placeholder filenames to add to the apk
-  #     (optional).
+  #   native_lib_placeholders: List of placeholder filenames to add to the apk.
+  #     These placeholders are necessary for some older devices - see
+  #     http://crbug.com/395038.
   #   apk_under_test: For an instrumentation test apk, this is the target of the
   #     tested apk.
   #   include_all_resources - If true include all resource IDs in all generated
@@ -1471,23 +1472,23 @@ if (enable_java_templates) {
     assert(defined(invoker.final_apk_path) || defined(invoker.apk_name))
     assert(defined(invoker.android_manifest))
     gen_dir = "$target_gen_dir/$target_name"
-    base_path = "$gen_dir/$target_name"
+    _base_path = "$gen_dir/$target_name"
     _build_config = "$target_gen_dir/$target_name.build_config"
-    resources_zip_path = "$base_path.resources.zip"
-    _all_resources_zip_path = "$base_path.resources.all.zip"
-    _jar_path = "$base_path.jar"
-    _lib_dex_path = "$base_path.dex.jar"
+    resources_zip_path = "$_base_path.resources.zip"
+    _all_resources_zip_path = "$_base_path.resources.all.zip"
+    _jar_path = "$_base_path.jar"
+    _lib_dex_path = "$_base_path.dex.jar"
     _rebased_lib_dex_path = rebase_path(_lib_dex_path, root_build_dir)
     _template_name = target_name
 
     enable_multidex =
         defined(invoker.enable_multidex) && invoker.enable_multidex
     if (enable_multidex) {
-      final_dex_path = "$gen_dir/classes.dex.zip"
+      _final_dex_path = "$gen_dir/classes.dex.zip"
     } else {
-      final_dex_path = "$gen_dir/classes.dex"
+      _final_dex_path = "$gen_dir/classes.dex"
     }
-    final_dex_target_name = "${_template_name}__final_dex"
+    _final_dex_target_name = "${_template_name}__final_dex"
 
     _final_apk_path = ""
     if (defined(invoker.final_apk_path)) {
@@ -1517,6 +1518,7 @@ if (enable_java_templates) {
     if (defined(invoker.version_name)) {
       _version_name = invoker.version_name
     }
+
     _keystore_path = android_keystore_path
     _keystore_name = android_keystore_name
     _keystore_password = android_keystore_password
@@ -1616,8 +1618,26 @@ if (enable_java_templates) {
 
     _proguard_enabled =
         defined(invoker.proguard_enabled) && invoker.proguard_enabled
+
+    # We are creating an .apk just for instrumentation tests to run on.
+    # This allows us to turn on optimizations for the 'real' .apk, optimizations which wouldn't cause
+    # any real issues, but would break instrumentation tests due to inlining (and possibly other harmless optimizations).
+    # Thus, we make a copy of the 'real' apk and apply another proguard file to it.
+    _create_apk_for_test = _proguard_enabled && !defined(invoker.apk_under_test)
+
     if (_proguard_enabled) {
-      _proguard_output_jar_path = "$base_path.proguard.jar"
+      _proguard_output_jar_path = "$_base_path.proguard.jar"
+    }
+    if (_create_apk_for_test) {
+      _template_name_for_test = "${_template_name}_for_test"
+      _proguard_output_jar_for_test_path = "${_base_path}_for_test.proguard.jar"
+      _final_dex_for_test_path = "$gen_dir/classes_for_test.dex"
+
+      assert(
+          !defined(invoker.final_apk_path),
+          "Setting final_apk_path for proguarded .apks is not yet implemented.")
+      _final_apk_for_test_path =
+          "$root_build_dir/apks/${invoker.apk_name}ForTest.apk"
     }
 
     _emma_never_instrument = defined(invoker.testonly) && invoker.testonly
@@ -1627,8 +1647,11 @@ if (enable_java_templates) {
       forward_variables_from(invoker, [ "apk_under_test" ])
       type = "android_apk"
       jar_path = _jar_path
-      dex_path = final_dex_path
+      dex_path = _final_dex_path
       apk_path = _final_apk_path
+      if (_create_apk_for_test) {
+        apk_for_test_path = _final_apk_for_test_path
+      }
       incremental_apk_path = "${_final_apk_path_no_ext}_incremental.apk"
       incremental_install_script_path = _incremental_install_script_path
       resources_zip = resources_zip_path
@@ -1667,7 +1690,7 @@ if (enable_java_templates) {
 
     _final_deps = []
 
-    _generated_proguard_config = "$base_path.resources.proguard.txt"
+    _generated_proguard_config = "$_base_path.resources.proguard.txt"
     process_resources_target = "${_template_name}__process_resources"
     process_resources(process_resources_target) {
       forward_variables_from(invoker,
@@ -1891,6 +1914,38 @@ if (enable_java_templates) {
           ":$_proguard_target",
         ]
       }
+
+      if (_create_apk_for_test) {
+        _proguard_for_test_target = "${_template_name_for_test}__proguard"
+        proguard(_proguard_for_test_target) {

[agrieve, 2016/07/07 01:32:12]

I liked how the create_apk() template turned out. Can you do the same for
proguard()? e.g.

proguard_helper(_proguard_for_test_target) {
  output_jar_path = ...
  proguard_configs = _proguard_configs + [
"//testing/android/proguard_for_test.flags" ]
}

[smaier, 2016/07/07 15:05:05]

Done.

+          forward_variables_from(invoker,
+                                 [
+                                   "alternative_android_sdk_jar",
+                                   "proguard_jar_path",
+                                 ])
+          deps = [
+            ":$build_config_target",
+            ":$java_target",
+            ":$process_resources_target",
+          ]
+          inputs = [
+                     _build_config,
+                     _jar_path,
+                   ] + _proguard_configs
+
+          output_jar_path = _proguard_output_jar_for_test_path
+
+          # This Proguard flags file will apply only to tested apks - true
+          # release apks are not affected.
+          _proguard_configs += [ "//testing/android/proguard_for_test.flags" ]
+          _rebased_proguard_configs =
+              rebase_path(_proguard_configs, root_build_dir)
+          args = [
+            "--proguard-configs=$_rebased_proguard_configs",
+            "--input-paths=@FileArg($_rebased_build_config:proguard:input_paths)",
+          ]
+        }
+      }
     } else {
       if (enable_multidex) {
         _dex_sources = [ _jar_path ]
@@ -1900,13 +1955,13 @@ if (enable_java_templates) {
       _dex_deps = [ ":$java_target" ]
     }
 
-    dex("$final_dex_target_name") {
+    dex("$_final_dex_target_name") {
       deps = _dex_deps + [ ":$build_config_target" ]
       inputs = [
         _build_config,
       ]
       sources = _dex_sources
-      output = final_dex_path
+      output = _final_dex_path
 
       # All deps are already included in _dex_sources when proguard is used.
       if (!_proguard_enabled) {
@@ -1920,6 +1975,23 @@ if (enable_java_templates) {
       }
     }
 
+    if (_create_apk_for_test) {
+      _final_dex_for_test_target_name = "${_final_dex_target_name}_for_test"
+      dex("$_final_dex_for_test_target_name") {
+        deps = [
+          ":$_proguard_for_test_target",
+          ":$build_config_target",
+        ]
+        inputs = [
+          _build_config,
+        ]
+        sources = [
+          _proguard_output_jar_for_test_path,
+        ]
+        output = _final_dex_for_test_path
+      }
+    }
+
     _native_libs_file_arg_dep = ":$build_config_target"
     _native_libs_file_arg = "@FileArg($_rebased_build_config:native:libraries)"
 
@@ -1991,78 +2063,120 @@ if (enable_java_templates) {
     if (defined(invoker.loadable_modules) && invoker.loadable_modules != []) {
       _extra_native_libs_even_when_incremental += invoker.loadable_modules
     }
+    assert(_extra_native_libs_even_when_incremental == [] ||

[agrieve, 2016/07/07 01:32:12]

nit: move below comment.

[smaier, 2016/07/07 15:05:05]

Done.

+           _extra_native_libs_even_when_incremental != [])  # Mark as used.
+
+    # These assertions are to mark variables as used that will be used by create_main_apk,
+    # since GN doesn't recognize they are being used. See crbug.com/395883.
+    assert(!defined(invoker.extensions_to_not_compress) ||
+           invoker.extensions_to_not_compress != "")  # Mark as used.
+    assert(!defined(invoker.uncompress_shared_libraries) ||
+           invoker.uncompress_shared_libraries == true ||
+           invoker.uncompress_shared_libraries == false)  # Mark as used.
+    assert(!defined(invoker.page_align_shared_libraries) ||
+           invoker.page_align_shared_libraries == true ||
+           invoker.page_align_shared_libraries == false)  # Mark as used.
+    assert(_version_code != "")  # Mark as used.
+    assert(_version_code != "")  # Mark as used.
+    assert(_version_name != "")  # Mark as used.
+    assert(_keystore_path != "")  # Mark as used.
+    assert(_keystore_name != "")  # Mark as used.
+    assert(_keystore_password != "")  # Mark as used.
+    template("create_main_apk") {
+      create_apk(target_name) {
+        _parent_invoker = invoker.invoker

[jbudorick, 2016/07/07 01:48:52]

:| I'm not crazy about the double invoker thing -- build/config/android appears
to be the only place in which it's used at all. Can we instead forward variables
down normally?

[agrieve, 2016/07/07 13:37:57]

Hmm, I suggested using invoker.invoker and think it's not the worst since it
still refers to the main template's invoker. But, looking at this again, I
wonder if we could get rid a lot of the Mark as used asserts by doing:

_create_apk_vars = [
  "alternative_android_sdk_jar",
  "android_aapt_path",
  "app_as_shared_lib",
  "deps",
  "extensions_to_not_compress",
  "language_splits",
  "native_lib_placeholders",
  "page_align_shared_libraries",
  "public_deps",
  "secondary_native_libs",
  "shared_resources",
  "uncompress_shared_libraries",
  "write_asset_list",
]

Then in create_main_apk:
forward_variables_from(invoker, _create_apk_vars + [ ... ])

Then when calling it:
create_main_apk("${_template_name}__create") {
  forward_variables_from(invoker, _create_apk_vars)
}

[smaier, 2016/07/07 15:05:04]

Done.

+        forward_variables_from(invoker,
+                               [
+                                 "apk_path",
+                                 "base_path",
+                                 "dex_path",
+                               ])
 
-    _final_deps += [ ":${_template_name}__create" ]
-    create_apk("${_template_name}__create") {
-      forward_variables_from(invoker,
-                             [
-                               "alternative_android_sdk_jar",
-                               "android_aapt_path",
-                               "app_as_shared_lib",
-                               "deps",
-                               "extensions_to_not_compress",
-                               "language_splits",
-                               "page_align_shared_libraries",
-                               "public_deps",
-                               "secondary_native_libs",
-                               "shared_resources",
-                               "uncompress_shared_libraries",
-                               "write_asset_list",
-                             ])
-      if (!defined(deps)) {
-        deps = []
+        forward_variables_from(_parent_invoker,
+                               [
+                                 "alternative_android_sdk_jar",
+                                 "android_aapt_path",
+                                 "app_as_shared_lib",
+                                 "deps",
+                                 "extensions_to_not_compress",
+                                 "language_splits",
+                                 "native_lib_placeholders",
+                                 "page_align_shared_libraries",
+                                 "public_deps",
+                                 "secondary_native_libs",
+                                 "shared_resources",
+                                 "uncompress_shared_libraries",
+                                 "write_asset_list",
+                               ])
+        if (!defined(deps)) {
+          deps = []
+        }
+
+        # This target generates the input file _all_resources_zip_path.
+        deps += _android_manifest_deps + [
+                  ":$build_config_target",
+                  ":$process_resources_target",
+                  invoker.dex_target,
+                ]
+        android_manifest = _android_manifest
+        assets_build_config = _build_config
+        resources_zip = _all_resources_zip_path
+        load_library_from_apk = _load_library_from_apk
+        create_density_splits = _create_density_splits
+        emma_instrument = emma_coverage && !_emma_never_instrument
+
+        if (!defined(extensions_to_not_compress)) {
+          # Allow icu data, v8 snapshots, and pak files to be loaded directly from
+          # the .apk.
+          # Note: These are actually suffix matches, not necessarily extensions.
+          extensions_to_not_compress = ".dat,.bin,.pak"
+        }
+
+        version_code = _version_code
+        version_name = _version_name
+
+        keystore_name = _keystore_name
+        keystore_path = _keystore_path
+        keystore_password = _keystore_password
+
+        # Incremental apk does not use native libs nor final dex.
+        incremental_deps = deps + _android_manifest_deps + [
+                             ":$build_config_target",
+                             ":$process_resources_target",
+                           ]
+        if ((_native_libs_deps != [] ||
+             _extra_native_libs_even_when_incremental != []) &&
+            !_create_abi_split) {
+          deps += _native_libs_deps + _extra_native_libs_deps +
+                  _extra_native_libs_even_when_incremental_deps +
+                  [ _native_libs_file_arg_dep ]
+          native_libs_filearg = _native_libs_file_arg
+          native_libs = _extra_native_libs
+          native_libs_even_when_incremental =
+              _extra_native_libs_even_when_incremental
+        }
       }
+    }
+
+    _final_deps += [ ":${_template_name}__create" ]
+    create_main_apk("${_template_name}__create") {
       apk_path = _final_apk_path
-      android_manifest = _android_manifest
-      assets_build_config = _build_config
-      resources_zip = _all_resources_zip_path
-      dex_path = final_dex_path
-      load_library_from_apk = _load_library_from_apk
-      create_density_splits = _create_density_splits
-      emma_instrument = emma_coverage && !_emma_never_instrument
-
-      if (!defined(extensions_to_not_compress)) {
-        # Allow icu data, v8 snapshots, and pak files to be loaded directly from
-        # the .apk.
-        # Note: These are actually suffix matches, not necessarily extensions.
-        extensions_to_not_compress = ".dat,.bin,.pak"
-      }
-
-      version_code = _version_code
-      version_name = _version_name
-
-      keystore_name = _keystore_name
-      keystore_path = _keystore_path
-      keystore_password = _keystore_password
-
-      # Incremental apk does not use native libs nor final dex.
-      incremental_deps = deps + _android_manifest_deps + [
-                           ":$build_config_target",
-                           ":$process_resources_target",
-                         ]
-
-      # This target generates the input file _all_resources_zip_path.
-      deps += _android_manifest_deps + [
-                ":$build_config_target",
-                ":$process_resources_target",
-                ":$final_dex_target_name",
-              ]
-
-      if ((_native_libs_deps != [] ||
-           _extra_native_libs_even_when_incremental != []) &&
-          !_create_abi_split) {
-        deps += _native_libs_deps + _extra_native_libs_deps +
-                _extra_native_libs_even_when_incremental_deps +
-                [ _native_libs_file_arg_dep ]
-        native_libs_filearg = _native_libs_file_arg
-        native_libs = _extra_native_libs
-        native_libs_even_when_incremental =
-            _extra_native_libs_even_when_incremental
-      }
+      base_path = _base_path
+      dex_path = _final_dex_path
+      dex_target = ":$_final_dex_target_name"
+    }
 
-      # Placeholders necessary for some older devices.
-      # http://crbug.com/395038
-      forward_variables_from(invoker, [ "native_lib_placeholders" ])
+    if (_create_apk_for_test) {
+      # We are going to create a second apk, very similiar to the above
+      # create_apk("${_template_name}__create"), but with this one being an
+      # externally visible rule which will build an .apk with the additional
+      # test-only proguard flags.
+      create_main_apk("${_template_name_for_test}") {
+        apk_path = _final_apk_for_test_path
+        base_path = _base_path + "_for_test"
+        dex_path = _final_dex_for_test_path
+        dex_target = ":$_final_dex_for_test_target_name"
+      }
     }
 
     if ((_native_libs_deps != [] ||
@@ -2313,7 +2427,11 @@ if (enable_java_templates) {
         ":${_apk_target_name}_dist_ijar",
       ]
       if (defined(invoker.apk_under_test)) {
-        public_deps += [ invoker.apk_under_test ]
+        if (defined(invoker.proguard_enabled) && invoker.proguard_enabled) {
+          public_deps += [ "${invoker.apk_under_test}_for_test" ]
+        } else {
+          public_deps += [ invoker.apk_under_test ]
+        }
       }
       if (defined(invoker.isolate_file)) {
         isolate_values = exec_script("//build/gypi_to_gn.py",