Reland of Deduplicate Monochrome locale .paks

Instead of using system webview's resource whitelist, now uses a
generated list of resource IDs that are actually packed into
Webview's locale paks. This fixes the missing strings issue.

Original issue:
https://codereview.chromium.org/2980773002/

TBR=agrieve@chromium.org,dpranke@chromium.org,thestig@chromium.org,sadrul@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days
ago.
BUG=724110, 742388

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

chrome/android/BUILD.gn

@@ -41,6 +41,10 @@ app_hooks_impl = "java/src/org/chromium/chrome/browser/AppHooksImpl.java"
 if (enable_resource_whitelist_generation) {
   monochrome_resource_whitelist =
       "$target_gen_dir/monochrome_resource_whitelist.txt"
+  system_webview_locale_resource_id_list =
+      "$target_gen_dir/system_webview_locale_resource_id_list.txt"
+  monochrome_locale_whitelist =
+      "$target_gen_dir/monochrome_locale_whitelist.txt"
 }
 
 jinja_template("chrome_public_android_manifest") {
@@ -728,9 +732,66 @@ if (current_toolchain == default_toolchain) {
               "/libmonochrome$shlib_extension.whitelist"
       output = monochrome_resource_whitelist
     }
+
+    # Use custom resource ID list instead of android_webview's compiler
+    # resource whitelist because //android_webview: generate_webui_resources
+    # and //android_webview: generate_components_resources use hand-written
+    # resource whitelists.
+    action("system_webview_locale_resource_id_list") {
+      script = "//tools/grit/pak_util.py"
+
+      _system_webview_en_US_locale_pak =
+          "$root_out_dir/android_webview/locales/en-US.pak"
+
+      inputs = [
+        _system_webview_en_US_locale_pak,
+      ]
+
+      outputs = [
+        system_webview_locale_resource_id_list,
+      ]
+
+      deps = [
+        "//android_webview:repack_locales",
+      ]
+
+      args = [
+        "list-id",
+        "--output",
+        rebase_path(system_webview_locale_resource_id_list, root_build_dir),
+        rebase_path(_system_webview_en_US_locale_pak, root_build_dir),
+      ]
+    }
+
+    action("monochrome_locale_whitelist") {
+      script = "//tools/resources/filter_resource_whitelist.py"
+
+      inputs = [
+        monochrome_resource_whitelist,
+        system_webview_locale_resource_id_list,
+      ]
+
+      outputs = [
+        monochrome_locale_whitelist,
+      ]
+
+      deps = [
+        ":monochrome_resource_whitelist",
+        ":system_webview_locale_resource_id_list",
+        "//android_webview:system_webview_pak_whitelist",
+      ]
+
+      args = [
+        "--input",
+        rebase_path(monochrome_resource_whitelist, root_build_dir),
+        "--filter",
+        rebase_path(system_webview_locale_resource_id_list, root_build_dir),
+        "--output",
+        rebase_path(monochrome_locale_whitelist, root_build_dir),
+      ]
+    } 
   }
 
-  # This target does not output locale paks.
   chrome_paks("monochrome_paks") {
     output_dir = "$target_gen_dir/$target_name"
 
@@ -739,12 +800,25 @@ if (current_toolchain == default_toolchain) {
       "//android_webview:generate_aw_resources",
     ]
 
-    exclude_locale_paks = true
-
     if (enable_resource_whitelist_generation) {
       repack_whitelist = monochrome_resource_whitelist
       deps += [ ":monochrome_resource_whitelist" ]
+      locale_whitelist = monochrome_locale_whitelist
+      deps += [ ":monochrome_locale_whitelist" ]
+    }
+  }
+
+  # This target is separate from monochrome_pak_assets because it does not
+  # disable compression.
+  android_assets("monochrome_locale_pak_assets") {
+    sources = []
+    foreach(_locale, locales - android_chrome_omitted_locales) {
+      sources += [ "$target_gen_dir/monochrome_paks/locales/$_locale.pak" ]
     }
+
+    deps = [
+      ":monochrome_paks",
+    ]
   }
 
   # This target explicitly includes locale paks via deps.
