[Android] Auto-generate only necessary v14 resources.

build/android/gyp/generate_v14_overlay_resources.py

Index: build/android/gyp/generate_v14_overlay_resources.py
diff --git a/build/android/gyp/generate_v14_resources.py b/build/android/gyp/generate_v14_overlay_resources.py
similarity index 50%
rename from build/android/gyp/generate_v14_resources.py
rename to build/android/gyp/generate_v14_overlay_resources.py
index a07a42a88d10a0f6c491d2e11bcc09173b852ead..966b84a81f6c22c49bb08988b0c23b21d5431a98 100755
--- a/build/android/gyp/generate_v14_resources.py
+++ b/build/android/gyp/generate_v14_overlay_resources.py
@@ -6,7 +6,7 @@
 
 """Convert Android xml resources to API 14 compatible.
 
-There are two reasons that we cannot just use API attributes,
+There are two reasons that we cannot just use API 17 attributes,
 so we are generating another set of resources by this script.
 
 1. paddingStart attribute can cause a crash on Galaxy Tab 2.
@@ -21,14 +21,16 @@ Please refer to http://crbug.com/235118 for the details.
 
 import optparse
 import os
-import re
+import shutil
 import sys
 import xml.dom.minidom as minidom
 
 from util import build_utils
 
+# Note that we are assuming 'android:' is an alias of
+# the namespace 'http://schemas.android.com/apk/res/android'.
 
-ATTRIBUTE_NAMESPACE = 'http://schemas.android.com/apk/res/android'
+GRAVITY_ATTRIBUTES = ('android:gravity', 'android:layout_gravity')
 
 # Almost all the attributes that has "Start" or "End" in
 # its name should be mapped.
@@ -45,12 +47,11 @@ ATTRIBUTES_TO_MAP = {'paddingStart' : 'paddingLeft',
                      'layout_alignParentEnd' : 'layout_alignParentRight',
                      'layout_toEndOf' : 'layout_toRightOf'}
 
-ATTRIBUTES_TO_MAP_NS = {}
+ATTRIBUTES_TO_MAP = dict(['android:' + k, 'android:' + v] for k, v
+                         in ATTRIBUTES_TO_MAP.iteritems())
 
-for k, v in ATTRIBUTES_TO_MAP.items():
-  ATTRIBUTES_TO_MAP_NS[(ATTRIBUTE_NAMESPACE, k)] = (ATTRIBUTE_NAMESPACE, v)
-
-ATTRIBUTES_TO_MAP_NS_VALUES = set(ATTRIBUTES_TO_MAP_NS.values())
+ATTRIBUTES_TO_MAP_REVERSED = dict([v,k] for k, v
+                                  in ATTRIBUTES_TO_MAP.iteritems())
 
 
 def IterateXmlElements(node):
@@ -63,85 +64,107 @@ def IterateXmlElements(node):
       yield child_node_element
 
 
-def GenerateV14StyleResource(dom, output_file):
+def WarnDeprecatedAttribute(name, value, filename):

[newt (away), 2013/05/10 17:10:01]

I'd change this function name to "WarnIfDeprecatedAttribute()", so it's clear
that it usually *doesn't* emit a warning

[Kibeom Kim (inactive), 2013/05/10 17:46:14]

Done.

+  """print a warning message if the given attribute is deprecated."""
+  if name in ATTRIBUTES_TO_MAP_REVERSED:
+    print >> sys.stderr, ('warning: ' + filename + ' should use ' +
+                          ATTRIBUTES_TO_MAP_REVERSED[name] +
+                          ' instead of ' + name)
+  elif name in GRAVITY_ATTRIBUTES and ('left' in value or 'right' in value):
+    print >> sys.stderr, ('warning: ' + filename +
+                          ' should use start/end instead of left/right for ' +
+                          name)
+
+
+def GenerateV14LayoutResource(dom, filename):
+  """Convert layout resource to API 14 compatible layout resource.
+
+  Args:
+    dom: parsed minidom object to be modified.
+  Returns:
+    True if dom is modified, False otherwise.
+  """
+  is_modified = False
+
+  # Iterate all the elements' attributes to find attributes to convert.
+  for element in IterateXmlElements(dom):
+    for name, value in list(element.attributes.items()):
+      # Convert any other API 17 Start/End attributes to Left/Right attributes.

[newt (away), 2013/05/10 17:10:01]

what does "other" mean? I'd remove this now.

[Kibeom Kim (inactive), 2013/05/10 17:46:14]

Done.

+      # For example, from paddingStart="10dp" to paddingLeft="10dp"
+      # Note: gravity attributes are not necessary to convert because
+      # start/end values are backward-compatible. Explained at
+      # https://plus.sandbox.google.com/+RomanNurik/posts/huuJd8iVVXY?e=Showroom
+      if name in ATTRIBUTES_TO_MAP:
+        element.setAttribute(ATTRIBUTES_TO_MAP[name], value)
+        del element.attributes[name]
+        is_modified = True
+      else:
+        WarnDeprecatedAttribute(name, value, filename)
+
+  return is_modified
+
+
+def GenerateV14StyleResource(dom, filename):
   """Convert style resource to API 14 compatible style resource.
 
