Merge "Ensure dexopt is executed only with consistent package data" into oc-mr1-dev

am: 5ba8e3dc78

Change-Id: I1236a8f4bc1a11abde32e9462aefcba6d88e1854

services/core/java/com/android/server/pm/PackageDexOptimizer.java

@@ -39,6 +39,7 @@ import com.android.server.pm.dex.PackageDexUsage;
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -149,6 +150,17 @@ public class PackageDexOptimizer {
         String[] classLoaderContexts = DexoptUtils.getClassLoaderContexts(
                 pkg.applicationInfo, sharedLibraries);
 
+        // Sanity check that we do not call dexopt with inconsistent data.
+        if (paths.size() != classLoaderContexts.length) {
+            String[] splitCodePaths = pkg.applicationInfo.getSplitCodePaths();
+            throw new IllegalStateException("Inconsistent information "
+                + "between PackageParser.Package and its ApplicationInfo. "
+                + "pkg.getAllCodePaths=" + paths
+                + " pkg.applicationInfo.getBaseCodePath=" + pkg.applicationInfo.getBaseCodePath()
+                + " pkg.applicationInfo.getSplitCodePaths="
+                + (splitCodePaths == null ? "null" : Arrays.toString(splitCodePaths)));
+        }
+
         int result = DEX_OPT_SKIPPED;
         for (int i = 0; i < paths.size(); i++) {
             // Skip paths that have no code.

services/core/java/com/android/server/pm/PackageManagerService.java

@@ -18606,15 +18606,35 @@ public class PackageManagerService extends IPackageManager.Stub
                     Slog.e(TAG, "updateAllSharedLibrariesLPw failed: " + e.getMessage());
                 }
             }
+        }
 
-            // dexopt can take some time to complete, so, for instant apps, we skip this
+        if (!args.doRename(res.returnCode, pkg, oldCodePath)) {
-            // step during installation. Instead, we'll take extra time the first time the
+            res.setError(INSTALL_FAILED_INSUFFICIENT_STORAGE, "Failed rename");
-            // instant app starts. It's preferred to do it this way to provide continuous
+            return;
-            // progress to the user instead of mysteriously blocking somewhere in the
+        }
+
+        // Verify if we need to dexopt the app.
+        //
+        // NOTE: it is *important* to call dexopt after doRename which will sync the
+        // package data from PackageParser.Package and its corresponding ApplicationInfo.
+        //
+        // We only need to dexopt if the package meets ALL of the following conditions:
+        //   1) it is not forward locked.
+        //   2) it is not on on an external ASEC container.
+        //   3) it is not an instant app or if it is then dexopt is enabled via gservices.
+        //
+        // Note that we do not dexopt instant apps by default. dexopt can take some time to
+        // complete, so we skip this step during installation. Instead, we'll take extra time
+        // the first time the instant app starts. It's preferred to do it this way to provide
+        // continuous progress to the useur instead of mysteriously blocking somewhere in the
         // middle of running an instant app. The default behaviour can be overridden
         // via gservices.
-            if (!instantApp || Global.getInt(
+        final boolean performDexopt = !forwardLocked
-                        mContext.getContentResolver(), Global.INSTANT_APP_DEXOPT_ENABLED, 0) != 0) {
+            && !pkg.applicationInfo.isExternalAsec()
+            && (!instantApp || Global.getInt(mContext.getContentResolver(),
+                    Global.INSTANT_APP_DEXOPT_ENABLED, 0) != 0);
+
+        if (performDexopt) {
             Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "dexopt");
             // Do not run PackageDexOptimizer through the local performDexOpt
             // method because `pkg` may not be in `mPackages` yet.
@@ -18633,15 +18653,9 @@ public class PackageManagerService extends IPackageManager.Stub
 
         // Notify BackgroundDexOptService that the package has been changed.
         // If this is an update of a package which used to fail to compile,
-            // BDOS will remove it from its blacklist.
+        // BackgroundDexOptService will remove it from its blacklist.
         // TODO: Layering violation
         BackgroundDexOptService.notifyPackageChanged(pkg.packageName);
-        }
-
-        if (!args.doRename(res.returnCode, pkg, oldCodePath)) {
-            res.setError(INSTALL_FAILED_INSUFFICIENT_STORAGE, "Failed rename");
-            return;
-        }
 
         startIntentFilterVerifications(args.user.getIdentifier(), replace, pkg);