@@ -756,7 +830,7 @@ if (current_toolchain == default_toolchain) {
     disable_compression = true
 
     deps = [
-      ":chrome_public_locale_pak_assets",
+      ":monochrome_locale_pak_assets",
       ":monochrome_paks",
       "//android_webview:locale_pak_assets",
     ]

chrome/app/chrome_main_delegate.cc

@@ -877,6 +877,16 @@ void ChromeMainDelegate::PreSandboxStartup() {
     ResourceBundle::InitSharedInstanceWithPakFileRegion(base::File(pak_fd),
                                                         pak_region);
 
+    // Load secondary locale .pak file if it exists.
+    pak_fd = global_descriptors->MaybeGet(kAndroidSecondaryLocalePakDescriptor);
+    if (pak_fd != -1) {
+      pak_region = global_descriptors->GetRegion(
+          kAndroidSecondaryLocalePakDescriptor);
+      ResourceBundle::GetSharedInstance().
+          LoadSecondaryLocaleDataWithPakFileRegion(
+              base::File(pak_fd), pak_region);
+    }
+
     int extra_pak_keys[] = {
       kAndroidChrome100PercentPakDescriptor,
       kAndroidUIResourcesPakDescriptor,

chrome/browser/chrome_browser_main_android.cc

@@ -80,6 +80,10 @@ int ChromeBrowserMainPartsAndroid::PreCreateThreads() {
             crash_dump_dir, kAndroidMinidumpDescriptor));
   }
 
+  // Auto-detect based on en-US whether secondary locale .pak files exist.
+  ui::SetLoadSecondaryLocalePaks(
+      !ui::GetPathForAndroidLocalePakWithinApk("en-US").empty());
+
   return ChromeBrowserMainParts::PreCreateThreads();
 }
 

chrome/browser/chrome_content_browser_client.cc

