GN: Fix ChromePublic crash on launch from misconfigured resource overrides

build/android/gyp/process_resources.py

 #!/usr/bin/env python
 #
 # Copyright (c) 2012 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.
 
 """Process Android resources to generate R.java, and prepare for packaging.
 
 This will crunch images and generate v14 compatible resources
 (see generate_v14_compatible_resources.py).
 """
 
 import codecs
+import collections
 import optparse
 import os
 import re
 import shutil
 import sys
 
 import generate_v14_compatible_resources
 
 from util import build_utils
 
 # Import jinja2 from third_party/jinja2
 sys.path.insert(1,
     os.path.join(os.path.dirname(__file__), '../../../third_party'))
 from jinja2 import Template # pylint: disable=F0401
 
 
+# Represents a line from a R.txt file.
+TextSymbolsEntry = collections.namedtuple('RTextEntry',
+    ('java_type', 'resource_type', 'name', 'value'))
+
+
 def ParseArgs(args):
   """Parses command line options.
 
   Returns:
     An options object as from optparse.OptionsParser.parse_args()
   """
   parser = optparse.OptionParser()
   build_utils.AddDepfileOption(parser)
 
   parser.add_option('--android-sdk', help='path to the Android SDK folder')
@@ skipping 88 matching lines @@
       package_r_java_dir = os.path.join(r_dir, *package.split('.'))
       build_utils.MakeDirectory(package_r_java_dir)
       package_r_java_path = os.path.join(package_r_java_dir, 'R.java')
       new_r_java = re.sub(r'package [.\w]*;', u'package %s;' % package,
                           r_java_contents)
       codecs.open(package_r_java_path, 'w', encoding='utf-8').write(new_r_java)
   else:
     if len(extra_packages) != len(extra_r_text_files):
       raise Exception('Need one R.txt file per extra package')
 
-    all_resources = {}
     r_txt_file = os.path.join(r_dir, 'R.txt')
     if not os.path.exists(r_txt_file):
       return
-    with open(r_txt_file) as f:
-      for line in f:
-        m = re.match(r'(int(?:\[\])?) (\w+) (\w+) (.+)$', line)
-        if not m:
-          raise Exception('Unexpected line in R.txt: %s' % line)
-        java_type, resource_type, name, value = m.groups()
-        all_resources[(resource_type, name)] = (java_type, value)
 
+    # Map of (resource_type, name) -> Entry.
+    # Contains the correct values for resources.
+    all_resources = {}
+    for entry in _ParseTextSymbolsFile(r_txt_file):
+      all_resources[(entry.resource_type, entry.name)] = entry
+
+    # Map of package_name->resource_type->entry
+    resources_by_package = (
+        collections.defaultdict(lambda: collections.defaultdict(list)))
+    seen_entries_by_package = collections.defaultdict(set)

[newt (away), 2015/09/15 17:07:53]

Are there legitimate cases where extra_packages contains the same package name
multiple times? If so, I understand why this exists... but I don't see why we'd
need to support duplicate package names in extra_packages.

[newt (away), 2015/09/15 17:14:01]

Ah, now I see. You're reusing org.chromium.chrome when overriding the resources
downstream. The alternative is to simply use a different package name when
overriding the resources. The package name is irrelevant after all when
overriding resources -- it's the resource names that matter.

+    # Build the R.java files using each package's R.txt file, but replacing
+    # each entry's placeholder value with correct values from all_resources.
     for package, r_text_file in zip(extra_packages, extra_r_text_files):
-      if os.path.exists(r_text_file):
-        package_r_java_dir = os.path.join(r_dir, *package.split('.'))
-        build_utils.MakeDirectory(package_r_java_dir)
-        package_r_java_path = os.path.join(package_r_java_dir, 'R.java')
-        CreateExtraRJavaFile(
-            package, package_r_java_path, r_text_file, all_resources,
-            shared_resources)
+      if not os.path.exists(r_text_file):
+        continue
+      resources_by_type = resources_by_package[package]
+      seen_entries = seen_entries_by_package[package]
+      # The sub-R.txt files have the wrong values at this point. Read them to
+      # figure out which entries belong to them, but use the values from the
+      # main R.txt file.
+      for entry in _ParseTextSymbolsFile(r_text_file):
+        entry_key = (entry.resource_type, entry.name)
+        if not entry_key in seen_entries:
+          seen_entries.add(entry_key)
+          entry = all_resources[entry_key]
+          resources_by_type[entry.resource_type].append(entry)
+
+    for package, resources_by_type in resources_by_package.iteritems():
+      package_r_java_dir = os.path.join(r_dir, *package.split('.'))
+      build_utils.MakeDirectory(package_r_java_dir)
+      package_r_java_path = os.path.join(package_r_java_dir, 'R.java')
+      java_file_contents = _CreateExtraRJavaFile(
+          package, resources_by_type, shared_resources)
+      with open(package_r_java_path, 'w') as f:
+        f.write(java_file_contents)
 
 
-def CreateExtraRJavaFile(
-      package, r_java_path, r_text_file, all_resources, shared_resources):
-  resources = {}
-  with open(r_text_file) as f:
+def _ParseTextSymbolsFile(path):
+  """Given an R.txt file, returns a list of TextSymbolsEntry."""
+  ret = []
+  with open(path) as f:
     for line in f:
