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
2020-03-16 14:54:02 -07:00
parent 4ea1e42889
commit 39cacf2de7
8 changed files with 198 additions and 63 deletions
--- 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<>();
--- 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);
                }
            }

@@ -328,6 +325,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`.
     *
--- 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();
        }
    }

--- 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)

--- 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
--- 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.
--- 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();
--- 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");