From acb50f2c6aa3772954af4733686f55689b3e4506 Mon Sep 17 00:00:00 2001 From: Yi-Ling Chuang Date: Thu, 18 Apr 2019 16:29:45 +0800 Subject: [PATCH] Force the adapter to rebind cards with a toggle. A card dismissal and scheduled card collection trigger a reload. During card reloading, we perform slices pre check. The pre check goes through the whole slice binding process, which means slices will be pinned and unpinned. Hence, the on screen slices will gets unpinned resulting to the unresponsive toggling. As we have DiffCallbck implmented, if the card list are the same, then bindView in the renderer will be ignored, which means the unpinned slice will have no chance to re-register slice callback. So here we force it to rebind the views. Fixes: 123174237 Test: robotests Change-Id: Id98bc16632bf024cbb611b40890e4d2629f08d7b --- .../contextualcards/ContextualCard.java | 14 +++ .../ContextualCardsDiffCallback.java | 6 +- .../contextualcards/EligibleCardChecker.java | 20 +++- .../ContextualCardsDiffCallbackTest.java | 95 ++++++++++++++++ .../EligibleCardCheckerTest.java | 103 ++++++++++++++++++ 5 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallbackTest.java create mode 100644 tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCard.java b/src/com/android/settings/homepage/contextualcards/ContextualCard.java index ede12fb00c1..ccfb22d3e17 100644 --- a/src/com/android/settings/homepage/contextualcards/ContextualCard.java +++ b/src/com/android/settings/homepage/contextualcards/ContextualCard.java @@ -72,6 +72,7 @@ public class ContextualCard { @LayoutRes private final int mViewType; private final boolean mIsPendingDismiss; + private final boolean mHasInlineAction; public String getName() { return mName; @@ -161,6 +162,10 @@ public class ContextualCard { return mIsPendingDismiss; } + public boolean hasInlineAction() { + return mHasInlineAction; + } + public Builder mutate() { return mBuilder; } @@ -187,6 +192,7 @@ public class ContextualCard { mIsLargeCard = builder.mIsLargeCard; mViewType = builder.mViewType; mIsPendingDismiss = builder.mIsPendingDismiss; + mHasInlineAction = builder.mHasInlineAction; } ContextualCard(Cursor c) { @@ -234,6 +240,8 @@ public class ContextualCard { mBuilder.setViewType(mViewType); mIsPendingDismiss = false; mBuilder.setIsPendingDismiss(mIsPendingDismiss); + mHasInlineAction = false; + mBuilder.setHasInlineAction(mHasInlineAction); } @Override @@ -286,6 +294,7 @@ public class ContextualCard { @LayoutRes private int mViewType; private boolean mIsPendingDismiss; + private boolean mHasInlineAction; public Builder setName(String name) { mName = name; @@ -387,6 +396,11 @@ public class ContextualCard { return this; } + public Builder setHasInlineAction(boolean hasInlineAction) { + mHasInlineAction = hasInlineAction; + return this; + } + public ContextualCard build() { return new ContextualCard(this); } diff --git a/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallback.java b/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallback.java index d1623cd8ef4..58d6a4197c0 100644 --- a/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallback.java +++ b/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallback.java @@ -20,7 +20,6 @@ import androidx.recyclerview.widget.DiffUtil; import java.util.List; -//TODO(b/117816826): add test cases for DiffUtil. /** * A DiffCallback to calculate the difference between old and new {@link ContextualCard} List. */ @@ -53,6 +52,11 @@ public class ContextualCardsDiffCallback extends DiffUtil.Callback { @Override public boolean areContentsTheSame(int oldCardPosition, int newCardPosition) { + // Slices with toggles needs to be updated continuously, which means their contents may + // change. So here we assume the content will always be different to force view rebinding. + if (mNewCards.get(newCardPosition).hasInlineAction()) { + return false; + } return mOldCards.get(oldCardPosition).equals(mNewCards.get(newCardPosition)); } } \ No newline at end of file diff --git a/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java b/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java index 811aaa2fa13..8558ee79566 100644 --- a/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java +++ b/src/com/android/settings/homepage/contextualcards/EligibleCardChecker.java @@ -26,11 +26,14 @@ import android.util.Log; import androidx.annotation.VisibleForTesting; import androidx.slice.Slice; +import androidx.slice.SliceMetadata; import androidx.slice.SliceViewManager; +import androidx.slice.core.SliceAction; import com.android.settings.overlay.FeatureFactory; import com.android.settingslib.core.instrumentation.MetricsFeatureProvider; +import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -41,7 +44,9 @@ public class EligibleCardChecker implements Callable { private static final long LATCH_TIMEOUT_MS = 200; private final Context mContext; - private final ContextualCard mCard; + + @VisibleForTesting + ContextualCard mCard; EligibleCardChecker(Context context, ContextualCard card) { mContext = context; @@ -93,6 +98,11 @@ public class EligibleCardChecker implements Callable { } final Slice slice = bindSlice(uri); + + if (isSliceToggleable(slice)) { + mCard = card.mutate().setHasInlineAction(true).build(); + } + if (slice == null || slice.hasHint(HINT_ERROR)) { Log.w(TAG, "Failed to bind slice, not eligible for display " + uri); return false; @@ -133,4 +143,12 @@ public class EligibleCardChecker implements Callable { } return returnSlice[0]; } + + @VisibleForTesting + boolean isSliceToggleable(Slice slice) { + final SliceMetadata metadata = SliceMetadata.from(mContext, slice); + final List toggles = metadata.getToggles(); + + return !toggles.isEmpty(); + } } diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallbackTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallbackTest.java new file mode 100644 index 00000000000..eb95f716b5c --- /dev/null +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/ContextualCardsDiffCallbackTest.java @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2019 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 android.net.Uri; + +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; + +@RunWith(RobolectricTestRunner.class) +public class ContextualCardsDiffCallbackTest { + + private static final Uri TEST_SLICE_URI = Uri.parse("content://test/test"); + + private ContextualCardsDiffCallback mDiffCallback; + private List mOldCards; + private List mNewCards; + + @Before + public void setUp() { + mOldCards = new ArrayList<>(); + mNewCards = new ArrayList<>(); + mOldCards.add(getContextualCard("test1")); + mNewCards.add(getContextualCard("test1")); + mNewCards.add(getContextualCard("test2")); + mDiffCallback = new ContextualCardsDiffCallback(mOldCards, mNewCards); + } + + @Test + public void getOldListSize_oneCard_returnOne() { + assertThat(mDiffCallback.getOldListSize()).isEqualTo(1); + } + + @Test + public void getNewListSize_twoCards_returnTwo() { + assertThat(mDiffCallback.getNewListSize()).isEqualTo(2); + } + + @Test + public void areItemsTheSame_sameItems_returnTrue() { + assertThat(mDiffCallback.areItemsTheSame(0, 0)).isTrue(); + } + + @Test + public void areItemsTheSame_differentItems_returnFalse() { + mOldCards.add(getContextualCard("test3")); + + assertThat(mDiffCallback.areItemsTheSame(1, 1)).isFalse(); + } + + @Test + public void areContentsTheSame_sameContents_returnTrue() { + assertThat(mDiffCallback.areContentsTheSame(0, 0)).isTrue(); + } + + @Test + public void areContentsTheSame_sliceWithToggle_returnFalse() { + final ContextualCard card = getContextualCard("test1").mutate() + .setHasInlineAction(true).build(); + mNewCards.add(0, card); + + assertThat(mDiffCallback.areContentsTheSame(0, 0)).isFalse(); + } + + private ContextualCard getContextualCard(String name) { + return new ContextualCard.Builder() + .setName(name) + .setRankingScore(0.5) + .setCardType(ContextualCard.CardType.SLICE) + .setSliceUri(TEST_SLICE_URI) + .build(); + } +} diff --git a/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java b/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java new file mode 100644 index 00000000000..4951705a60e --- /dev/null +++ b/tests/robotests/src/com/android/settings/homepage/contextualcards/EligibleCardCheckerTest.java @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2019 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.Mockito.doReturn; +import static org.mockito.Mockito.spy; + +import android.content.Context; +import android.net.Uri; + +import androidx.slice.Slice; +import androidx.slice.SliceProvider; +import androidx.slice.widget.SliceLiveData; + +import com.android.settings.homepage.contextualcards.deviceinfo.EmergencyInfoSlice; +import com.android.settings.wifi.slice.ContextualWifiSlice; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; + +@RunWith(RobolectricTestRunner.class) +public class EligibleCardCheckerTest { + + private static final Uri TEST_SLICE_URI = Uri.parse("content://test/test"); + + private Context mContext; + private EligibleCardChecker mEligibleCardChecker; + + @Before + public void setUp() { + mContext = RuntimeEnvironment.application; + mEligibleCardChecker = + spy(new EligibleCardChecker(mContext, getContextualCard(TEST_SLICE_URI))); + SliceProvider.setSpecs(SliceLiveData.SUPPORTED_SPECS); + } + + @Test + public void isSliceToggleable_cardWithToggle_returnTrue() { + final ContextualWifiSlice wifiSlice = new ContextualWifiSlice(mContext); + final Slice slice = wifiSlice.getSlice(); + + assertThat(mEligibleCardChecker.isSliceToggleable(slice)).isTrue(); + } + + @Test + public void isSliceToggleable_cardWithoutToggle_returnFalse() { + final EmergencyInfoSlice emergencyInfoSlice = new EmergencyInfoSlice(mContext); + final Slice slice = emergencyInfoSlice.getSlice(); + + assertThat(mEligibleCardChecker.isSliceToggleable(slice)).isFalse(); + } + + @Test + public void isCardEligibleToDisplay_toggleSlice_hasInlineActionShouldBeTrue() { + final ContextualWifiSlice wifiSlice = new ContextualWifiSlice(mContext); + final Slice slice = wifiSlice.getSlice(); + doReturn(slice).when(mEligibleCardChecker).bindSlice(any(Uri.class)); + + mEligibleCardChecker.isCardEligibleToDisplay(getContextualCard(TEST_SLICE_URI)); + + assertThat(mEligibleCardChecker.mCard.hasInlineAction()).isTrue(); + } + + @Test + public void isCardEligibleToDisplay_notToggleSlice_hasInlineActionShouldBeFalse() { + final EmergencyInfoSlice emergencyInfoSlice = new EmergencyInfoSlice(mContext); + final Slice slice = emergencyInfoSlice.getSlice(); + doReturn(slice).when(mEligibleCardChecker).bindSlice(any(Uri.class)); + + mEligibleCardChecker.isCardEligibleToDisplay(getContextualCard(TEST_SLICE_URI)); + + assertThat(mEligibleCardChecker.mCard.hasInlineAction()).isFalse(); + } + + private ContextualCard getContextualCard(Uri sliceUri) { + return new ContextualCard.Builder() + .setName("test_card") + .setRankingScore(0.5) + .setCardType(ContextualCard.CardType.SLICE) + .setSliceUri(sliceUri) + .build(); + } +}