Allow OTA even when ART Service is enabled.

Make sure we don't get LegacyDexoptDisabledException during the OTA
preinstall, more specifically when
OtaDexoptService.generatePackageDexopts runs.

This code only collects command invocations for the otapreopt_chroot
binary in frameworks/native/cmds/installd, which through another
otapreopt binary uses the legacy installd dexopt code. IOW,
LegacyDexoptDisabledException does not guard that legacy code from
being executed in the prereboot OTA stage, but it only affects dexopt
artifacts for the other OTA slot.

Test: m dist
      system/update_engine/scripts/update_device.py out/dist/*-ota-eng.*.zip
      # Check logcat that update_engine completes
      adb shell setprop persist.pm.mock-upgrade true
      adb reboot
      # Check logcat that update_engine completes
  with dalvik.vm.useartservice=true
Test: m AppUsingOtherApp AppUsedByOtherApp
      adb install out/target/product/vsoc_x86_64/testcases/AppUsedByOtherApp/x86_64/AppUsedByOtherApp.apk
      adb shell pm snapshot-profile android.compilation.cts.appusedbyotherapp
      adb pull data/misc/profman/android.compilation.cts.appusedbyotherapp.prof
      mv android.compilation.cts.appusedbyotherapp.prof primary.prof
      zip - primary.prof > AppUsedByOtherApp.dm
      adb install-multiple out/target/product/vsoc_x86_64/testcases/AppUsedByOtherApp/x86_64/AppUsedByOtherApp.apk AppUsedByOtherApp.dm
      adb install out/target/product/vsoc_x86_64/testcases/AppUsingOtherApp/x86_64/AppUsingOtherApp.apk
      adb shell am instrument -w --user 0 android.compilation.cts.appusingotherapp
  with dalvik.vm.useartservice=true
Bug: 251903639
Change-Id: If7a91ba3d8038d1717c734c5afb092ab989bca49

3 files changed, 64 insertions, 8 deletions

diff --git a/services/core/java/com/android/server/pm/Installer.java b/services/core/java/com/android/server/pm/Installer.java
index 9329f063aee5..0d417e457509 100644
--- a/services/core/java/com/android/server/pm/Installer.java
+++ b/services/core/java/com/android/server/pm/Installer.java
@@ -136,9 +136,7 @@ public class Installer extends SystemService {
     }
 
     /**
-     * @param isolated indicates if this object should <em>not</em> connect to
-     *            the real {@code installd}. All remote calls will be ignored
-     *            unless you extend this class and intercept them.
+     * @param isolated Make the installer isolated. See {@link isIsolated}.
      */
     public Installer(Context context, boolean isolated) {
         super(context);
@@ -153,6 +151,15 @@ public class Installer extends SystemService {
         mWarnIfHeld = warnIfHeld;
     }
 
+    /**
+     * Returns true if the installer is isolated, i.e. if this object should <em>not</em> connect to
+     * the real {@code installd}. All remote calls will be ignored unless you extend this class and
+     * intercept them.
+     */
+    public boolean isIsolated() {
+        return mIsolated;
+    }
+
     @Override
     public void onStart() {
         if (mIsolated) {
diff --git a/services/core/java/com/android/server/pm/OtaDexoptService.java b/services/core/java/com/android/server/pm/OtaDexoptService.java
index 767c0a73bc54..6a2ddc8f94b0 100644
--- a/services/core/java/com/android/server/pm/OtaDexoptService.java
+++ b/services/core/java/com/android/server/pm/OtaDexoptService.java
@@ -16,6 +16,7 @@
 
 package com.android.server.pm;
 
+import static com.android.server.pm.DexOptHelper.useArtService;
 import static com.android.server.pm.InstructionSets.getAppDexInstructionSets;
 import static com.android.server.pm.InstructionSets.getDexCodeInstructionSets;
 import static com.android.server.pm.PackageManagerService.PLATFORM_PACKAGE_NAME;
@@ -301,6 +302,15 @@ public class OtaDexoptService extends IOtaDexopt.Stub {
                     throws InstallerException {
                 final StringBuilder builder = new StringBuilder();
 
+                if (useArtService()) {
+                    if ((dexFlags & DEXOPT_SECONDARY_DEX) != 0) {
+                        // installd may change the reference profile in place for secondary dex
+                        // files, which isn't safe with the lock free approach in ART Service.
+                        throw new IllegalArgumentException(
+                                "Invalid OTA dexopt call for secondary dex");
+                    }
+                }
+
                 // The current version. For v10, see b/115993344.
                 builder.append("10 ");
 
@@ -353,7 +363,6 @@ public class OtaDexoptService extends IOtaDexopt.Stub {
         PackageDexOptimizer optimizer = new OTADexoptPackageDexOptimizer(
                 collectingInstaller, mPackageManagerService.mInstallLock, mContext);
 
-        // TODO(b/251903639): Allow this use of legacy dexopt code even when ART Service is enabled.
         try {
             optimizer.performDexOpt(pkg, pkgSetting, null /* ISAs */,
                     null /* CompilerStats.PackageStats */,
@@ -362,9 +371,19 @@ public class OtaDexoptService extends IOtaDexopt.Stub {
                     new DexoptOptions(pkg.getPackageName(), compilationReason,
                             DexoptOptions.DEXOPT_BOOT_COMPLETE));
         } catch (LegacyDexoptDisabledException e) {
-            throw new RuntimeException(e);
+            // OTA is still allowed to use the legacy dexopt code even when ART Service is enabled.
+            // The installer is isolated and won't call into installd, and the dexopt() method is
+            // overridden to only collect the command above. Hence we shouldn't go into any code
+            // path where this exception is thrown.
+            Slog.wtf(TAG, e);
         }
 
+        // ART Service compat note: These commands are consumed by the otapreopt binary, which uses
+        // the same legacy dexopt code as installd to invoke dex2oat. It provides output path
+        // implementations (see calculate_odex_file_path and create_cache_path in
+        // frameworks/native/cmds/installd/otapreopt.cpp) to write to different odex files than
+        // those used by ART Service in its ordinary operations, so it doesn't interfere with ART
+        // Service even when dalvik.vm.useartservice is true.
         return commands;
     }
 
diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java
index 0a90e7a30db6..8a4080ff029d 100644
--- a/services/core/java/com/android/server/pm/PackageDexOptimizer.java
+++ b/services/core/java/com/android/server/pm/PackageDexOptimizer.java
@@ -18,6 +18,7 @@ package com.android.server.pm;
 
 import static android.content.pm.ApplicationInfo.HIDDEN_API_ENFORCEMENT_DISABLED;
 
+import static com.android.server.pm.DexOptHelper.useArtService;
 import static com.android.server.pm.Installer.DEXOPT_BOOTCOMPLETE;
 import static com.android.server.pm.Installer.DEXOPT_DEBUGGABLE;
 import static com.android.server.pm.Installer.DEXOPT_ENABLE_HIDDEN_API_CHECKS;
@@ -329,8 +330,22 @@ public class PackageDexOptimizer {
 
             String profileName = ArtManager.getProfileName(
                     i == 0 ? null : pkg.getSplitNames()[i - 1]);
-            final boolean isUsedByOtherApps = options.isDexoptAsSharedLibrary()
-                    || packageUseInfo.isUsedByOtherApps(path);
+
+            final boolean isUsedByOtherApps;
+            if (options.isDexoptAsSharedLibrary()) {
+                isUsedByOtherApps = true;
+            } else if (useArtService()) {
+                // We get here when collecting dexopt commands in OTA preopt, even when ART Service
+                // is in use. packageUseInfo isn't useful in that case since the legacy dex use
+                // database hasn't been updated. So we'd have to query ART Service instead, but it
+                // doesn't provide that API. Just cop-out and bypass the cloud profile handling.
+                // That means such apps will get preopted wrong, and we'll leave it to a later
+                // background dexopt after reboot instead.
+                isUsedByOtherApps = false;
+            } else {
+                isUsedByOtherApps = packageUseInfo.isUsedByOtherApps(path);
+            }
+
             String compilerFilter = getRealCompilerFilter(pkg, options.getCompilerFilter());
             // If the app is used by other apps, we must not use the existing profile because it
             // may contain user data, unless the profile is newly created on install.
@@ -446,6 +461,14 @@ public class PackageDexOptimizer {
     private boolean prepareCloudProfile(AndroidPackage pkg, String profileName, String path,
             @Nullable String dexMetadataPath) throws LegacyDexoptDisabledException {
         if (dexMetadataPath != null) {
+            if (mInstaller.isIsolated()) {
+                // If the installer is isolated, the two calls to it below will return immediately,
+                // so this only short-circuits that a bit. We need to do it to avoid the
+                // LegacyDexoptDisabledException getting thrown first, when we get here during OTA
+                // preopt and ART Service is enabled.
+                return true;
+            }
+
             try {
                 // Make sure we don't keep any existing contents.
                 mInstaller.deleteReferenceProfile(pkg.getPackageName(), profileName);
@@ -879,7 +902,12 @@ public class PackageDexOptimizer {
     private int getDexoptNeeded(String packageName, String path, String isa, String compilerFilter,
             String classLoaderContext, int profileAnalysisResult, boolean downgrade,
             int dexoptFlags, String oatDir) throws LegacyDexoptDisabledException {
-        Installer.checkLegacyDexoptDisabled();
+        // Allow calls from OtaDexoptService even when ART Service is in use. The installer is
+        // isolated in that case so later calls to it won't call into installd anyway.
+        if (!mInstaller.isIsolated()) {
+            Installer.checkLegacyDexoptDisabled();
+        }
+
         final boolean shouldBePublic = (dexoptFlags & DEXOPT_PUBLIC) != 0;
         final boolean isProfileGuidedFilter = (dexoptFlags & DEXOPT_PROFILE_GUIDED) != 0;
         boolean newProfile = profileAnalysisResult == PROFILE_ANALYSIS_OPTIMIZE;
@@ -948,6 +976,8 @@ public class PackageDexOptimizer {
      */
     private int analyseProfiles(AndroidPackage pkg, int uid, String profileName,
             String compilerFilter) throws LegacyDexoptDisabledException {
+        Installer.checkLegacyDexoptDisabled();
+
         // Check if we are allowed to merge and if the compiler filter is profile guided.
         if (!isProfileGuidedCompilerFilter(compilerFilter)) {
             return PROFILE_ANALYSIS_DONT_OPTIMIZE_SMALL_DELTA;