Revert "Revert "[res] Don't create extra asset provider when not..."

Revert submission 31216694-revert-31021037-TXGRFMNZKV

Reason for revert: relanding with a fix
The fix is to still return a null AssetsProvider when failing
to open the supplied apk, instead of ending up with an
empty provider that will fail later

Reverted changes: /q/submissionid:31216694-revert-31021037-TXGRFMNZKV

Change-Id: Ia8e2cdaddf81683cd58547942d1da3c5df93b087

3 files changed, 83 insertions, 47 deletions

diff --git a/core/jni/android_content_res_ApkAssets.cpp b/core/jni/android_content_res_ApkAssets.cpp
index 0da9b188ac5a..66c65d0ac1aa 100644
--- a/core/jni/android_content_res_ApkAssets.cpp
+++ b/core/jni/android_content_res_ApkAssets.cpp
@@ -111,9 +111,8 @@ static void DeleteGuardedApkAssets(Guarded<AssetManager2::ApkAssetsPtr>& apk_ass
 class LoaderAssetsProvider : public AssetsProvider {
  public:
   static std::unique_ptr<AssetsProvider> Create(JNIEnv* env, jobject assets_provider) {
-    return (!assets_provider) ? EmptyAssetsProvider::Create()
-                              : std::unique_ptr<AssetsProvider>(new LoaderAssetsProvider(
-                                    env, assets_provider));
+      return std::unique_ptr<AssetsProvider>{
+              assets_provider ? new LoaderAssetsProvider(env, assets_provider) : nullptr};
   }
 
   bool ForEachFile(const std::string& /* root_path */,
@@ -212,7 +211,7 @@ class LoaderAssetsProvider : public AssetsProvider {
     auto string_result = static_cast<jstring>(env->CallObjectMethod(
         assets_provider_, gAssetsProviderOffsets.toString));
     ScopedUtfChars str(env, string_result);
-    debug_name_ = std::string(str.c_str(), str.size());
+    debug_name_ = std::string(str.c_str());
   }
 
   // The global reference to the AssetsProvider
@@ -233,9 +232,9 @@ static jlong NativeLoad(JNIEnv* env, jclass /*clazz*/, const format_type_t forma
   AssetManager2::ApkAssetsPtr apk_assets;
   switch (format) {
     case FORMAT_APK: {
-        auto assets = MultiAssetsProvider::Create(std::move(loader_assets),
-                                                  ZipAssetsProvider::Create(path.c_str(),
-                                                                            property_flags));
+        auto assets = AssetsProvider::CreateWithOverride(ZipAssetsProvider::Create(path.c_str(),
+                                                                                   property_flags),
+                                                         std::move(loader_assets));
         apk_assets = ApkAssets::Load(std::move(assets), property_flags);
         break;
     }
@@ -243,15 +242,17 @@ static jlong NativeLoad(JNIEnv* env, jclass /*clazz*/, const format_type_t forma
       apk_assets = ApkAssets::LoadOverlay(path.c_str(), property_flags);
       break;
     case FORMAT_ARSC:
-      apk_assets = ApkAssets::LoadTable(AssetsProvider::CreateAssetFromFile(path.c_str()),
-                                        std::move(loader_assets),
-                                        property_flags);
-      break;
+        apk_assets =
+                ApkAssets::LoadTable(AssetsProvider::CreateAssetFromFile(path.c_str()),
+                                     AssetsProvider::CreateFromNullable(std::move(loader_assets)),
+                                     property_flags);
+        break;
     case FORMAT_DIRECTORY: {
-      auto assets = MultiAssetsProvider::Create(std::move(loader_assets),
-                                                DirectoryAssetsProvider::Create(path.c_str()));
-      apk_assets = ApkAssets::Load(std::move(assets), property_flags);
-      break;
+        auto assets =
+                AssetsProvider::CreateWithOverride(DirectoryAssetsProvider::Create(path.c_str()),
+                                                   std::move(loader_assets));
+        apk_assets = ApkAssets::Load(std::move(assets), property_flags);
+        break;
     }
     default:
       const std::string error_msg = base::StringPrintf("Unsupported format type %d", format);
@@ -308,18 +309,21 @@ static jlong NativeLoadFromFd(JNIEnv* env, jclass /*clazz*/, const format_type_t
   switch (format) {
     case FORMAT_APK: {
         auto assets =
-                MultiAssetsProvider::Create(std::move(loader_assets),
-                                            ZipAssetsProvider::Create(std::move(dup_fd),
-                                                                      friendly_name_utf8.c_str(),
-                                                                      property_flags));
+                AssetsProvider::CreateWithOverride(ZipAssetsProvider::Create(std::move(dup_fd),
+                                                                             friendly_name_utf8
+                                                                                     .c_str(),
+                                                                             property_flags),
+                                                   std::move(loader_assets));
         apk_assets = ApkAssets::Load(std::move(assets), property_flags);
         break;
     }
     case FORMAT_ARSC:
-      apk_assets = ApkAssets::LoadTable(
-          AssetsProvider::CreateAssetFromFd(std::move(dup_fd), nullptr /* path */),
-          std::move(loader_assets), property_flags);
-      break;
+        apk_assets =
+                ApkAssets::LoadTable(AssetsProvider::CreateAssetFromFd(std::move(dup_fd),
+                                                                       nullptr /* path */),
+                                     AssetsProvider::CreateFromNullable(std::move(loader_assets)),
+                                     property_flags);
+        break;
     default:
       const std::string error_msg = base::StringPrintf("Unsupported format type %d", format);
       jniThrowException(env, "java/lang/IllegalArgumentException", error_msg.c_str());
@@ -375,23 +379,28 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_
   switch (format) {
     case FORMAT_APK: {
         auto assets =
-                MultiAssetsProvider::Create(std::move(loader_assets),
-                                            ZipAssetsProvider::Create(std::move(dup_fd),
-                                                                      friendly_name_utf8.c_str(),
-                                                                      property_flags,
-                                                                      static_cast<off64_t>(offset),
-                                                                      static_cast<off64_t>(
-                                                                              length)));
+                AssetsProvider::CreateWithOverride(ZipAssetsProvider::Create(std::move(dup_fd),
+                                                                             friendly_name_utf8
+                                                                                     .c_str(),
+                                                                             property_flags,
+                                                                             static_cast<off64_t>(
+                                                                                     offset),
+                                                                             static_cast<off64_t>(
+                                                                                     length)),
+                                                   std::move(loader_assets));
         apk_assets = ApkAssets::Load(std::move(assets), property_flags);
         break;
     }
     case FORMAT_ARSC:
-      apk_assets = ApkAssets::LoadTable(
-          AssetsProvider::CreateAssetFromFd(std::move(dup_fd), nullptr /* path */,
-                                            static_cast<off64_t>(offset),
-                                            static_cast<off64_t>(length)),
-          std::move(loader_assets), property_flags);
-      break;
+        apk_assets =
+                ApkAssets::LoadTable(AssetsProvider::CreateAssetFromFd(std::move(dup_fd),
+                                                                       nullptr /* path */,
+                                                                       static_cast<off64_t>(offset),
+                                                                       static_cast<off64_t>(
+                                                                               length)),
+                                     AssetsProvider::CreateFromNullable(std::move(loader_assets)),
+                                     property_flags);
+        break;
     default:
       const std::string error_msg = base::StringPrintf("Unsupported format type %d", format);
       jniThrowException(env, "java/lang/IllegalArgumentException", error_msg.c_str());
@@ -408,13 +417,16 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_
 }
 
 static jlong NativeLoadEmpty(JNIEnv* env, jclass /*clazz*/, jint flags, jobject assets_provider) {
-  auto apk_assets = ApkAssets::Load(LoaderAssetsProvider::Create(env, assets_provider), flags);
-  if (apk_assets == nullptr) {
-    const std::string error_msg =
-        base::StringPrintf("Failed to load empty assets with provider %p", (void*)assets_provider);
-    jniThrowException(env, "java/io/IOException", error_msg.c_str());
-    return 0;
-  }
+    auto apk_assets = ApkAssets::Load(AssetsProvider::CreateFromNullable(
+                                              LoaderAssetsProvider::Create(env, assets_provider)),
+                                      flags);
+    if (apk_assets == nullptr) {
+        const std::string error_msg =
+                base::StringPrintf("Failed to load empty assets with provider %p",
+                                   (void*)assets_provider);
+        jniThrowException(env, "java/io/IOException", error_msg.c_str());
+        return 0;
+    }
   return CreateGuardedApkAssets(std::move(apk_assets));
 }
 
diff --git a/libs/androidfw/AssetsProvider.cpp b/libs/androidfw/AssetsProvider.cpp
index 59c6aba7b8b0..808509120462 100644
--- a/libs/androidfw/AssetsProvider.cpp
+++ b/libs/androidfw/AssetsProvider.cpp
@@ -24,9 +24,27 @@
 #include <ziparchive/zip_archive.h>
 
 namespace android {
-namespace {
-constexpr const char* kEmptyDebugString = "<empty>";
-} // namespace
+
+static constexpr std::string_view kEmptyDebugString = "<empty>";
+
+std::unique_ptr<AssetsProvider> AssetsProvider::CreateWithOverride(
+    std::unique_ptr<AssetsProvider> provider, std::unique_ptr<AssetsProvider> override) {
+  if (provider == nullptr) {
+    return {};
+  }
+  if (override == nullptr) {
+    return provider;
+  }
+  return MultiAssetsProvider::Create(std::move(override), std::move(provider));
+}
+
+std::unique_ptr<AssetsProvider> AssetsProvider::CreateFromNullable(
+    std::unique_ptr<AssetsProvider> nullable) {
+  if (nullable) {
+    return nullable;
+  }
+  return EmptyAssetsProvider::Create();
+}
 
 std::unique_ptr<Asset> AssetsProvider::Open(const std::string& path, Asset::AccessMode mode,
                                             bool* file_exists) const {
@@ -425,7 +443,7 @@ const std::string& EmptyAssetsProvider::GetDebugName() const {
   if (path_.has_value()) {
     return *path_;
   }
-  const static std::string kEmpty = kEmptyDebugString;
+  constexpr static std::string kEmpty{kEmptyDebugString};
   return kEmpty;
 }
 
diff --git a/libs/androidfw/include/androidfw/AssetsProvider.h b/libs/androidfw/include/androidfw/AssetsProvider.h
index 46d7a57ab763..037f684f5b78 100644
--- a/libs/androidfw/include/androidfw/AssetsProvider.h
+++ b/libs/androidfw/include/androidfw/AssetsProvider.h
@@ -36,6 +36,12 @@ namespace android {
 struct AssetsProvider {
   static constexpr off64_t kUnknownLength = -1;
 
+  static std::unique_ptr<AssetsProvider> CreateWithOverride(
+      std::unique_ptr<AssetsProvider> provider, std::unique_ptr<AssetsProvider> override);
+
+  static std::unique_ptr<AssetsProvider> CreateFromNullable(
+      std::unique_ptr<AssetsProvider> nullable);
+
   // Opens a file for reading. If `file_exists` is not null, it will be set to `true` if the file
   // exists. This is useful for determining if the file exists but was unable to be opened due to
   // an I/O error.