Log an ignore non-fatal errors in deleteRuntimeArtifacts.

This is to comform with the javadoc on this method, which says "Throws
fatal errors. Logs and ignores non-fatal errors." Also, the binder
caller does not expect non-fatal errors and does not handle them.

Test: m test-art-host-gtest-art_artd_tests
Change-Id: If8419e7c3439be35719a09bd14e83342ae4c2442

2 files changed, 40 insertions, 17 deletions

diff --git a/artd/artd.cc b/artd/artd.cc
index 42eb45094e..078afd99ac 100644
--- a/artd/artd.cc
+++ b/artd/artd.cc
@@ -510,17 +510,24 @@ std::ostream& operator<<(std::ostream& os, const FdLogger& fd_logger) {
 
 }  // namespace
 
-#define OR_RETURN_ERROR(func, expr)         \
-  ({                                        \
-    decltype(expr)&& tmp = (expr);          \
-    if (!tmp.ok()) {                        \
-      return (func)(tmp.error().message()); \
-    }                                       \
-    std::move(tmp).value();                 \
+#define OR_RETURN_ERROR(func, expr)                           \
+  ({                                                          \
+    decltype(expr)&& __or_return_error_tmp = (expr);          \
+    if (!__or_return_error_tmp.ok()) {                        \
+      return (func)(__or_return_error_tmp.error().message()); \
+    }                                                         \
+    std::move(__or_return_error_tmp).value();                 \
   })
 
 #define OR_RETURN_FATAL(expr)     OR_RETURN_ERROR(Fatal, expr)
 #define OR_RETURN_NON_FATAL(expr) OR_RETURN_ERROR(NonFatal, expr)
+#define OR_LOG_AND_RETURN_OK(expr)     \
+  OR_RETURN_ERROR(                     \
+      [](const std::string& message) { \
+        LOG(ERROR) << message;         \
+        return ScopedAStatus::ok();    \
+      },                               \
+      expr)
 
 ScopedAStatus Artd::isAlive(bool* _aidl_return) {
   *_aidl_return = true;
@@ -1281,8 +1288,9 @@ ScopedAStatus Artd::isInDalvikCache(const std::string& in_dexFile, bool* _aidl_r
 ScopedAStatus Artd::deleteRuntimeArtifacts(const RuntimeArtifactsPath& in_runtimeArtifactsPath,
                                            int64_t* _aidl_return) {
   OR_RETURN_FATAL(ValidateRuntimeArtifactsPath(in_runtimeArtifactsPath));
-  std::string android_data = OR_RETURN_NON_FATAL(GetAndroidDataOrError());
-  std::string android_expand = OR_RETURN_NON_FATAL(GetAndroidExpandOrError());
+  *_aidl_return = 0;
+  std::string android_data = OR_LOG_AND_RETURN_OK(GetAndroidDataOrError());
+  std::string android_expand = OR_LOG_AND_RETURN_OK(GetAndroidExpandOrError());
   for (const std::string& file :
        ListRuntimeArtifactsFiles(android_data, android_expand, in_runtimeArtifactsPath)) {
     *_aidl_return += GetSizeAndDeleteFile(file);
@@ -1309,14 +1317,10 @@ ScopedAStatus Artd::getRuntimeArtifactsSize(const RuntimeArtifactsPath& in_runti
                                             int64_t* _aidl_return) {
   OR_RETURN_FATAL(ValidateRuntimeArtifactsPath(in_runtimeArtifactsPath));
   *_aidl_return = 0;
-  Result<std::string> android_data = GetAndroidDataOrError();
-  Result<std::string> android_expand = GetAndroidExpandOrError();
-  if (!android_data.ok() || !android_expand.ok()) {
-    LOG(ERROR) << "Failed to get the path to ANDROID_DATA or ANDROID_EXPAND";
-    return ScopedAStatus::ok();
-  }
-  for (const std::string& file : ListRuntimeArtifactsFiles(
-           android_data.value(), android_expand.value(), in_runtimeArtifactsPath)) {
+  std::string android_data = OR_LOG_AND_RETURN_OK(GetAndroidDataOrError());
+  std::string android_expand = OR_LOG_AND_RETURN_OK(GetAndroidExpandOrError());
+  for (const std::string& file :
+       ListRuntimeArtifactsFiles(android_data, android_expand, in_runtimeArtifactsPath)) {
     *_aidl_return += GetSize(file).value_or(0);
   }
   return ScopedAStatus::ok();
diff --git a/artd/artd_test.cc b/artd/artd_test.cc
index 79e6858819..6e6dad476d 100644
--- a/artd/artd_test.cc
+++ b/artd/artd_test.cc
@@ -2285,6 +2285,25 @@ TEST_F(ArtdTest, deleteRuntimeArtifacts) {
   }
 }
 
+TEST_F(ArtdTest, deleteRuntimeArtifactsAndroidDataNotExist) {
+  // Will be cleaned up by `android_data_env_`
+  setenv("ANDROID_DATA", "/non-existing", /*replace=*/1);
+
+  auto scoped_set_logger = ScopedSetLogger(mock_logger_.AsStdFunction());
+  EXPECT_CALL(mock_logger_,
+              Call(_, _, _, _, _, HasSubstr("Failed to find directory /non-existing")));
+
+  int64_t aidl_return;
+  ASSERT_TRUE(
+      artd_
+          ->deleteRuntimeArtifacts(
+              {.packageName = "com.android.foo", .dexPath = "/a/b/base.apk", .isa = "arm64"},
+              &aidl_return)
+          .isOk());
+
+  EXPECT_EQ(aidl_return, 0);
+}
+
 TEST_F(ArtdTest, deleteRuntimeArtifactsSpecialChars) {
   std::vector<std::string> removed_files;
   std::vector<std::string> kept_files;