Fix ClassLoader mismatch for modules which depend on the chrome module

I noticed most of the exceptions when loading the vr module have logs
from SplitCompatAppComponentFactory, which means the chrome ClassLoader
is messed up in the ClassLoader cache. When the vr module is loaded, it
uses the wrong chrome ClassLoader as the vr ClassLoader parent, which
ends up causing a ClassCastException. This CL detects that case and
creates a fresh ClassLoader with the correct parent.

I tested this locally by faking the situation where the chrome
ClassLoader is incorrect, and this loads the vr module and other DFMs
which depend on chrome (e.g. image_editor) without crashing.

Bug: 1146745
Change-Id: I5960fa2817604a13b90d80b10517b3b416c46013
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2620939Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842167}

base/android/java/src/org/chromium/base/BundleUtils.java

@@ -5,17 +5,22 @@
 package org.chromium.base;
 
 import android.content.Context;
+import android.content.ContextWrapper;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.os.Build;
 
 import androidx.annotation.Nullable;
+import androidx.collection.SimpleArrayMap;
 
 import dalvik.system.BaseDexClassLoader;
+import dalvik.system.PathClassLoader;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.compat.ApiHelperForO;
+import org.chromium.base.metrics.RecordHistogram;
 
+import java.lang.reflect.Field;
 import java.util.Arrays;
 
 /**
@@ -38,6 +43,11 @@ import java.util.Arrays;
 public final class BundleUtils {
     private static Boolean sIsBundle;
 
+    // This cache is needed to support the workaround for b/172602571, see
+    // createIsolatedSplitContext() for more info.
+    private static final SimpleArrayMap<String, ClassLoader> sCachedClassLoaders =
+            new SimpleArrayMap<>();
+
     /**
      * {@link BundleUtils#isBundle()}  is not called directly by native because
      * {@link CalledByNative} prevents inlining, causing the bundle support lib to not be
@@ -98,12 +108,56 @@ public final class BundleUtils {
         }
 
         try {
-            return ApiHelperForO.createContextForSplit(base, splitName);
+            Context context = ApiHelperForO.createContextForSplit(base, splitName);
+            ClassLoader parent = context.getClassLoader().getParent();
+            Context appContext = ContextUtils.getApplicationContext();
+            // If the ClassLoader from the newly created context does not equal either the
+            // BundleUtils ClassLoader (the base module ClassLoader) or the app context ClassLoader
+            // (the chrome module ClassLoader) there must be something messed up in the ClassLoader
+            // cache, see b/172602571. This should be solved for the chrome ClassLoader by
+            // SplitCompatAppComponentFactory, but modules which depend on the chrome module need
+            // special handling here to make sure they have the correct parent.
+            boolean shouldReplaceClassLoader = isolatedSplitsEnabled()
+                    && !parent.equals(BundleUtils.class.getClassLoader()) && appContext != null
+                    && !parent.equals(appContext.getClassLoader());
+            if (shouldReplaceClassLoader) {
+                if (!sCachedClassLoaders.containsKey(splitName)) {
+                    String[] splitNames = ApiHelperForO.getSplitNames(context.getApplicationInfo());
+                    int idx = Arrays.binarySearch(splitNames, splitName);
+                    assert idx >= 0;
+                    // The librarySearchPath argument to PathClassLoader is not needed here because
+                    // the framework doesn't pass it either, see b/171269960.
+                    sCachedClassLoaders.put(splitName,
+                            new PathClassLoader(context.getApplicationInfo().splitSourceDirs[idx],
+                                    appContext.getClassLoader()));
+                }
+                replaceClassLoader(context, sCachedClassLoaders.get(splitName));
+            }
+            RecordHistogram.recordBooleanHistogram(
+                    "Android.IsolatedSplits.ClassLoaderReplaced." + splitName,
+                    shouldReplaceClassLoader);
+            return context;
         } catch (PackageManager.NameNotFoundException e) {
             throw new RuntimeException(e);
         }
     }
 
+    /** Replaces the ClassLoader of the passed in Context. */
+    public static void replaceClassLoader(Context baseContext, ClassLoader classLoader) {
+        while (baseContext instanceof ContextWrapper) {
+            baseContext = ((ContextWrapper) baseContext).getBaseContext();
+        }
+
+        try {
+            // baseContext should now be an instance of ContextImpl.
+            Field classLoaderField = baseContext.getClass().getDeclaredField("mClassLoader");
+            classLoaderField.setAccessible(true);
+            classLoaderField.set(baseContext, classLoader);
+        } catch (ReflectiveOperationException e) {
+            throw new RuntimeException("Error setting ClassLoader.", e);
+        }
+    }
+
     /* Returns absolute path to a native library in a feature module. */
     @CalledByNative
     @Nullable

