[Build] Add size config JSON files and supports.

Previously, generate_commit_size_analysis.py takes:
* --apk-name for SuperSize "main file" input (.apk or .minimal.apks).
* --mapping-name for "mapping file" input.

This worked fine for Monochrome. However, for Trichrome, the SuperSize
"main file" is now an .ssargs file that specifies 3 "container files",
which for Monochrome, coincided with the "main file".

To centralize the specification of all these files from BUILD files,
we define "size config JSON" file to specify the following:
* Mapping files (as a list).
* Arguments to be passed to resource_sizes.py (which already supports
  Trichrome).
* Supersize main file, which can be {.apk, .minimal.apks, .ssargs}.
* Version numeric string (for late ruse).

This CL adds size config JSON support. Details:
* Add GN template "android_resource_size_config" to specify size config
  JSON files. Files are generated during "gn gen", and written to
  config/ under Chromium output dir.
* Add GN targets to generate size config JSON files for Monochrome and
  Trichrome
  * resource_size_config_monochrome_public_minimal_apks
    -> config/MonochromePublic_size_config.json
  * resource_size_config_trichrome -> config/Trichrome_size_config.json
* In generate_commit_size_analysis.py:
  * Add --size-config-json to specify size config JSON path.
    * If specified, the file is copied to the staging dir.
  * If the "main file" is .ssargs, then it's also copied to the
    staging dir.
  * Add Trichrome support to call resource_sizes.py
* In trybot_commit_size_checker.py:
  * Add --size-config-json-name to specify size config JSON *filename*,
    which will be searched in {before, after} dirs.
    * If specified, "mapping files" are directly extracted from the
      size config JSON files for "ForTest" symbol extraction.
    * This will replace the --apk-name argument.

With this CL, size config JSON files and supports are available but
unused. Usages will be added in follow-up work.

Bug: 1040645
Change-Id: Ie3328ac413af3a15cd1cbb0957881da7d2d1f839
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518126
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824562}

build/android/resource_sizes.gni

@@ -69,3 +69,32 @@ template("android_resource_sizes_test") {
     }
   }
 }
+
+# Generates "size config JSON file" to specify data to be passed from bots via
+# recipe to Python scripts for binary size measurement. All filenames are
+# relative to $root_build_dir. The resulting JSON file is written to
+# "$root_build_dir/config/${invoker.name}_size_config.json".
+#
+# Variables:
+#   name: The name of the path to the generated size config JSON file.
+#   mapping_files: List of mapping files.
+#   to_resource_sizes_py: Scope containing data to pass to resource_sizes.py,
+#     processed by generate_commit_size_analysis.py.
+#   supersize_input_file: Main input for SuperSize.
+#   version: Version number, uses to notify build bots that significant changes
+#     has occurred, so that gs:// should be ignored.
+template("android_resource_size_config") {
+  _full_target_name = get_label_info(target_name, "label_no_toolchain")
+  _out_json = {
+    _HEADER = "Written by build target '${_full_target_name}'"
+    forward_variables_from(invoker,
+                           [
+                             "mapping_files",
+                             "to_resource_sizes_py",
+                             "supersize_input_file",
+                             "version",
+                           ])
+  }
+  _output_json_path = "$root_build_dir/config/${invoker.name}_size_config.json"
+  write_file(_output_json_path, _out_json, "json")
+}

chrome/android/BUILD.gn

@@ -3013,10 +3013,23 @@ if (is_official_build) {
     deps = [ ":monochrome_public_bundle" ]
     bundle_path = "$root_build_dir/apks/MonochromePublic.aab"
   }
+
+  _minimal_apks_filename = "MonochromePublic.minimal.apks"
   android_resource_sizes_test("resource_sizes_monochrome_public_minimal_apks") {
-    file_path = "$root_build_dir/apks/MonochromePublic.minimal.apks"
+    file_path = "$root_build_dir/apks/${_minimal_apks_filename}"
     data_deps = [ ":monochrome_public_minimal_apks" ]
   }
