Creating new gn arg: enable_all_proguard_optimizations.

build/config/android/rules.gni

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import("//build/config/android/config.gni")
 import("//build/config/android/internal_rules.gni")
 import("//build/toolchain/toolchain.gni")
 
 assert(is_android)
 
@@ skipping 1453 matching lines @@
   #       ":my_shared_lib",
   #     ]
   #   }
   template("android_apk") {
     set_sources_assignment_filter([])
     forward_variables_from(invoker, [ "testonly" ])
 
     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"
     } else {
       final_dex_path = "$gen_dir/classes.dex"
     }
@@ skipping 119 matching lines @@
     _create_density_splits =
         defined(invoker.create_density_splits) && invoker.create_density_splits
     _create_language_splits =
         defined(invoker.language_splits) && invoker.language_splits != []
 
     # Help GN understand that _create_abi_split is not unused (bug in GN).
     assert(_create_abi_split || true)
 
     _proguard_enabled =
         defined(invoker.proguard_enabled) && invoker.proguard_enabled
+    _is_testable_apk = _proguard_enabled && !defined(invoker.apk_under_test)

[Yaron, 2016/07/06 14:01:41]

Should at minimum have a comment but I feel like in general this is hard to grok
(or will be after the fact). Rather than assuming there will be other
differences, it might make it easier to understand if we just call this the
"unoptimized" apk.

[agrieve, 2016/07/06 14:18:49]

nit: non-proguarded apks are testable as well. How about "_create_apk_for_test"

This is probably also a good spot to add a comment saying why it's necessary to
do this.

[smaier, 2016/07/06 19:09:37]

Done.

     if (_proguard_enabled) {
-      _proguard_output_jar_path = "$base_path.proguard.jar"
+      _proguard_output_jar_path = "$_base_path.proguard.jar"
+    }
+    if (_is_testable_apk) {
+      _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"

[agrieve, 2016/07/06 14:18:49]

nit: prefix with _

[smaier, 2016/07/06 19:09:37]

Done.

+
+      assert(!defined(invoker.final_apk_path),
+             "Cannot set final_apk_path for tested apk")

[agrieve, 2016/07/06 14:18:49]

nit: this message misses the "why". How about:

"Setting final_apk_path for proguarded apks is not yet implemented."

[smaier, 2016/07/06 19:09:37]

Done.

+      _final_apk_for_test_path =
+          "$root_build_dir/apks/${invoker.apk_name}_for_test.apk"

[agrieve, 2016/07/06 14:18:49]

nit: apk_names are AllInCamelCase. So how about: ${invoker.apk_name}ForTest.apk

[smaier, 2016/07/06 19:09:37]

Done.

     }
 
     _emma_never_instrument = defined(invoker.testonly) && invoker.testonly
 
     build_config_target = "${_template_name}__build_config"
     write_build_config(build_config_target) {
       forward_variables_from(invoker, [ "apk_under_test" ])
       type = "android_apk"
       jar_path = _jar_path
       dex_path = final_dex_path
       apk_path = _final_apk_path
+      if (_is_testable_apk) {
+        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
       build_config = _build_config
       android_manifest = _android_manifest
 
       deps = _android_manifest_deps
 
       if (defined(invoker.deps)) {
         possible_config_deps = invoker.deps
@@ skipping 18 matching lines @@
       # build the native libraries just to create the .build_config file.
       # The dep is unnecessary since the runtime_deps file is created by gn gen
       # and the runtime_deps file is added to write_build_config.py's depfile.
       if (_native_libs_deps != []) {
         shared_libraries_runtime_deps_file = _runtime_deps_file
       }
     }
 
     _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,
                              [
                                "alternative_android_sdk_jar",
                                "android_aapt_path",
                                "app_as_shared_lib",
                                "include_all_resources",
                                "shared_resources",
                              ])
@@ skipping 203 matching lines @@
         sources = [
           "$_proguard_output_jar_path.mapping",
         ]
         outputs = [
           "$_final_apk_path.mapping",
         ]
         deps = [
           ":$_proguard_target",
         ]
       }
+
+      if (_is_testable_apk) {
+        _proguard_for_test_target = "${_template_name_for_test}__proguard"
+        proguard(_proguard_for_test_target) {
+          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 +=
+              [ "//chrome/android/java/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)",
+          ]
+        }
+        _dex_sources_for_test = [ _proguard_output_jar_for_test_path ]

[agrieve, 2016/07/06 14:18:49]

nit: these are both used only once. Would be better to inline them.

[smaier, 2016/07/06 19:09:37]

Done.

+        _dex_deps_for_test = [ ":$_proguard_for_test_target" ]
+      }
     } else {
       if (enable_multidex) {
         _dex_sources = [ _jar_path ]
       } else {
         _dex_sources = [ _lib_dex_path ]
       }
       _dex_deps = [ ":$java_target" ]
     }
 
     dex("$final_dex_target_name") {
       deps = _dex_deps + [ ":$build_config_target" ]
       inputs = [
         _build_config,
       ]
       sources = _dex_sources
       output = final_dex_path
 
       # All deps are already included in _dex_sources when proguard is used.
       if (!_proguard_enabled) {
         if (enable_multidex) {
           _dex_arg_key = "${_rebased_build_config}:dist_jar:dependency_jars"
         } else {
           _dex_arg_key =
               "${_rebased_build_config}:final_dex:dependency_dex_files"
         }
         args = [ "--inputs=@FileArg($_dex_arg_key)" ]
       }
     }
 