@@ -2727,6 +2727,12 @@ void ChromeContentBrowserClient::GetAdditionalMappedFilesForChildProcess(
   fd = ui::GetLocalePackFd(&region);
   mappings->ShareWithRegion(kAndroidLocalePakDescriptor, fd, region);
 
+  // Optional secondary locale .pak file.
+  fd = ui::GetSecondaryLocalePackFd(&region);
+  if (fd != -1) {
+    mappings->ShareWithRegion(kAndroidSecondaryLocalePakDescriptor, fd, region);
+  }
+
   breakpad::CrashDumpObserver::GetInstance()->BrowserChildProcessStarted(
       child_process_id, mappings);
 

chrome/chrome_paks.gni

@@ -184,7 +184,7 @@ template("chrome_extra_paks") {
 #   output_dir [required]: Directory to output .pak files. Locale .pak files
 #       will always be place in $output_dir/locales
 #   additional_extra_paks: List of extra .pak sources for resources.pak.
-#   exclude_locale_paks: if set to true, skip chrome_repack_locales.
+#   locale_whitelist: if set, override repack_whitelist for locale .pak files.
 #   copy_data_to_bundle:
 #   deps:
 #   output_dir:
@@ -234,24 +234,26 @@ template("chrome_paks") {
     }
   }
 
-  if (!defined(invoker.exclude_locale_paks) || !invoker.exclude_locale_paks) {
-    chrome_repack_locales("${target_name}_locales") {
-      forward_variables_from(invoker,
-                             [
-                               "copy_data_to_bundle",
-                               "deps",
-                               "repack_whitelist",
-                               "visibility",
-                             ])
+  chrome_repack_locales("${target_name}_locales") {
+    forward_variables_from(invoker,
+                           [
+                             "copy_data_to_bundle",
+                             "deps",
+                             "visibility",
+                           ])
+    if (defined(invoker.locale_whitelist)) {
+      repack_whitelist = invoker.locale_whitelist
+    } else if (defined(invoker.repack_whitelist)) {
+      repack_whitelist = invoker.repack_whitelist
+    }
 
-      input_locales = locales
-      output_dir = "${invoker.output_dir}/locales"
+    input_locales = locales
+    output_dir = "${invoker.output_dir}/locales"
 
-      if (is_mac) {
-        output_locales = locales_as_mac_outputs
-      } else {
-        output_locales = locales
-      }
+    if (is_mac) {
+      output_locales = locales_as_mac_outputs
+    } else {
+      output_locales = locales
     }
   }
 
@@ -260,10 +262,8 @@ template("chrome_paks") {
     public_deps = [
       ":${target_name}_100_percent",
       ":${target_name}_extra",
+      ":${target_name}_locales",
     ]
-    if (!defined(invoker.exclude_locale_paks) || !invoker.exclude_locale_paks) {
-      public_deps += [ ":${target_name}_locales" ]
-    }
     if (enable_hidpi) {
       public_deps += [ ":${target_name}_200_percent" ]
     }

chrome/common/descriptors_android.h

@@ -11,6 +11,7 @@
 enum {
 #if defined(OS_ANDROID)
   kAndroidLocalePakDescriptor = kContentIPCDescriptorMax + 1,
+  kAndroidSecondaryLocalePakDescriptor,
   kAndroidChrome100PercentPakDescriptor,
   kAndroidUIResourcesPakDescriptor,
   kAndroidMinidumpDescriptor,

tools/grit/pak_util.py

@@ -62,6 +62,12 @@ def _PrintMain(args):
     print line.encode('utf-8')
 
 
+def _ListMain(args):
+  resources, _ = data_pack.ReadDataPack(args.pak_file)
+  for resource_id in sorted(resources.keys()):
+    args.output.write('%d\n' % resource_id)
+
+
 def main():
   parser = argparse.ArgumentParser(
       description=__doc__, formatter_class=argparse.RawTextHelpFormatter)
@@ -88,6 +94,14 @@ def main():
       help='Prints all pak IDs and contents. Useful for diffing.')
   sub_parser.add_argument('pak_file')
   sub_parser.set_defaults(func=_PrintMain)
+
+  sub_parser = sub_parsers.add_parser('list-id',
+      help='Outputs all resource IDs to a file.')
+  sub_parser.add_argument('pak_file')
+  sub_parser.add_argument('--output', type=argparse.FileType('w'),
+      default=sys.stdout,
+      help='The resource list path to write (default stdout)')
+  sub_parser.set_defaults(func=_ListMain)
 
   if len(sys.argv) == 1:
     parser.print_help()

tools/resources/OWNERS

@@ -1,2 +1,4 @@
 per-file generate_resource_whitelist.*=agrieve@chromium.org
 per-file generate_resource_whitelist.*=estevenson@chromium.org
+per-file filter_resource_whitelist.*=agrieve@chromium.org
+per-file filter_resource_whitelist.*=zpeng@chromium.org

tools/resources/filter_resource_whitelist.py

@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+# Copyright 2017 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.
+
+"""filter_resource_whitelist.py [-h] [--input INPUT] [--filter FILTER]
+[--output OUTPUT]
+
+INPUT specifies a resource whitelist file containing resource IDs that should
+be whitelisted, where each line of INPUT contains a single resource ID.
+
+FILTER specifies a resource whitelist file containing resource IDs that should
+not be whitelisted, where each line of FILTER contains a single resource ID.
+
+Filters a resource whitelist by removing resource IDs that are contained in a
+another resource whitelist.
+
+This script is used to generate Monochrome's locale paks.
+"""
+
+import argparse
+import sys
+
+
+def main():
+  parser = argparse.ArgumentParser(usage=__doc__)
+  parser.add_argument(
+      '--input', type=argparse.FileType('r'), required=True,
+      help='A resource whitelist where each line contains one resource ID. '
+           'These IDs, excluding the ones in FILTER, are to be included.')
+  parser.add_argument(
+      '--filter', type=argparse.FileType('r'), required=True,
+      help='A resource whitelist where each line contains one resource ID. '
+           'These IDs are to be excluded.')
+  parser.add_argument(
+      '--output', type=argparse.FileType('w'), default=sys.stdout,
+      help='The resource list path to write (default stdout)')
+
+  args = parser.parse_args()
+
+  input_resources = list(int(resource_id) for resource_id in args.input)
+  filter_resources = set(int(resource_id) for resource_id in args.filter)
+  output_resources = [resource_id for resource_id in input_resources
+                      if resource_id not in filter_resources]
+
+  for resource_id in sorted(output_resources):
+    args.output.write('%d\n' % resource_id)
+
+if __name__ == '__main__':
+  main()