Don't cleanup artifacts if compilation is skipped.

Bug: 280776418
Test: atest odsign_e2e_tests_full
Change-Id: I5ce05e077b6810f61f4fdd404cd68d61933bcab7
diff --git a/odrefresh/odrefresh.cc b/odrefresh/odrefresh.cc
index 1777be4..608c321 100644
--- a/odrefresh/odrefresh.cc
+++ b/odrefresh/odrefresh.cc
@@ -1620,12 +1620,6 @@
     metrics.SetTrigger(data_result.GetTrigger());
   }
 
-  // If partial compilation is disabled, we should compile everything regardless of what's in
-  // `compilation_options`.
-  if (compilation_required && !config_.GetPartialCompilation()) {
-    return cleanup_and_compile_all();
-  }
-
   // Always keep the cache info.
   checked_artifacts.push_back(cache_info_filename_);
 
@@ -1995,10 +1989,20 @@
 }
 
 WARN_UNUSED ExitCode OnDeviceRefresh::Compile(OdrMetrics& metrics,
-                                              const CompilationOptions& compilation_options) const {
+                                              CompilationOptions compilation_options) const {
   const char* staging_dir = nullptr;
   metrics.SetStage(OdrMetrics::Stage::kPreparation);
 
+  // If partial compilation is disabled, we should compile everything regardless of what's in
+  // `compilation_options`.
+  if (!config_.GetPartialCompilation()) {
+    compilation_options = CompilationOptions::CompileAll(*this);
+    if (!RemoveArtifactsDirectory()) {
+      metrics.SetStatus(OdrMetrics::Status::kIoError);
+      return ExitCode::kCleanupFailed;
+    }
+  }
+
   if (!EnsureDirectoryExists(config_.GetArtifactDirectory())) {
     LOG(ERROR) << "Failed to prepare artifact directory";
     metrics.SetStatus(errno == EPERM ? OdrMetrics::Status::kDalvikCachePermissionDenied :
diff --git a/odrefresh/odrefresh.h b/odrefresh/odrefresh.h
index 1b296ef..57b04da 100644
--- a/odrefresh/odrefresh.h
+++ b/odrefresh/odrefresh.h
@@ -173,8 +173,7 @@
   CheckArtifactsAreUpToDate(OdrMetrics& metrics,
                             /*out*/ CompilationOptions* compilation_options) const;
 
-  WARN_UNUSED ExitCode Compile(OdrMetrics& metrics,
-                               const CompilationOptions& compilation_options) const;
+  WARN_UNUSED ExitCode Compile(OdrMetrics& metrics, CompilationOptions compilation_options) const;
 
   WARN_UNUSED bool RemoveArtifactsDirectory() const;
 
diff --git a/test/odsign/test-src/com/android/tests/odsign/DeviceState.java b/test/odsign/test-src/com/android/tests/odsign/DeviceState.java
index 9295831..62adcab 100644
--- a/test/odsign/test-src/com/android/tests/odsign/DeviceState.java
+++ b/test/odsign/test-src/com/android/tests/odsign/DeviceState.java
@@ -158,6 +158,13 @@
         pushAndBindMount(localFile, "/system/framework/services.jar");
     }
 
+    /** Simulates that a system server jar is bad. */
+    public void simulateBadSystemServerJar() throws Exception {
+        File tempFile = File.createTempFile("empty", ".jar");
+        tempFile.deleteOnExit();
+        pushAndBindMount(tempFile, "/system/framework/services.jar");
+    }
+
     public void makeDex2oatFail() throws Exception {
         setProperty("dalvik.vm.boot-dex2oat-threads", "-1");
     }
diff --git a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
index 88bc465..8f74799 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
@@ -24,6 +24,7 @@
 import com.android.tradefed.testtype.junit4.AfterClassWithInfo;
 import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
 import com.android.tradefed.testtype.junit4.BeforeClassWithInfo;
+import com.android.tradefed.util.CommandResult;
 
 import org.junit.After;
 import org.junit.Before;
@@ -452,6 +453,36 @@
         mTestUtils.assertFilesNotExist(mTestUtils.getSystemServerExpectedArtifacts());
     }
 
+    /**
+     * If the compilation is skipped because a previous attempt partially failed, odrefresh should
+     * not clear existing artifacts.
+     */
+    @Test
+    public void verifyArtifactsKeptWhenCompilationSkippedNoPartialCompilation() throws Exception {
+        // Simulate that the compilation is partially failed.
+        mDeviceState.simulateBadSystemServerJar();
+        long timeMs = mTestUtils.getCurrentTimeMs();
+        mTestUtils.runOdrefreshNoPartialCompilation();
+
+        // Verify the test setup: boot images are still generated.
+        mTestUtils.assertModifiedAfter(mTestUtils.getExpectedPrimaryBootImage(), timeMs);
+        mTestUtils.assertModifiedAfter(mTestUtils.getExpectedBootImageMainlineExtension(), timeMs);
+
+        // Rerun odrefresh. The compilation is skipped this time.
+        timeMs = mTestUtils.getCurrentTimeMs();
+        CommandResult result = mTestUtils.runOdrefreshNoPartialCompilation();
+
+        // Existing artifacts should be kept.
+        mTestUtils.assertNotModifiedAfter(mTestUtils.getExpectedPrimaryBootImage(), timeMs);
+        mTestUtils.assertNotModifiedAfter(
+                mTestUtils.getExpectedBootImageMainlineExtension(), timeMs);
+
+        // Note that the existing artifacts may be manipulated (CVE-2021-39689). Make sure odrefresh
+        // returns `kOkay` rather than `kCompilationSuccess` or `kCompilationFailed`, so that odsign
+        // only verifies the artifacts but not sign them.
+        assertThat(result.getExitCode()).isEqualTo(0);
+    }
+
     private Set<String> simulateMissingArtifacts() throws Exception {
         Set<String> missingArtifacts = new HashSet<>();
         String sample = mTestUtils.getSystemServerExpectedArtifacts().iterator().next();
diff --git a/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java b/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java
index 41e45ea..8b61905 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java
@@ -480,9 +480,9 @@
         runOdrefresh("" /* extraArgs */);
     }
 
-    public void runOdrefresh(String extraArgs) throws Exception {
+    public CommandResult runOdrefresh(String extraArgs) throws Exception {
         mTestInfo.getDevice().executeShellV2Command(ODREFRESH_BIN + " --check");
-        mTestInfo.getDevice().executeShellV2Command(ODREFRESH_BIN
+        return mTestInfo.getDevice().executeShellV2Command(ODREFRESH_BIN
                 + " --partial-compilation=true --no-refresh " + extraArgs + " --compile");
     }
 
@@ -490,9 +490,9 @@
      * Simulates how odsign invokes odrefresh on a device that doesn't have the security fix for
      * CVE-2021-39689 (b/206090748).
      */
-    public void runOdrefreshNoPartialCompilation() throws Exception {
+    public CommandResult runOdrefreshNoPartialCompilation() throws Exception {
         // Note that odsign doesn't call `odrefresh --check` on such a device.
-        mTestInfo.getDevice().executeShellV2Command(
+        return mTestInfo.getDevice().executeShellV2Command(
                 ODREFRESH_BIN + " --partial-compilation=false --no-refresh --compile");
     }