Android: Plumb enabled_proguard_obfuscation to L8

Makes the arg take effect more universally by having it be a flag on
proguard.py rather than work by having targets add the correct proguard
configs.

Also enabled -repackageclasses '' for L8, which means it should
produce a smaller .dex file.

Bug: 1168672
Change-Id: I9c07d337755efedbdf1fabbe8822a9fdfba49fa6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2641306
Commit-Queue: David Trainor <dtrainor@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845894}

android_webview/system_webview_apk_tmpl.gni

@@ -218,13 +218,6 @@ template("system_webview_apk_or_module_tmpl") {
         "//base/android/proguard/chromium_apk.flags",
         "//base/android/proguard/chromium_code.flags",
       ]
-      if (enable_proguard_obfuscation) {
-        proguard_configs +=
-            [ "//base/android/proguard/enable_obfuscation.flags" ]
-      } else {
-        proguard_configs +=
-            [ "//base/android/proguard/disable_all_obfuscation.flags" ]
-      }
       png_to_webp = true
     }
 

base/android/proguard/disable_all_obfuscation.flags

-# Copyright 2016 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.
-
-# Disables obfuscation while still allowing optimizations.
--keepnames,allowoptimization class *** {
-  *;
-}

base/android/proguard/enable_obfuscation.flags

-# Copyright 2016 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.
-
-# As of August 11, 2016, obfuscation was found to save 660kb on our .dex size
-# and 53kb memory/process (through shrinking method/string counts).
--repackageclasses ''

build/android/gyp/dex_jdk_libs.py

@@ -34,9 +34,14 @@ def _ParseArgs(args):
   return options
 
 
