Revert "Revert "[DexLoadReporter] Report classloader contexts di..."

Original commit:
[DexLoadReporter] Report classloader contexts directly from classloader

At the moment classloader contexts are incorrectly computed in the
PackageManager for secondary dex files. There are two issues:

(1) The wrong computed classLoaderContext will be reported for a secondary
    dex file if it was loaded at the same time as a primary dex file
    - This is due to the continue statement that doesn't increment
      dexPathIndex
(2) If a secondary dex file was loaded with a shared library then that
    shared library info isn't passed through the dex load reporting
    infrastructure, and thus its classloader context is incorrectly computed
    in PackageManager.

In order to fix the issues described above & prevent further classloader
context computation divergences between the package manager and the
runtime, lets compute the classloader context in the runtime at dex load
time and report the expected classloader context directly to
DexLoadReporter (and thus the package manager).

Notes: This is mostly just a refactor (i.e. there are a lot of line
changes, but functionally speaking this set of CLs doesn't do much
except change where the classloader context is computed)

Addendum: The bugs described above could also be fixed by:
- changing DexLoadReporter to report information about shared libraries that
  the reported classloaders depend on to PackageManager
- Teach DexoptUtils.processContextForDexLoad about shared libraries
- Fix dexPathIndex calculation in DexManager

I opted for this set of changes instead because this reduces the
possibility of context computation divergence between the framework and the
runtime. Additionally it feels more "solid" that the classloader context
is now computed directly when a dex file is loaded, rather than the
context recreated later on in the PackageManager.

Test: atest com.android.server.pm.dex.DexManagerTests
Test: atest com.android.server.pm.PackageManagerServiceTest
Test: Install app depending on shared library & uses secondary dex
files; adb shell pm bg-dexopt-job; launch app and see odex file
successfully loaded (from smaps/no logcat errors)

Bug: 148494302
Exempt-From-Owner-Approval: This is a pure re-revert, previously owner approved.
Reason for revert: Re-land
Reverted Changes:
I295a6e99e:Revert "Fix shared libraries not being reported vi...
Ib58066e8f:Revert "[DexLoadReporter] Report classloader conte...

Change-Id: I8d1af791f93a3f8fa6eca78df50891cd2ebbb4a3

6 files changed, 137 insertions, 100 deletions

diff --git a/core/java/android/app/DexLoadReporter.java b/core/java/android/app/DexLoadReporter.java
index 229bee55e911..5bc999240a66 100644
--- a/core/java/android/app/DexLoadReporter.java
+++ b/core/java/android/app/DexLoadReporter.java
@@ -28,9 +28,8 @@ import dalvik.system.VMRuntime;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -87,50 +86,32 @@ import java.util.Set;
     }
 
     @Override
