From e0327ee583626294f05da8ab949fabc474c8134a Mon Sep 17 00:00:00 2001 From: Jason Chiu Date: Fri, 17 Apr 2020 19:05:08 +0800 Subject: [PATCH] Reload homepage cards when necessary Many users leave Settings app by pressing Home key, but Settings remains in the same card status and doesn't update when users come back, which may lead to a bad UX. This change reloads cards and resets the UI session for some events, including home key, recent app key, and screen off. Fixes: 151789260 Test: robotest Change-Id: Idb575cef4a58894984cb42238d7b3b43c49389a3 --- .../ContextualCardManager.java | 22 +- .../ContextualCardsAdapter.java | 2 - .../ContextualCardsDiffCallback.java | 14 +- .../ContextualCardsFragment.java | 115 ++++++++++- .../slices/DarkThemeSlice.java | 62 +----- .../slices/SliceContextualCardRenderer.java | 8 +- .../ContextualCardManagerTest.java | 59 +++++- .../ContextualCardsDiffCallbackTest.java | 22 +- .../ContextualCardsFragmentTest.java | 192 ++++++++++++++++++ 9 files changed, 419 insertions(+), 77 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsFragmentTest.java diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCardManager.java b/src/com/android/settings/homepage/contextualcards/ContextualCardManager.java index 0931a8083d8..fc666ec7319 100644 --- a/src/com/android/settings/homepage/contextualcards/ContextualCardManager.java +++ b/src/com/android/settings/homepage/contextualcards/ContextualCardManager.java @@ -117,13 +117,13 @@ public class ContextualCardManager implements ContextualCardLoader.CardContentLo } else { mSavedCards = savedInstanceState.getStringArrayList(KEY_CONTEXTUAL_CARDS); } - //for data provided by Settings + // for data provided by Settings for (@ContextualCard.CardType int cardType : getSettingsCards()) { setupController(cardType); } } - void loadContextualCards(LoaderManager loaderManager) { + void loadContextualCards(LoaderManager loaderManager, boolean restartLoaderNeeded) { if (mContext.getResources().getBoolean(R.bool.config_use_legacy_suggestion)) { Log.w(TAG, "Legacy suggestion contextual card enabled, skipping contextual cards."); return; @@ -132,9 +132,17 @@ public class ContextualCardManager implements ContextualCardLoader.CardContentLo final CardContentLoaderCallbacks cardContentLoaderCallbacks = new CardContentLoaderCallbacks(mContext); cardContentLoaderCallbacks.setListener(this); - // Use the cached data when navigating back to the first page and upon screen rotation. - loaderManager.initLoader(CARD_CONTENT_LOADER_ID, null /* bundle */, - cardContentLoaderCallbacks); + if (!restartLoaderNeeded) { + // Use the cached data when navigating back to the first page and upon screen rotation. + loaderManager.initLoader(CARD_CONTENT_LOADER_ID, null /* bundle */, + cardContentLoaderCallbacks); + } else { + // Reload all cards when navigating back after pressing home key, recent app key, or + // turn off screen. + mIsFirstLaunch = true; + loaderManager.restartLoader(CARD_CONTENT_LOADER_ID, null /* bundle */, + cardContentLoaderCallbacks); + } } private void loadCardControllers() { @@ -146,7 +154,7 @@ public class ContextualCardManager implements ContextualCardLoader.CardContentLo @VisibleForTesting int[] getSettingsCards() { if (!FeatureFlagUtils.isEnabled(mContext, FeatureFlags.CONDITIONAL_CARDS)) { - return new int[]{ContextualCard.CardType.LEGACY_SUGGESTION}; + return new int[] {ContextualCard.CardType.LEGACY_SUGGESTION}; } return new int[] {ContextualCard.CardType.CONDITIONAL, ContextualCard.CardType.LEGACY_SUGGESTION}; @@ -194,7 +202,7 @@ public class ContextualCardManager implements ContextualCardLoader.CardContentLo // except Conditional cards, all other cards are from the database. So when the map sent // here is empty, we only keep Conditional cards. if (cardTypes.isEmpty()) { - final Set conditionalCardTypes = new TreeSet() {{ + final Set conditionalCardTypes = new TreeSet() {{ add(ContextualCard.CardType.CONDITIONAL); add(ContextualCard.CardType.CONDITIONAL_HEADER); add(ContextualCard.CardType.CONDITIONAL_FOOTER); diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCardsAdapter.java b/src/com/android/settings/homepage/contextualcards/ContextualCardsAdapter.java index 4e010fd2066..b9bc43b32b5 100644 --- a/src/com/android/settings/homepage/contextualcards/ContextualCardsAdapter.java +++ b/src/com/android/settings/homepage/contextualcards/ContextualCardsAdapter.java @@ -135,8 +135,6 @@ public class ContextualCardsAdapter extends RecyclerView.Adapter cards = new ArrayList<>(); + cards.add(buildContextualCard(CustomSliceRegistry.CONTEXTUAL_WIFI_SLICE_URI, + ContextualCardProto.ContextualCard.Category.STICKY_VALUE, 1.02f)); + cards.add(buildContextualCard(CustomSliceRegistry.BLUETOOTH_DEVICES_SLICE_URI, + ContextualCardProto.ContextualCard.Category.STICKY_VALUE, 1.01f)); + cards.add(buildContextualCard(CustomSliceRegistry.LOW_STORAGE_SLICE_URI, + ContextualCardProto.ContextualCard.Category.SUGGESTION_VALUE, 0.01f)); + + final List sortedCards = mManager.sortCards(cards); + + assertThat(sortedCards.get(cards.size() - 1).getSliceUri()) + .isEqualTo(CustomSliceRegistry.BLUETOOTH_DEVICES_SLICE_URI); + assertThat(sortedCards.get(cards.size() - 2).getSliceUri()) + .isEqualTo(CustomSliceRegistry.CONTEXTUAL_WIFI_SLICE_URI); + } + @Test public void onContextualCardUpdated_emptyMapWithExistingCards_shouldOnlyKeepConditionalCard() { mManager.mContextualCards.add(new ConditionalContextualCard.Builder().build()); @@ -686,6 +730,17 @@ public class ContextualCardManagerTest { .build(); } + private ContextualCard buildContextualCard(Uri uri, int category, double rankingScore) { + return new ContextualCard.Builder() + .setName(uri.toString()) + .setCardType(ContextualCard.CardType.SLICE) + .setSliceUri(uri) + .setViewType(VIEW_TYPE_FULL_WIDTH) + .setCategory(category) + .setRankingScore(rankingScore) + .build(); + } + private List buildCategoriedCards(List cards, List categories) { final List result = new ArrayList<>(); diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallbackTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallbackTest.java index eb95f716b5c..1e17a0bae6b 100644 --- a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallbackTest.java +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallbackTest.java @@ -16,6 +16,9 @@ package com.android.settings.homepage.contextualcards; +import static com.android.settings.intelligence.ContextualCardProto.ContextualCard.Category.IMPORTANT_VALUE; +import static com.android.settings.intelligence.ContextualCardProto.ContextualCard.Category.STICKY_VALUE; + import static com.google.common.truth.Truth.assertThat; import android.net.Uri; @@ -24,7 +27,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; -import org.robolectric.RuntimeEnvironment; import java.util.ArrayList; import java.util.List; @@ -84,6 +86,24 @@ public class ContextualCardsDiffCallbackTest { assertThat(mDiffCallback.areContentsTheSame(0, 0)).isFalse(); } + @Test + public void areContentsTheSame_stickySlice_returnFalse() { + final ContextualCard card = getContextualCard("test1").mutate() + .setCategory(STICKY_VALUE).build(); + mNewCards.add(0, card); + + assertThat(mDiffCallback.areContentsTheSame(0, 0)).isFalse(); + } + + @Test + public void areContentsTheSame_importantSlice_returnFalse() { + final ContextualCard card = getContextualCard("test1").mutate() + .setCategory(IMPORTANT_VALUE).build(); + mNewCards.add(0, card); + + assertThat(mDiffCallback.areContentsTheSame(0, 0)).isFalse(); + } + private ContextualCard getContextualCard(String name) { return new ContextualCard.Builder() .setName(name) diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsFragmentTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsFragmentTest.java new file mode 100644 index 00000000000..d1277913f04 --- /dev/null +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsFragmentTest.java @@ -0,0 +1,192 @@ +/* + * Copyright (C) 2020 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 com.android.settings.homepage.contextualcards; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; + +import androidx.fragment.app.FragmentActivity; +import androidx.lifecycle.LifecycleOwner; +import androidx.lifecycle.ViewModelStoreOwner; +import androidx.loader.app.LoaderManager; + +import com.android.settings.homepage.contextualcards.ContextualCardsFragment.ScreenOffReceiver; +import com.android.settings.slices.SlicesFeatureProvider; +import com.android.settings.testutils.FakeFeatureFactory; +import com.android.settings.testutils.shadow.ShadowFragment; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; + +@RunWith(RobolectricTestRunner.class) +@Config(shadows = {ShadowFragment.class, ContextualCardsFragmentTest.ShadowLoaderManager.class, + ContextualCardsFragmentTest.ShadowContextualCardManager.class}) +public class ContextualCardsFragmentTest { + + @Mock + private FragmentActivity mActivity; + private Context mContext; + private ContextualCardsFragment mFragment; + private SlicesFeatureProvider mSlicesFeatureProvider; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mContext = RuntimeEnvironment.application; + mSlicesFeatureProvider = FakeFeatureFactory.setupForTest().slicesFeatureProvider; + + mFragment = spy(new ContextualCardsFragment()); + doReturn(mActivity).when(mFragment).getActivity(); + mFragment.onCreate(null); + } + + @Test + public void onStart_shouldRegisterBothReceivers() { + mFragment.onStart(); + + verify(mActivity).registerReceiver(eq(mFragment.mKeyEventReceiver), + any(IntentFilter.class)); + verify(mActivity).registerReceiver(eq(mFragment.mScreenOffReceiver), + any(IntentFilter.class)); + } + + @Test + public void onStop_shouldUnregisterKeyEventReceiver() { + mFragment.onStart(); + mFragment.onStop(); + + verify(mActivity).unregisterReceiver(eq(mFragment.mKeyEventReceiver)); + } + + @Test + public void onDestroy_shouldUnregisterScreenOffReceiver() { + mFragment.onStart(); + mFragment.onDestroy(); + + verify(mActivity).unregisterReceiver(any(ScreenOffReceiver.class)); + } + + @Test + public void onStart_needRestartLoader_shouldClearRestartLoaderNeeded() { + mFragment.sRestartLoaderNeeded = true; + + mFragment.onStart(); + + assertThat(mFragment.sRestartLoaderNeeded).isFalse(); + } + + @Test + public void onReceive_homeKey_shouldResetSession() { + final Intent intent = new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + intent.putExtra("reason", "homekey"); + mFragment.onStart(); + + mFragment.mKeyEventReceiver.onReceive(mContext, intent); + + assertThat(mFragment.sRestartLoaderNeeded).isTrue(); + verify(mSlicesFeatureProvider, times(2)).newUiSession(); + verify(mActivity).unregisterReceiver(any(ScreenOffReceiver.class)); + } + + @Test + public void onReceive_recentApps_shouldResetSession() { + final Intent intent = new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + intent.putExtra("reason", "recentapps"); + mFragment.onStart(); + + mFragment.mKeyEventReceiver.onReceive(mContext, intent); + + assertThat(mFragment.sRestartLoaderNeeded).isTrue(); + verify(mSlicesFeatureProvider, times(2)).newUiSession(); + verify(mActivity).unregisterReceiver(any(ScreenOffReceiver.class)); + } + + @Test + public void onReceive_otherKey_shouldNotResetSession() { + final Intent intent = new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + intent.putExtra("reason", "other"); + mFragment.onStart(); + + mFragment.mKeyEventReceiver.onReceive(mContext, intent); + + assertThat(mFragment.sRestartLoaderNeeded).isFalse(); + verify(mSlicesFeatureProvider).newUiSession(); + verify(mActivity, never()).unregisterReceiver(any(ScreenOffReceiver.class)); + } + + @Test + public void onReceive_screenOff_shouldResetSession() { + final Intent intent = new Intent(Intent.ACTION_SCREEN_OFF); + mFragment.onStart(); + + mFragment.mScreenOffReceiver.onReceive(mContext, intent); + + assertThat(mFragment.sRestartLoaderNeeded).isTrue(); + verify(mSlicesFeatureProvider, times(2)).newUiSession(); + verify(mActivity).unregisterReceiver(any(ScreenOffReceiver.class)); + } + + @Implements(value = LoaderManager.class) + static class ShadowLoaderManager { + + @Mock + private static LoaderManager sLoaderManager; + + @Implementation + public static LoaderManager getInstance( + T owner) { + return sLoaderManager; + } + } + + @Implements(value = ContextualCardManager.class) + public static class ShadowContextualCardManager { + + public ShadowContextualCardManager() { + } + + @Implementation + protected void setupController(int cardType) { + // do nothing + } + + @Implementation + protected void loadContextualCards(LoaderManager loaderManager, + boolean restartLoaderNeeded) { + // do nothing + } + } +}