Merge "Modify PropertyInvalidatedCache" into sc-dev am: c1ad465c8a

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/14922569

Change-Id: I45de8590816dae29ef5973a59c0f0c38d7630ce4

core/java/android/app/PropertyInvalidatedCache.java

@@ -34,6 +34,7 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -259,13 +260,20 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
     @GuardedBy("sCorkLock")
     private static final HashMap<String, Integer> sCorks = new HashMap<>();
 
+    /**
+     * A map of cache keys that have been disabled in the local process.  When a key is
+     * disabled locally, existing caches are disabled and the key is saved in this map.
+     * Future cache instances that use the same key will be disabled in their constructor.
+     */
+    @GuardedBy("sCorkLock")
+    private static final HashSet<String> sDisabledKeys = new HashSet<>();
+
     /**
      * Weakly references all cache objects in the current process, allowing us to iterate over
      * them all for purposes like issuing debug dumps and reacting to memory pressure.
      */
     @GuardedBy("sCorkLock")
-    private static final WeakHashMap<PropertyInvalidatedCache, Void> sCaches =
+    private static final WeakHashMap<PropertyInvalidatedCache, Void> sCaches = new WeakHashMap<>();
-            new WeakHashMap<>();
 
     private final Object mLock = new Object();
 
@@ -348,6 +356,9 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
             };
         synchronized (sCorkLock) {
             sCaches.put(this, null);
+            if (sDisabledKeys.contains(mCacheName)) {
+                disableInstance();
+            }
         }
     }
 
@@ -371,6 +382,14 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
      */
     protected abstract Result recompute(Query query);
 
+    /**
+     * Return true if the query should bypass the cache.  The default behavior is to
+     * always use the cache but the method can be overridden for a specific class.
+     */
+    protected boolean bypass(Query query) {
+        return false;
+    }
+
     /**
      * Determines if a pair of responses are considered equal. Used to determine whether
      * a cache is inadvertently returning stale results when VERIFY is set to true.
@@ -414,13 +433,37 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
     /**
      * Disable the use of this cache in this process.
      */
-    public final void disableLocal() {
+    public final void disableInstance() {
         synchronized (mLock) {
             mDisabled = true;
             clear();
         }
     }
 
+    /**
+     * Disable the local use of all caches with the same name.  All currently registered caches
+     * using the key will be disabled now, and all future cache instances that use the key will be
+     * disabled in their constructor.
+     */
+    public static final void disableLocal(@NonNull String name) {
+        synchronized (sCorkLock) {
+            sDisabledKeys.add(name);
+            for (PropertyInvalidatedCache cache : sCaches.keySet()) {
+                if (name.equals(cache.mCacheName)) {
+                    cache.disableInstance();
+                }
+            }
+        }
+    }
+
+    /**
+     * Disable this cache in the current process, and all other caches that use the same
+     * property.
+     */
+    public final void disableLocal() {
+        disableLocal(mCacheName);
+    }
+
     /**
      * Return whether the cache is disabled in this process.
      */
@@ -435,8 +478,8 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
         // Let access to mDisabled race: it's atomic anyway.
         long currentNonce = (!isDisabledLocal()) ? getCurrentNonce() : NONCE_DISABLED;
         for (;;) {
-            if (currentNonce == NONCE_DISABLED || currentNonce == NONCE_UNSET ||
+            if (currentNonce == NONCE_DISABLED || currentNonce == NONCE_UNSET
-                currentNonce == NONCE_CORKED) {
+                    || currentNonce == NONCE_CORKED || bypass(query)) {
                 if (!mDisabled) {
                     // Do not bother collecting statistics if the cache is
                     // locally disabled.
@@ -874,6 +917,15 @@ public abstract class PropertyInvalidatedCache<Query, Result> {
         sEnabled = false;
     }
 
+    /**
+     * Report the disabled status of this cache instance.  The return value does not
+     * reflect status of the property key.
+     */
+    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
+    public boolean getDisabledState() {
+        return isDisabledLocal();
+    }
+
     /**
      * Returns a list of caches alive at the current time.
      */

core/tests/coretests/src/android/app/PropertyInvalidatedCacheTests.java

@@ -0,0 +1,117 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.app;
+
+import static org.junit.Assert.assertEquals;
+
+import androidx.test.filters.SmallTest;
+
+import org.junit.Test;
+
+/**
+ * Test for verifying the behavior of {@link PropertyInvalidatedCache}.  This test does
+ * not use any actual binder calls - it is entirely self-contained.
+ * <p>
+ * Build/Install/Run:
+ *  atest FrameworksCoreTests:PropertyInvalidatedCacheTests
+ */
+@SmallTest
+public class PropertyInvalidatedCacheTests {
+
+    static final String CACHE_PROPERTY = "cache_key.cache_test_a";
+
+    // This class is a proxy for binder calls.  It contains a counter that increments
+    // every time the class is queried.
+    private static class ServerProxy {
+        // The number of times this class was queried.
+        private int mCount = 0;
+
+        // A single query.  The key behavior is that the query count is incremented.
+        boolean query(int x) {
+            mCount++;
+            return value(x);
+        }
+
+        // Return the expected value of an input, without incrementing the query count.
+        boolean value(int x) {
+            return x % 3 == 0;
+        }
+
+        // Verify the count.
+        void verify(int x) {
+            assertEquals(x, mCount);
+        }
+    }
+
+    @Test
+    public void testDisableCache1() {
+
+        // A stand-in for the binder.  The test verifies that calls are passed through to
+        // this class properly.
+        ServerProxy tester = new ServerProxy();
+
+        // Three caches, all using the same system property but one uses a different name.
+        PropertyInvalidatedCache<Integer, Boolean> cache1 =
+                new PropertyInvalidatedCache<>(4, CACHE_PROPERTY) {
+                    @Override
+                    protected Boolean recompute(Integer x) {
+                        return tester.query(x);
+                    }
+                };
+        PropertyInvalidatedCache<Integer, Boolean> cache2 =
+                new PropertyInvalidatedCache<>(4, CACHE_PROPERTY) {
+                    @Override
+                    protected Boolean recompute(Integer x) {
+                        return tester.query(x);
+                    }
+                };
+        PropertyInvalidatedCache<Integer, Boolean> cache3 =
+                new PropertyInvalidatedCache<>(4, CACHE_PROPERTY, "cache3") {
+                    @Override
+                    protected Boolean recompute(Integer x) {
+                        return tester.query(x);
+                    }
+                };
+
+        // Caches are enabled upon creation.
+        assertEquals(false, cache1.getDisabledState());
+        assertEquals(false, cache2.getDisabledState());
+        assertEquals(false, cache3.getDisabledState());
+
+        // Disable the cache1 instance.  Only cache1 is disabled
+        cache1.disableInstance();
+        assertEquals(true, cache1.getDisabledState());
+        assertEquals(false, cache2.getDisabledState());
+        assertEquals(false, cache3.getDisabledState());
+
+        // Disable cache1.  This will disable cache1 and cache2 because they share the
+        // same name.  cache3 has a different name and will not be disabled.
+        cache1.disableLocal();
+        assertEquals(true, cache1.getDisabledState());
+        assertEquals(true, cache2.getDisabledState());
+        assertEquals(false, cache3.getDisabledState());
+
+        // Create a new cache1.  Verify that the new instance is disabled.
+        cache1 = new PropertyInvalidatedCache<>(4, CACHE_PROPERTY) {
+                    @Override
+                    protected Boolean recompute(Integer x) {
+                        return tester.query(x);
+                    }
+                };
+        assertEquals(true, cache1.getDisabledState());
+    }
+}