Add DiffCallback to handle Contextual Cards update.

Use DiffCallback in ContextualCardsAdapter to only update items that
are changed instead of touching those unchanged ones.

Also fix a bug where ConditionContexualCardCard#onStart is incorrectly
skipped.

Fixes: 112245748
Bug: 118165942
Test: robotests
Change-Id: I7989d621764fe40a3fceb8c9f40baced840818ba
This commit is contained in:
Emily Chuang
2018-10-16 18:49:29 +08:00
committed by Fan Zhang
parent 0a546d1916
commit 18a99faaf1
6 changed files with 80 additions and 17 deletions

View File

@@ -115,10 +115,6 @@ public class ContextualCardManager implements CardContentLoader.CardContentLoade
@Override @Override
public void onContextualCardUpdated(Map<Integer, List<ContextualCard>> updateList) { public void onContextualCardUpdated(Map<Integer, List<ContextualCard>> updateList) {
//TODO(b/112245748): Should implement a DiffCallback.
//Keep the old list for comparison.
final List<ContextualCard> prevCards = mContextualCards;
final Set<Integer> cardTypes = updateList.keySet(); final Set<Integer> cardTypes = updateList.keySet();
//Remove the existing data that matches the certain cardType before inserting new data. //Remove the existing data that matches the certain cardType before inserting new data.
final List<ContextualCard> cardsToKeep = mContextualCards final List<ContextualCard> cardsToKeep = mContextualCards

View File

@@ -17,11 +17,13 @@
package com.android.settings.homepage; package com.android.settings.homepage;
import android.content.Context; import android.content.Context;
import android.util.Log;
import android.view.LayoutInflater; import android.view.LayoutInflater;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import androidx.lifecycle.LifecycleOwner; import androidx.lifecycle.LifecycleOwner;
import androidx.recyclerview.widget.DiffUtil;
import androidx.recyclerview.widget.GridLayoutManager; import androidx.recyclerview.widget.GridLayoutManager;
import androidx.recyclerview.widget.RecyclerView; import androidx.recyclerview.widget.RecyclerView;
@@ -108,14 +110,15 @@ public class ContextualCardsAdapter extends RecyclerView.Adapter<RecyclerView.Vi
@Override @Override
public void onContextualCardUpdated(Map<Integer, List<ContextualCard>> cards) { public void onContextualCardUpdated(Map<Integer, List<ContextualCard>> cards) {
final List<ContextualCard> contextualCards = cards.get(ContextualCard.CardType.DEFAULT); final List<ContextualCard> contextualCards = cards.get(ContextualCard.CardType.DEFAULT);
//TODO(b/112245748): Should implement a DiffCallback so we can use notifyItemChanged()
// instead.
if (contextualCards == null) { if (contextualCards == null) {
mContextualCards.clear(); mContextualCards.clear();
notifyDataSetChanged();
} else { } else {
final DiffUtil.DiffResult diffResult = DiffUtil.calculateDiff(
new ContextualCardsDiffCallback(mContextualCards, contextualCards));
mContextualCards.clear(); mContextualCards.clear();
mContextualCards.addAll(contextualCards); mContextualCards.addAll(contextualCards);
} diffResult.dispatchUpdatesTo(this);
notifyDataSetChanged(); }
} }
} }

View File

@@ -0,0 +1,58 @@
/*
* Copyright (C) 2018 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;
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.
*/
public class ContextualCardsDiffCallback extends DiffUtil.Callback {
private final List<ContextualCard> mOldCards;
private final List<ContextualCard> mNewCards;
public ContextualCardsDiffCallback(List<ContextualCard> oldCards,
List<ContextualCard> newCards) {
mOldCards = oldCards;
mNewCards = newCards;
}
@Override
public int getOldListSize() {
return mOldCards.size();
}
@Override
public int getNewListSize() {
return mNewCards.size();
}
@Override
public boolean areItemsTheSame(int oldCardPosition, int newCardPosition) {
return mOldCards.get(oldCardPosition).getName().equals(
mNewCards.get(newCardPosition).getName());
}
@Override
public boolean areContentsTheSame(int oldCardPosition, int newCardPosition) {
return mOldCards.get(oldCardPosition).equals(mNewCards.get(newCardPosition));
}
}

View File

@@ -37,6 +37,8 @@ import java.util.Map;
public class ConditionContextualCardController implements ContextualCardController, public class ConditionContextualCardController implements ContextualCardController,
ConditionListener, LifecycleObserver, OnStart, OnStop { ConditionListener, LifecycleObserver, OnStart, OnStop {
private static final String TAG = "ConditionCtxCardCtrl";
private final Context mContext; private final Context mContext;
private final ConditionManager mConditionManager; private final ConditionManager mConditionManager;

View File

@@ -107,13 +107,13 @@ public class ConditionManager {
*/ */
public void startMonitoringStateChange() { public void startMonitoringStateChange() {
if (mIsListeningToStateChange) { if (mIsListeningToStateChange) {
Log.d(TAG, "Already listening to condition state changes, skipping"); Log.d(TAG, "Already listening to condition state changes, skipping monitor setup");
return; } else {
}
mIsListeningToStateChange = true; mIsListeningToStateChange = true;
for (ConditionalCardController controller : mCardControllers) { for (ConditionalCardController controller : mCardControllers) {
controller.startMonitoringStateChange(); controller.startMonitoringStateChange();
} }
}
// Force a refresh on listener // Force a refresh on listener
onConditionChanged(); onConditionChanged();
} }

View File

@@ -19,6 +19,8 @@ package com.android.settings.homepage.conditional;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.never; 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 static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
@@ -53,7 +55,7 @@ public class ConditionManagerTest {
public void setUp() { public void setUp() {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
mContext = RuntimeEnvironment.application; mContext = RuntimeEnvironment.application;
mManager = new ConditionManager(mContext, mConditionListener); mManager = spy(new ConditionManager(mContext, mConditionListener));
assertThat(mManager.mCandidates.size()).isEqualTo(mManager.mCardControllers.size()); assertThat(mManager.mCandidates.size()).isEqualTo(mManager.mCardControllers.size());
@@ -94,11 +96,13 @@ public class ConditionManagerTest {
@Test @Test
public void startMonitoringStateChange_multipleTimes_shouldRegisterOnce() { public void startMonitoringStateChange_multipleTimes_shouldRegisterOnce() {
final int loopCount = 10;
for (int i = 0; i < loopCount; i++) {
mManager.startMonitoringStateChange(); mManager.startMonitoringStateChange();
mManager.startMonitoringStateChange(); }
mManager.startMonitoringStateChange();
verify(mController).startMonitoringStateChange(); verify(mController).startMonitoringStateChange();
verify(mManager, times(loopCount)).onConditionChanged();
} }
@Test @Test