-    public void report(List<ClassLoader> classLoadersChain, List<String> classPaths) {
-        if (classLoadersChain.size() != classPaths.size()) {
-            Slog.wtf(TAG, "Bad call to DexLoadReporter: argument size mismatch");
-            return;
-        }
-        if (classPaths.isEmpty()) {
-            Slog.wtf(TAG, "Bad call to DexLoadReporter: empty dex paths");
-            return;
-        }
-
-        // The first element of classPaths is the list of dex files that should be registered.
-        // The classpath is represented as a list of dex files separated by File.pathSeparator.
-        String[] dexPathsForRegistration = classPaths.get(0).split(File.pathSeparator);
-        if (dexPathsForRegistration.length == 0) {
-            // No dex files to register.
+    public void report(Map<String, String> classLoaderContextMap) {
+        if (classLoaderContextMap.isEmpty()) {
+            Slog.wtf(TAG, "Bad call to DexLoadReporter: empty classLoaderContextMap");
             return;
         }
 
         // Notify the package manager about the dex loads unconditionally.
         // The load might be for either a primary or secondary dex file.
-        notifyPackageManager(classLoadersChain, classPaths);
+        notifyPackageManager(classLoaderContextMap);
         // Check for secondary dex files and register them for profiling if possible.
         // Note that we only register the dex paths belonging to the first class loader.
-        registerSecondaryDexForProfiling(dexPathsForRegistration);
+        registerSecondaryDexForProfiling(classLoaderContextMap.keySet());
     }
 
-    private void notifyPackageManager(List<ClassLoader> classLoadersChain,
-            List<String> classPaths) {
+    private void notifyPackageManager(Map<String, String> classLoaderContextMap) {
         // Get the class loader names for the binder call.
-        List<String> classLoadersNames = new ArrayList<>(classPaths.size());
-        for (ClassLoader classLoader : classLoadersChain) {
-            classLoadersNames.add(classLoader.getClass().getName());
-        }
         String packageName = ActivityThread.currentPackageName();
         try {
-            ActivityThread.getPackageManager().notifyDexLoad(
-                    packageName, classLoadersNames, classPaths,
-                    VMRuntime.getRuntime().vmInstructionSet());
+            ActivityThread.getPackageManager().notifyDexLoad(packageName,
+                    classLoaderContextMap, VMRuntime.getRuntime().vmInstructionSet());
         } catch (RemoteException re) {
             Slog.e(TAG, "Failed to notify PM about dex load for package " + packageName, re);
         }
     }
 
-    private void registerSecondaryDexForProfiling(String[] dexPaths) {
+    private void registerSecondaryDexForProfiling(Set<String> dexPaths) {
         if (!SystemProperties.getBoolean("dalvik.vm.dexopt.secondary", false)) {
             return;
         }
diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl
index 429a6e51ccb4..6d051e47c716 100644
--- a/core/java/android/content/pm/IPackageManager.aidl
+++ b/core/java/android/content/pm/IPackageManager.aidl
@@ -529,19 +529,12 @@ interface IPackageManager {
      * Notify the package manager that a list of dex files have been loaded.
      *
      * @param loadingPackageName the name of the package who performs the load
-     * @param classLoadersNames the names of the class loaders present in the loading chain. The
-     *    list encodes the class loader chain in the natural order. The first class loader has
-     *    the second one as its parent and so on. The dex files present in the class path of the
-     *    first class loader will be recorded in the usage file.
-     * @param classPaths the class paths corresponding to the class loaders names from
-     *     {@param classLoadersNames}. The the first element corresponds to the first class loader
-     *     and so on. A classpath is represented as a list of dex files separated by
-     *     {@code File.pathSeparator}, or null if the class loader's classpath is not known.
-     *     The dex files found in the first class path will be recorded in the usage file.
+     * @param classLoaderContextMap a map from file paths to dex files that have been loaded to
+     *     the class loader context that was used to load them.
      * @param loaderIsa the ISA of the loader process
      */
-    oneway void notifyDexLoad(String loadingPackageName, in List<String> classLoadersNames,
-            in List<String> classPaths, String loaderIsa);
+    oneway void notifyDexLoad(String loadingPackageName,
+            in Map<String, String> classLoaderContextMap, String loaderIsa);
 
     /**
      * Register an application dex module with the package manager.
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index d8f5dfbbaf48..709811ed520d 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -9764,8 +9764,8 @@ public class PackageManagerService extends IPackageManager.Stub
     }
 
     @Override
-    public void notifyDexLoad(String loadingPackageName, List<String> classLoaderNames,
-            List<String> classPaths, String loaderIsa) {
+    public void notifyDexLoad(String loadingPackageName, Map<String, String> classLoaderContextMap,
+            String loaderIsa) {
         int userId = UserHandle.getCallingUserId();
         ApplicationInfo ai = getApplicationInfo(loadingPackageName, /*flags*/ 0, userId);
         if (ai == null) {
@@ -9773,7 +9773,7 @@ public class PackageManagerService extends IPackageManager.Stub
                 + loadingPackageName + ", user=" + userId);
             return;
         }
-        mDexManager.notifyDexLoad(ai, classLoaderNames, classPaths, loaderIsa, userId);
+        mDexManager.notifyDexLoad(ai, classLoaderContextMap, loaderIsa, userId);
     }
 
     @Override
diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java
index 9e86a4bd2880..441fa75cd912 100644
--- a/services/core/java/com/android/server/pm/dex/DexManager.java
+++ b/services/core/java/com/android/server/pm/dex/DexManager.java
@@ -44,6 +44,8 @@ import com.android.server.pm.PackageDexOptimizer;
 import com.android.server.pm.PackageManagerService;
 import com.android.server.pm.PackageManagerServiceUtils;
 
+import dalvik.system.VMRuntime;
+
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
@@ -143,22 +145,15 @@ public class DexManager {
      * return as fast as possible.
      *
      * @param loadingAppInfo the package performing the load
-     * @param classLoadersNames the names of the class loaders present in the loading chain. The
-     *    list encodes the class loader chain in the natural order. The first class loader has
-     *    the second one as its parent and so on. The dex files present in the class path of the
-     *    first class loader will be recorded in the usage file.
-     * @param classPaths the class paths corresponding to the class loaders names from
-     *     {@param classLoadersNames}. The the first element corresponds to the first class loader
-     *     and so on. A classpath is represented as a list of dex files separated by
-     *     {@code File.pathSeparator}, or null if the class loader's classpath is not known.
-     *     The dex files found in the first class path will be recorded in the usage file.
+     * @param classLoaderContextMap a map from file paths to dex files that have been loaded to
+     *     the class loader context that was used to load them.
      * @param loaderIsa the ISA of the app loading the dex files
      * @param loaderUserId the user id which runs the code loading the dex files
      */
-    public void notifyDexLoad(ApplicationInfo loadingAppInfo, List<String> classLoadersNames,
-            List<String> classPaths, String loaderIsa, int loaderUserId) {
+    public void notifyDexLoad(ApplicationInfo loadingAppInfo,
+            Map<String, String> classLoaderContextMap, String loaderIsa, int loaderUserId) {
         try {
-            notifyDexLoadInternal(loadingAppInfo, classLoadersNames, classPaths, loaderIsa,
+            notifyDexLoadInternal(loadingAppInfo, classLoaderContextMap, loaderIsa,
                     loaderUserId);
         } catch (Exception e) {
             Slog.w(TAG, "Exception while notifying dex load for package " +
@@ -168,46 +163,23 @@ public class DexManager {
 
     @VisibleForTesting
     /*package*/ void notifyDexLoadInternal(ApplicationInfo loadingAppInfo,
-            List<String> classLoaderNames, List<String> classPaths, String loaderIsa,
+            Map<String, String> classLoaderContextMap, String loaderIsa,
             int loaderUserId) {
-        if (classLoaderNames.size() != classPaths.size()) {
-            Slog.wtf(TAG, "Bad call to noitfyDexLoad: args have different size");
+        if (classLoaderContextMap == null) {
             return;
         }
-        if (classLoaderNames.isEmpty()) {
+        if (classLoaderContextMap.isEmpty()) {
             Slog.wtf(TAG, "Bad call to notifyDexLoad: class loaders list is empty");
             return;
         }
         if (!PackageManagerServiceUtils.checkISA(loaderIsa)) {
-            Slog.w(TAG, "Loading dex files " + classPaths + " in unsupported ISA: " +
-                    loaderIsa + "?");
+            Slog.w(TAG, "Loading dex files " + classLoaderContextMap.keySet()
+                    + " in unsupported ISA: " + loaderIsa + "?");
             return;
         }
 
-        // The first classpath should never be null because the first classloader
-        // should always be an instance of BaseDexClassLoader.
-        String firstClassPath = classPaths.get(0);
-        if (firstClassPath == null) {
-            return;
-        }
-        // The classpath is represented as a list of dex files separated by File.pathSeparator.
-        String[] dexPathsToRegister = firstClassPath.split(File.pathSeparator);
-
-        // Encode the class loader contexts for the dexPathsToRegister.
-        String[] classLoaderContexts = DexoptUtils.processContextForDexLoad(
-                classLoaderNames, classPaths);
-
-        // A null classLoaderContexts means that there are unsupported class loaders in the
-        // chain.
-        if (classLoaderContexts == null) {
-            if (DEBUG) {
-                Slog.i(TAG, loadingAppInfo.packageName +
-                        " uses unsupported class loader in " + classLoaderNames);
-            }
-        }
-
-        int dexPathIndex = 0;
-        for (String dexPath : dexPathsToRegister) {
+        for (Map.Entry<String, String> mapping : classLoaderContextMap.entrySet()) {
+            String dexPath = mapping.getKey();
             // Find the owning package name.
             DexSearchResult searchResult = getDexPackage(loadingAppInfo, dexPath, loaderUserId);
 
@@ -229,7 +201,6 @@ public class DexManager {
                     // If the dex file is the primary apk (or a split) and not isUsedByOtherApps
                     // do not record it. This case does not bring any new usable information
                     // and can be safely skipped.
-                    dexPathIndex++;
                     continue;
                 }
 
@@ -239,13 +210,13 @@ public class DexManager {
                             searchResult.mOwningPackageName, loadingAppInfo.packageName);
                 }
 
-                if (classLoaderContexts != null) {
-
+                String classLoaderContext = mapping.getValue();
+                if (classLoaderContext != null
+                        && VMRuntime.isValidClassLoaderContext(classLoaderContext)) {
                     // Record dex file usage. If the current usage is a new pattern (e.g. new
                     // secondary, or UsedByOtherApps), record will return true and we trigger an
                     // async write to disk to make sure we don't loose the data in case of a reboot.
 
-                    String classLoaderContext = classLoaderContexts[dexPathIndex];
                     if (mPackageDexUsage.record(searchResult.mOwningPackageName,
                             dexPath, loaderUserId, loaderIsa, isUsedByOtherApps, primaryOrSplit,
                             loadingAppInfo.packageName, classLoaderContext)) {
@@ -259,7 +230,6 @@ public class DexManager {
                     Slog.i(TAG, "Could not find owning package for dex file: " + dexPath);
                 }
             }
-            dexPathIndex++;
         }
     }
 
diff --git a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java
index e68c2386e03b..08763e729c71 100644
--- a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java
+++ b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java
@@ -83,8 +83,9 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
             "=UnknownClassLoaderContext=";
 
     // The marker used for unsupported class loader contexts (no longer written, may occur in old
-    // files so discarded on read).
-    private static final String UNSUPPORTED_CLASS_LOADER_CONTEXT =
+    // files so discarded on read). Note: this matches
+    // ClassLoaderContext::kUnsupportedClassLoaderContextEncoding in the runtime.
+    /*package*/ static final String UNSUPPORTED_CLASS_LOADER_CONTEXT =
             "=UnsupportedClassLoaderContext=";
 
     /**
@@ -133,6 +134,9 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
         if (classLoaderContext == null) {
             throw new IllegalArgumentException("Null classLoaderContext");
         }
+        if (classLoaderContext.equals(UNSUPPORTED_CLASS_LOADER_CONTEXT)) {
+            return false;
+        }
 
         synchronized (mPackageUseInfoMap) {
             PackageUseInfo packageUseInfo = mPackageUseInfoMap.get(owningPackageName);
@@ -843,10 +847,11 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
             boolean updateLoadingPackages = mLoadingPackages.addAll(dexUseInfo.mLoadingPackages);
 
             String oldClassLoaderContext = mClassLoaderContext;
-            if (UNKNOWN_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext)) {
+            if (isUnknownOrUnsupportedContext(mClassLoaderContext)) {
                 // Can happen if we read a previous version.
                 mClassLoaderContext = dexUseInfo.mClassLoaderContext;
-            } else if (!Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) {
+            } else if (!isUnknownOrUnsupportedContext(dexUseInfo.mClassLoaderContext)
+                        && !Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) {
                 // We detected a context change.
                 mClassLoaderContext = VARIABLE_CLASS_LOADER_CONTEXT;
             }
@@ -857,6 +862,13 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
                     || !Objects.equals(oldClassLoaderContext, mClassLoaderContext);
         }
 
+        private static boolean isUnknownOrUnsupportedContext(String context) {
+            // TODO: Merge UNKNOWN_CLASS_LOADER_CONTEXT & UNSUPPORTED_CLASS_LOADER_CONTEXT cases
+            // into UNSUPPORTED_CLASS_LOADER_CONTEXT.
+            return UNKNOWN_CLASS_LOADER_CONTEXT.equals(context)
+                    || UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(context);
+        }
+
         public boolean isUsedByOtherApps() {
             return mIsUsedByOtherApps;
         }
@@ -878,7 +890,7 @@ public class PackageDexUsage extends AbstractStatsBase<Void> {
         public boolean isUnknownClassLoaderContext() {
             // The class loader context may be unknown if we loaded the data from a previous version
             // which didn't save the context.
-            return UNKNOWN_CLASS_LOADER_CONTEXT.equals(mClassLoaderContext);
+            return isUnknownOrUnsupportedContext(mClassLoaderContext);
         }
 
         public boolean isVariableClassLoaderContext() {
diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java
index a4ba056b96a8..cb20b65c9f9b 100644
--- a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java
+++ b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java
@@ -532,6 +532,61 @@ public class DexManagerTests {
         assertHasDclInfo(mBarUser0, mBarUser0, secondaries);
     }
 
+    @Test
+    public void testPrimaryAndSecondaryDexLoad() {
+        // Foo loads both primary and secondary dexes
+        List<String> fooSecondaries = mFooUser0.getSecondaryDexPaths();
+        List<String> fooDexes = new ArrayList<>(mFooUser0.getBaseAndSplitDexPaths());
+        int primaryCount = fooDexes.size();
+        fooDexes.addAll(fooSecondaries);
+
+        notifyDexLoad(mFooUser0, fooDexes, mUser0);
+
+        PackageUseInfo pui = getPackageUseInfo(mFooUser0);
+        assertIsUsedByOtherApps(mFooUser0, pui, false);
+        assertEquals(fooSecondaries.size(), pui.getDexUseInfoMap().size());
+
+        // Below we want to verify that the secondary dex files within fooDexes have been correctly
+        // reported and their class loader contexts were correctly recorded.
+        //
+        // In order to achieve this we first use DexoptUtils.processContextForDexLoad to compute the
+        // class loader contexts for all the dex files.
+        String[] allClassLoaderContexts = DexoptUtils.processContextForDexLoad(
+                Arrays.asList(mFooUser0.mClassLoader),
+                Arrays.asList(String.join(File.pathSeparator, fooDexes)));
+        // Next we filter out the class loader contexts corresponding to non-secondary dex files.
+        String[] secondaryClassLoaderContexts = Arrays.copyOfRange(allClassLoaderContexts,
+                primaryCount, allClassLoaderContexts.length);
+        assertSecondaryUse(mFooUser0, pui, fooSecondaries, /*isUsedByOtherApps*/false, mUser0,
+                secondaryClassLoaderContexts);
+
+        assertHasDclInfo(mFooUser0, mFooUser0, fooSecondaries);
+    }
+
+    @Test
+    public void testNotifySecondary_withSharedLibrary() {
+        // Foo loads its own secondary files.
+        List<String> fooSecondaries = mFooUser0.getSecondaryDexPaths();
+
+        String contextSuffix = "{PCL[/system/framework/org.apache.http.legacy.jar]}";
+        String[] expectedContexts = DexoptUtils.processContextForDexLoad(
+                Arrays.asList(mFooUser0.mClassLoader),
+                Arrays.asList(String.join(File.pathSeparator, fooSecondaries)));
+        for (int i = 0; i < expectedContexts.length; i++) {
+            expectedContexts[i] += contextSuffix;
+        }
+
+        notifyDexLoad(mFooUser0, fooSecondaries, expectedContexts, mUser0);
+
+        PackageUseInfo pui = getPackageUseInfo(mFooUser0);
+        assertIsUsedByOtherApps(mFooUser0, pui, false);
+        assertEquals(fooSecondaries.size(), pui.getDexUseInfoMap().size());
+        assertSecondaryUse(mFooUser0, pui, fooSecondaries, /*isUsedByOtherApps*/false, mUser0,
+                expectedContexts);
+
+        assertHasDclInfo(mFooUser0, mFooUser0, fooSecondaries);
+    }
+
     private void assertSecondaryUse(TestData testData, PackageUseInfo pui,
             List<String> secondaries, boolean isUsedByOtherApps, int ownerUserId,
             String[] expectedContexts) {
@@ -572,17 +627,43 @@ public class DexManagerTests {
         // By default, assume a single class loader in the chain.
         // This makes writing tests much easier.
         List<String> classLoaders = Arrays.asList(testData.mClassLoader);
-        List<String> classPaths = (dexPaths == null)
-                                  ? Arrays.asList((String) null)
-                                  : Arrays.asList(String.join(File.pathSeparator, dexPaths));
+        List<String> classPaths = dexPaths != null
+                ? Arrays.<String>asList(String.join(File.pathSeparator, dexPaths)) : null;
         notifyDexLoad(testData, classLoaders, classPaths, loaderUserId);
     }
 
     private void notifyDexLoad(TestData testData, List<String> classLoaders,
             List<String> classPaths, int loaderUserId) {
+        String[] classLoaderContexts = computeClassLoaderContexts(classLoaders, classPaths);
         // We call the internal function so any exceptions thrown cause test failures.
-        mDexManager.notifyDexLoadInternal(testData.mPackageInfo.applicationInfo, classLoaders,
-                classPaths, testData.mLoaderIsa, loaderUserId);
+        List<String> dexPaths = classPaths != null
+                ? Arrays.asList(classPaths.get(0).split(File.pathSeparator)) : Arrays.asList();
+        notifyDexLoad(testData, dexPaths, classLoaderContexts, loaderUserId);
+    }
+
+    private void notifyDexLoad(TestData testData, List<String> dexPaths,
+            String[] classLoaderContexts, int loaderUserId) {
+        assertTrue(dexPaths.size() == classLoaderContexts.length);
+        HashMap<String, String> dexPathMapping = new HashMap<>(dexPaths.size());
+        for (int i = 0; i < dexPaths.size(); i++) {
+            dexPathMapping.put(dexPaths.get(i), classLoaderContexts != null
+                    ? classLoaderContexts[i] : PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT);
+        }
+        mDexManager.notifyDexLoadInternal(testData.mPackageInfo.applicationInfo, dexPathMapping,
+                testData.mLoaderIsa, loaderUserId);
+    }
+
+    private String[] computeClassLoaderContexts(List<String> classLoaders,
+            List<String> classPaths) {
+        if (classPaths == null) {
+            return new String[0];
+        }
+        String[] results = DexoptUtils.processContextForDexLoad(classLoaders, classPaths);
+        if (results == null) {
+            results = new String[classPaths.get(0).split(File.pathSeparator).length];
+            Arrays.fill(results, PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT);
+        }
+        return results;
     }
 
     private PackageUseInfo getPackageUseInfo(TestData testData) {