Reland#2 Use Aapt2 for final apk resource packaging

This reverts commit c63b9f73.

Reason for reland: Fix non-determinism in aapt2 resources.asrc output

TBR=agrieve@chromium.org

Bug: 782316
Change-Id: Iea2d6f63874a40a40084abd880ee5462b1e51de8
Reviewed-on: https://chromium-review.googlesource.com/830766
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525252}

base/android/jni_generator/AndroidManifest.xml

+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  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.
+-->
+
+<manifest xmlns:android="http://schemas.android.com/apk/res/android" package="org.jni.generator">
+
+    <uses-sdk android:minSdkVersion="16" android:targetSdkVersion="24" />
+    <application></application>
+
+</manifest>

base/android/jni_generator/BUILD.gn

@@ -64,7 +64,7 @@ shared_library("jni_sample_lib") {
 
 android_apk("sample_jni_apk") {
   apk_name = "SampleJni"
-  android_manifest = "//build/android/AndroidManifest.xml"
+  android_manifest = "AndroidManifest.xml"
   deps = [
     ":jni_sample_java",
     "//base:base_java",

build/android/gyp/process_resources.py

@@ -19,8 +19,9 @@ import re
 import shutil
 import subprocess
 import sys
-import xml.etree.ElementTree
 import zipfile
+from xml.etree import ElementTree
+
 
 import generate_v14_compatible_resources
 
@@ -454,6 +455,13 @@ def _ZipResources(resource_dirs, zip_path):
         files_to_zip[archive_path] = path
   build_utils.DoZip(files_to_zip.iteritems(), zip_path)
 
+def _SortZip(original_path, sorted_path):
+  with zipfile.ZipFile(sorted_path, 'w') as sorted_zip, \
+      zipfile.ZipFile(original_path, 'r') as original_zip:
+    for info in sorted(original_zip.infolist(), key=lambda i: i.filename):
+      sorted_zip.writestr(info, original_zip.read(info))
+
+
 
 def _DuplicateZhResources(resource_dirs):
   for resource_dir in resource_dirs:
@@ -465,7 +473,7 @@ def _DuplicateZhResources(resource_dirs):
         shutil.copyfile(path, hk_path)
 
 def _ExtractPackageFromManifest(manifest_path):
-  doc = xml.etree.ElementTree.parse(manifest_path)
+  doc = ElementTree.parse(manifest_path)
   return doc.getroot().get('package')
 
 
@@ -553,45 +561,62 @@ def _CheckForMissedConfigs(apk_path, check_density, languages):
                            'config (trigger=%s)') % (name, trigger.pattern))
 
 
-def _CreatePackageApkArgs(options):
-  package_command = [
+def _CreateLinkApkArgs(options):
+  link_command = [
+    options.aapt_path + '2',
+    'link',
     '--version-code', options.version_code,
     '--version-name', options.version_name,
-    '-f',
-    '-F', options.apk_path,
+    '--auto-add-overlay',
+    '--no-version-vectors',
+    '-I', options.android_sdk_jar,
+    '-o', options.apk_path,
   ]
 
   if options.proguard_file:
-    package_command += ['-G', options.proguard_file]
+    link_command += ['--proguard', options.proguard_file]
   if options.proguard_file_main_dex:
-    package_command += ['-D', options.proguard_file_main_dex]
+    link_command += ['--proguard-main-dex', options.proguard_file_main_dex]
 
   if options.no_compress:
     for ext in options.no_compress.split(','):
-      package_command += ['-0', ext]
+      link_command += ['-0', ext]
 
   if options.shared_resources:
-    package_command.append('--shared-lib')
-  if options.app_as_shared_lib:
-    package_command.append('--app-as-shared-lib')
+    link_command.append('--shared-lib')
 
   if options.create_density_splits:
     for config in _DENSITY_SPLITS.itervalues():
-      package_command.extend(('--split', ','.join(config)))
+      link_command.extend(('--split', ','.join(config)))
 
   if options.language_splits:
     for lang in options.language_splits:
-      package_command.extend(('--split', lang))
-
-  if options.debuggable:
-    package_command += ['--debug-mode']
+      link_command.extend(('--split', lang))
 
   if options.locale_whitelist:
     aapt_locales = _ToAaptLocales(
         options.locale_whitelist, options.support_zh_hk)
-    package_command += ['-c', ','.join(aapt_locales)]
+    link_command += ['-c', ','.join(aapt_locales)]
+
+  return link_command
+
+
+def _EnableDebugInManifest(manifest_path, temp_dir):
+  debug_manifest_path = os.path.join(temp_dir, 'AndroidManifest.xml')
+  _ANDROID_NAMESPACE = 'http://schemas.android.com/apk/res/android'
+  _TOOLS_NAMESPACE = 'http://schemas.android.com/tools'
+  ElementTree.register_namespace('android', _ANDROID_NAMESPACE)
+  ElementTree.register_namespace('tools', _TOOLS_NAMESPACE)
+  original_manifest = ElementTree.parse(manifest_path)
 
-  return package_command
+  app_node = original_manifest.find('application')
+  app_node.set('{%s}%s' % (_ANDROID_NAMESPACE, 'debuggable'), 'true')
+
+  with open(debug_manifest_path, 'w') as debug_manifest:
+    debug_manifest.write(ElementTree.tostring(
+        original_manifest.getroot(), encoding='UTF-8'))
+
+  return debug_manifest_path
 
 
 def _ResourceNameFromPath(path):
@@ -636,10 +661,37 @@ def _ConvertToWebP(webp_binary, png_files):
   pool.join()
 
 
-def _PackageApk(options, package_command, dep_subdirs):
-  _DuplicateZhResources(dep_subdirs)
+def _CompileDeps(aapt_path, dep_subdirs, temp_dir):
+  partials_dir = os.path.join(temp_dir, 'partials')
+  build_utils.MakeDirectory(partials_dir)
+  partial_compile_command = [
+      aapt_path + '2',
+      'compile',
+      '--no-crunch',
+  ]
+  pool = multiprocessing.pool.ThreadPool(10)
+  def compile_partial(directory):
+    dirname = os.path.basename(directory)
+    partial_path = os.path.join(partials_dir, dirname + '.zip')
+    compile_command = (partial_compile_command +
+                       ['--dir', directory, '-o', partial_path])
+    build_utils.CheckOutput(compile_command)
+
+    # Sorting the files in the partial ensures deterministic output from the
+    # aapt2 link step which uses order of files in the partial.
+    sorted_partial_path = os.path.join(partials_dir, dirname + '.sorted.zip')
+    _SortZip(partial_path, sorted_partial_path)
 
-  package_command += _CreatePackageApkArgs(options)
+    return sorted_partial_path
+
+  partials = pool.map(compile_partial, dep_subdirs)
+  pool.close()
+  pool.join()
+  return partials
+
+
+def _PackageApk(options, dep_subdirs, temp_dir, gen_dir, r_txt_path):
+  _DuplicateZhResources(dep_subdirs)
 
   keep_predicate = _CreateKeepPredicate(
       dep_subdirs, options.exclude_xxxhdpi, options.xxxhdpi_whitelist)
@@ -655,10 +707,26 @@ def _PackageApk(options, package_command, dep_subdirs):
   for directory in dep_subdirs:
     _MoveImagesToNonMdpiFolders(directory)
 
+  link_command = _CreateLinkApkArgs(options)
+  link_command += ['--output-text-symbols', r_txt_path]
+  link_command += ['--java', gen_dir]
+
+  if options.debuggable:
+    debug_manifest = _EnableDebugInManifest(options.android_manifest, temp_dir)
+    link_command += ['--manifest', debug_manifest]
+  else:
+    link_command += ['--manifest', options.android_manifest]
+
+  partials = _CompileDeps(options.aapt_path, dep_subdirs, temp_dir)
+  # It only works if partials are reversed (resource clobbering). This could be
+  # due to aapt2 processes the partials in reversed order (compared to aapt)
+  for partial in reversed(partials):
+    link_command += ['-R', partial]
+
   # Creates a .zip with AndroidManifest.xml, resources.arsc, res/*
   # Also creates R.txt
   build_utils.CheckOutput(
-      package_command, print_stdout=False, print_stderr=False)
+      link_command, print_stdout=False, print_stderr=False)
 
   if options.create_density_splits or options.language_splits:
     _CheckForMissedConfigs(options.apk_path, options.create_density_splits,
@@ -668,10 +736,38 @@ def _PackageApk(options, package_command, dep_subdirs):
     _RenameDensitySplits(options.apk_path)
 
 
-def _PackageLibrary(options, package_command, temp_dir):
+# _PackageLibrary uses aapt rather than aapt2 because aapt2 compile does not
+# support outputting an R.txt file.
+def _PackageLibrary(options, dep_subdirs, temp_dir, gen_dir):
   v14_dir = os.path.join(temp_dir, 'v14')
   build_utils.MakeDirectory(v14_dir)
 
+  # Generate R.java. This R.java contains non-final constants and is used only
+  # while compiling the library jar (e.g. chromium_content.jar). When building
+  # an apk, a new R.java file with the correct resource -> ID mappings will be
+  # generated by merging the resources from all libraries and the main apk
+  # project.
+  package_command = [options.aapt_path,
+                     'package',
+                     '-m',
+                     '-M', options.android_manifest,
+                     '--no-crunch',
+                     '--auto-add-overlay',
+                     '--no-version-vectors',
+                     '-I', options.android_sdk_jar,
+                     '--output-text-symbols', gen_dir,
+                     '-J', gen_dir,  # Required for R.txt generation.
+                     '--ignore-assets', build_utils.AAPT_IGNORE_PATTERN]
+
+  # Adding all dependencies as sources is necessary for @type/foo references
+  # to symbols within dependencies to resolve. However, it has the side-effect
+  # that all Java symbols from dependencies are copied into the new R.java.
+  # E.g.: It enables an arguably incorrect usage of
+  # "mypackage.R.id.lib_symbol" where "libpackage.R.id.lib_symbol" would be
+  # more correct. This is just how Android works.
+  for d in dep_subdirs:
+    package_command += ['-S', d]
+
   input_resource_dirs = options.resource_dirs
 
   for d in input_resource_dirs:
@@ -764,36 +860,10 @@ def _OnStaleMd5(options):
 
     dep_subdirs = _ExtractDeps(options.dependencies_res_zips, deps_dir)
 
-    # Generate R.java. This R.java contains non-final constants and is used only
-    # while compiling the library jar (e.g. chromium_content.jar). When building
-    # an apk, a new R.java file with the correct resource -> ID mappings will be
-    # generated by merging the resources from all libraries and the main apk
-    # project.
-    package_command = [options.aapt_path,
-                       'package',
-                       '-m',
-                       '-M', options.android_manifest,
-                       '--no-crunch',
-                       '--auto-add-overlay',
-                       '--no-version-vectors',
-                       '-I', options.android_sdk_jar,
-                       '--output-text-symbols', gen_dir,
-                       '-J', gen_dir,  # Required for R.txt generation.
-                       '--ignore-assets', build_utils.AAPT_IGNORE_PATTERN]
-
-    # Adding all dependencies as sources is necessary for @type/foo references
-    # to symbols within dependencies to resolve. However, it has the side-effect
-    # that all Java symbols from dependencies are copied into the new R.java.
-    # E.g.: It enables an arguably incorrect usage of
-    # "mypackage.R.id.lib_symbol" where "libpackage.R.id.lib_symbol" would be
-    # more correct. This is just how Android works.
-    for d in dep_subdirs:
-      package_command += ['-S', d]
-
     if options.apk_path:
-      _PackageApk(options, package_command, dep_subdirs)
+      _PackageApk(options, dep_subdirs, temp_dir, gen_dir, r_txt_path)
     else:
-      _PackageLibrary(options, package_command, temp_dir)
+      _PackageLibrary(options, dep_subdirs, temp_dir, gen_dir)
 
     _CreateRTxtAndSrcJar(options, r_txt_path, srcjar_dir)
 
@@ -832,12 +902,13 @@ def main(args):
     options.v14_skip,
     options.exclude_xxxhdpi,
     options.xxxhdpi_whitelist,
+    str(options.debuggable),
     str(options.png_to_webp),
     str(options.support_zh_hk),
   ]
 
   if options.apk_path:
-    input_strings.extend(_CreatePackageApkArgs(options))
+    input_strings.extend(_CreateLinkApkArgs(options))
 
   input_paths = [
     options.aapt_path,

build/android/pylib/instrumentation/instrumentation_test_instance.py

@@ -568,10 +568,10 @@ class InstrumentationTestInstance(test_instance.TestInstance):
     self._test_package = self._test_apk.GetPackageName()
     all_instrumentations = self._test_apk.GetAllInstrumentations()
     all_junit3_runner_classes = [
-        x for x in all_instrumentations if ('true' not in x.get(
+        x for x in all_instrumentations if ('0xffffffff' not in x.get(
             'chromium-junit4', ''))]
     all_junit4_test_runner_classes = [
-        x for x in all_instrumentations if ('true' in x.get(
+        x for x in all_instrumentations if ('0xffffffff' in x.get(
             'chromium-junit4', ''))]
 
     if len(all_junit3_runner_classes) > 1:

chrome/android/java/res/layout/data_reduction_main_menu_item.xml

@@ -25,7 +25,7 @@
 
         <ImageView
             android:id="@+id/chart_icon"
-            android:src="@+drawable/data_reduction_main_menu_icon"
+            android:src="@drawable/data_reduction_main_menu_icon"
             android:layout_width="@dimen/data_reduction_main_menu_icon_width"
             android:layout_height="match_parent"
             android:layout_gravity="start|center_vertical"

testing/android/empty_apk/AndroidManifest.xml

@@ -5,4 +5,5 @@
 
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
   package="{{package}}">
+  <application></application>
 </manifest>