-  It's mostly a simple replacement, s/Start/Left s/End/Right,
-  on the attribute names specified by <item> element.
+  Args:
+    dom: parsed minidom object to be modified.
+  Returns:
+    True if dom is modified, False otherwise.
   """
+  is_modified = False
+
   for style_element in dom.getElementsByTagName('style'):
     for item_element in style_element.getElementsByTagName('item'):
-      namespace, name = item_element.attributes['name'].value.split(':')
-      # Note: namespace == 'android' is not precise because
-      # we are looking for 'http://schemas.android.com/apk/res/android' and
-      # 'android' can be aliased to another name in layout xml files where
-      # this style is used. e.g. xmlns:android="http://crbug.com/".
-      if namespace == 'android' and name in ATTRIBUTES_TO_MAP:
-        mapped_name = ATTRIBUTES_TO_MAP[name]
-        item_element.attributes['name'] = namespace + ':' + mapped_name
+      name = item_element.attributes['name'].value
+      value = item_element.childNodes[0].nodeValue
+      if name in ATTRIBUTES_TO_MAP:
+        item_element.attributes['name'].value = ATTRIBUTES_TO_MAP[name]
+        is_modified = True
+      else:
+        WarnDeprecatedAttribute(name, value, filename)
 
-  build_utils.MakeDirectory(os.path.dirname(output_file))
-  with open(output_file, 'w') as f:
-    dom.writexml(f, '', '  ', '\n', encoding='utf-8')
+  return is_modified
 
 
-def GenerateV14LayoutResource(input_file, output_file):
-  """Convert layout resource to API 14 compatible layout resource.
+def GenerateV14Resource(input_filename,

[newt (away), 2013/05/10 17:10:01]

slightly crazy functional idea:

you could pass GenerateV14StyleResource or GenerateV14LayoutResource as an
argument called "generateResourceFunction" to this method. then you could
replace lines 143-147 with:

    generateResourceFunction(dom, input_filename)

I probably wouldn't actually do this though...

I would add a comment though explaining that line 145 corresponds to XML files
in "values" and line 147 is for XML files in "layouts" or (sadly) "xml"

[Kibeom Kim (inactive), 2013/05/10 17:46:14]

Yeah. I like what you suggested and looks more elegant. But maybe decrease
readability slightly? Because this is not a huge logic, I'll keep plain.

Added comments at 145, 147. Done.


On 2013/05/10 17:10:01, newt wrote:

[newt (away), 2013/05/10 18:06:27]

thanks. I agree about the decrease in readability of the other option,
especially since it's python and the types aren't clear at all.

+                        output_v14_filename,
+                        output_v17_filename):
+  """Convert layout/style resource to API 14 compatible layout/style resource.
 
   It's mostly a simple replacement, s/Start/Left s/End/Right,
   on the attribute names.
+  If the generated resource is identical to the original resource,
+  don't do anything. If not, write the generated resource to
+  output_v14_filename, and copy the original resource to output_v17_filename.
   """
-  dom = minidom.parse(input_file)
+  dom = minidom.parse(input_filename)
 
-  for element in IterateXmlElements(dom):
-    all_names = element.attributes.keysNS()
+  root_node = IterateXmlElements(dom).next()
+  if root_node.nodeName == 'resources':
+    is_modified = GenerateV14StyleResource(dom, input_filename)
+  else:
+    is_modified = GenerateV14LayoutResource(dom, input_filename)
 
-    # Iterate all the attributes to find attributes to convert.
-    # Note that name variable is actually a tuple that has namespace and name.
-    # For example,
-    # name == ('http://schemas.android.com/apk/res/android', 'paddingStart')
-    for name, value in list(element.attributes.itemsNS()):
-      # Note: gravity attributes are not necessary to convert because
-      # start/end values are backward-compatible. Explained at
-      # https://plus.sandbox.google.com/+RomanNurik/posts/huuJd8iVVXY?e=Showroom
-
-      # Convert any other API 17 Start/End attributes to Left/Right attributes.
-      # For example, from paddingStart="10dp" to paddingLeft="10dp"
-      if name in ATTRIBUTES_TO_MAP_NS:
-        mapped_name = ATTRIBUTES_TO_MAP_NS[name]
-
-        # Add the new mapped attribute and remove the original attribute.
-        # For example, add paddingLeft and remove paddingStart.
-        # Note that instead of element.setAttribute(...), this is more correct.
-        # element.setAttributeNS(mapped_name[0], mapped_name[1], value)
-        # However, there is a minidom bug that doesn't print namespace set by
-        # setAttributeNS. Hence this workaround.
-        # This is a similar bug discussion about minidom namespace normalizing.
-        # http://stackoverflow.com/questions/863774/how-to-generate-xml-documents-with-namespaces-in-python
-        element.setAttribute('android:' + mapped_name[1], value)
-        del element.attributes[name]
-      elif name in ATTRIBUTES_TO_MAP_NS_VALUES:
-        # TODO(kkimlabs): Enable  warning once layouts have been converted
-        # print >> sys.stderror, 'Warning: layout should use xxx instead of yyy'
-        pass
+  if is_modified:
+    # Write the generated resource.
+    build_utils.MakeDirectory(os.path.dirname(output_v14_filename))
+    with open(output_v14_filename, 'w') as f:
+      dom.writexml(f, '', '  ', '\n', encoding='utf-8')
 
