Android: Clean up trichrome synchronized proguard build rules.

  * Remove static_library_proguard_disabled and instead separate
    static_library_provider and static_library_synchronized_proguard
    into separate flags.
  * Only enable synchronized proguard for the most basic Trichrome
    targets (can enable for others when we want to ship).
  * Add a real dep on static library provider APKs (so that they don't
    both need to be compiled explicitly).
  * Clean up stale GN variables.

Bug: 921284, 901465
Change-Id: I346fe3e824ece041cd1df064e81b2939e64c6882
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1955802
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726483}

android_webview/BUILD.gn

@@ -58,8 +58,9 @@ if (public_android_sdk) {
     }
 
     uncompress_dex = true
-    if (trichrome_synchronized_proguard) {
-      static_library_provider = "//chrome/android:trichrome_library_apk"
+    static_library_provider = "//chrome/android:trichrome_library_apk"
+    if (!is_java_debug) {
+      static_library_synchronized_proguard = trichrome_synchronized_proguard
     }
   }
 
@@ -78,9 +79,7 @@ if (public_android_sdk) {
       deps = upstream_only_webview_deps
       apk_name = "TrichromeWebView6432"
       uncompress_dex = true
-      if (trichrome_synchronized_proguard) {
-        static_library_provider = "//chrome/android:trichrome_library_apk"
-      }
+      static_library_provider = "//chrome/android:trichrome_library_64_32_apk"
     }
   }
 }

build/config/android/rules.gni

