Allow using loaders on non-RM Resources instances

Currently there is a limitation where ResourcesLoaders cannot be  used
on Resources object not created through ResourcesManager. This change
creates an update handler for Resources objects that are not registered
with ResourcesManager.

The handler changes the loaders on the asset manager owned by the
Resources instance.

Bug: 151666644
Test: atest ResourceLoaderValuesTest
Change-Id: I5a89f686386bdb088dc964014e7becc0c2b4770f
diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java
index 60f61ce..5f75603 100644
--- a/core/java/android/app/ResourcesManager.java
+++ b/core/java/android/app/ResourcesManager.java
@@ -1288,7 +1288,8 @@
          * instance uses.
          */
         @Override
-        public void onLoadersChanged(Resources resources, List<ResourcesLoader> newLoader) {
+        public void onLoadersChanged(@NonNull Resources resources,
+                @NonNull List<ResourcesLoader> newLoader) {
             synchronized (ResourcesManager.this) {
                 final ResourcesKey oldKey = findKeyForResourceImplLocked(resources.getImpl());
                 if (oldKey == null) {
@@ -1316,7 +1317,7 @@
          * {@code loader} to apply any changes of the set of {@link ApkAssets}.
          **/
         @Override
-        public void onLoaderUpdated(ResourcesLoader loader) {
+        public void onLoaderUpdated(@NonNull ResourcesLoader loader) {
             synchronized (ResourcesManager.this) {
                 final ArrayMap<ResourcesImpl, ResourcesKey> updatedResourceImplKeys =
                         new ArrayMap<>();
diff --git a/core/java/android/content/res/AssetManager.java b/core/java/android/content/res/AssetManager.java
index 7b2b939..d2103af 100644
--- a/core/java/android/content/res/AssetManager.java
+++ b/core/java/android/content/res/AssetManager.java
@@ -148,12 +148,9 @@
                 final List<ApkAssets> currentLoaderApkAssets = mLoaders.get(i).getApkAssets();
                 for (int j = currentLoaderApkAssets.size() - 1; j >= 0; j--) {
                     final ApkAssets apkAssets = currentLoaderApkAssets.get(j);
-                    if (uniqueLoaderApkAssets.contains(apkAssets)) {
-                        continue;
+                    if (uniqueLoaderApkAssets.add(apkAssets)) {
+                        loaderApkAssets.add(0, apkAssets);
                     }
-
-                    uniqueLoaderApkAssets.add(apkAssets);
-                    loaderApkAssets.add(0, apkAssets);
                 }
             }
 
@@ -329,6 +326,42 @@
     }
 
     /**
+     * Changes the {@link ResourcesLoader ResourcesLoaders} used in this AssetManager.
+     * @hide
+     */
+    void setLoaders(@NonNull List<ResourcesLoader> newLoaders) {
+        Objects.requireNonNull(newLoaders, "newLoaders");
+
+        final ArrayList<ApkAssets> apkAssets = new ArrayList<>();
+        for (int i = 0; i < mApkAssets.length; i++) {
+            // Filter out the previous loader apk assets.
+            if (!mApkAssets[i].isForLoader()) {
+                apkAssets.add(mApkAssets[i]);
+            }
+        }
+
+        if (!newLoaders.isEmpty()) {
+            // Filter so that assets provided by multiple loaders are only included once
+            // in the final assets list. The last appearance of the ApkAssets dictates its load
+            // order.
+            final int loaderStartIndex = apkAssets.size();
+            final ArraySet<ApkAssets> uniqueLoaderApkAssets = new ArraySet<>();
+            for (int i = newLoaders.size() - 1; i >= 0; i--) {
+                final List<ApkAssets> currentLoaderApkAssets = newLoaders.get(i).getApkAssets();
+                for (int j = currentLoaderApkAssets.size() - 1; j >= 0; j--) {
+                    final ApkAssets loaderApkAssets = currentLoaderApkAssets.get(j);
+                    if (uniqueLoaderApkAssets.add(loaderApkAssets)) {
+                        apkAssets.add(loaderStartIndex, loaderApkAssets);
+                    }
+                }
+            }
+        }
+
+        mLoaders = newLoaders.toArray(new ResourcesLoader[0]);
+        setApkAssets(apkAssets.toArray(new ApkAssets[0]), true /* invalidate_caches */);
+    }
+
+    /**
      * Invalidates the caches in this AssetManager according to the bitmask `diff`.
      *
      * @param diff The bitmask of changes generated by {@link Configuration#diff(Configuration)}.
diff --git a/core/java/android/content/res/Resources.java b/core/java/android/content/res/Resources.java
index e77d8af..d6a9f69 100644
--- a/core/java/android/content/res/Resources.java
+++ b/core/java/android/content/res/Resources.java
@@ -66,6 +66,7 @@
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.ArrayUtils;
 import com.android.internal.util.GrowingArrayUtils;
+import com.android.internal.util.Preconditions;
 import com.android.internal.util.XmlUtils;
 
 import org.xmlpull.v1.XmlPullParser;
@@ -244,7 +245,36 @@
          * @param resources the instance being updated
          * @param newLoaders the new set of loaders for the instance
          */
-        void onLoadersChanged(Resources resources, List<ResourcesLoader> newLoaders);
+        void onLoadersChanged(@NonNull Resources resources,
+                @NonNull List<ResourcesLoader> newLoaders);
+    }
+
+    /**
+     * Handler that propagates updates of the {@link Resources} instance to the underlying
+     * {@link AssetManager} when the Resources is not registered with a
+     * {@link android.app.ResourcesManager}.
+     * @hide
+     */
+    public class AssetManagerUpdateHandler implements UpdateCallbacks{
+
+        @Override
+        public void onLoadersChanged(@NonNull Resources resources,
+                @NonNull List<ResourcesLoader> newLoaders) {
+            Preconditions.checkArgument(Resources.this == resources);
+            final ResourcesImpl impl = mResourcesImpl;
+            impl.clearAllCaches();
+            impl.getAssets().setLoaders(newLoaders);
+        }
+
+        @Override
+        public void onLoaderUpdated(@NonNull ResourcesLoader loader) {
+            final ResourcesImpl impl = mResourcesImpl;
+            final AssetManager assets = impl.getAssets();
+            if (assets.getLoaders().contains(loader)) {
+                impl.clearAllCaches();
+                assets.setLoaders(assets.getLoaders());
+            }
+        }
     }
 
     /**
@@ -2367,8 +2397,9 @@
 
     private void checkCallbacksRegistered() {
         if (mCallbacks == null) {
-            throw new IllegalArgumentException("Cannot modify resource loaders of Resources"
-                    + " instances created outside of ResourcesManager");
+            // Fallback to updating the underlying AssetManager if the Resources is not associated
+            // with a ResourcesManager.
+            mCallbacks = new AssetManagerUpdateHandler();
         }
     }
 
diff --git a/core/tests/ResourceLoaderTests/src/android/content/res/loader/test/ResourceLoaderTestBase.kt b/core/tests/ResourceLoaderTests/src/android/content/res/loader/test/ResourceLoaderTestBase.kt
index 4764c10..ec6a605 100644
--- a/core/tests/ResourceLoaderTests/src/android/content/res/loader/test/ResourceLoaderTestBase.kt
+++ b/core/tests/ResourceLoaderTests/src/android/content/res/loader/test/ResourceLoaderTestBase.kt
@@ -68,8 +68,7 @@
     open lateinit var dataType: DataType
 
     protected lateinit var context: Context
-    protected open val resources: Resources
-        get() = context.resources
+    protected lateinit var resources: Resources
 
     // Track opened streams and ResourcesProviders to close them after testing
     private val openedObjects = mutableListOf<Closeable>()
@@ -78,6 +77,7 @@
     fun setUpBase() {
         context = InstrumentationRegistry.getTargetContext()
                 .createConfigurationContext(Configuration())
+        resources = context.resources
     }
 
     @After
@@ -92,42 +92,42 @@
         }
     }
 
-    protected fun String.openProvider(dataType: DataType, assetsProvider: MemoryAssetsProvider?)
-            :ResourcesProvider {
+    protected fun String.openProvider(dataType: DataType,
+                                      assetsProvider: MemoryAssetsProvider?): ResourcesProvider {
         if (assetsProvider != null) {
             openedObjects += assetsProvider
         }
         return when (dataType) {
             DataType.APK_DISK_FD -> {
-                val file = context.copiedAssetFile("${this}.apk")
+                val file = context.copiedAssetFile("$this.apk")
                 ResourcesProvider.loadFromApk(ParcelFileDescriptor.fromFd(file.fd),
                         assetsProvider).apply {
                     file.close()
                 }
             }
             DataType.APK_DISK_FD_OFFSETS -> {
-                val asset = context.assets.openFd("${this}.apk")
+                val asset = context.assets.openFd("$this.apk")
                 ResourcesProvider.loadFromApk(asset.parcelFileDescriptor, asset.startOffset,
                         asset.length, assetsProvider).apply {
                     asset.close()
                 }
             }
             DataType.ARSC_DISK_FD -> {
-                val file = context.copiedAssetFile("${this}.arsc")
+                val file = context.copiedAssetFile("$this.arsc")
                 ResourcesProvider.loadFromTable(ParcelFileDescriptor.fromFd(file.fd),
                         assetsProvider).apply {
                     file.close()
                 }
             }
             DataType.ARSC_DISK_FD_OFFSETS -> {
-                val asset = context.assets.openFd("${this}.arsc")
+                val asset = context.assets.openFd("$this.arsc")
                 ResourcesProvider.loadFromTable(asset.parcelFileDescriptor, asset.startOffset,
                         asset.length, assetsProvider).apply {
                     asset.close()
                 }
             }
             DataType.APK_RAM_OFFSETS -> {
-                val asset = context.assets.openFd("${this}.apk")
+                val asset = context.assets.openFd("$this.apk")
                 val leadingGarbageSize = 100L
                 val trailingGarbageSize = 55L
                 val fd = loadAssetIntoMemory(asset, leadingGarbageSize.toInt(),
@@ -139,7 +139,7 @@
                 }
             }
             DataType.APK_RAM_FD -> {
-                val asset = context.assets.openFd("${this}.apk")
+                val asset = context.assets.openFd("$this.apk")
                 var fd = loadAssetIntoMemory(asset)
                 ResourcesProvider.loadFromApk(fd, assetsProvider).apply {
                     asset.close()
@@ -147,7 +147,7 @@
                 }
             }
             DataType.ARSC_RAM_MEMORY -> {
-                val asset = context.assets.openFd("${this}.arsc")
+                val asset = context.assets.openFd("$this.arsc")
                 var fd = loadAssetIntoMemory(asset)
                 ResourcesProvider.loadFromTable(fd, assetsProvider).apply {
                     asset.close()
@@ -155,7 +155,7 @@
                 }
             }
             DataType.ARSC_RAM_MEMORY_OFFSETS -> {
-                val asset = context.assets.openFd("${this}.arsc")
+                val asset = context.assets.openFd("$this.arsc")
                 val leadingGarbageSize = 100L
                 val trailingGarbageSize = 55L
                 val fd = loadAssetIntoMemory(asset, leadingGarbageSize.toInt(),
@@ -175,7 +175,7 @@
                 }
             }
             DataType.DIRECTORY -> {
-                ResourcesProvider.loadFromDirectory(zipToDir("${this}.apk").absolutePath,
+                ResourcesProvider.loadFromDirectory(zipToDir("$this.apk").absolutePath,
                         assetsProvider)
             }
             DataType.SPLIT -> {
@@ -186,9 +186,9 @@
 
     class EmptyAssetsProvider : AssetsProvider
 
-    /** */
-    inner class ZipAssetsProvider(val providerName : String) : AssetsProvider {
-        val root: File = zipToDir("${providerName}.apk")
+    /** An AssetsProvider that reads from a zip asset. */
+    inner class ZipAssetsProvider(val providerName: String) : AssetsProvider {
+        val root: File = zipToDir("$providerName.apk")
 
         override fun loadAssetFd(path: String, accessMode: Int): AssetFileDescriptor? {
             val f = File(root, path)
@@ -203,7 +203,7 @@
     class MemoryAssetsProvider : AssetsProvider, Closeable {
         var loadAssetResults = HashMap<String, FileDescriptor>()
 
-        fun addLoadAssetFdResult(path : String, value : String) = apply {
+        fun addLoadAssetFdResult(path: String, value: String) = apply {
             val fd = Os.memfd_create(path, 0)
             val valueBytes = value.toByteArray()
             Os.write(fd, valueBytes, 0, valueBytes.size)
@@ -224,7 +224,7 @@
     }
 
     /** Extracts an archive-based asset into a directory on disk. */
-    private fun zipToDir(name : String) : File {
+    private fun zipToDir(name: String): File {
         val root = File(context.filesDir, name.split('.')[0])
         if (root.exists()) {
             return root
@@ -252,13 +252,13 @@
         return root
     }
 
-
     /** Loads the asset into a temporary file stored in RAM. */
-    private fun loadAssetIntoMemory(asset: AssetFileDescriptor,
-                                    leadingGarbageSize: Int = 0,
-                                    trailingGarbageSize: Int = 0
+    private fun loadAssetIntoMemory(
+        asset: AssetFileDescriptor,
+        leadingGarbageSize: Int = 0,
+        trailingGarbageSize: Int = 0
     ): ParcelFileDescriptor {
-        val originalFd = Os.memfd_create(asset.toString(), 0 /* flags */);
+        val originalFd = Os.memfd_create(asset.toString(), 0 /* flags */)
         val fd = ParcelFileDescriptor.dup(originalFd)
         Os.close(originalFd)
 
diff --git a/core/tests/ResourceLoaderTests/src/android/content/res/loader/test/ResourceLoaderValuesTest.kt b/core/tests/ResourceLoaderTests/src/android/content/res/loader/test/ResourceLoaderValuesTest.kt
index a994536..5aa8814 100644
--- a/core/tests/ResourceLoaderTests/src/android/content/res/loader/test/ResourceLoaderValuesTest.kt
+++ b/core/tests/ResourceLoaderTests/src/android/content/res/loader/test/ResourceLoaderValuesTest.kt
@@ -19,6 +19,7 @@
 import android.app.Activity
 import android.content.Context
 import android.content.Intent
+import android.content.res.AssetManager
 import android.content.res.Configuration
 import android.content.res.Resources
 import android.content.res.loader.ResourcesLoader
@@ -63,22 +64,40 @@
                             },
                             "getAdditional" to { res ->
                                 res.getString(0x7f0400fe /* R.string.additional */)
+                            },
+                            "getIdentifier" to { res ->
+                                res.getString(res.getIdentifier("test", "string",
+                                        "android.content.res.loader.test"))
+                            },
+                            "getIdentifierAdditional" to { res ->
+                                res.getString(res.getIdentifier("additional", "string",
+                                        "android.content.res.loader.test"))
                             }
                     )),
                     mapOf("getOverlaid" to "Not overlaid",
-                            "getAdditional" to "NotFoundException"),
+                            "getAdditional" to "NotFoundException",
+                            "getIdentifier" to "Not overlaid",
+                            "getIdentifierAdditional" to "NotFoundException"),
 
                     mapOf("getOverlaid" to "One",
-                            "getAdditional" to "One"),
+                            "getAdditional" to "One",
+                            "getIdentifier" to "One",
+                            "getIdentifierAdditional" to "One"),
 
                     mapOf("getOverlaid" to "Two",
-                            "getAdditional" to "Two"),
+                            "getAdditional" to "Two",
+                            "getIdentifier" to "Two",
+                            "getIdentifierAdditional" to "Two"),
 
                     mapOf("getOverlaid" to "Three",
-                            "getAdditional" to "Three"),
+                            "getAdditional" to "Three",
+                            "getIdentifier" to "Three",
+                            "getIdentifierAdditional" to "Three"),
 
                     mapOf("getOverlaid" to "Four",
-                            "getAdditional" to "Four"),
+                            "getAdditional" to "Four",
+                            "getIdentifier" to "Four",
+                            "getIdentifierAdditional" to "Four"),
                     listOf(DataType.APK_DISK_FD, DataType.APK_DISK_FD_OFFSETS, DataType.APK_RAM_FD,
                             DataType.APK_RAM_OFFSETS, DataType.ARSC_DISK_FD,
                             DataType.ARSC_DISK_FD_OFFSETS, DataType.ARSC_RAM_MEMORY,
@@ -224,7 +243,7 @@
     lateinit var parameter: Parameter
 
     private val valueOriginal by lazy { mapToString(parameter.valueOriginal) }
-    private val valueOne by lazy {  mapToString(parameter.valueOne) }
+    private val valueOne by lazy { mapToString(parameter.valueOne) }
     private val valueTwo by lazy { mapToString(parameter.valueTwo) }
     private val valueThree by lazy { mapToString(parameter.valueThree) }
     private val valueFour by lazy { mapToString(parameter.valueFour) }
@@ -241,6 +260,7 @@
 
     // Class method for syntax highlighting purposes
     private fun getValue(c: Context = context) = parameter.getValue(c.resources)
+    private fun getValue(r: Resources) = parameter.getValue(r)
 
     @Test
     fun assertValueUniqueness() {
@@ -713,28 +733,79 @@
         provider.close()
     }
 
+    @Test
+    fun addLoadersRepeatedlyCustomResources() {
+        val res = Resources(AssetManager::class.java.newInstance(), resources.displayMetrics,
+                resources.configuration!!)
+        val originalValue = getValue(res)
+        val testOne = openOne()
+        val testTwo = openTwo()
+        val loader1 = ResourcesLoader()
+        val loader2 = ResourcesLoader()
+
+        res.addLoaders(loader1)
+        loader1.addProvider(testOne)
+        assertEquals(valueOne, getValue(res))
+
+        res.addLoaders(loader2)
+        loader2.addProvider(testTwo)
+        assertEquals(valueTwo, getValue(res))
+
+        res.removeLoaders(loader1)
+        res.addLoaders(loader1)
+        assertEquals(valueOne, getValue(res))
+
+        res.removeLoaders(loader1)
+        assertEquals(valueTwo, getValue(res))
+
+        res.removeLoaders(loader2)
+        assertEquals(originalValue, getValue(res))
+    }
+
+    @Test
+    fun setMultipleProvidersCustomResources() {
+        val res = Resources(AssetManager::class.java.newInstance(), resources.displayMetrics,
+                resources.configuration!!)
+        val originalValue = getValue(res)
+        val testOne = openOne()
+        val testTwo = openTwo()
+        val loader = ResourcesLoader()
+
+        res.addLoaders(loader)
+        loader.providers = listOf(testOne, testTwo)
+        assertEquals(valueTwo, getValue(res))
+
+        loader.removeProvider(testTwo)
+        assertEquals(valueOne, getValue(res))
+
+        loader.providers = Collections.emptyList()
+        assertEquals(originalValue, getValue(res))
+    }
+
     data class Parameter(
-            val testPrefix: String,
-            val getValue: Resources.() -> String,
-            val valueOriginal: Map<String, String>,
-            val valueOne: Map<String, String>,
-            val assetProviderOne: (() -> MemoryAssetsProvider)? = null,
-            val valueTwo: Map<String, String>,
-            val assetProviderTwo: (() -> MemoryAssetsProvider)? = null,
-            val valueThree: Map<String, String>,
-            val assetProviderThree: (() -> MemoryAssetsProvider)? = null,
-            val valueFour: Map<String, String>,
-            val assetProviderFour: (() -> MemoryAssetsProvider)? = null,
-            val dataTypes: List<DataType>
+        val testPrefix: String,
+        val getValue: Resources.() -> String,
+        val valueOriginal: Map<String, String>,
+        val valueOne: Map<String, String>,
+        val assetProviderOne: (() -> MemoryAssetsProvider)? = null,
+        val valueTwo: Map<String, String>,
+        val assetProviderTwo: (() -> MemoryAssetsProvider)? = null,
+        val valueThree: Map<String, String>,
+        val assetProviderThree: (() -> MemoryAssetsProvider)? = null,
+        val valueFour: Map<String, String>,
+        val assetProviderFour: (() -> MemoryAssetsProvider)? = null,
+        val dataTypes: List<DataType>
     ) {
-        constructor(testPrefix: String,
-                    getValue: Resources.() -> String,
-                    valueOriginal : Map<String, String>,
-                    valueOne: Map<String, String>,
-                    valueTwo: Map<String, String>,
-                    valueThree: Map<String, String>,
-                    valueFour: Map<String, String>,
-                    dataTypes: List<DataType>): this(testPrefix, getValue, valueOriginal, valueOne,
+        constructor(
+            testPrefix: String,
+            getValue: Resources.() -> String,
+            valueOriginal: Map<String, String>,
+            valueOne: Map<String, String>,
+            valueTwo: Map<String, String>,
+            valueThree: Map<String, String>,
+            valueFour: Map<String, String>,
+            dataTypes: List<DataType>
+        ): this(testPrefix, getValue, valueOriginal, valueOne,
                 null, valueTwo, null, valueThree, null, valueFour, null, dataTypes)
 
         override fun toString() = testPrefix
diff --git a/libs/androidfw/ApkAssets.cpp b/libs/androidfw/ApkAssets.cpp
index 202651d..918e7af 100644
--- a/libs/androidfw/ApkAssets.cpp
+++ b/libs/androidfw/ApkAssets.cpp
@@ -471,7 +471,7 @@
   bool resources_asset_exists = false;
   auto resources_asset_ = assets->Open(kResourcesArsc, Asset::AccessMode::ACCESS_BUFFER,
                                        &resources_asset_exists);
-  
+
   assets = MultiAssetsProvider::Create(std::move(override_assets), std::move(assets));
 
   // Wrap the handle in a unique_ptr so it gets automatically closed.
diff --git a/libs/androidfw/tests/ApkAssets_test.cpp b/libs/androidfw/tests/ApkAssets_test.cpp
index ce9e532..19db25c 100644
--- a/libs/androidfw/tests/ApkAssets_test.cpp
+++ b/libs/androidfw/tests/ApkAssets_test.cpp
@@ -50,8 +50,7 @@
   unique_fd fd(::open(path.c_str(), O_RDONLY | O_BINARY));
   ASSERT_THAT(fd.get(), Ge(0));
 
-  std::unique_ptr<const ApkAssets> loaded_apk =
-      ApkAssets::LoadFromFd(std::move(fd), path, false /*system*/, false /*force_shared_lib*/);
+  std::unique_ptr<const ApkAssets> loaded_apk = ApkAssets::LoadFromFd(std::move(fd), path);
   ASSERT_THAT(loaded_apk, NotNull());
 
   const LoadedArsc* loaded_arsc = loaded_apk->GetLoadedArsc();
diff --git a/libs/androidfw/tests/Theme_test.cpp b/libs/androidfw/tests/Theme_test.cpp
index be5ecd9..16b9c75 100644
--- a/libs/androidfw/tests/Theme_test.cpp
+++ b/libs/androidfw/tests/Theme_test.cpp
@@ -36,7 +36,7 @@
 class ThemeTest : public ::testing::Test {
  public:
   void SetUp() override {
-    system_assets_ = ApkAssets::Load(GetTestDataPath() + "/system/system.apk", true /*system*/);
+    system_assets_ = ApkAssets::Load(GetTestDataPath() + "/system/system.apk", PROPERTY_SYSTEM);
     ASSERT_NE(nullptr, system_assets_);
 
     style_assets_ = ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk");