Fix OatFileAssistant non-determinism in choosing best oat files.

https://r.android.com/3474808 added a file existence check in
GetBestInfo. This introduced a potential inconsistency between two calls
to GetBestInfo because the file existence may change between the two
calls (e.g., a new file is generated by bg-dexopt during that period).
This CL fixes it by only guarding the logging by the file existence
check.

In addition, because `IsUseable` is no longer guarded by the file
existence check, we need to add a file existence check in
ZipArchive::Open, to prevent `OpenArchive` from logging unnecessary
warnings on the absence of DM files.

Bug: 396649891
Bug: 377474232
Bug: 345762752
Test: adb shell pm art dump
Change-Id: I5a7079bed76fb1d89bfaa791bbdd14bb6981d563

2 files changed, 23 insertions, 27 deletions

diff --git a/libartbase/base/zip_archive.cc b/libartbase/base/zip_archive.cc
index fef23e9105..5e441a2aab 100644
--- a/libartbase/base/zip_archive.cc
+++ b/libartbase/base/zip_archive.cc
@@ -233,6 +233,13 @@ static void SetCloseOnExec(int fd) {
 ZipArchive* ZipArchive::Open(const char* filename, std::string* error_msg) {
   DCHECK(filename != nullptr);
 
+  // Don't call into `OpenArchive` on file absence. `OpenArchive` prints a warning even if the file
+  // absence is expected.
+  if (!OS::FileExists(filename)) {
+    *error_msg = StringPrintf("Failed to open '%s': File not found", filename);
+    return nullptr;
+  }
+
   ZipArchiveHandle handle;
   const int32_t error = OpenArchive(filename, &handle);
   if (error != 0) {
diff --git a/runtime/oat/oat_file_assistant.cc b/runtime/oat/oat_file_assistant.cc
index e6f9cb15a3..b6276f52d2 100644
--- a/runtime/oat/oat_file_assistant.cc
+++ b/runtime/oat/oat_file_assistant.cc
@@ -788,7 +788,7 @@ OatFileAssistant::OatFileInfo& OatFileAssistant::GetBestInfo() {
   ScopedTrace trace("GetBestInfo");
 
   auto log_status = [&](std::string_view location, OatFileInfo* info) {
-    if (!VLOG_IS_ON(oat)) {
+    if (!VLOG_IS_ON(oat) || !info->FileExists()) {
       return;
     }
     std::string error_msg;
@@ -810,42 +810,31 @@ OatFileAssistant::OatFileInfo& OatFileAssistant::GetBestInfo() {
   // If the oat location is useable, take it. This must be an app on a readonly filesystem
   // (typically, a system app or an incremental app). This must be prioritized over the odex
   // location, because the odex location probably has the dexpreopt artifacts.
-  if (oat_.FileExists()) {
-    log_status("odex in dalvik-cache", &oat_);
-    if (oat_.IsUseable()) {
-      return oat_;
-    }
+  log_status("odex in dalvik-cache", &oat_);
+  if (oat_.IsUseable()) {
+    return oat_;
   }
 
   // The odex location, which is the most common.
-  if (odex_.FileExists()) {
-    log_status("odex next to the dex file", &odex_);
-    if (odex_.IsUseable()) {
-      return odex_;
-    }
+  log_status("odex next to the dex file", &odex_);
+  if (odex_.IsUseable()) {
+    return odex_;
   }
 
   // No odex/oat available, look for a useable vdex file.
-  if (vdex_for_oat_.FileExists()) {
-    log_status("vdex in dalvik-cache", &vdex_for_oat_);
-    if (vdex_for_oat_.IsUseable()) {
-      return vdex_for_oat_;
-    }
+  log_status("vdex in dalvik-cache", &vdex_for_oat_);
+  if (vdex_for_oat_.IsUseable()) {
+    return vdex_for_oat_;
   }
-  if (vdex_for_odex_.FileExists()) {
-    VLOG(oat) << ART_FORMAT("GetBestInfo checking vdex next to the dex file ({})",
-                            vdex_for_odex_.DisplayFilename());
-    if (vdex_for_odex_.IsUseable()) {
-      return vdex_for_odex_;
-    }
+  log_status("vdex next to the dex file", &vdex_for_odex_);
+  if (vdex_for_odex_.IsUseable()) {
+    return vdex_for_odex_;
   }
 
   // A .dm file may be available, look for it.
-  if (dm_.FileExists()) {
-    log_status("dm", &dm_);
-    if (dm_.IsUseable()) {
-      return dm_;
-    }
+  log_status("dm", &dm_);
+  if (dm_.IsUseable()) {
+    return dm_;
   }
 
   // No usable artifact. Pick the odex if it exists, or the oat if not.