[res] Don't create extra asset provider when not needed

MultiAssetsProvider is useful to combine a ResourcesLoader with
a file-based provider, but most of the time it's used without
any loaders, and ends up with an EmptyAssetsProvider object that
just wastes cycles and RAM.

This CL optimizes that out, only creating the extra layer when
we have a real loader, and falling back to the only remaining
provider otherwise

Bug: 319137634
Test: build + boot + manual + atest libandroidfw_tests
Flag: EXEMPT optimization
Change-Id: Ibc5ac60adc5e008b70a62481a747758c89083ff9

3 files changed, 43 insertions, 32 deletions

diff --git a/core/jni/android_content_res_ApkAssets.cpp b/core/jni/android_content_res_ApkAssets.cpp
index 0da9b188ac5a..e6364a96bd9f 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
@@ -243,10 +242,10 @@ 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()),
+                                          MultiAssetsProvider::Create(std::move(loader_assets)),
+                                          property_flags);
+        break;
     case FORMAT_DIRECTORY: {
       auto assets = MultiAssetsProvider::Create(std::move(loader_assets),
                                                 DirectoryAssetsProvider::Create(path.c_str()));
@@ -316,10 +315,11 @@ static jlong NativeLoadFromFd(JNIEnv* env, jclass /*clazz*/, const format_type_t
         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 */),
+                                          MultiAssetsProvider::Create(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());
@@ -386,12 +386,15 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_
         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)),
+                                     MultiAssetsProvider::Create(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 +411,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(MultiAssetsProvider::Create(
+                                              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..11b12eb030a6 100644
--- a/libs/androidfw/AssetsProvider.cpp
+++ b/libs/androidfw/AssetsProvider.cpp
@@ -24,9 +24,8 @@
 #include <ziparchive/zip_archive.h>
 
 namespace android {
-namespace {
-constexpr const char* kEmptyDebugString = "<empty>";
-} // namespace
+
+static constexpr std::string_view kEmptyDebugString = "<empty>";
 
 std::unique_ptr<Asset> AssetsProvider::Open(const std::string& path, Asset::AccessMode mode,
                                             bool* file_exists) const {
@@ -356,8 +355,14 @@ MultiAssetsProvider::MultiAssetsProvider(std::unique_ptr<AssetsProvider>&& prima
 
 std::unique_ptr<AssetsProvider> MultiAssetsProvider::Create(
     std::unique_ptr<AssetsProvider>&& primary, std::unique_ptr<AssetsProvider>&& secondary) {
-  if (primary == nullptr || secondary == nullptr) {
-    return nullptr;
+  if (primary == nullptr && secondary == nullptr) {
+    return EmptyAssetsProvider::Create();
+  }
+  if (!primary) {
+    return secondary;
+  }
+  if (!secondary) {
+    return primary;
   }
   return std::unique_ptr<MultiAssetsProvider>(new MultiAssetsProvider(std::move(primary),
                                                                       std::move(secondary)));
@@ -425,7 +430,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..e3b3ae41f7f4 100644
--- a/libs/androidfw/include/androidfw/AssetsProvider.h
+++ b/libs/androidfw/include/androidfw/AssetsProvider.h
@@ -164,7 +164,7 @@ struct DirectoryAssetsProvider : public AssetsProvider {
 // `secondary` asset provider if the asset cannot be found in the `primary`.
 struct MultiAssetsProvider : public AssetsProvider {
   static std::unique_ptr<AssetsProvider> Create(std::unique_ptr<AssetsProvider>&& primary,
-                                                std::unique_ptr<AssetsProvider>&& secondary);
+                                                std::unique_ptr<AssetsProvider>&& secondary = {});
 
   bool ForEachFile(const std::string& root_path,
                    base::function_ref<void(StringPiece, FileType)> f) const override;