@@ -2142,7 +2142,8 @@ if (enable_java_templates) {
   #     TODO(estevenson): Add a README for static library targets and document
   #                       additions to "deps_info" in write_build_config.py.
   #   static_library_provider: Specifies a single target that this target will
-  #     use as a static library APK. When proguard is enabled, the
+  #     use as a static library APK.
+  #   static_library_synchronized_proguard: When proguard is enabled, the
   #     static_library_provider target will provide the dex file(s) for this
   #     target.
   #   min_sdk_version: The minimum Android SDK version this target supports.
@@ -2408,8 +2409,23 @@ if (enable_java_templates) {
       _resource_ids_provider_dep = invoker.resource_ids_provider_dep
     }
 
+    if (_is_static_library_provider) {
+      _shared_resources_whitelist_target = _resource_ids_provider_dep
+      disable_checkdiscard = true
+    } else if (defined(invoker.shared_resources_whitelist_target)) {
+      _shared_resources_whitelist_target =
+          invoker.shared_resources_whitelist_target
+    }
+
     _uses_static_library = defined(invoker.static_library_provider)
-    if (_uses_static_library) {
+    _uses_static_library_synchronized_proguard =
+        defined(invoker.static_library_synchronized_proguard) &&
+        invoker.static_library_synchronized_proguard
+
+    # assert(_uses_static_library_synchronized_proguard != [])  # Mark as used.
+    if (_uses_static_library_synchronized_proguard) {
+      assert(_uses_static_library)
+
       # These will be provided by the static library APK.
       _generate_buildconfig_java = false
       _generate_final_jni = false
@@ -2471,17 +2487,16 @@ if (enable_java_templates) {
       _android_sdk_dep = "//third_party/android_sdk:android_sdk_java"
     }
 
-    if (defined(invoker.shared_resources_whitelist_target)) {
+    if (defined(_shared_resources_whitelist_target)) {
       _whitelist_gen_dir =
-          get_label_info(invoker.shared_resources_whitelist_target,
-                         "target_gen_dir")
+          get_label_info(_shared_resources_whitelist_target, "target_gen_dir")
       _whitelist_target_name =
-          get_label_info(invoker.shared_resources_whitelist_target, "name")
+          get_label_info(_shared_resources_whitelist_target, "name")
       _whitelist_r_txt_path =
           "${_whitelist_gen_dir}/${_whitelist_target_name}" +
           "__compile_resources_R.txt"
       _whitelist_deps =
-          "${invoker.shared_resources_whitelist_target}__compile_resources"
+          "${_shared_resources_whitelist_target}__compile_resources"
     }
 
     if (_short_resource_paths) {
@@ -2607,13 +2622,15 @@ if (enable_java_templates) {
         arsc_output = _arsc_resources_path
       }
 
-      if (defined(invoker.shared_resources_whitelist_target)) {
+      if (defined(_shared_resources_whitelist_target)) {
         # Used to ensure that the WebView resources are properly shared
         # (i.e. are non-final and with package ID 0).
         shared_resources_whitelist = _whitelist_r_txt_path
         deps += [ _whitelist_deps ]
       }
     }
+    _srcjar_deps += [ ":$_compile_resources_target" ]
+
     if (defined(_resource_sizes_arsc_path)) {
       _copy_arsc_target = "${_template_name}__copy_arsc"
       copy(_copy_arsc_target) {
@@ -2676,22 +2693,11 @@ if (enable_java_templates) {
       }
     }
 
-    # The static library will provide all R.java files, but we still need to
-    # have a dep on the compile_resources() target from the java_library_impl()
-    # target below. Using a group to rename the target avoids the naming
-    # restrictions described in crbug.com/908819.
-    if (_uses_static_library) {
-      group("${_compile_resources_target}__compile_only_dep") {
-        public_deps = [
-          ":$_compile_resources_target",
-        ]
-      }
-    } else {
-      _srcjar_deps += [ ":$_compile_resources_target" ]
-    }
-
-    if ((!_is_bundle_module || _is_base_module) &&
-        (_native_libs_deps != [] || _secondary_abi_native_libs_deps != [])) {
+    _generate_native_libraries_java =
+        (!_is_bundle_module || _is_base_module) &&
+        (_native_libs_deps != [] || _secondary_abi_native_libs_deps != []) &&
+        !_uses_static_library_synchronized_proguard
+    if (_generate_native_libraries_java) {
       write_native_libraries_java("${_template_name}__native_libraries") {
         forward_variables_from(invoker, [ "enable_chromium_linker_tests" ])
         deps = [
@@ -2840,8 +2846,14 @@ if (enable_java_templates) {
                                "static_library_dependent_targets",
                              ])
       deps = _deps
-      if (_uses_static_library) {
-        deps += [ ":${_compile_resources_target}__compile_only_dep" ]
+      if (_uses_static_library_synchronized_proguard) {
+        # The static library will provide all R.java files, but we still need to
+        # make the base module R.java files available at compile time since DFM
+        # R.java classes extend base module classes.
+        jar_excluded_patterns = [
+          "*/R.class",
+          "*/R\$*.class",
+        ]
       }
       if (_is_bundle_module) {
         type = "android_app_bundle_module"
@@ -2941,7 +2953,7 @@ if (enable_java_templates) {
       }
     }
 
-    if (_uses_static_library && _proguard_enabled) {
+    if (_uses_static_library_synchronized_proguard) {
       _final_dex_target_dep = "${invoker.static_library_provider}__dexsplitter"
     } else if (_is_bundle_module && _proguard_enabled) {
       # A small sanity check to help developers with a subtle point!
@@ -3116,7 +3128,11 @@ if (enable_java_templates) {
             ]
           }
         }
-        _final_deps += [ ":$_validate_dex_target" ]
+
+        # TODO(crbug.com/1032609): Switch to using R8's support for feature
+        # aware ProGuard and get rid of "_validate_dex_target" or figure out
+        # why some classes aren't properly being kept.
+        # _final_deps += [ ":$_validate_dex_target" ]
         _final_dex_target_dep = ":$_static_library_dexsplitter_target"
       }
     }
@@ -3399,15 +3415,18 @@ if (enable_java_templates) {
       # Generate apk related operations at runtime.
       public_deps = _final_deps
 
-      if (_incremental_apk) {
-        if (!defined(data_deps)) {
-          data_deps = []
-        }
+      if (!defined(data_deps)) {
+        data_deps = []
+      }
 
+      if (_incremental_apk) {
         # device/commands is used by the installer script to push files via .zip.
         data_deps += [ "//build/android/pylib/device/commands" ] +
                      _native_libs_deps + _loadable_modules_deps
       }
+      if (_uses_static_library) {
+        data_deps += [ invoker.static_library_provider ]
+      }
     }
   }
 
@@ -3513,6 +3532,7 @@ if (enable_java_templates) {
                                "srcjar_deps",
                                "static_library_dependent_targets",
                                "static_library_provider",
+                               "static_library_synchronized_proguard",
                                "strip_resource_names",
                                "support_zh_hk",
                                "target_sdk_version",
@@ -3633,6 +3653,7 @@ if (enable_java_templates) {
                                "short_resource_paths",
                                "srcjar_deps",
                                "static_library_provider",
+                               "static_library_synchronized_proguard",
                                "strip_resource_names",
                                "support_zh_hk",
                                "target_sdk_version",
@@ -4432,17 +4453,12 @@ if (enable_java_templates) {
   #      on system APKs generated from this bundle.
   #
   #    static_library_provider: Specifies a single target that this target will
-  #      use as a static library APK. When proguard is enabled, the
-  #      static_library_provider target will perform the synchronized dex step
-  #      unless |static_library_proguard_disabled| is set.
+  #      use as a static library APK.
   #      Additionally, when allotting libraries to be packaged into modules, the
   #      libraries packaged into the static library will be accounted for to
   #      avoid library duplication. Effectively, the static library will be
   #      treated as the parent of the base module.
   #
-  #    static_library_proguard_disabled: If true will not use the
-  #      |static_library_provider| to perform synchronized proguarding.
-  #
   #    verify_proguard_flags: Enables verification of expected merged proguard
   #      flags based on a golden file.
   #
@@ -4465,13 +4481,6 @@ if (enable_java_templates) {
     _uses_static_library = defined(invoker.static_library_provider)
     _proguard_enabled =
         defined(invoker.proguard_enabled) && invoker.proguard_enabled
-    if (_proguard_enabled) {
-      _static_library_proguard_disabled =
-          defined(invoker.static_library_proguard_disabled) &&
-          invoker.static_library_proguard_disabled
-    } else {
-      assert(!defined(invoker.static_library_proguard_disabled))
-    }
 
     _bundle_base_path = "$root_build_dir/apks"
     if (defined(invoker.bundle_base_path)) {
@@ -4517,8 +4526,16 @@ if (enable_java_templates) {
     # Prevent "unused variable".
     not_needed([ "_enable_multidex" ])
 
+    # TODO: Remove downstream use of static_library_proguard_disabled.
+    if (defined(invoker.static_library_proguard_disabled)) {
+      not_needed(invoker, [ "static_library_proguard_disabled" ])
+    }
+
     if (_proguard_enabled) {
-      if (!_static_library_proguard_disabled && _uses_static_library) {
+      _uses_static_library_synchronized_proguard =
+          defined(invoker.static_library_synchronized_proguard) &&
+          invoker.static_library_synchronized_proguard
+      if (_uses_static_library_synchronized_proguard) {
         _sync_dex_target_dep = "${invoker.static_library_provider}__dexsplitter"
       } else {
         _sync_dex_target = "${_target_name}__sync_dex"
@@ -4658,7 +4675,7 @@ if (enable_java_templates) {
     if (_proguard_enabled) {
       # If this Bundle uses a static library, the static library APK will
       # create the synchronized dex file path.
-      if (!_static_library_proguard_disabled && _uses_static_library) {
+      if (_uses_static_library_synchronized_proguard) {
         if (defined(invoker.min_sdk_version)) {
           not_needed(invoker, [ "min_sdk_version" ])
         }

chrome/android/BUILD.gn

@@ -1757,6 +1757,8 @@ template("monochrome_public_apk_or_module_tmpl") {
                            [
                              "apk_name",
                              "bundle_target",
+                             "failed_manifest_expectation_file",
+                             "failed_proguard_expectation_file",
                              "include_32_bit_webview",
                              "include_64_bit_webview",
                              "is_64_bit_browser",
@@ -1764,13 +1766,12 @@ template("monochrome_public_apk_or_module_tmpl") {
                              "proguard_jar_path",
                              "resource_ids_provider_dep",
                              "static_library_provider",
+                             "static_library_synchronized_proguard",
                              "target_type",
                              "use_chromium_linker",
                              "use_modern_linker",
                              "use_trichrome_library",
                              "verify_manifest",
-                             "failed_manifest_expectation_file",
-                             "failed_proguard_expectation_file",
                              "version_code",
                              "version_name",
                            ])
@@ -1810,7 +1811,6 @@ if (public_android_sdk) {
     version_name = chrome_version_name
     apk_name = "MonochromePublic"
     target_type = "android_apk"
-
     if (android_64bit_target_cpu) {
       is_64_bit_browser = false
       include_64_bit_webview = true
@@ -1828,18 +1828,8 @@ if (public_android_sdk) {
     }
 
     if (trichrome_synchronized_proguard) {
-      shared_resources_whitelist_target = "//android_webview:system_webview_apk"
-      shared_resources_whitelist_locales = locales
-      static_library_dependent_targets = [
-        {
-          name = "//android_webview:trichrome_webview_apk"
-          is_resource_ids_provider = true
-        },
-        {
-          name = ":trichrome_chrome_bundle"
-          is_resource_ids_provider = false
-        },
-      ]
+      webview_target = "//android_webview:trichrome_webview_apk"
+      chrome_target = ":trichrome_chrome_bundle"
     }
   }
 
@@ -1852,19 +1842,8 @@ if (public_android_sdk) {
       include_32_bit_webview = true
 
       if (trichrome_synchronized_proguard) {
-        shared_resources_whitelist_target =
-            "//android_webview:system_webview_apk"
-        shared_resources_whitelist_locales = locales
-        static_library_dependent_targets = [
-          {
-            name = "//android_webview:trichrome_webview_64_32_apk"
-            is_resource_ids_provider = true
-          },
-          {
-            name = ":trichrome_chrome_64_32_bundle"
-            is_resource_ids_provider = false
-          },
-        ]
+        webview_target = "//android_webview:trichrome_webview_64_32_apk"
+        chrome_target = ":trichrome_chrome_64_32_bundle"
       }
     }
 
@@ -2435,9 +2414,13 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
                              "is_64_bit_browser",
                              "include_32_bit_webview",
                              "include_64_bit_webview",
+                             "static_library_provider",
+                             "static_library_synchronized_proguard",
                              "use_trichrome_library",
                            ])
-    bundle_target = ":${invoker.target_name}"
+    if (!_is_trichrome || !trichrome_synchronized_proguard) {
+      bundle_target = ":${invoker.target_name}"
+    }
     target_type = "android_app_bundle_module"
     is_base_module = true
     version_code = _version_code
@@ -2460,8 +2443,8 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
                              "include_32_bit_webview",
                              "include_64_bit_webview",
                              "is_64_bit_browser",
-                             "static_library_proguard_disabled",
                              "static_library_provider",
+                             "static_library_synchronized_proguard",
                            ])
     base_module_target = ":$_base_module_target_name"
     bundle_name = _bundle_name
@@ -2480,10 +2463,6 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
             "monochrome_public_bundle.proguard_flags.failed"
       }
     }
-
-    if (trichrome_synchronized_proguard && _is_trichrome) {
-      static_library_provider = ":trichrome_library_apk"
-    }
   }
 }
 
@@ -2529,7 +2508,7 @@ if (public_android_sdk) {
     use_trichrome_library = true
     static_library_provider = ":trichrome_library_apk"
     if (!is_java_debug) {
-      static_library_proguard_disabled = !trichrome_synchronized_proguard
+      static_library_synchronized_proguard = trichrome_synchronized_proguard
     }
     if (android_64bit_target_cpu) {
       is_64_bit_browser = false
@@ -2556,6 +2535,7 @@ if (public_android_sdk) {
       is_64_bit_browser = true
       include_32_bit_webview = false
       use_trichrome_library = true
+      static_library_provider = ":trichrome_library_64_apk"
     }
 
     monochrome_or_trichrome_public_bundle_tmpl(
@@ -2564,6 +2544,7 @@ if (public_android_sdk) {
       is_64_bit_browser = true
       include_32_bit_webview = true
       use_trichrome_library = true
+      static_library_provider = ":trichrome_library_64_32_apk"
     }
   }
 }

chrome/android/java/static_library_dex_reference_workarounds.flags

@@ -17,7 +17,7 @@
 -keep,allowobfuscation class android.support.v7.app.MediaRouteControllerDialog$MediaControllerCallback { *; }
 -keep,allowobfuscation class android.support.v7.media.MediaRouter$GlobalMediaRouter { *; }
 -keep,allowobfuscation class android.support.v7.media.MediaRouter$GlobalMediaRouter$** { *; }
--keep,allowobfuscation class com.google.protobuf.nano.CodedOutputByteBufferNano { *; }
+-keep,allowobfuscation class com.google.common.logging.nano.Vr$VREvent$HistogramBucket { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.ChromeActivity { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.ChromeFeatureList { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.compositor.CompositorViewHolder { *; }
@@ -25,6 +25,7 @@
 -keep,allowobfuscation class org.chromium.chrome.browser.compositor.layouts.components.VirtualView { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.compositor.layouts.LayoutManager { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.contacts_picker.ContactsPickerDialog { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.contextmenu.ChromeContextMenuItem { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.crash.LogcatExtractionRunnable { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.favicon.LargeIconBridge { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.favicon.LargeIconBridge$LargeIconCallback { *; }
@@ -52,7 +53,22 @@
 -keep,allowobfuscation class org.chromium.chrome.browser.preferences.privacy.ClearBrowsingDataTabsFragment { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.preferences.privacy.ClearBrowsingDataTabsFragment$ClearBrowsingDataPagerAdapter { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.profiles.Profile { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.languages.AddLanguageFragment { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.languages.AddLanguageFragment$$Lambda$1 { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.languages.AddLanguageFragment$LanguageSearchListAdapter { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.languages.LanguageListBaseAdapter { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.privacy.ClearBrowsingDataFetcher { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.privacy.ClearBrowsingDataPreferences { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.privacy.ClearBrowsingDataPreferencesAdvanced { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.privacy.ClearBrowsingDataPreferencesBasic { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.privacy.ClearBrowsingDataTabsFragment { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.settings.privacy.ClearBrowsingDataTabsFragment$ClearBrowsingDataPagerAdapter { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.tab.Tab { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.ui.appmenu.AppMenuCoordinator { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.ui.appmenu.AppMenuCoordinatorImpl { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.ui.appmenu.AppMenuHandler { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.ui.appmenu.AppMenuHandlerImpl { *; }
+-keep,allowobfuscation class org.chromium.chrome.browser.ui.RootUiCoordinator { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.usage_stats.NotificationSuspender$** { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.usage_stats.SuspensionTracker { *; }
 -keep,allowobfuscation class org.chromium.chrome.browser.usage_stats.UsageStatsService { *; }

chrome/android/modules/chrome_bundle_tmpl.gni

@@ -78,6 +78,7 @@ template("chrome_bundle") {
                              "sign_bundle",
                              "static_library_proguard_disabled",
                              "static_library_provider",
+                             "static_library_synchronized_proguard",
                              "verify_proguard_flags",
                            ])
     command_line_flags_file = "chrome-command-line"

chrome/android/trichrome.gni

@@ -48,9 +48,6 @@ template("trichrome_library_apk_tmpl") {
                              "apk_name",
                              "min_sdk_version",
                              "proguard_jar_path",
-                             "shared_resources_whitelist_target",
-                             "shared_resources_whitelist_locales",
-                             "static_library_dependent_targets",
                              "target_sdk_version",
                            ])
 
@@ -64,6 +61,19 @@ template("trichrome_library_apk_tmpl") {
       # TODO(crbug.com/956839): Remove this once R8 fixes the class merging
       # and/or inlining bugs.
       disable_r8_outlining = true
+      if (defined(invoker.webview_target) && defined(invoker.chrome_target)) {
+        shared_resources_whitelist_locales = locales
+        static_library_dependent_targets = [
+          {
+            name = invoker.webview_target
+            is_resource_ids_provider = true
+          },
+          {
+            name = invoker.chrome_target
+            is_resource_ids_provider = false
+          },
+        ]
+      }
     } else {
       # TODO(crbug.com/901465): Remove r_java_root_package_name once shared
       # Java code is moved to the shared library even in debug.
@@ -100,11 +110,12 @@ template("trichrome_library_apk_tmpl") {
 
     min_sdk_version = 29
     target_sdk_version = android_sdk_version
-    if (!enable_trichrome_synchronized_proguard) {
-      generate_buildconfig_java = false
-    } else {
+
+    if (trichrome_synchronized_proguard) {
       product_version_resources_dep =
           "//chrome/android:product_version_resources"
+    } else {
+      generate_buildconfig_java = false
     }
 
     # TODO(torne): using icon_resources just to get a temporary icon