-  build_utils.MakeDirectory(os.path.dirname(output_file))
-  with open(output_file, 'w') as f:
-    dom.writexml(f, '', '  ', '\n', encoding='utf-8')
+    # Copy the original resource.
+    build_utils.MakeDirectory(os.path.dirname(output_v17_filename))
+    shutil.copy2(input_filename, output_v17_filename)
 
 
-def GenerateV14XmlResourcesInDir(input_dir, output_dir, only_styles=False):
+def GenerateV14XmlResourcesInDir(input_dir, output_v14_dir, output_v17_dir):
   """Convert resources to API 14 compatible XML resources in the directory."""
-  for input_file in build_utils.FindInDirectory(input_dir, '*.xml'):
-    output_file = os.path.join(output_dir,
-                               os.path.relpath(input_file, input_dir))
-    if only_styles:
-      dom = minidom.parse(input_file)
-      if not dom.getElementsByTagName('style'):
-        continue
-      GenerateV14StyleResource(dom, output_file)
-    else:
-      GenerateV14LayoutResource(input_file, output_file)
+  for input_filename in build_utils.FindInDirectory(input_dir, '*.xml'):
+    rel_filename = os.path.relpath(input_filename, input_dir)
+    output_v14_filename = os.path.join(output_v14_dir, rel_filename)
+    output_v17_filename = os.path.join(output_v17_dir, rel_filename)
+    GenerateV14Resource(input_filename, output_v14_filename,
+                        output_v17_filename)
 
 
 def ParseArgs():
@@ -154,7 +177,7 @@ def ParseArgs():
   parser.add_option('--res-dir',
                     help='directory containing resources '
                          'used to generate v14 resources')

[newt (away), 2013/05/10 18:06:27]

be sure to update these:

"v14 compatible resources"

[Kibeom Kim (inactive), 2013/05/10 18:42:55]

Done.

-  parser.add_option('--res-v14-dir',
+  parser.add_option('--res-v14-overlay-dir',
                     help='output directory into which '
                          'v14 resources will be generated')
   parser.add_option('--stamp', help='File to touch on success')
@@ -165,7 +188,7 @@ def ParseArgs():
     parser.error('No positional arguments should be given.')
 
   # Check that required options have been provided.
-  required_options = ('res_dir', 'res_v14_dir')
+  required_options = ('res_dir', 'res_v14_overlay_dir')
   build_utils.CheckOptions(options, parser, required=required_options)
   return options
 
@@ -173,8 +196,8 @@ def ParseArgs():
 def main(argv):
   options = ParseArgs()
 
-  build_utils.DeleteDirectory(options.res_v14_dir)
-  build_utils.MakeDirectory(options.res_v14_dir)
+  build_utils.DeleteDirectory(options.res_v14_overlay_dir)
+  build_utils.MakeDirectory(options.res_v14_overlay_dir)
 
   for name in os.listdir(options.res_dir):
     if not os.path.isdir(os.path.join(options.res_dir, name)):
@@ -188,17 +211,18 @@ def main(argv):
     if 'ldrtl' in qualifiers:
       continue
 
+    # We also need to copy the original v17 resource to *-v17 directory
+    # because the generated v14 resource will hide the original resource.
     input_dir = os.path.join(options.res_dir, name)
-    output_dir = os.path.join(options.res_v14_dir, name)
+    output_v14_dir = os.path.join(options.res_v14_overlay_dir, name)
+    output_v17_dir = os.path.join(options.res_v14_overlay_dir, name + '-v17')
 
     # We only convert resources under layout*/, xml*/,
     # and style resources under values*/.
     # TODO(kkimlabs): don't process xml directly once all layouts have
     # been moved out of XML directory. see http://crbug.com/238458
-    if resource_type in ('layout', 'xml'):
-      GenerateV14XmlResourcesInDir(input_dir, output_dir)
-    elif resource_type in ('values'):
-      GenerateV14XmlResourcesInDir(input_dir, output_dir, only_styles=True)
+    if resource_type in ('layout', 'xml', 'values'):
+      GenerateV14XmlResourcesInDir(input_dir, output_v14_dir, output_v17_dir)
 
   if options.stamp:
     build_utils.Touch(options.stamp)