chrome/android/java/src/org/chromium/chrome/browser/base/SplitChromeApplication.java

@@ -9,19 +9,17 @@ import static org.chromium.chrome.browser.base.SplitCompatUtils.CHROME_SPLIT_NAM
 import android.annotation.SuppressLint;
 import android.app.Activity;
 import android.content.Context;
-import android.content.ContextWrapper;
 import android.content.pm.PackageManager;
 import android.os.Build;
 import android.os.SystemClock;
 
 import org.chromium.base.ActivityState;
 import org.chromium.base.ApplicationStatus;
+import org.chromium.base.BundleUtils;
 import org.chromium.base.JNIUtils;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.metrics.RecordHistogram;
 
-import java.lang.reflect.Field;
-
 /**
  * Application class to use for Chrome when //chrome code is in an isolated split. This class will
  * perform any necessary initialization for non-browser processes without loading code from the
@@ -101,7 +99,10 @@ public class SplitChromeApplication extends SplitCompatApplication {
             // chromeContext will have the same ClassLoader as the base context, so no need to
             // replace the ClassLoaders here.
             if (!context.getClassLoader().equals(chromeContext.getClassLoader())) {
-                replaceClassLoader(this, chromeContext.getClassLoader());
+                // Replace the application Context's ClassLoader with the chrome ClassLoader,
+                // because the application ClassLoader is expected to be able to access all chrome
+                // classes.
+                BundleUtils.replaceClassLoader(this, chromeContext.getClassLoader());
                 JNIUtils.setClassLoader(chromeContext.getClassLoader());
             }
         });
@@ -143,24 +144,9 @@ public class SplitChromeApplication extends SplitCompatApplication {
                             return;
                         }
 
-                        replaceClassLoader(
+                        BundleUtils.replaceClassLoader(
                                 activity.getBaseContext(), activity.getClass().getClassLoader());
                     }
                 });
     }
-
-    private static void replaceClassLoader(Context baseContext, ClassLoader classLoader) {
-        while (baseContext instanceof ContextWrapper) {
-            baseContext = ((ContextWrapper) baseContext).getBaseContext();
-        }
-
-        try {
-            // baseContext should now be an instance of ContextImpl.
-            Field classLoaderField = baseContext.getClass().getDeclaredField("mClassLoader");
-            classLoaderField.setAccessible(true);
-            classLoaderField.set(baseContext, classLoader);
-        } catch (ReflectiveOperationException e) {
-            throw new RuntimeException("Error setting ClassLoader.", e);
-        }
-    }
 }

tools/metrics/histograms/histograms_xml/android/histograms.xml

@@ -1188,6 +1188,18 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram base="true" name="Android.IsolatedSplits.ClassLoaderReplaced"
+    enum="BooleanYesNo" expires_after="2022-01-10">
+<!-- Name completed by histogram_suffixes name="AndroidFeatureModuleName" -->
+
+  <owner>cduvall@chromium.org</owner>
+  <owner>agrieve@chromium.org</owner>
+  <summary>
+    Whether a split Context has had its ClassLoader replaced due to b/172602571.
+    This is recorded every time a split Context is created.
+  </summary>
+</histogram>
+
 <histogram base="true" name="Android.IsolatedSplits.ContextCreateTime"
     units="ms" expires_after="2021-12-07">
 <!-- Name completed by histogram_suffixes name="AndroidFeatureModuleName" -->

tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml

@@ -775,6 +775,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <suffix name="chrome" label="Chrome Module"/>
   <suffix name="dev_ui" label="Developer UI Module"/>
   <suffix name="extra_icu" label="Extra ICU Module"/>
+  <suffix name="feedv2" label="Feed V2 Module"/>
   <suffix name="image_editor" label="Image Editor Module"/>
   <suffix name="stack_unwinder" label="Stack Unwinder Module"/>
   <suffix name="tab_ui" label="Tab Management Module"/>
@@ -828,6 +829,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
       name="Android.FeatureModules.UncachedInstallDuration.PendingDownload"/>
   <affected-histogram
       name="Android.FeatureModules.UncachedInstallDuration.PendingInstall"/>
+  <affected-histogram name="Android.IsolatedSplits.ClassLoaderReplaced"/>
   <affected-histogram name="Android.IsolatedSplits.ContextCreateTime"/>
   <affected-histogram name="Android.IsolatedSplits.PreloadWaitTime"/>
 </histogram_suffixes>