Merge "Continue dexopt for other packages when a package reports an error" into tm-dev

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

@@ -91,17 +91,21 @@ public final class BackgroundDexOptService {
 
     // Possible return codes of individual optimization steps.
     /** Ok status: Optimizations finished, All packages were processed, can continue */
-    private static final int STATUS_OK = 0;
+    /* package */ static final int STATUS_OK = 0;
     /** Optimizations should be aborted. Job scheduler requested it. */
-    private static final int STATUS_ABORT_BY_CANCELLATION = 1;
+    /* package */ static final int STATUS_ABORT_BY_CANCELLATION = 1;
     /** Optimizations should be aborted. No space left on device. */
-    private static final int STATUS_ABORT_NO_SPACE_LEFT = 2;
+    /* package */ static final int STATUS_ABORT_NO_SPACE_LEFT = 2;
     /** Optimizations should be aborted. Thermal throttling level too high. */
-    private static final int STATUS_ABORT_THERMAL = 3;
+    /* package */ static final int STATUS_ABORT_THERMAL = 3;
     /** Battery level too low */
-    private static final int STATUS_ABORT_BATTERY = 4;
+    /* package */ static final int STATUS_ABORT_BATTERY = 4;
-    /** {@link PackageDexOptimizer#DEX_OPT_FAILED} case */
+    /**
-    private static final int STATUS_DEX_OPT_FAILED = 5;
+     * {@link PackageDexOptimizer#DEX_OPT_FAILED} case. This state means some packages have failed
+     * compilation during the job. Note that the failure will not be permanent as the next dexopt
+     * job will exclude those failed packages.
+     */
+    /* package */ static final int STATUS_DEX_OPT_FAILED = 5;
 
     @IntDef(prefix = {"STATUS_"}, value = {
             STATUS_OK,
@@ -525,7 +529,10 @@ public final class BackgroundDexOptService {
         }
     }
 
-    /** Returns true if completed */
+    /**
+     * Returns whether we've successfully run the job. Note that it will return true even if some
+     * packages may have failed compiling.
+     */
     private boolean runIdleOptimization(PackageManagerService pm, List<String> pkgs,
             boolean isPostBootUpdate) {
         synchronized (mLock) {
@@ -541,7 +548,7 @@ public final class BackgroundDexOptService {
             mLastExecutionDurationMs = SystemClock.elapsedRealtime() - mLastExecutionStartTimeMs;
         }
 
-        return status == STATUS_OK;
+        return status == STATUS_OK || status == STATUS_DEX_OPT_FAILED;
     }
 
     /** Gets the size of the directory. It uses recursion to go over all files. */
@@ -661,6 +668,11 @@ public final class BackgroundDexOptService {
             ArraySet<String> updatedPackages, boolean isPostBootUpdate) {
         boolean supportSecondaryDex = mInjector.supportSecondaryDex();
 
+        // Keep the error if there is any error from any package.
+        @Status int status = STATUS_OK;
+
+        // Other than cancellation, all packages will be processed even if an error happens
+        // in a package.
         for (String pkg : pkgs) {
             int abortCode = abortIdleOptimizations(lowStorageThreshold);
             if (abortCode != STATUS_OK) {
@@ -670,10 +682,13 @@ public final class BackgroundDexOptService {
 
             @DexOptResult int primaryResult =
                     optimizePackage(pkg, true /* isForPrimaryDex */, isPostBootUpdate);
+            if (primaryResult == PackageDexOptimizer.DEX_OPT_CANCELLED) {
+                return STATUS_ABORT_BY_CANCELLATION;
+            }
             if (primaryResult == PackageDexOptimizer.DEX_OPT_PERFORMED) {
                 updatedPackages.add(pkg);
-            } else if (primaryResult != PackageDexOptimizer.DEX_OPT_SKIPPED) {
+            } else if (primaryResult == PackageDexOptimizer.DEX_OPT_FAILED) {
-                return convertPackageDexOptimizerStatusToInternal(primaryResult);
+                status = convertPackageDexOptimizerStatusToInternal(primaryResult);
             }
 
             if (!supportSecondaryDex) {
@@ -682,12 +697,14 @@ public final class BackgroundDexOptService {
 
             @DexOptResult int secondaryResult =
                     optimizePackage(pkg, false /* isForPrimaryDex */, isPostBootUpdate);
-            if (secondaryResult != PackageDexOptimizer.DEX_OPT_PERFORMED
+            if (secondaryResult == PackageDexOptimizer.DEX_OPT_CANCELLED) {
-                    && secondaryResult != PackageDexOptimizer.DEX_OPT_SKIPPED) {
+                return STATUS_ABORT_BY_CANCELLATION;
-                return convertPackageDexOptimizerStatusToInternal(secondaryResult);
+            }
+            if (secondaryResult == PackageDexOptimizer.DEX_OPT_FAILED) {
+                status = convertPackageDexOptimizerStatusToInternal(secondaryResult);
             }
         }
-        return STATUS_OK;
+        return status;
     }
 
     /**
@@ -779,9 +796,9 @@ public final class BackgroundDexOptService {
     @DexOptResult
     private int performDexOptPrimary(String pkg, int reason,
             int dexoptFlags) {
+        DexoptOptions dexoptOptions = new DexoptOptions(pkg, reason, dexoptFlags);
         return trackPerformDexOpt(pkg, /*isForPrimaryDex=*/ true,
-                () -> mDexOptHelper.performDexOptWithStatus(
+                () -> mDexOptHelper.performDexOptWithStatus(dexoptOptions));
-                        new DexoptOptions(pkg, reason, dexoptFlags)));
     }
 
     @DexOptResult
@@ -791,7 +808,7 @@ public final class BackgroundDexOptService {
                 dexoptFlags | DexoptOptions.DEXOPT_ONLY_SECONDARY_DEX);
         return trackPerformDexOpt(pkg, /*isForPrimaryDex=*/ false,
                 () -> mDexOptHelper.performDexOpt(dexoptOptions)
-                    ? PackageDexOptimizer.DEX_OPT_PERFORMED : PackageDexOptimizer.DEX_OPT_FAILED
+                        ? PackageDexOptimizer.DEX_OPT_PERFORMED : PackageDexOptimizer.DEX_OPT_FAILED
         );
     }
 

services/tests/mockingservicestests/src/com/android/server/pm/BackgroundDexOptServiceUnitTest.java

@@ -16,6 +16,9 @@
 
 package com.android.server.pm;
 
+import static com.android.server.pm.BackgroundDexOptService.STATUS_DEX_OPT_FAILED;
+import static com.android.server.pm.BackgroundDexOptService.STATUS_OK;
+
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.mockito.ArgumentMatchers.any;
@@ -23,12 +26,14 @@ import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.testng.Assert.assertThrows;
 
+import android.annotation.Nullable;
 import android.app.job.JobInfo;
 import android.app.job.JobParameters;
 import android.app.job.JobScheduler;
@@ -38,10 +43,13 @@ import android.content.Intent;
 import android.content.IntentFilter;
 import android.os.HandlerThread;
 import android.os.PowerManager;
+import android.util.Log;
 
+import com.android.internal.util.IndentingPrintWriter;
 import com.android.server.LocalServices;
 import com.android.server.PinnerService;
 import com.android.server.pm.dex.DexManager;
+import com.android.server.pm.dex.DexoptOptions;
 
 import org.junit.After;
 import org.junit.Before;
@@ -52,7 +60,11 @@ import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
+import java.io.ByteArrayOutputStream;
+import java.io.PrintWriter;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.stream.Collectors;
@@ -66,8 +78,12 @@ public final class BackgroundDexOptServiceUnitTest {
 
     private static final long TEST_WAIT_TIMEOUT_MS = 10_000;
 
-    private static final List<String> DEFAULT_PACKAGE_LIST = List.of("aaa", "bbb");
+    private static final String PACKAGE_AAA = "aaa";
-    private static final List<String> EMPTY_PACKAGE_LIST = List.of();
+    private static final List<String> DEFAULT_PACKAGE_LIST = List.of(PACKAGE_AAA, "bbb");
+    private int mDexOptResultForPackageAAA = PackageDexOptimizer.DEX_OPT_PERFORMED;
+
+    // Store expected dexopt sequence for verification.
+    private ArrayList<DexOptInfo> mDexInfoSequence = new ArrayList<>();
 
     @Mock
     private Context mContext;
@@ -116,14 +132,23 @@ public final class BackgroundDexOptServiceUnitTest {
         when(mInjector.getDexOptThermalCutoff()).thenReturn(PowerManager.THERMAL_STATUS_CRITICAL);
         when(mInjector.getCurrentThermalStatus()).thenReturn(PowerManager.THERMAL_STATUS_NONE);
         when(mInjector.supportSecondaryDex()).thenReturn(true);
-        when(mDexOptHelper.getOptimizablePackages(any())).thenReturn(DEFAULT_PACKAGE_LIST);
+        setupDexOptHelper();
-        when(mDexOptHelper.performDexOptWithStatus(any())).thenReturn(
-                PackageDexOptimizer.DEX_OPT_PERFORMED);
-        when(mDexOptHelper.performDexOpt(any())).thenReturn(true);
 
         mService = new BackgroundDexOptService(mInjector);
     }
 
+    private void setupDexOptHelper() {
+        when(mDexOptHelper.getOptimizablePackages(any())).thenReturn(DEFAULT_PACKAGE_LIST);
+        when(mDexOptHelper.performDexOptWithStatus(any())).thenAnswer(inv -> {
+            DexoptOptions opt = inv.getArgument(0);
+            if (opt.getPackageName().equals(PACKAGE_AAA)) {
+                return mDexOptResultForPackageAAA;
+            }
+            return PackageDexOptimizer.DEX_OPT_PERFORMED;
+        });
+        when(mDexOptHelper.performDexOpt(any())).thenReturn(true);
+    }
+
     @After
     public void tearDown() throws Exception {
         LocalServices.removeServiceForTest(BackgroundDexOptService.class);
@@ -159,7 +184,7 @@ public final class BackgroundDexOptServiceUnitTest {
     @Test
     public void testNoExecutionForNoOptimizablePackages() {
         initUntilBootCompleted();
-        when(mDexOptHelper.getOptimizablePackages(any())).thenReturn(EMPTY_PACKAGE_LIST);
+        when(mDexOptHelper.getOptimizablePackages(any())).thenReturn(Collections.emptyList());
 
         assertThat(mService.onStartJob(mJobServiceForPostBoot,
                 mJobParametersForPostBoot)).isFalse();
@@ -170,15 +195,70 @@ public final class BackgroundDexOptServiceUnitTest {
     public void testPostBootUpdateFullRun() {
         initUntilBootCompleted();
 
-        runFullJob(mJobServiceForPostBoot, mJobParametersForPostBoot, false, 1);
+        runFullJob(mJobServiceForPostBoot, mJobParametersForPostBoot,
+                /* expectedReschedule= */ false, /* expectedStatus= */ STATUS_OK,
+                /* totalJobFinishedWithParams= */ 1, /* expectedSkippedPackage= */ null);
+    }
+
+    @Test
+    public void testPostBootUpdateFullRunWithPackageFailure() {
+        mDexOptResultForPackageAAA = PackageDexOptimizer.DEX_OPT_FAILED;
+
+        initUntilBootCompleted();
+
+        runFullJob(mJobServiceForPostBoot, mJobParametersForPostBoot,
+                /* expectedReschedule= */ false, /* expectedStatus= */ STATUS_DEX_OPT_FAILED,
+                /* totalJobFinishedWithParams= */ 1, /* expectedSkippedPackage= */ PACKAGE_AAA);
+
+        assertThat(getFailedPackageNamesPrimary()).containsExactly(PACKAGE_AAA);
+        assertThat(getFailedPackageNamesSecondary()).isEmpty();
     }
 
     @Test
     public void testIdleJobFullRun() {
         initUntilBootCompleted();
+        runFullJob(mJobServiceForPostBoot, mJobParametersForPostBoot,
+                /* expectedReschedule= */ false, /* expectedStatus= */ STATUS_OK,
+                /* totalJobFinishedWithParams= */ 1, /* expectedSkippedPackage= */ null);
+        runFullJob(mJobServiceForIdle, mJobParametersForIdle,
+                /* expectedReschedule= */ true, /* expectedStatus= */ STATUS_OK,
+                /* totalJobFinishedWithParams= */ 1, /* expectedSkippedPackage= */ null);
+    }
 
-        runFullJob(mJobServiceForPostBoot, mJobParametersForPostBoot, false, 1);
+    @Test
-        runFullJob(mJobServiceForIdle, mJobParametersForIdle, true, 2);
+    public void testIdleJobFullRunWithFailureOnceAndSuccessAfterUpdate() {
+        mDexOptResultForPackageAAA = PackageDexOptimizer.DEX_OPT_FAILED;
+
+        initUntilBootCompleted();
+
+        runFullJob(mJobServiceForPostBoot, mJobParametersForPostBoot,
+                /* expectedReschedule= */ false, /* expectedStatus= */ STATUS_DEX_OPT_FAILED,
+                /* totalJobFinishedWithParams= */ 1, /* expectedSkippedPackage= */ PACKAGE_AAA);
+
+        assertThat(getFailedPackageNamesPrimary()).containsExactly(PACKAGE_AAA);
+        assertThat(getFailedPackageNamesSecondary()).isEmpty();
+
+        runFullJob(mJobServiceForIdle, mJobParametersForIdle,
+                /* expectedReschedule= */ true, /* expectedStatus= */ STATUS_OK,
+                /* totalJobFinishedWithParams= */ 1, /* expectedSkippedPackage= */ PACKAGE_AAA);
+
+        assertThat(getFailedPackageNamesPrimary()).containsExactly(PACKAGE_AAA);
+        assertThat(getFailedPackageNamesSecondary()).isEmpty();
+
+        mService.notifyPackageChanged(PACKAGE_AAA);
+
+        assertThat(getFailedPackageNamesPrimary()).isEmpty();
+        assertThat(getFailedPackageNamesSecondary()).isEmpty();
+
+        // Succeed this time.
+        mDexOptResultForPackageAAA = PackageDexOptimizer.DEX_OPT_PERFORMED;
+
+        runFullJob(mJobServiceForIdle, mJobParametersForIdle,
+                /* expectedReschedule= */ true, /* expectedStatus= */ STATUS_OK,
+                /* totalJobFinishedWithParams= */ 2, /* expectedSkippedPackage= */ null);
+
+        assertThat(getFailedPackageNamesPrimary()).isEmpty();
+        assertThat(getFailedPackageNamesSecondary()).isEmpty();
     }
 
     @Test
@@ -404,8 +484,10 @@ public final class BackgroundDexOptServiceUnitTest {
     }
 
     private void runFullJob(BackgroundDexOptJobService jobService, JobParameters params,
-            boolean expectedReschedule, int totalJobRuns) {
+            boolean expectedReschedule, int expectedStatus, int totalJobFinishedWithParams,
+            @Nullable String expectedSkippedPackage) {
         when(mInjector.createAndStartThread(any(), any())).thenReturn(Thread.currentThread());
+        addFullRunSequence(expectedSkippedPackage);
         assertThat(mService.onStartJob(jobService, params)).isTrue();
 
         ArgumentCaptor<Runnable> argThreadRunnable = ArgumentCaptor.forClass(Runnable.class);
@@ -413,20 +495,99 @@ public final class BackgroundDexOptServiceUnitTest {
 
         argThreadRunnable.getValue().run();
 
-        verify(jobService).jobFinished(params, expectedReschedule);
+        verify(jobService, times(totalJobFinishedWithParams)).jobFinished(params,
+                expectedReschedule);
         // Never block
         verify(mDexOptHelper, never()).controlDexOptBlocking(true);
-        verifyPerformDexOpt(DEFAULT_PACKAGE_LIST, totalJobRuns);
+        verifyPerformDexOpt();
+        assertThat(getLastExecutionStatus()).isEqualTo(expectedStatus);
     }
 
-    private void verifyPerformDexOpt(List<String> pkgs, int expectedRuns) {
+    private void verifyPerformDexOpt() {
         InOrder inOrder = inOrder(mDexOptHelper);
-        for (int i = 0; i < expectedRuns; i++) {
+        inOrder.verify(mDexOptHelper).getOptimizablePackages(any());
-            for (String pkg : pkgs) {
+        for (DexOptInfo info : mDexInfoSequence) {
-                inOrder.verify(mDexOptHelper, times(1)).performDexOptWithStatus(argThat((option) ->
+            if (info.isPrimary) {
-                        option.getPackageName().equals(pkg) && !option.isDexoptOnlySecondaryDex()));
+                verify(mDexOptHelper).performDexOptWithStatus(
-                inOrder.verify(mDexOptHelper, times(1)).performDexOpt(argThat((option) ->
+                        argThat((option) -> option.getPackageName().equals(info.packageName)
-                        option.getPackageName().equals(pkg) && option.isDexoptOnlySecondaryDex()));
+                                && !option.isDexoptOnlySecondaryDex()));
+            } else {
+                inOrder.verify(mDexOptHelper).performDexOpt(
+                        argThat((option) -> option.getPackageName().equals(info.packageName)
+                                && option.isDexoptOnlySecondaryDex()));
+            }
+        }
+
+        // Even InOrder cannot check the order if the same call is made multiple times.
+        // To check the order across multiple runs, we reset the mock so that order can be checked
+        // in each call.
+        mDexInfoSequence.clear();
+        reset(mDexOptHelper);
+        setupDexOptHelper();
+    }
+
+    private String findDumpValueForKey(String key) {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        PrintWriter pw = new PrintWriter(out, true);
+        IndentingPrintWriter writer = new IndentingPrintWriter(pw, "");
+        try {
+            mService.dump(writer);
+            writer.flush();
+            Log.i(TAG, "dump output:" + out.toString());
+            for (String line : out.toString().split(System.lineSeparator())) {
+                String[] vals = line.split(":");
+                if (vals[0].equals(key)) {
+                    if (vals.length == 2) {
+                        return vals[1].strip();
+                    } else {
+                        break;
+                    }
+                }
+            }
+            return "";
+        } finally {
+            writer.close();
+        }
+    }
+
+    List<String> findStringListFromDump(String key) {
+        String values = findDumpValueForKey(key);
+        if (values.isEmpty()) {
+            return Collections.emptyList();
+        }
+        return Arrays.asList(values.split(","));
+    }
+
+    private List<String> getFailedPackageNamesPrimary() {
+        return findStringListFromDump("mFailedPackageNamesPrimary");
+    }
+
+    private List<String> getFailedPackageNamesSecondary() {
+        return findStringListFromDump("mFailedPackageNamesSecondary");
+    }
+
+    private int getLastExecutionStatus() {
+        return Integer.parseInt(findDumpValueForKey("mLastExecutionStatus"));
+    }
+
+    private static class DexOptInfo {
+        public final String packageName;
+        public final boolean isPrimary;
+
+        private DexOptInfo(String packageName, boolean isPrimary) {
+            this.packageName = packageName;
+            this.isPrimary = isPrimary;
+        }
+    }
+
+    private void addFullRunSequence(@Nullable String expectedSkippedPackage) {
+        for (String packageName : DEFAULT_PACKAGE_LIST) {
+            if (packageName.equals(expectedSkippedPackage)) {
+                // only fails primary dexopt in mocking but add secodary
+                mDexInfoSequence.add(new DexOptInfo(packageName, /* isPrimary= */ false));
+            } else {
+                mDexInfoSequence.add(new DexOptInfo(packageName, /* isPrimary= */ true));
+                mDexInfoSequence.add(new DexOptInfo(packageName, /* isPrimary= */ false));
             }
         }
     }