Add check to ensure DexSplitter does not cause runtime errors

If an interface has only one implementer, R8 will merge the interface
with the implementer. This causes problems if DexSplitter moves this
class into a DFM, as the parent dex may have a reference to the
interface, which is now a reference to the implementation in the DFM.
This will cause a LinkageError at runtime, since the parent ClassLoader
will try to load the reference to a class found in the child
ClassLoader.

I timed the proguard step for monochrome_bundle on my machine locally,
the whole thing takes about 100s while _VerifySplitDexFiles takes about
3s. The verify step will only be run for bundles with isolated splits
enabled which use the chrome module (monochrome/trichrome chrome
bundles).

Bug: 1154065
Change-Id: I453f10354ca20f355c8a946ed611a9ff186ab50f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2567476
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832657}

build/android/gyp/proguard.py

@@ -325,10 +325,6 @@ def _OptimizeWithR8(options,
       raise build_utils.CalledProcessError(err.cwd, err.args,
                                            err.output + debugging_link)
 
-    if options.uses_split:
-      _SplitChildFeatures(options, feature_contexts, tmp_dir, tmp_mapping_path,
-                          print_stdout)
-
     base_has_imported_lib = False
     if options.desugar_jdk_libs_json:
       logging.debug('Running L8')
@@ -342,6 +338,10 @@ def _OptimizeWithR8(options,
           options.desugared_library_keep_rule_output, jdk_dex_output,
           options.warnings_as_errors)
 
+    if options.uses_split:
+      _SplitChildFeatures(options, feature_contexts, base_dex_context, tmp_dir,
+                          tmp_mapping_path, print_stdout)
+
     logging.debug('Collecting ouputs')
     base_dex_context.CreateOutput(base_has_imported_lib,
                                   options.desugared_library_keep_rule_output)
@@ -355,7 +355,11 @@ def _OptimizeWithR8(options,
       out_file.writelines(l for l in in_file if not l.startswith('#'))
 
 
-def _CheckForMissingSymbols(r8_path, dex_files, classpath, warnings_as_errors):
+def _CheckForMissingSymbols(r8_path,
+                            dex_files,
+                            classpath,
+                            warnings_as_errors,
+                            error_message=None):
   cmd = build_utils.JavaCmd(warnings_as_errors) + [
       '-cp', r8_path, 'com.android.tools.r8.tracereferences.TraceReferences',
       '--map-diagnostics:MissingDefinitionsDiagnostic', 'error', 'warning',
@@ -423,7 +427,8 @@ def _CheckForMissingSymbols(r8_path, dex_files, classpath, warnings_as_errors):
         stderr, '|'.join(re.escape(x) for x in ignored_lines))
     if stderr:
       if '  ' in stderr:
-        stderr = """
+        if error_message is None:
+          stderr = """
 DEX contains references to non-existent symbols after R8 optimization.
 Tip: Build with:
         is_java_debug=false
@@ -435,6 +440,8 @@ Tip: Build with:
        third_party/android_sdk/public/build-tools/*/dexdump -d \
 out/Release/apks/YourApk.apk > dex.txt
 """ + stderr
+        else:
+          stderr = error_message + stderr
       elif had_unfiltered_items:
         # Left only with empty headings. All indented items filtered out.
         stderr = ''
@@ -447,8 +454,8 @@ out/Release/apks/YourApk.apk > dex.txt
                           fail_on_output=warnings_as_errors)
 
 
-def _SplitChildFeatures(options, feature_contexts, tmp_dir, mapping_path,
-                        print_stdout):
+def _SplitChildFeatures(options, feature_contexts, base_dex_context, tmp_dir,
+                        mapping_path, print_stdout):
   feature_map = {f.name: f for f in feature_contexts}
   parent_to_child = defaultdict(list)
   for child, parent in options.uses_split.items():
@@ -505,6 +512,37 @@ def _SplitChildFeatures(options, feature_contexts, tmp_dir, mapping_path,
         shutil.move(os.path.join(child_split_output, dex_file),
                     os.path.join(child_staging_dir, dex_file))
 
+  if not options.disable_checks:
+    logging.debug('Verifying dex files')
+    _VerifySplitDexFiles(parent_to_child, feature_map, base_dex_context,
+                         options)
+
+
+def _VerifySplitDexFiles(parent_to_child, feature_map, base_dex_context,
+                         options):
+  def list_dex_files(feature):
+    staging_dir = feature.staging_dir
+    return [os.path.join(staging_dir, f) for f in os.listdir(staging_dir)]
+
+  # This list will only have "chrome" as the parent for now, unless other splits
+  # are used as targets of uses_split in the future. We only care about running
+  # on the chrome split because DexSplitter was used to split out the DFMs that
+  # depend on it, and may have pulled too much into the DFM. This is not a
+  # problem for the base split because DFMs are pulled out of base using R8,
+  # which shouldn't mess anything up.
+  for parent in parent_to_child:
+    error_message = """
+Classes in a DFM may have been merged into the interface they implement.
+DexSplitter does not support unmerging interfaces, so @DoNotInline may need to
+be added to the interfaces implemented by the classes below.
+"""
+    _CheckForMissingSymbols(options.r8_path,
+                            list_dex_files(feature_map[parent]) +
+                            list_dex_files(base_dex_context),
+                            options.classpath,
+                            options.warnings_as_errors,
+                            error_message=error_message)
+
 
 def _CombineConfigs(configs, dynamic_config_data, exclude_generated=False):
   ret = []