Fix the logic about re-compiling for boot image.

There were some logic errors in
`OatFileAssistant::ShouldRecompileForFilter` that triggered
recompilation after reboot and caused the artifacts generated by
otapreopt to be overwritten. This CL fixes the logic.

Bug: 294044824
Test: m test-art-host-gtest-art_runtime_tests
Test: -
  1. m dist
  2. system/update_engine/scripts/update_device.py out/dist/cf_x86_64_phone-ota-*.zip
  3. adb reboot
  4. See that UpdatePackagesIfNeeded doesn't recompile any package.
  5. Check that `adb shell dumpsys package dexopt` shows
     "reason=ab-ota".
Change-Id: Ie909190b8c790ec696d3e1a89e74ec18701fb16c

2 files changed, 156 insertions, 1 deletions

diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc
index 47a3b341bc..9f39e05fc1 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -1182,12 +1182,21 @@ bool OatFileAssistant::OatFileInfo::ShouldRecompileForFilter(CompilerFilter::Fil
     return true;
   }
 
+  // Don't regress the compiler filter for the triggers handled below.
+  if (CompilerFilter::IsBetter(current, target)) {
+    VLOG(oat) << "Should not recompile: current filter is better";
+    return false;
+  }
+
   if (dexopt_trigger.primaryBootImageBecomesUsable &&
-      CompilerFilter::DependsOnImageChecksum(current)) {
+      CompilerFilter::IsAotCompilationEnabled(current)) {
     // If the oat file has been compiled without an image, and the runtime is
     // now running with an image loaded from disk, return that we need to
     // re-compile. The recompilation will generate a better oat file, and with an app
     // image for profile guided compilation.
+    // However, don't recompile for "verify". Although verification depends on the boot image, the
+    // penalty of being verified without a boot image is low. Consider the case where a dex file
+    // is verified by "ab-ota", we don't want it to be re-verified by "boot-after-ota".
     const char* oat_boot_class_path_checksums =
         file->GetOatHeader().GetStoreValueByKey(OatHeader::kBootClassPathChecksumsKey);
     if (oat_boot_class_path_checksums != nullptr &&
diff --git a/runtime/oat_file_assistant_test.cc b/runtime/oat_file_assistant_test.cc
index 1a5c57f902..e162ec98e7 100644
--- a/runtime/oat_file_assistant_test.cc
+++ b/runtime/oat_file_assistant_test.cc
@@ -2106,6 +2106,152 @@ TEST_P(OatFileAssistantTest, VdexNoDex) {
       &oat_file_assistant, "unknown", "unknown", "io-error-no-apk");
 }
 
+// Case: We have a VDEX file, generated without a boot image, and we now have a boot image.
+// Expect: Dexopt only if the target compiler filter >= "speed-profile".
+TEST_P(OatFileAssistantTest, ShouldRecompileForImageFromVdex) {
+  std::string dex_location = GetScratchDir() + "/TestDex.jar";
+  std::string odex_location = GetOdexDir() + "/TestDex.odex";
+  std::string vdex_location = GetOdexDir() + "/TestDex.vdex";
+  Copy(GetMultiDexSrc1(), dex_location);
+
+  // Compile without a boot image.
+  GenerateOdexForTest(dex_location,
+                      odex_location,
+                      CompilerFilter::kVerify,
+                      "install",
+                      {"--boot-image=/nonx/boot.art"});
+
+  // Delete the odex file and only keep the vdex.
+  ASSERT_EQ(0, unlink(odex_location.c_str()));
+
+  auto scoped_maybe_without_runtime = ScopedMaybeWithoutRuntime();
+
+  OatFileAssistant oat_file_assistant = CreateOatFileAssistant(dex_location.c_str());
+
+  VerifyGetDexOptNeeded(&oat_file_assistant,
+                        CompilerFilter::kSpeed,
+                        default_trigger_,
+                        /*expected_dexopt_needed=*/true,
+                        /*expected_is_vdex_usable=*/true,
+                        /*expected_location=*/OatFileAssistant::kLocationOdex);
+  EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter,
+            oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed));
+
+  VerifyGetDexOptNeeded(&oat_file_assistant,
+                        CompilerFilter::kSpeedProfile,
+                        default_trigger_,
+                        /*expected_dexopt_needed=*/true,
+                        /*expected_is_vdex_usable=*/true,
+                        /*expected_location=*/OatFileAssistant::kLocationOdex);
+  EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter,
+            oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeedProfile));
+
+  VerifyGetDexOptNeeded(&oat_file_assistant,
+                        CompilerFilter::kVerify,
+                        default_trigger_,
+                        /*expected_dexopt_needed=*/false,
+                        /*expected_is_vdex_usable=*/true,
+                        /*expected_location=*/OatFileAssistant::kLocationOdex);
+  EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
+            oat_file_assistant.GetDexOptNeeded(CompilerFilter::kVerify));
+}
+
+// Case: We have an ODEX file, generated without a boot image (filter: "verify"), and we now have a
+// boot image.
+// Expect: Dexopt only if the target compiler filter >= "speed-profile".
+TEST_P(OatFileAssistantTest, ShouldRecompileForImageFromVerify) {
+  std::string dex_location = GetScratchDir() + "/TestDex.jar";
+  std::string odex_location = GetOdexDir() + "/TestDex.odex";
+  std::string vdex_location = GetOdexDir() + "/TestDex.vdex";
+  Copy(GetMultiDexSrc1(), dex_location);
+
+  // Compile without a boot image.
+  GenerateOdexForTest(dex_location,
+                      odex_location,
+                      CompilerFilter::kVerify,
+                      "install",
+                      {"--boot-image=/nonx/boot.art"});
+
+  auto scoped_maybe_without_runtime = ScopedMaybeWithoutRuntime();
+
+  OatFileAssistant oat_file_assistant = CreateOatFileAssistant(dex_location.c_str());
+
+  VerifyGetDexOptNeeded(&oat_file_assistant,
+                        CompilerFilter::kSpeed,
+                        default_trigger_,
+                        /*expected_dexopt_needed=*/true,
+                        /*expected_is_vdex_usable=*/true,
+                        /*expected_location=*/OatFileAssistant::kLocationOdex);
+  EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter,
+            oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed));
+
+  VerifyGetDexOptNeeded(&oat_file_assistant,
+                        CompilerFilter::kSpeedProfile,
+                        default_trigger_,
+                        /*expected_dexopt_needed=*/true,
+                        /*expected_is_vdex_usable=*/true,
+                        /*expected_location=*/OatFileAssistant::kLocationOdex);
+  EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter,
+            oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeedProfile));
+
+  VerifyGetDexOptNeeded(&oat_file_assistant,
+                        CompilerFilter::kVerify,
+                        default_trigger_,
+                        /*expected_dexopt_needed=*/false,
+                        /*expected_is_vdex_usable=*/true,
+                        /*expected_location=*/OatFileAssistant::kLocationOdex);
+  EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
+            oat_file_assistant.GetDexOptNeeded(CompilerFilter::kVerify));
+}
+
+// Case: We have an ODEX file, generated without a boot image (filter: "speed-profile"), and we now
+// have a boot image.
+// Expect: Dexopt only if the target compiler filter >= "speed-profile".
+TEST_P(OatFileAssistantTest, ShouldRecompileForImageFromSpeedProfile) {
+  std::string dex_location = GetScratchDir() + "/TestDex.jar";
+  std::string odex_location = GetOdexDir() + "/TestDex.odex";
+  std::string vdex_location = GetOdexDir() + "/TestDex.vdex";
+  Copy(GetMultiDexSrc1(), dex_location);
+
+  // Compile without a boot image.
+  GenerateOdexForTest(dex_location,
+                      odex_location,
+                      CompilerFilter::kSpeedProfile,
+                      "install",
+                      {"--boot-image=/nonx/boot.art"});
+
+  auto scoped_maybe_without_runtime = ScopedMaybeWithoutRuntime();
+
+  OatFileAssistant oat_file_assistant = CreateOatFileAssistant(dex_location.c_str());
+
+  VerifyGetDexOptNeeded(&oat_file_assistant,
+                        CompilerFilter::kSpeed,
+                        default_trigger_,
+                        /*expected_dexopt_needed=*/true,
+                        /*expected_is_vdex_usable=*/true,
+                        /*expected_location=*/OatFileAssistant::kLocationOdex);
+  EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter,
+            oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed));
+
+  VerifyGetDexOptNeeded(&oat_file_assistant,
+                        CompilerFilter::kSpeedProfile,
+                        default_trigger_,
+                        /*expected_dexopt_needed=*/true,
+                        /*expected_is_vdex_usable=*/true,
+                        /*expected_location=*/OatFileAssistant::kLocationOdex);
+  EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter,
+            oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeedProfile));
+
+  VerifyGetDexOptNeeded(&oat_file_assistant,
+                        CompilerFilter::kVerify,
+                        default_trigger_,
+                        /*expected_dexopt_needed=*/false,
+                        /*expected_is_vdex_usable=*/true,
+                        /*expected_location=*/OatFileAssistant::kLocationOdex);
+  EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded,
+            oat_file_assistant.GetDexOptNeeded(CompilerFilter::kVerify));
+}
+
 // Test that GetLocation of a dex file is the same whether the dex
 // filed is backed by an oat file or not.
 TEST_F(OatFileAssistantBaseTest, GetDexLocation) {