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

8 files changed, 198 insertions, 63 deletions

diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java
index 60f61cef0b61..5f756033390b 100644
--- a/core/java/android/app/ResourcesManager.java
+++ b/core/java/android/app/ResourcesManager.java
@@ -1288,7 +1288,8 @@ public class ResourcesManager {
          * 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 @@ public class ResourcesManager {
          * {@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 7b2b93949a9f..d2103af1d247 100644
--- a/core/java/android/content/res/AssetManager.java
+++ b/core/java/android/content/res/AssetManager.java
@@ -148,12 +148,9 @@ public final class AssetManager implements AutoCloseable {
                 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 @@ public final class AssetManager implements AutoCloseable {
     }
 
     /**
+     * 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 e77d8af49873..d6a9f6990abe 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.GuardedBy;
 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 @@ public class Resources {
          * @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 @@ public class Resources {
 
     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 4764c1008d2f..ec6a605340ae 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 @@ abstract class ResourceLoaderTestBase {
     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 @@ abstract class ResourceLoaderTestBase {
     fun setUpBase() {
         context = InstrumentationRegistry.getTargetContext()
                 .createConfigurationContext(Configuration())
+        resources = context.resources
     }
 
     @After
@@ -92,42 +92,42 @@ abstract class ResourceLoaderTestBase {
         }
     }
 
-    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 @@ abstract class ResourceLoaderTestBase {
                 }
             }
             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 @@ abstract class ResourceLoaderTestBase {
                 }
             }
             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 @@ abstract class ResourceLoaderTestBase {
                 }
             }
             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 @@ abstract class ResourceLoaderTestBase {
                 }
             }
             DataType.DIRECTORY -> {
-                ResourcesProvider.loadFromDirectory(zipToDir("${this}.apk").absolutePath,
+                ResourcesProvider.loadFromDirectory(zipToDir("$this.apk").absolutePath,
                         assetsProvider)
             }
             DataType.SPLIT -> {
@@ -186,9 +186,9 @@ abstract class ResourceLoaderTestBase {
 
     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 @@ abstract class ResourceLoaderTestBase {
     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 @@ abstract class ResourceLoaderTestBase {
     }
 
     /** 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 @@ abstract class ResourceLoaderTestBase {
         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 a9945369d1df..5aa8814c7481 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 @@ package android.content.res.loader.test
 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 @@ class ResourceLoaderValuesTest : ResourceLoaderTestBase() {
                             },
                             "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 @@ class ResourceLoaderValuesTest : ResourceLoaderTestBase() {
     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 ResourceLoaderValuesTest : ResourceLoaderTestBase() {
 
     // 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 @@ class ResourceLoaderValuesTest : ResourceLoaderTestBase() {
         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 202651dc86d5..918e7af12d31 100644
--- a/libs/androidfw/ApkAssets.cpp
+++ b/libs/androidfw/ApkAssets.cpp
@@ -471,7 +471,7 @@ std::unique_ptr<const ApkAssets> ApkAssets::LoadImpl(
   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 ce9e53244801..19db25ce8811 100644
--- a/libs/androidfw/tests/ApkAssets_test.cpp
+++ b/libs/androidfw/tests/ApkAssets_test.cpp
@@ -50,8 +50,7 @@ TEST(ApkAssetsTest, LoadApkFromFd) {
   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 be5ecd94a588..16b9c75982fb 100644
--- a/libs/androidfw/tests/Theme_test.cpp
+++ b/libs/androidfw/tests/Theme_test.cpp
@@ -36,7 +36,7 @@ namespace android {
 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");