From 4913183defc66c8877b98fc5f26a0a5115841fc6 Mon Sep 17 00:00:00 2001 From: Jason Monk Date: Tue, 24 Apr 2018 17:08:26 -0400 Subject: [PATCH] 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 --- .../slices/SettingsSliceProvider.java | 30 +++++++++-- .../slices/SettingsSliceProviderTest.java | 50 ++++++++++++++++++- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/com/android/settings/slices/SettingsSliceProvider.java b/src/com/android/settings/slices/SettingsSliceProvider.java index 70e9d763f64..f4496057e0b 100644 --- a/src/com/android/settings/slices/SettingsSliceProvider.java +++ b/src/com/android/settings/slices/SettingsSliceProvider.java @@ -29,6 +29,7 @@ import android.provider.SettingsSlicesContract; 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 com.android.settingslib.utils.ThreadUtils; 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 @@ public class SettingsSliceProvider extends SliceProvider { SlicesDatabaseAccessor mSlicesDatabaseAccessor; @VisibleForTesting + Map mSliceWeakDataCache; Map mSliceDataCache; @Override public boolean onCreateSliceProvider() { mSlicesDatabaseAccessor = new SlicesDatabaseAccessor(getContext()); - mSliceDataCache = new WeakHashMap<>(); + mSliceDataCache = new ArrayMap<>(); + mSliceWeakDataCache = new WeakHashMap<>(); return true; } @@ -131,6 +135,17 @@ public class SettingsSliceProvider extends SliceProvider { } } + @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(); @@ -141,14 +156,16 @@ public class SettingsSliceProvider extends SliceProvider { 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 @@ public class SettingsSliceProvider extends SliceProvider { long startBuildTime = System.currentTimeMillis(); final SliceData sliceData = mSlicesDatabaseAccessor.getSliceDataFromUri(uri); - mSliceDataCache.put(uri, sliceData); + List 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 b5399eaa951..0fda33e07f2 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 android.content.ContentResolver.SCHEME_CONTENT; 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 org.robolectric.RuntimeEnvironment; import androidx.slice.Slice; +import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; /** @@ -65,17 +70,22 @@ public class SettingsSliceProviderTest { 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 @@ -98,6 +108,30 @@ public class SettingsSliceProviderTest { insertSpecialCase(KEY); 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); @@ -108,11 +142,23 @@ public class SettingsSliceProviderTest { @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(); }