+
+  android_resource_size_config(
+      "resource_size_config_monochrome_public_minimal_apks") {
+    name = "MonochromePublic"
+    mapping_files = [ "apks/MonochromePublic.aab.mapping" ]
+    to_resource_sizes_py = {
+      apk_name = "apks/$_minimal_apks_filename"
+    }
+    supersize_input_file = "apks/$_minimal_apks_filename"
+    version = "1"
+  }
 }
 
 monochrome_or_trichrome_public_bundle_tmpl("trichrome_chrome_bundle") {
@@ -3056,9 +3069,10 @@ if (is_official_build) {
   _trichrome_library_basename = "TrichromeLibrary.apk"
   _trichrome_chrome_basename = "TrichromeChrome.minimal.apks"
   _trichrome_webview_basename = "TrichromeWebView.minimal.apks"
+  _ssargs_filename = "Trichrome.ssargs"
 
   write_ssargs_trichrome("ssargs_trichrome") {
-    ssargs_path = "$root_build_dir/apks/Trichrome.ssargs"
+    ssargs_path = "$root_build_dir/apks/${_ssargs_filename}"
     trichrome_library_basename = _trichrome_library_basename
     trichrome_chrome_basename = _trichrome_chrome_basename
     trichrome_webview_basename = _trichrome_webview_basename
@@ -3085,6 +3099,23 @@ if (is_official_build) {
     trichrome_webview_path = "$root_build_dir/apks/$_trichrome_webview_basename"
     data_deps = [ ":trichrome_minimal_apks" ]
   }
+
+  android_resource_size_config("resource_size_config_trichrome") {
+    name = "Trichrome"
+    mapping_files = [
+      "apks/TrichromeLibrary.apk.mapping",
+      "apks/TrichromeChrome.aab.mapping",
+      "apks/TrichromeWebView.aab.mapping",
+    ]
+    to_resource_sizes_py = {
+      apk_name = "apks/resource_size_config_trichrome"
+      trichrome_library = "apks/$_trichrome_library_basename"
+      trichrome_chrome = "apks/$_trichrome_chrome_basename"
+      trichrome_webview = "apks/$_trichrome_webview_basename"
+    }
+    supersize_input_file = "apks/$_ssargs_filename"
+    version = "1"
+  }
 }
 
 if (android_64bit_target_cpu) {

tools/binary_size/generate_commit_size_analysis.py

@@ -7,8 +7,9 @@
 """Creates files required to feed into trybot_commit_size_checker"""
 
 import argparse
-import os
+import json
 import logging
+import os
 import shutil
 import subprocess
 
@@ -21,38 +22,41 @@ _CLANG_UPDATE_PATH = os.path.join(_SRC_ROOT, 'tools', 'clang', 'scripts',
                                   'update.py')
 
 
-def _extract_proguard_mapping(apk_name, mapping_name_list, staging_dir,
-                              chromium_output_directory):
-  """Copies proguard mapping files to staging_dir"""
-  for mapping_name in mapping_name_list:
-    mapping_path = os.path.join(chromium_output_directory, 'apks', mapping_name)
-    shutil.copy(mapping_path, os.path.join(staging_dir, apk_name + '.mapping'))
+def _copy_files_to_staging_dir(files_to_copy, make_staging_path):
+  """Copies files from output directory to staging_dir"""
+  for filename in files_to_copy:
+    shutil.copy(filename, make_staging_path(filename))
 
 
-def _generate_resource_sizes(apk_name, staging_dir, chromium_output_directory):
+def _generate_resource_sizes(to_resource_sizes_py, make_chromium_output_path,
+                             make_staging_path):
   """Creates results-chart.json file in staging_dir"""
-  apk_path = os.path.join(chromium_output_directory, 'apks', apk_name)
-
-  subprocess.run(
-      [
-          _RESOURCE_SIZES_PATH,
-          apk_path,
-          '--output-format=chartjson',
-          '--output-dir',
-          staging_dir,
-          '--chromium-output-directory',
-          chromium_output_directory,
-      ],
-      check=True,
-  )
-
-
-def _generate_supersize_archive(apk_name, staging_dir,
-                                chromium_output_directory):
+  cmd = [
+      _RESOURCE_SIZES_PATH,
+      make_chromium_output_path(to_resource_sizes_py['apk_name']),
+      '--output-format=chartjson',
+      '--chromium-output-directory',
+      make_chromium_output_path(),
+      '--output-dir',
+      make_staging_path(),
+  ]
+  FORWARDED_PARAMS = [
+      ('--trichrome-library', make_chromium_output_path, 'trichrome_library'),
+      ('--trichrome-chrome', make_chromium_output_path, 'trichrome_chrome'),
+      ('--trichrome-webview', make_chromium_output_path, 'trichrome_webview'),
+  ]
+  for switch, fun, key in FORWARDED_PARAMS:
+    if key in to_resource_sizes_py:
+      cmd += [switch, fun(to_resource_sizes_py[key])]
+  subprocess.run(cmd, check=True)
+
+
+def _generate_supersize_archive(supersize_input_file, make_chromium_output_path,
+                                make_staging_path):
   """Creates a .size file for the given .apk or .minimal.apks"""
   subprocess.run([_CLANG_UPDATE_PATH, '--package=objdump'], check=True)
-  apk_path = os.path.join(chromium_output_directory, 'apks', apk_name)
-  size_path = os.path.join(staging_dir, apk_name + '.size')
+  supersize_input_path = make_chromium_output_path(supersize_input_file)
+  size_path = make_staging_path(supersize_input_file) + '.size'
 
   supersize_script_path = os.path.join(_BINARY_SIZE_DIR, 'supersize')
 
@@ -62,7 +66,7 @@ def _generate_supersize_archive(apk_name, staging_dir,
           'archive',
           size_path,
           '-f',
-          apk_path,
+          supersize_input_path,
           '-v',
       ],
       check=True,
@@ -71,22 +75,47 @@ def _generate_supersize_archive(apk_name, staging_dir,
 
 def main():
   parser = argparse.ArgumentParser()
+
+  # A size config JSON specifies files relative to --chromium-output-directory.
+  # Its fields are:
+  # * mapping_files: A list of .mapping files, to be copied to --staging-dir for
+  #   SuperSize and trybot_commit_size_checker.py (indirectly). SuperSize
+  #   deduces mapping filenames; there's no need to pass these to it directly.
+  # * resource_size_args: A dict of arguments for resource_sizes.py. Its
+  #   sub-fields are:
+  #   * apk_name: Required main input, although for Trichrome this can be a
+  #     placeholder name.
+  #   * trichrome_library: --trichrome-library param (Trichrome only).
+  #   * trichrome_chrome: --trichrome-chrome param (Trichrome only).
+  #   * trichrome_webview: --trichrome-webview param (Trichrome only).
+  # * supersize_input_file: Main input for SuperSize, and can be {.apk,
+  #   .minimal.apks, .ssargs}. If .ssargs, then the file is copied to the
+  #   staging dir.
+  # * version: (Unused by this script) Used by build bots to determine whether
+  #   significant binary package restructure has occurred.
+
+  # --size-config-json will replace {--apk-name, --mapping-name}.
+  parser.add_argument('--size-config-json',
+                      help='Path to JSON file with configs for binary size '
+                      'measurement.')
+
+  # Deprecated.
   parser.add_argument(
       '--apk-name',
-      required=True,
       help='Name of the apk (ex. Name.apk)',
   )
-  parser.add_argument(
-      '--chromium-output-directory',
-      required=True,
-      help='Location of the build artifacts.',
-  )
+  # Deprecated.
   parser.add_argument(
       '--mapping-name',
-      required=True,
       action='append',
       help='Filename of the proguard mapping file.',
   )
+
+  parser.add_argument(
+      '--chromium-output-directory',
+      required=True,
+      help='Location of the build artifacts.',
+  )
   parser.add_argument(
       '--staging-dir',
       required=True,
@@ -95,21 +124,52 @@ def main():
 
   args = parser.parse_args()
 
-  _extract_proguard_mapping(
-      args.apk_name,
-      args.mapping_name,
-      args.staging_dir,
-      args.chromium_output_directory,
-  )
+  assert bool(args.size_config_json) != bool(
+      args.apk_name), ('Require exactly one of --size-config-json or the'
+                       ' {--apk-name, --mapping-name} group.')
+  assert bool(args.apk_name) == bool(args.mapping_name), (
+      'Require {--apk-name, --mapping-name} to be specified together.')
+
+  if args.size_config_json:
+    with open(args.size_config_json, 'rt') as fh:
+      config = json.load(fh)
+    to_resource_sizes_py = config['to_resource_sizes_py']
+    mapping_files = config['mapping_files']
+    supersize_input_file = config['supersize_input_file']
+  else:
+    # Deprecated flow. Add 'apks/' prefix for compatibility.
+    to_resource_sizes_py = {'apk_name': os.path.join('apks', args.apk_name)}
+    mapping_files = [os.path.join('apks', name) for name in args.mapping_name]
+    supersize_input_file = os.path.join('apks', args.apk_name)
+
+  def make_chromium_output_path(path_rel_to_output=None):
+    if path_rel_to_output is None:
+      return args.chromium_output_directory
+    return os.path.join(args.chromium_output_directory, path_rel_to_output)
+
+  # N.B. os.path.basename() usage.
+  def make_staging_path(path_rel_to_output=None):
+    if path_rel_to_output is None:
+      return args.staging_dir
+    return os.path.join(args.staging_dir, os.path.basename(path_rel_to_output))
+
+  files_to_copy = [make_chromium_output_path(f) for f in mapping_files]
+  # Copy size config JSON and .ssargs to staging dir to save settings used.
+  if args.size_config_json:
+    files_to_copy.append(args.size_config_json)
+  if supersize_input_file.endswith('.ssargs'):
+    files_to_copy.append(make_chromium_output_path(supersize_input_file))
+  _copy_files_to_staging_dir(files_to_copy, make_staging_path)
+
   _generate_resource_sizes(
-      args.apk_name,
-      args.staging_dir,
-      args.chromium_output_directory,
+      to_resource_sizes_py,
+      make_chromium_output_path,
+      make_staging_path,
   )
   _generate_supersize_archive(
-      args.apk_name,
-      args.staging_dir,
-      args.chromium_output_directory,
+      supersize_input_file,
+      make_chromium_output_path,
+      make_staging_path,
   )
 
 

tools/binary_size/trybot_commit_size_checker.py

@@ -123,9 +123,9 @@ def _CreateResourceSizesDelta(before_dir, after_dir):
       sizes_diff.summary_stat.value)
 
 
-def _CreateSupersizeDiff(apk_name, before_dir, after_dir):
-  before_size_path = os.path.join(before_dir, apk_name + '.size')
-  after_size_path = os.path.join(after_dir, apk_name + '.size')
+def _CreateSupersizeDiff(main_file_name, before_dir, after_dir):
+  before_size_path = os.path.join(before_dir, main_file_name + '.size')
+  after_size_path = os.path.join(after_dir, main_file_name + '.size')
   before = archive.LoadAndPostProcessSizeInfo(before_size_path)
   after = archive.LoadAndPostProcessSizeInfo(after_size_path)
   size_info_delta = diff.Diff(before, after, sort=True)
@@ -146,8 +146,7 @@ def _CreateUncompressedPakSizeDeltas(symbols):
   ]
 
 
-def _ExtractForTestingSymbolsFromMapping(mapping_path):
-  symbols = set()
+def _ExtractForTestingSymbolsFromSingleMapping(mapping_path):
   with open(mapping_path) as f:
     proguard_mapping_lines = f.readlines()
     current_class_orig = None
@@ -168,20 +167,26 @@ def _ExtractForTestingSymbolsFromMapping(mapping_path):
         method_symbol = '{}#{}'.format(
             match.group('original_method_class') or current_class_orig,
             match.group('original_method_name'))
-        symbols.add(method_symbol)
+        yield method_symbol
 
       match = _PROGUARD_FIELD_MAPPING_RE.search(line)
       if (match is not None
           and match.group('original_name').find('ForTest') > -1):
         field_symbol = '{}#{}'.format(current_class_orig,
                                       match.group('original_name'))
-        symbols.add(field_symbol)
+        yield field_symbol
+
+
+def _ExtractForTestingSymbolsFromMappings(mapping_paths):
+  symbols = set()
+  for mapping_path in mapping_paths:
+    symbols.update(_ExtractForTestingSymbolsFromSingleMapping(mapping_path))
   return symbols
 
 
-def _CreateTestingSymbolsDeltas(before_mapping_path, after_mapping_path):
-  before_symbols = _ExtractForTestingSymbolsFromMapping(before_mapping_path)
-  after_symbols = _ExtractForTestingSymbolsFromMapping(after_mapping_path)
+def _CreateTestingSymbolsDeltas(before_mapping_paths, after_mapping_paths):
+  before_symbols = _ExtractForTestingSymbolsFromMappings(before_mapping_paths)
+  after_symbols = _ExtractForTestingSymbolsFromMappings(after_mapping_paths)
   added_symbols = list(after_symbols.difference(before_symbols))
   removed_symbols = list(before_symbols.difference(after_symbols))
   lines = []
@@ -197,13 +202,14 @@ def _CreateTestingSymbolsDeltas(before_mapping_path, after_mapping_path):
                            len(added_symbols) - len(removed_symbols))
 
 
-def _GuessMappingFilename(results_dir, apk_name):
-  guess = apk_name + '.mapping'
-  if os.path.exists(os.path.join(results_dir, guess)):
+def _GuessMappingFilename(to_result_path, container_filename):
+  guess = to_result_path(container_filename + '.mapping')
+  if os.path.exists(guess):
     return guess
-  guess = (apk_name.replace('minimal.apks', '.aab').replace('.apks', '.aab') +
-           '.mapping')
-  if os.path.exists(os.path.join(results_dir, guess)):
+  guess = to_result_path(
+      container_filename.replace('.minimal.apks', '.aab').replace(
+          '.apks', '.aab') + '.mapping')
+  if os.path.exists(guess):
     return guess
   return None
 
@@ -249,8 +255,16 @@ def _FormatNumber(number):
 def main():
   parser = argparse.ArgumentParser()
   parser.add_argument('--author', required=True, help='CL author')
-  parser.add_argument(
-      '--apk-name', required=True, help='Name of the apk (ex. Name.apk)')
+
+  name_parser = parser.add_mutually_exclusive_group(required=True)
+
+  # --size-config-json-name will replace --apk-name.
+  name_parser.add_argument('--size-config-json-name',
+                           help='Filename of JSON with configs for '
+                           'binary size measurement.')
+  # Deprecated.
+  name_parser.add_argument('--apk-name', help='Name of the apk (ex. Name.apk).')
+
   parser.add_argument(
       '--before-dir',
       required=True,
@@ -273,9 +287,31 @@ def main():
   if args.verbose:
     logging.basicConfig(level=logging.INFO)
 
+  to_before_path = lambda p: os.path.join(args.before_dir, os.path.basename(p))
+  to_after_path = lambda p: os.path.join(args.after_dir, os.path.basename(p))
+  if args.size_config_json_name:
+    with open(to_after_path(args.size_config_json_name), 'rt') as fh:
+      config = json.load(fh)
+    supersize_input_name = os.path.basename(config['supersize_input_file'])
+    before_mapping_paths = [to_before_path(f) for f in config['mapping_files']]
+    after_mapping_paths = [to_after_path(f) for f in config['mapping_files']]
+  else:
+    supersize_input_name = args.apk_name
+    # Guess separately for "before" and "after" to be robust against naming
+    # glitches as generate_commit_size_analysis.py's renaming scheme change.
+    before_mapping_path = _GuessMappingFilename(to_before_path, args.apk_name)
+    if not before_mapping_path:
+      raise Exception('Cannot find "before" proguard mapping file.')
+    before_mapping_paths = [before_mapping_path]
+
+    after_mapping_path = _GuessMappingFilename(to_after_path, args.apk_name)
+    if not after_mapping_path:
+      raise Exception('Cannot find "after" proguard mapping file.')
+    after_mapping_paths = [after_mapping_path]
+
   logging.info('Creating Supersize diff')
   supersize_diff_lines, delta_size_info = _CreateSupersizeDiff(
-      args.apk_name, args.before_dir, args.after_dir)
+      supersize_input_name, args.before_dir, args.after_dir)
 
   changed_symbols = delta_size_info.raw_symbols.WhereDiffStatusIs(
       models.DIFF_STATUS_UNCHANGED).Inverted()
@@ -295,16 +331,10 @@ def main():
   size_deltas.add(mutable_constants_delta)
   metrics.add((mutable_constants_delta, _MUTABLE_CONSTANTS_LOG))
 
-  # Look for symbols with 'ForTesting' in their name.
+  # Look for symbols with 'ForTest' in their name.
   logging.info('Checking for DEX symbols named "ForTest"')
-  mapping_name = _GuessMappingFilename(args.before_dir, args.apk_name)
-  if not mapping_name:
-    raise Exception('Cannot find proguard mapping file.')
-
-  before_mapping = os.path.join(args.before_dir, mapping_name)
-  after_mapping = os.path.join(args.after_dir, mapping_name)
   testing_symbols_lines, test_symbols_delta = (_CreateTestingSymbolsDeltas(
-      before_mapping, after_mapping))
+      before_mapping_paths, after_mapping_paths))
   size_deltas.add(test_symbols_delta)
   metrics.add((test_symbols_delta, _FOR_TESTING_LOG))