+    if (_is_testable_apk) {
+      final_dex_for_test_target_name = "${final_dex_target_name}_for_test"

[agrieve, 2016/07/06 14:18:49]

nit: prefix with _

[smaier, 2016/07/06 19:09:37]

Done.

+      dex("$final_dex_for_test_target_name") {
+        deps = _dex_deps_for_test + [ ":$build_config_target" ]
+        inputs = [
+          _build_config,
+        ]
+        sources = _dex_sources_for_test
+        output = final_dex_for_test_path
+      }
+    }
+
     _native_libs_file_arg_dep = ":$build_config_target"
     _native_libs_file_arg = "@FileArg($_rebased_build_config:native:libraries)"
 
     if (_native_libs_deps != [] && _enable_relocation_packing) {
       _prepare_native_target_name = "${_template_name}__prepare_native"
       _native_libs_dir = "$gen_dir/packed-libs"
       _native_libs_json = "$gen_dir/packed-libs/filelist.json"
       _rebased_native_libs_json = rebase_path(_native_libs_json, root_build_dir)
 
       _native_libs_file_arg_dep = ":$_prepare_native_target_name"
@@ skipping 73 matching lines @@
                                "public_deps",
                                "secondary_native_libs",
                                "shared_resources",
                                "uncompress_shared_libraries",
                                "write_asset_list",
                              ])
       if (!defined(deps)) {
         deps = []
       }
       apk_path = _final_apk_path
+      base_path = _base_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
@@ skipping 32 matching lines @@
         native_libs = _extra_native_libs
         native_libs_even_when_incremental =
             _extra_native_libs_even_when_incremental
       }
 
       # Placeholders necessary for some older devices.
       # http://crbug.com/395038
       forward_variables_from(invoker, [ "native_lib_placeholders" ])
     }
 
+    if (_is_testable_apk) {
+      # We are going to create a second apk, very similiar to the above

[Yaron, 2016/07/06 14:01:41]

seems like an awful lot of copy/paste :(
is using a new template unwieldy?

[smaier, 2016/07/06 19:09:37]

Done.

+      # 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_apk("${_template_name_for_test}") {

[agrieve, 2016/07/06 14:18:49]

We can eliminate this copy / paste using a nested template() I think (e.g.
define it at line 2055).

It would have to go something like:

template("create_main_apk") {
  _parent_invoker = invoker.invoker
  ...
}

create_main_apk(_template_name_for_test) {
  apk_path = ...
  base_path = ...
  dex_path = ...
  deps = ...
}

[smaier, 2016/07/06 19:09:37]

Done.

+        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 = []
+        }
+        apk_path = _final_apk_for_test_path
+        base_path = _base_path + "_for_test"
+        android_manifest = _android_manifest
+        assets_build_config = _build_config
+        resources_zip = _all_resources_zip_path
+        dex_path = final_dex_for_test_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_for_test_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
+        }
+
+        # Placeholders necessary for some older devices.
+        # http://crbug.com/395038
+        forward_variables_from(invoker, [ "native_lib_placeholders" ])

[agrieve, 2016/07/06 14:18:49]

nit: since you're here, can you put this in with the forward_variables_from()
all above, and move the note about http://crbug.com/395038 to appear in the
template argument comments on line 1429?

[smaier, 2016/07/06 19:09:37]

Done.

+      }
+    }
+
     if ((_native_libs_deps != [] ||
          _extra_native_libs_even_when_incremental != []) && _create_abi_split) {
       _manifest_rule =
           "${_template_name}__split_manifest_abi_${android_app_abi}"
       generate_split_manifest(_manifest_rule) {
         main_manifest = _android_manifest
         out_manifest =
             "$gen_dir/split-manifests/${android_app_abi}/AndroidManifest.xml"
         split_name = "abi_${android_app_abi}"
         deps = _android_manifest_deps
@@ skipping 228 matching lines @@
 
     group(target_name) {
       public_deps = [
         ":$_apk_target_name",
         ":$_test_runner_target_name",
 
         # Required by test runner to enumerate test list.
         ":${_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",
                                      [
                                        rebase_path(invoker.isolate_file),
                                        "--replace",
                                        "<(DEPTH)=/",
                                      ],
                                      "scope",
                                      [ invoker.isolate_file ])
@@ skipping 232 matching lines @@
     android_library(target_name) {
       chromium_code = false
       java_files = []
       srcjar_deps = [ ":${_template_name}__protoc_java" ]
       deps = [
         "//third_party/android_protobuf:protobuf_nano_javalib",
       ]
     }
   }
 }