Fix settings slice caching
Previously if there were multiple clients it would trigger an infinite
loop as the cache gits dropped after the first bind, and the second
client would trigger another load. Instead now cache as long as slices
are pinned since thats the intended behavior of caching.
Test: make RunSettingsRoboTests
Change-Id: I7d29fab87573120b34cd55e1696c4c5b70fc891c
Fixes: 78471335
diff --git a/src/com/android/settings/slices/SettingsSliceProvider.java b/src/com/android/settings/slices/SettingsSliceProvider.java
index 70e9d76..f449605 100644
--- a/src/com/android/settings/slices/SettingsSliceProvider.java
+++ b/src/com/android/settings/slices/SettingsSliceProvider.java
@@ -29,6 +29,7 @@
import android.support.annotation.VisibleForTesting;
import android.support.v4.graphics.drawable.IconCompat;
import android.text.TextUtils;
+import android.util.ArrayMap;
import android.util.Log;
import android.util.Pair;
@@ -38,6 +39,7 @@
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.WeakHashMap;
@@ -111,12 +113,14 @@
SlicesDatabaseAccessor mSlicesDatabaseAccessor;
@VisibleForTesting
+ Map<Uri, SliceData> mSliceWeakDataCache;
Map<Uri, SliceData> mSliceDataCache;
@Override
public boolean onCreateSliceProvider() {
mSlicesDatabaseAccessor = new SlicesDatabaseAccessor(getContext());
- mSliceDataCache = new WeakHashMap<>();
+ mSliceDataCache = new ArrayMap<>();
+ mSliceWeakDataCache = new WeakHashMap<>();
return true;
}
@@ -132,6 +136,17 @@
}
@Override
+ public void onSlicePinned(Uri sliceUri) {
+ // Start warming the slice, we expect someone will want it soon.
+ loadSliceInBackground(sliceUri);
+ }
+
+ @Override
+ public void onSliceUnpinned(Uri sliceUri) {
+ mSliceDataCache.remove(sliceUri);
+ }
+
+ @Override
public Slice onBindSlice(Uri sliceUri) {
String path = sliceUri.getPath();
// If adding a new Slice, do not directly match Slice URIs.
@@ -141,14 +156,16 @@
return createWifiSlice(sliceUri);
}
- SliceData cachedSliceData = mSliceDataCache.get(sliceUri);
+ SliceData cachedSliceData = mSliceWeakDataCache.get(sliceUri);
if (cachedSliceData == null) {
loadSliceInBackground(sliceUri);
return getSliceStub(sliceUri);
}
// Remove the SliceData from the cache after it has been used to prevent a memory-leak.
- mSliceDataCache.remove(sliceUri);
+ if (!mSliceDataCache.containsKey(sliceUri)) {
+ mSliceWeakDataCache.remove(sliceUri);
+ }
return SliceBuilderUtils.buildSlice(getContext(), cachedSliceData);
}
@@ -236,7 +253,12 @@
long startBuildTime = System.currentTimeMillis();
final SliceData sliceData = mSlicesDatabaseAccessor.getSliceDataFromUri(uri);
- mSliceDataCache.put(uri, sliceData);
+ List<Uri> pinnedSlices = getContext().getSystemService(
+ SliceManager.class).getPinnedSlices();
+ if (pinnedSlices.contains(uri)) {
+ mSliceDataCache.put(uri, sliceData);
+ }
+ mSliceWeakDataCache.put(uri, sliceData);
getContext().getContentResolver().notifyChange(uri, null /* content observer */);
Log.d(TAG, "Built slice (" + uri + ") in: " +
diff --git a/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java b/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java
index b5399ea..0fda33e 100644
--- a/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java
+++ b/tests/robotests/src/com/android/settings/slices/SettingsSliceProviderTest.java
@@ -21,9 +21,12 @@
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
+import android.app.slice.SliceManager;
import android.content.ContentResolver;
import android.content.ContentValues;
import android.content.Context;
@@ -43,7 +46,9 @@
import androidx.slice.Slice;
+import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
/**
@@ -65,17 +70,22 @@
private Context mContext;
private SettingsSliceProvider mProvider;
private SQLiteDatabase mDb;
+ private SliceManager mManager;
@Before
public void setUp() {
mContext = spy(RuntimeEnvironment.application);
mProvider = spy(new SettingsSliceProvider());
+ mProvider.mSliceWeakDataCache = new HashMap<>();
mProvider.mSliceDataCache = new HashMap<>();
mProvider.mSlicesDatabaseAccessor = new SlicesDatabaseAccessor(mContext);
when(mProvider.getContext()).thenReturn(mContext);
mDb = SlicesDatabaseHelper.getInstance(mContext).getWritableDatabase();
SlicesDatabaseHelper.getInstance(mContext).setIndexedState();
+ mManager = mock(SliceManager.class);
+ when(mContext.getSystemService(SliceManager.class)).thenReturn(mManager);
+ when(mManager.getPinnedSlices()).thenReturn(Collections.emptyList());
}
@After
@@ -99,6 +109,30 @@
Uri uri = SliceBuilderUtils.getUri(INTENT_PATH, false);
mProvider.loadSlice(uri);
+ SliceData data = mProvider.mSliceWeakDataCache.get(uri);
+
+ assertThat(data.getKey()).isEqualTo(KEY);
+ assertThat(data.getTitle()).isEqualTo(TITLE);
+ }
+
+ @Test
+ public void testLoadSlice_doesntCacheWithoutPin() {
+ insertSpecialCase(KEY);
+ Uri uri = SliceBuilderUtils.getUri(INTENT_PATH, false);
+
+ mProvider.loadSlice(uri);
+ SliceData data = mProvider.mSliceDataCache.get(uri);
+
+ assertThat(data).isNull();
+ }
+
+ @Test
+ public void testLoadSlice_cachesWithPin() {
+ insertSpecialCase(KEY);
+ Uri uri = SliceBuilderUtils.getUri(INTENT_PATH, false);
+ when(mManager.getPinnedSlices()).thenReturn(Arrays.asList(uri));
+
+ mProvider.loadSlice(uri);
SliceData data = mProvider.mSliceDataCache.get(uri);
assertThat(data.getKey()).isEqualTo(KEY);
@@ -108,11 +142,23 @@
@Test
public void testLoadSlice_cachedEntryRemovedOnBuild() {
SliceData data = getDummyData();
- mProvider.mSliceDataCache.put(data.getUri(), data);
+ mProvider.mSliceWeakDataCache.put(data.getUri(), data);
mProvider.onBindSlice(data.getUri());
insertSpecialCase(data.getKey());
- SliceData cachedData = mProvider.mSliceDataCache.get(data.getUri());
+ SliceData cachedData = mProvider.mSliceWeakDataCache.get(data.getUri());
+
+ assertThat(cachedData).isNull();
+ }
+
+ @Test
+ public void testLoadSlice_cachedEntryRemovedOnUnpin() {
+ SliceData data = getDummyData();
+ mProvider.mSliceDataCache.put(data.getUri(), data);
+ mProvider.onSliceUnpinned(data.getUri());
+ insertSpecialCase(data.getKey());
+
+ SliceData cachedData = mProvider.mSliceWeakDataCache.get(data.getUri());
assertThat(cachedData).isNull();
}