-      m = re.match(r'int(?:\[\])? (\w+) (\w+) ', line)
+      m = re.match(r'(int(?:\[\])?) (\w+) (\w+) (.+)$', line)
       if not m:
         raise Exception('Unexpected line in R.txt: %s' % line)
-      resource_type, name = m.groups()
-      java_type, value = all_resources[(resource_type, name)]
-      if resource_type not in resources:
-        resources[resource_type] = []
-      resources[resource_type].append((name, java_type, value))
+      java_type, resource_type, name, value = m.groups()
+      ret.append(TextSymbolsEntry(java_type, resource_type, name, value))
+  return ret
 
+
+def _CreateExtraRJavaFile(package, resources_by_type, shared_resources):
+  """Generates the contents of a R.java file."""
   template = Template("""/* AUTO-GENERATED FILE.  DO NOT MODIFY. */
 
 package {{ package }};
 
 public final class R {
     {% for resource_type in resources %}
     public static final class {{ resource_type }} {
-        {% for name, java_type, value in resources[resource_type] %}
+        {% for entry in resources[resource_type] %}
         {% if shared_resources %}
-        public static {{ java_type }} {{ name }} = {{ value }};
+        public static {{ entry.java_type }} {{ entry.name }} = {{ entry.value }};
         {% else %}
-        public static final {{ java_type }} {{ name }} = {{ value }};
+        public static final {{ entry.java_type }} {{ entry.name }} = {{ entry.value }};
         {% endif %}
         {% endfor %}
     }
     {% endfor %}
     {% if shared_resources %}
     public static void onResourcesLoaded(int packageId) {
         {% for resource_type in resources %}
-        {% for name, java_type, value in resources[resource_type] %}
-        {% if java_type == 'int[]' %}
-        for(int i = 0; i < {{ resource_type }}.{{ name }}.length; ++i) {
-            {{ resource_type }}.{{ name }}[i] =
-                    ({{ resource_type }}.{{ name }}[i] & 0x00ffffff)
+        {% for entry in resources[resource_type] %}
+        {% if entry.java_type == 'int[]' %}
+        for(int i = 0; i < {{ entry.resource_type }}.{{ entry.name }}.length; ++i) {
+            {{ entry.resource_type }}.{{ entry.name }}[i] =
+                    ({{ entry.resource_type }}.{{ entry.name }}[i] & 0x00ffffff)
                     | (packageId << 24);
         }
         {% else %}
-        {{ resource_type }}.{{ name }} =
-                ({{ resource_type }}.{{ name }} & 0x00ffffff)
+        {{ entry.resource_type }}.{{ entry.name }} =
+                ({{ entry.resource_type }}.{{ entry.name }} & 0x00ffffff)
                 | (packageId << 24);
         {% endif %}
         {% endfor %}
         {% endfor %}
     }
     {% endif %}
 }
 """, trim_blocks=True, lstrip_blocks=True)
 
-  output = template.render(package=package, resources=resources,
-                           shared_resources=shared_resources)
-  with open(r_java_path, 'w') as f:
-    f.write(output)
+  return template.render(package=package, resources=resources_by_type,
+                         shared_resources=shared_resources)
 
 
 def CrunchDirectory(aapt, input_dir, output_dir):
   """Crunches the images in input_dir and its subdirectories into output_dir.
 
   If an image is already optimized, crunching often increases image size. In
   this case, the crunched image is overwritten with the original image.
   """
   aapt_cmd = [aapt,
               'crunch',
@@ skipping 183 matching lines @@
   if options.depfile:
     input_files += build_utils.GetPythonDependencies()
     build_utils.WriteDepfile(options.depfile, input_files)
 
   if options.stamp:
     build_utils.Touch(options.stamp)
 
 
 if __name__ == '__main__':
   main()