-def DexJdkLibJar(r8_path, min_api, desugar_jdk_libs_json, desugar_jdk_libs_jar,
-                 desugar_jdk_libs_configuration_jar, keep_rule_file, output,
-                 warnings_as_errors):
+def DexJdkLibJar(r8_path,
+                 min_api,
+                 desugar_jdk_libs_json,
+                 desugar_jdk_libs_jar,
+                 desugar_jdk_libs_configuration_jar,
+                 output,
+                 warnings_as_errors,
+                 config_paths=None):
   # TODO(agrieve): Spews a lot of stderr about missing classes.
   with build_utils.TempDir() as tmp_dir:
     cmd = build_utils.JavaCmd(warnings_as_errors) + [
@@ -53,8 +58,9 @@ def DexJdkLibJar(r8_path, min_api, desugar_jdk_libs_json, desugar_jdk_libs_jar,
 
     # If no desugaring is required, no keep rules are generated, and the keep
     # file will not be created.
-    if keep_rule_file is not None and os.path.exists(keep_rule_file):
-      cmd += ['--pg-conf', keep_rule_file]
+    if config_paths is not None:
+      for path in config_paths:
+        cmd += ['--pg-conf', path]
 
     cmd += [
         '--output', tmp_dir, desugar_jdk_libs_jar,
@@ -79,7 +85,7 @@ def main(args):
   options = _ParseArgs(args)
   DexJdkLibJar(options.r8_path, options.min_api, options.desugar_jdk_libs_json,
                options.desugar_jdk_libs_jar,
-               options.desugar_jdk_libs_configuration_jar, None, options.output,
+               options.desugar_jdk_libs_configuration_jar, options.output,
                options.warnings_as_errors)
 
 

build/android/gyp/proguard.py

@@ -77,6 +77,9 @@ def _ParseOptions():
       '- only works with R8.')
   parser.add_argument(
       '--min-api', help='Minimum Android API level compatibility.')
+  parser.add_argument('--enable-obfuscation',
+                      action='store_true',
+                      help='Minify symbol names')
   parser.add_argument(
       '--verbose', '-v', action='store_true', help='Print all ProGuard output')
   parser.add_argument(
@@ -210,10 +213,10 @@ def _OptimizeWithR8(options,
                     print_stdout=False):
   with build_utils.TempDir() as tmp_dir:
     if dynamic_config_data:
-      tmp_config_path = os.path.join(tmp_dir, 'proguard_config.txt')
-      with open(tmp_config_path, 'w') as f:
+      dynamic_config_path = os.path.join(tmp_dir, 'dynamic_config.flags')
+      with open(dynamic_config_path, 'w') as f:
         f.write(dynamic_config_data)
-      config_paths = config_paths + [tmp_config_path]
+      config_paths = config_paths + [dynamic_config_path]
 
     tmp_mapping_path = os.path.join(tmp_dir, 'mapping.txt')
     # If there is no output (no classes are kept), this prevents this script
@@ -338,12 +341,20 @@ def _OptimizeWithR8(options,
       existing_files = build_utils.FindInDirectory(base_dex_context.staging_dir)
       jdk_dex_output = os.path.join(base_dex_context.staging_dir,
                                     'classes%d.dex' % (len(existing_files) + 1))
+      # Use -applymapping to avoid name collisions.
+      l8_dynamic_config_path = os.path.join(tmp_dir, 'l8_dynamic_config.flags')
+      with open(l8_dynamic_config_path, 'w') as f:
+        f.write("-applymapping '{}'\n".format(tmp_mapping_path))
+      # Pass the dynamic config so that obfuscation options are picked up.
+      l8_config_paths = [dynamic_config_path, l8_dynamic_config_path]
+      if os.path.exists(options.desugared_library_keep_rule_output):
+        l8_config_paths.append(options.desugared_library_keep_rule_output)
+
       base_has_imported_lib = dex_jdk_libs.DexJdkLibJar(
           options.r8_path, options.min_api, options.desugar_jdk_libs_json,
           options.desugar_jdk_libs_jar,
-          options.desugar_jdk_libs_configuration_jar,
-          options.desugared_library_keep_rule_output, jdk_dex_output,
-          options.warnings_as_errors)
+          options.desugar_jdk_libs_configuration_jar, jdk_dex_output,
+          options.warnings_as_errors, l8_config_paths)
       if int(options.min_api) >= 24 and base_has_imported_lib:
         with open(jdk_dex_output, 'rb') as f:
           dexfile = dex_parser.DexFile(bytearray(f.read()))
@@ -591,10 +602,13 @@ def _CreateDynamicConfig(options):
     ret.append("-renamesourcefileattribute '%s' # OMIT FROM EXPECTATIONS" %
                options.sourcefile)
 
+  if options.enable_obfuscation:
+    ret.append("-repackageclasses ''")
+  else:
+    ret.append("-keepnames,allowoptimization class *** { *; }")
+
   if options.apply_mapping:
-    ret.append("-applymapping '%s'" % os.path.abspath(options.apply_mapping))
-  if options.repackage_classes:
-    ret.append("-repackageclasses '%s'" % options.repackage_classes)
+    ret.append("-applymapping '%s'" % options.apply_mapping)
 
   _min_api = int(options.min_api) if options.min_api else 0
   for api_level, version_code in _API_LEVEL_VERSION_CODE:

build/config/android/internal_rules.gni

@@ -1145,13 +1145,10 @@ if (enable_java_templates) {
     if (defined(invoker.inputs)) {
       _inputs += invoker.inputs
     }
-    if (defined(invoker.mapping_path)) {
-      _mapping_path = invoker.mapping_path
-    } else {
-      _mapping_path = "${invoker.output_path}.mapping"
-    }
     if (defined(invoker.proguard_mapping_path)) {
       _mapping_path = invoker.proguard_mapping_path
+    } else {
+      _mapping_path = "${invoker.output_path}.mapping"
     }
 
     _enable_jdk_library_desugaring = enable_jdk_library_desugaring
@@ -1159,10 +1156,6 @@ if (enable_java_templates) {
         !invoker.supports_jdk_library_desugaring) {
       _enable_jdk_library_desugaring = false
     }
-    _proguard_sourcefile_suffix = ""
-    if (defined(invoker.proguard_sourcefile_suffix)) {
-      _proguard_sourcefile_suffix = "-${invoker.proguard_sourcefile_suffix}"
-    }
 
     _rebased_build_config = rebase_path(invoker.build_config, root_build_dir)
     _args = [
@@ -1184,16 +1177,23 @@ if (enable_java_templates) {
       args += [ "--classpath=${_rebased_desugar_jars_paths}" ]
     }
 
-    if (enable_proguard_obfuscation) {
+    if ((!defined(invoker.proguard_enable_obfuscation) ||
+         invoker.proguard_enable_obfuscation) && enable_proguard_obfuscation) {
+      _proguard_sourcefile_suffix = ""
+      if (defined(invoker.proguard_sourcefile_suffix)) {
+        _proguard_sourcefile_suffix = "-${invoker.proguard_sourcefile_suffix}"
+      }
+
       # This is generally the apk name, and serves to identify the mapping
       # file that would be required to deobfuscate a stacktrace.
       _mapping_id = get_path_info(_mapping_path, "name")
       _args += [
+        "--enable-obfuscation",
         "--sourcefile",
         "chromium-${_mapping_id}${_proguard_sourcefile_suffix}",
       ]
-    } else {
-      not_needed([ "_proguard_sourcefile_suffix" ])
+    } else if (defined(invoker.proguard_sourcefile_suffix)) {
+      not_needed(invoker, [ "proguard_sourcefile_suffix" ])
     }
 
     if (defined(invoker.modules)) {
@@ -1482,6 +1482,7 @@ if (enable_java_templates) {
                                      "ignore_desugar_missing_deps",
                                      "is_static_library",
                                      "modules",
+                                     "proguard_enable_obfuscation",
                                      "proguard_mapping_path",
                                      "proguard_sourcefile_suffix",
                                      "supports_jdk_library_desugaring",
@@ -1516,13 +1517,6 @@ if (enable_java_templates) {
           args += invoker.proguard_args
         }
 
-        if (defined(invoker.repackage_classes)) {
-          args += [
-            "--repackage-classes",
-            "${invoker.repackage_classes}",
-          ]
-        }
-
         if (defined(invoker.apply_mapping)) {
           _rebased_apply_mapping_path =
               rebase_path(invoker.apply_mapping, root_build_dir)
@@ -1547,8 +1541,8 @@ if (enable_java_templates) {
 
         if (defined(invoker.output)) {
           output_path = invoker.output
-        } else {
-          mapping_path = "$target_out_dir/$target_name.mapping"
+        } else if (!defined(proguard_mapping_path)) {
+          proguard_mapping_path = "$target_out_dir/$target_name.mapping"
         }
       }
     } else {  # !_proguard_enabled
@@ -4115,7 +4109,11 @@ if (enable_java_templates) {
 
     if (_build_device_jar) {
       dex("${target_name}__dex") {
-        forward_variables_from(invoker, [ "desugar_jars_paths" ])
+        forward_variables_from(invoker,
+                               [
+                                 "desugar_jars_paths",
+                                 "proguard_enable_obfuscation",
+                               ])
         input_class_jars = [ _device_processed_jar_path ]
         enable_desugar = _enable_desugar
         ignore_desugar_missing_deps = !_enable_bytecode_checks

build/config/android/rules.gni

@@ -1644,6 +1644,7 @@ if (enable_java_templates) {
   #   output: Path to the output dex.
   #   proguard_enabled: Whether to enable R8.
   #   proguard_configs: List of proguard configs.
+  #   proguard_enable_obfuscation: Whether to enable obfuscation (default=true).
   #
   # Example
   #   dist_dex("lib_fatjar") {
@@ -1681,6 +1682,7 @@ if (enable_java_templates) {
                                    "data_deps",
                                    "proguard_configs",
                                    "proguard_enabled",
+                                   "proguard_enable_obfuscation",
                                    "min_sdk_version",
                                  ])
       deps = _deps
@@ -2917,6 +2919,7 @@ if (enable_java_templates) {
                                  "disable_r8_outlining",
                                  "dexlayout_profile",
                                  "enable_proguard_checks",
+                                 "proguard_enable_obfuscation",
                                ])
         min_sdk_version = _min_sdk_version
         proguard_enabled = _proguard_enabled
@@ -3516,6 +3519,7 @@ if (enable_java_templates) {
                                "product_version_resources_dep",
                                "proguard_configs",
                                "proguard_enabled",
+                               "proguard_enable_obfuscation",
                                "r_java_root_package_name",
                                "resource_exclusion_exceptions",
                                "resource_exclusion_regex",
@@ -3649,6 +3653,7 @@ if (enable_java_templates) {
                                "product_version_resources_dep",
                                "proguard_configs",
                                "proguard_enabled",
+                               "proguard_enable_obfuscation",
                                "resource_exclusion_exceptions",
                                "resource_exclusion_regex",
                                "resource_ids_provider_dep",
@@ -3878,6 +3883,7 @@ if (enable_java_templates) {
                                "modules",
                                "never_incremental",
                                "proguard_enabled",
+                               "proguard_enable_obfuscation",
                                "public_deps",
                                "use_webview_provider",
                              ])
@@ -4491,6 +4497,8 @@ if (enable_java_templates) {
   #      bundle. Default is to enable this only for release builds. Note that
   #      this will always perform synchronized proguarding.
   #
+  #    proguard_enable_obfuscation: Whether to enable obfuscation (default=true)
+  #
   #    enable_multidex: Optional. Enable multidexing of optimized modules jars
   #      when using synchronized proguarding. Only applies to base module.
   #
@@ -4750,6 +4758,7 @@ if (enable_java_templates) {
                                    "expected_proguard_config",
                                    "expected_proguard_config_base",
                                    "min_sdk_version",
+                                   "proguard_enable_obfuscation",
                                  ])
           if (defined(expected_proguard_config)) {
             top_target_name = _target_name

chrome/android/BUILD.gn

@@ -2731,10 +2731,6 @@ if (enable_arcore) {
     }
 
     additional_apks = [ "//net/android:net_test_support_apk" ]
-    proguard_enabled = !is_java_debug
-    if (proguard_enabled && !enable_proguard_obfuscation) {
-      proguard_configs = [ "//base/android/proguard/enable_obfuscation.flags" ]
-    }
   }
 }
 

chrome/android/chrome_public_apk_tmpl.gni

@@ -290,13 +290,6 @@ template("chrome_public_common_apk_or_module_tmpl") {
         "//base/android/proguard/chromium_apk.flags",
         "//base/android/proguard/chromium_code.flags",
       ]
-      if (enable_proguard_obfuscation) {
-        proguard_configs +=
-            [ "//base/android/proguard/enable_obfuscation.flags" ]
-      } else {
-        proguard_configs +=
-            [ "//base/android/proguard/disable_all_obfuscation.flags" ]
-      }
       if (_enable_chrome_module) {
         proguard_configs +=
             [ "//chrome/android/proguard/isolated_splits.flags" ]

chrome/android/expectations/monochrome_public_bundle.proguard_flags.expected

@@ -257,15 +257,6 @@
   @org.chromium.base.annotations.RemovableInRelease *;
 }
 
-# File: ../../base/android/proguard/enable_obfuscation.flags
-# Copyright 2016 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.
-
-# As of August 11, 2016, obfuscation was found to save 660kb on our .dex size
-# and 53kb memory/process (through shrinking method/string counts).
--repackageclasses ''
-
 # File: ../../chrome/android/proguard/isolated_splits.flags
 # Copyright 2020 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
@@ -863,6 +854,7 @@
 
 # File: //build/android/gyp/proguard.py (generated rules)
 # THIS LINE WAS OMITTED
+-repackageclasses ''
 -keep @interface org.chromium.base.annotations.VerifiesOnNMR1
 -if @org.chromium.base.annotations.VerifiesOnNMR1 class * {
     *** *(...);

chrome/android/trichrome.gni

@@ -199,8 +199,7 @@ template("trichrome_library_apk_tmpl") {
         # workaround for crbug.com/1012842. There were naming conflicts between
         # Library and Chrome, since each Proguard run doesn't know about the
         # other, and thus handed out the first names (a, b, c) to both.
-        proguard_configs +=
-            [ "//base/android/proguard/disable_all_obfuscation.flags" ]
+        proguard_enable_obfuscation = false
       }
     }
     deps += [ "//chrome/android:trichrome_dummy_resources" ]