Revert "Remove old and duplicated logic in picking up the best artifact."

This reverts commit 4f6801ea633f9d94945bcedb8a1db79be6cec10b.

Reason for revert: has issues and probably caused app startup regression.

Bug: 308248092
Change-Id: I5b6f1c933ffa5912fa8b96826fde79ff544680f9

3 files changed, 108 insertions, 27 deletions

diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc
index 812fbbe604..e8047af952 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -208,6 +208,24 @@ OatFileAssistant::OatFileAssistant(const char* dex_location,
                    << error_msg;
     }
   }
+
+  // Check if the dex directory is writable.
+  // This will be needed in most uses of OatFileAssistant and so it's OK to
+  // compute it eagerly. (the only use which will not make use of it is
+  // OatFileAssistant::GetStatusDump())
+  size_t pos = dex_location_.rfind('/');
+  if (pos == std::string::npos) {
+    LOG(WARNING) << "Failed to determine dex file parent directory: " << dex_location_;
+  } else if (!UseFdToReadFiles()) {
+    // We cannot test for parent access when using file descriptors. That's ok
+    // because in this case we will always pick the odex file anyway.
+    std::string parent = dex_location_.substr(0, pos);
+    if (access(parent.c_str(), W_OK) == 0) {
+      dex_parent_writable_ = true;
+    } else {
+      VLOG(oat) << "Dex parent of " << dex_location_ << " is not writable: " << strerror(errno);
+    }
+  }
 }
 
 std::unique_ptr<OatFileAssistant> OatFileAssistant::Create(
@@ -861,43 +879,78 @@ bool OatFileAssistant::IsPrimaryBootImageUsable() {
 
 OatFileAssistant::OatFileInfo& OatFileAssistant::GetBestInfo() {
   ScopedTrace trace("GetBestInfo");
-
-  // We first try the odex location, which is the most common.
-  VLOG(oat) << ART_FORMAT("GetBestInfo checking odex next to the dex file ({})",
-                          odex_.DisplayFilename());
-  if (odex_.IsUseable()) {
+  // TODO(calin): Document the side effects of class loading when
+  // running dalvikvm command line.
+  if (dex_parent_writable_ || UseFdToReadFiles()) {
+    // If the parent of the dex file is writable it means that we can
+    // create the odex file. In this case we unconditionally pick the odex
+    // as the best oat file. This corresponds to the regular use case when
+    // apps gets installed or when they load private, secondary dex file.
+    // For apps on the system partition the odex location will not be
+    // writable and thus the oat location might be more up to date.
+
+    // If the odex is not useable, and we have a useable vdex, return the vdex
+    // instead.
+    VLOG(oat) << ART_FORMAT("GetBestInfo checking odex next to the dex file ({})",
+                            odex_.DisplayFilename());
+    if (!odex_.IsUseable()) {
+      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_;
+      }
+      VLOG(oat) << ART_FORMAT("GetBestInfo checking dm ({})", dm_for_odex_.DisplayFilename());
+      if (dm_for_odex_.IsUseable()) {
+        return dm_for_odex_;
+      }
+    }
     return odex_;
   }
 
+  // We cannot write to the odex location. This must be a system app.
+
   // If the oat location is useable take it.
   VLOG(oat) << ART_FORMAT("GetBestInfo checking odex in dalvik-cache ({})", oat_.DisplayFilename());
   if (oat_.IsUseable()) {
     return oat_;
   }
 
-  // No odex/oat available, look for a useable vdex file.
-  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_;
+  // The oat file is not useable but the odex file might be up to date.
+  // This is an indication that we are dealing with an up to date prebuilt
+  // (that doesn't need relocation).
+  VLOG(oat) << ART_FORMAT("GetBestInfo checking odex next to the dex file ({})",
+                          odex_.DisplayFilename());
+  if (odex_.IsUseable()) {
+    return odex_;
   }
+
+  // Look for a useable vdex file.
   VLOG(oat) << ART_FORMAT("GetBestInfo checking vdex in dalvik-cache ({})",
                           vdex_for_oat_.DisplayFilename());
   if (vdex_for_oat_.IsUseable()) {
     return vdex_for_oat_;
   }
-
-  // A .dm file may be available, look for it.
-  VLOG(oat) << ART_FORMAT("GetBestInfo checking dm ({})", dm_for_odex_.DisplayFilename());
-  if (dm_for_odex_.IsUseable()) {
-    return dm_for_odex_;
+  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_;
   }
   VLOG(oat) << ART_FORMAT("GetBestInfo checking dm ({})", dm_for_oat_.DisplayFilename());
   if (dm_for_oat_.IsUseable()) {
     return dm_for_oat_;
   }
+  // TODO(jiakaiz): Is this the same as above?
+  VLOG(oat) << ART_FORMAT("GetBestInfo checking dm ({})", dm_for_odex_.DisplayFilename());
+  if (dm_for_odex_.IsUseable()) {
+    return dm_for_odex_;
+  }
 
-  // No usable artifact. Pick the odex if it exists, or the oat if not.
+  // We got into the worst situation here:
+  // - the oat location is not useable
+  // - the prebuild odex location is not up to date
+  // - the vdex-only file is not useable
+  // - and we don't have the original dex file anymore (stripped).
+  // Pick the odex if it exists, or the oat if not.
   VLOG(oat) << "GetBestInfo no usable artifacts";
   return (odex_.Status() == kOatCannotOpen) ? oat_ : odex_;
 }
diff --git a/runtime/oat_file_assistant.h b/runtime/oat_file_assistant.h
index 137e2fc721..402e9aef1a 100644
--- a/runtime/oat_file_assistant.h
+++ b/runtime/oat_file_assistant.h
@@ -532,6 +532,9 @@ class OatFileAssistant {
   // skipped.
   ClassLoaderContext* context_;
 
+  // Whether or not the parent directory of the dex file is writable.
+  bool dex_parent_writable_ = false;
+
   // In a properly constructed OatFileAssistant object, isa_ should be either
   // the 32 or 64 bit variant for the current device.
   const InstructionSet isa_ = InstructionSet::kNone;
diff --git a/tools/art b/tools/art
index a4ae3d57e5..47724de9f9 100755
--- a/tools/art
+++ b/tools/art
@@ -127,6 +127,34 @@ find_cp_in_args() {
   done
 }
 
+# Delete the 'oat' directories relative to the classpath's dex files.
+# e.g. (foo/classes.dex bar/classes.dex) would delete (foo/oat bar/oat) directories.
+cleanup_oat_directory() {
+  local classpath
+  classpath=("$@")
+
+  local dirpath
+
+  for path in "${classpath[@]}"; do
+    dirpath="$(dirname "$path")"
+    [[ -d "$dirpath" ]] && verbose_run rm -rf "$dirpath/oat"
+  done
+}
+
+# Parse -cp <CP>, -classpath <CP>, and $CLASSPATH to find the dex files.
+# Each dex file's directory will have an 'oat' file directory, delete it.
+# Input: Command line arguments to the art script.
+# e.g. -cp foo/classes.dex:bar/classes.dex would delete (foo/oat bar/oat) directories.
+cleanup_oat_directory_for_classpath() {
+  if [ "$CLEAN_OAT_FILES" = "yes" ]; then
+    # First try: Use $CLASSPATH environment variable.
+    parse_classpath "$CLASSPATH"
+    # Second try: Look for latest -cp or -classpath arg which will take precedence.
+    find_cp_in_args "$@"
+
+    cleanup_oat_directory "${PARSE_CLASSPATH_RESULT[@]}"
+  fi
+}
 
 # Attempt to find $ANDROID_ROOT/framework/<isa>/core.art' without knowing what <isa> is.
 function check_if_boot_image_file_exists() {
@@ -170,17 +198,11 @@ function run_dex2oat() {
     done
     # Create oat file directory.
     verbose_run mkdir -p $(dirname "$dex_file")/oat/$ISA
-    local oat_file="${dex_file:1}"
-    local oat_file="${oat_file//\//@}"
-    if [[ $dex_file != *.dex ]]; then
-      local oat_file=$ANDROID_DATA/dalvik-cache/$ISA/${oat_file}@classes.dex
-    else
-      local oat_file=$ANDROID_DATA/dalvik-cache/$ISA/${oat_file}
-    fi
+    local oat_file=$(basename "$dex_file")
+    local oat_file=$(dirname "$dex_file")/oat/$ISA/${oat_file%.*}.odex
     if [ "$GENERATE_APP_IMAGE" = "yes" ]; then
-      local art_file="${dex_file:1}"
-      local art_file="${art_file//\//@}"
-      local art_file=$ANDROID_DATA/dalvik-cache/$ISA/${art_file%.*}.art
+      local art_file=$(basename "$dex_file")
+      local art_file=$(dirname "$dex_file")/oat/$ISA/${art_file%.*}.art
       DEX2OAT_FLAGS+=("--app-image-file=$art_file")
     fi
 
@@ -536,7 +558,7 @@ if [ "$ANDROID_DATA" = "/data" ] || [ "$ANDROID_DATA" = "" ]; then
     # by default.
     ANDROID_DATA="$ANDROID_DATA/local/tmp/android-data$$"
   fi
-  mkdir -p "$ANDROID_DATA/dalvik-cache/$ISA"
+  mkdir -p "$ANDROID_DATA"
   DELETE_ANDROID_DATA="yes"
 fi
 
@@ -607,6 +629,9 @@ if [ "$ALLOW_DEFAULT_JDWP" = "no" ]; then
   EXTRA_OPTIONS+=(-XjdwpProvider:none)
 fi
 
+# First cleanup any left-over 'oat' files from the last time dalvikvm was run.
+cleanup_oat_directory_for_classpath "$@"
+
 # Protect additional arguments in quotes to preserve whitespaces (used by
 # run-jdwp-test.sh when running on device), '$' (may be used as part of
 # classpath) and other special characters when evaluated.