Fix race condition and optimize categoryUpdater refresh
- In SettingsActivity, do not call updateCategories() if nothing
changed.
- In SummaryLoader, create a mapping between tile key and summary. This
is necessary to handle a race condition where category is refreshed
after summary load.
- In DashboardSummary, refresh Tile's summary to latest cache value
everytime category is refreshed.
Change-Id: I61389b8ba614ba7e34939325bada6e1bd6fa6709
Fix: 63149109
Test: robotests
diff --git a/src/com/android/settings/SettingsActivity.java b/src/com/android/settings/SettingsActivity.java
index 556dbfb..a7d9afc 100644
--- a/src/com/android/settings/SettingsActivity.java
+++ b/src/com/android/settings/SettingsActivity.java
@@ -45,7 +45,6 @@
import android.text.TextUtils;
import android.transition.TransitionManager;
import android.util.Log;
-import android.view.Menu;
import android.view.View;
import android.view.View.OnClickListener;
import android.view.ViewGroup;
@@ -64,7 +63,6 @@
import com.android.settings.overlay.FeatureFactory;
import com.android.settings.search.DynamicIndexableContentMonitor;
import com.android.settings.search.SearchActivity;
-import com.android.settings.search.SearchFeatureProvider;
import com.android.settings.wfd.WifiDisplaySettings;
import com.android.settings.widget.SwitchBar;
import com.android.settingslib.drawer.DashboardCategory;
@@ -770,68 +768,81 @@
PackageManager pm = getPackageManager();
final UserManager um = UserManager.get(this);
final boolean isAdmin = um.isAdminUser();
-
+ boolean somethingChanged = false;
String packageName = getPackageName();
- setTileEnabled(new ComponentName(packageName, WifiSettingsActivity.class.getName()),
- pm.hasSystemFeature(PackageManager.FEATURE_WIFI), isAdmin);
+ somethingChanged = setTileEnabled(
+ new ComponentName(packageName, WifiSettingsActivity.class.getName()),
+ pm.hasSystemFeature(PackageManager.FEATURE_WIFI), isAdmin) || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.BluetoothSettingsActivity.class.getName()),
- pm.hasSystemFeature(PackageManager.FEATURE_BLUETOOTH), isAdmin);
+ pm.hasSystemFeature(PackageManager.FEATURE_BLUETOOTH), isAdmin)
+ || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.DataUsageSummaryActivity.class.getName()),
- Utils.isBandwidthControlEnabled(), isAdmin);
+ Utils.isBandwidthControlEnabled(), isAdmin)
+ || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.SimSettingsActivity.class.getName()),
- Utils.showSimCardTile(this), isAdmin);
+ Utils.showSimCardTile(this), isAdmin)
+ || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.PowerUsageSummaryActivity.class.getName()),
- mBatteryPresent, isAdmin);
+ mBatteryPresent, isAdmin) || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.UserSettingsActivity.class.getName()),
UserHandle.MU_ENABLED && UserManager.supportsMultipleUsers()
- && !Utils.isMonkeyRunning(), isAdmin);
+ && !Utils.isMonkeyRunning(), isAdmin)
+ || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.NetworkDashboardActivity.class.getName()),
- !UserManager.isDeviceInDemoMode(this), isAdmin);
+ !UserManager.isDeviceInDemoMode(this), isAdmin)
+ || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.ConnectedDeviceDashboardActivity.class.getName()),
- !UserManager.isDeviceInDemoMode(this), isAdmin);
+ !UserManager.isDeviceInDemoMode(this), isAdmin)
+ || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.DateTimeSettingsActivity.class.getName()),
- !UserManager.isDeviceInDemoMode(this), isAdmin);
+ !UserManager.isDeviceInDemoMode(this), isAdmin)
+ || somethingChanged;
NfcAdapter adapter = NfcAdapter.getDefaultAdapter(this);
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.PaymentSettingsActivity.class.getName()),
pm.hasSystemFeature(PackageManager.FEATURE_NFC)
&& pm.hasSystemFeature(PackageManager.FEATURE_NFC_HOST_CARD_EMULATION)
- && adapter != null && adapter.isEnabled(), isAdmin);
+ && adapter != null && adapter.isEnabled(), isAdmin)
+ || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.PrintSettingsActivity.class.getName()),
- pm.hasSystemFeature(PackageManager.FEATURE_PRINTING), isAdmin);
+ pm.hasSystemFeature(PackageManager.FEATURE_PRINTING), isAdmin)
+ || somethingChanged;
final boolean showDev = mDevelopmentPreferences.getBoolean(
DevelopmentSettings.PREF_SHOW, android.os.Build.TYPE.equals("eng"))
&& !um.hasUserRestriction(UserManager.DISALLOW_DEBUGGING_FEATURES);
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.DevelopmentSettingsActivity.class.getName()),
- showDev, isAdmin);
+ showDev, isAdmin)
+ || somethingChanged;
// Enable/disable backup settings depending on whether the user is admin.
- setTileEnabled(new ComponentName(packageName,
- BackupSettingsActivity.class.getName()), true, isAdmin);
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
+ BackupSettingsActivity.class.getName()), true, isAdmin)
+ || somethingChanged;
- setTileEnabled(new ComponentName(packageName,
+ somethingChanged = setTileEnabled(new ComponentName(packageName,
Settings.WifiDisplaySettingsActivity.class.getName()),
- WifiDisplaySettings.isAvailable(this), isAdmin);
+ WifiDisplaySettings.isAvailable(this), isAdmin)
+ || somethingChanged;
if (UserHandle.MU_ENABLED && !isAdmin) {
@@ -848,7 +859,8 @@
SettingsGateway.SETTINGS_FOR_RESTRICTED, name);
if (packageName.equals(component.getPackageName())
&& !isEnabledForRestricted) {
- setTileEnabled(component, false, isAdmin);
+ somethingChanged = setTileEnabled(component, false, isAdmin)
+ || somethingChanged;
}
}
}
@@ -856,16 +868,24 @@
}
// Final step, refresh categories.
- updateCategories();
+ if (somethingChanged) {
+ Log.d(LOG_TAG, "Enabled state changed for some tiles, reloading all categories");
+ updateCategories();
+ } else {
+ Log.d(LOG_TAG, "No enabled state changed, skipping updateCategory call");
+ }
}
- private void setTileEnabled(ComponentName component, boolean enabled, boolean isAdmin) {
+ /**
+ * @return whether or not the enabled state actually changed.
+ */
+ private boolean setTileEnabled(ComponentName component, boolean enabled, boolean isAdmin) {
if (UserHandle.MU_ENABLED && !isAdmin && getPackageName().equals(component.getPackageName())
&& !ArrayUtils.contains(SettingsGateway.SETTINGS_FOR_RESTRICTED,
component.getClassName())) {
enabled = false;
}
- setTileEnabled(component, enabled);
+ return setTileEnabled(component, enabled);
}
private void getMetaData() {
diff --git a/src/com/android/settings/dashboard/DashboardSummary.java b/src/com/android/settings/dashboard/DashboardSummary.java
index 71a4d8f..ae99106 100644
--- a/src/com/android/settings/dashboard/DashboardSummary.java
+++ b/src/com/android/settings/dashboard/DashboardSummary.java
@@ -294,10 +294,12 @@
final DashboardCategory category = mDashboardFeatureProvider.getTilesForCategory(
CategoryKey.CATEGORY_HOMEPAGE);
+ mSummaryLoader.updateSummaryToCache(category);
if (suggestions != null) {
mAdapter.setCategoriesAndSuggestions(category, suggestions);
} else {
mAdapter.setCategory(category);
}
}
+
}
diff --git a/src/com/android/settings/dashboard/SummaryLoader.java b/src/com/android/settings/dashboard/SummaryLoader.java
index df21168..4586a55 100644
--- a/src/com/android/settings/dashboard/SummaryLoader.java
+++ b/src/com/android/settings/dashboard/SummaryLoader.java
@@ -34,7 +34,6 @@
import com.android.settings.SettingsActivity;
import com.android.settings.overlay.FeatureFactory;
import com.android.settingslib.drawer.DashboardCategory;
-import com.android.settingslib.drawer.SettingsDrawerActivity;
import com.android.settingslib.drawer.Tile;
import java.lang.reflect.Field;
@@ -47,7 +46,8 @@
public static final String SUMMARY_PROVIDER_FACTORY = "SUMMARY_PROVIDER_FACTORY";
private final Activity mActivity;
- private final ArrayMap<SummaryProvider, ComponentName> mSummaryMap = new ArrayMap<>();
+ private final ArrayMap<SummaryProvider, ComponentName> mSummaryProviderMap = new ArrayMap<>();
+ private final ArrayMap<String, CharSequence> mSummaryTextMap = new ArrayMap<>();
private final DashboardFeatureProvider mDashboardFeatureProvider;
private final String mCategoryKey;
@@ -111,13 +111,13 @@
}
public void setSummary(SummaryProvider provider, final CharSequence summary) {
- final ComponentName component = mSummaryMap.get(provider);
+ final ComponentName component = mSummaryProviderMap.get(provider);
mHandler.post(new Runnable() {
@Override
public void run() {
final Tile tile = getTileFromCategory(
- mDashboardFeatureProvider.getTilesForCategory(mCategoryKey), component);
+ mDashboardFeatureProvider.getTilesForCategory(mCategoryKey), component);
if (tile == null) {
if (DEBUG) {
@@ -142,6 +142,7 @@
}
return;
}
+ mSummaryTextMap.put(mDashboardFeatureProvider.getDashboardKeyForTile(tile), summary);
tile.summary = summary;
if (mSummaryConsumer != null) {
mSummaryConsumer.notifySummaryChanged(tile);
@@ -223,11 +224,27 @@
});
}
+ /**
+ * Updates all tile's summary to latest cached version. This is necessary to handle the case
+ * where category is updated after summary change.
+ */
+ public void updateSummaryToCache(DashboardCategory category) {
+ if (category == null) {
+ return;
+ }
+ for (Tile tile : category.tiles) {
+ final String key = mDashboardFeatureProvider.getDashboardKeyForTile(tile);
+ if (mSummaryTextMap.containsKey(key)) {
+ tile.summary = mSummaryTextMap.get(key);
+ }
+ }
+ }
+
private synchronized void setListeningW(boolean listening) {
if (mWorkerListening == listening) return;
mWorkerListening = listening;
if (DEBUG) Log.d(TAG, "Listening " + listening);
- for (SummaryProvider p : mSummaryMap.keySet()) {
+ for (SummaryProvider p : mSummaryProviderMap.keySet()) {
try {
p.setListening(listening);
} catch (Exception e) {
@@ -240,28 +257,10 @@
SummaryProvider provider = getSummaryProvider(tile);
if (provider != null) {
if (DEBUG) Log.d(TAG, "Creating " + tile);
- mSummaryMap.put(provider, tile.intent.getComponent());
+ mSummaryProviderMap.put(provider, tile.intent.getComponent());
}
}
- private Tile getTileFromCategory(List<DashboardCategory> categories, ComponentName component) {
- if (categories == null) {
- if (DEBUG) {
- Log.d(TAG, "Category is null, can't find tile");
- }
- return null;
- }
- final int categorySize = categories.size();
- for (int i = 0; i < categorySize; i++) {
- final DashboardCategory category = categories.get(i);
- final Tile tile = getTileFromCategory(category, component);
- if (tile != null) {
- return tile;
- }
- }
- return null;
- }
-
private Tile getTileFromCategory(DashboardCategory category, ComponentName component) {
if (category == null || category.tiles == null) {
return null;
@@ -276,6 +275,8 @@
return null;
}
+
+
public interface SummaryProvider {
void setListening(boolean listening);
}
diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardSummaryTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardSummaryTest.java
index c6446f1..093d5a2 100644
--- a/tests/robotests/src/com/android/settings/dashboard/DashboardSummaryTest.java
+++ b/tests/robotests/src/com/android/settings/dashboard/DashboardSummaryTest.java
@@ -19,11 +19,12 @@
import android.app.Activity;
import android.support.v7.widget.LinearLayoutManager;
-import com.android.settings.testutils.SettingsRobolectricTestRunner;
import com.android.settings.TestConfig;
import com.android.settings.dashboard.conditional.ConditionManager;
import com.android.settings.dashboard.conditional.FocusRecyclerView;
+import com.android.settings.testutils.SettingsRobolectricTestRunner;
import com.android.settingslib.drawer.CategoryKey;
+import com.android.settingslib.drawer.DashboardCategory;
import com.android.settingslib.drawer.Tile;
import org.junit.Before;
@@ -34,6 +35,7 @@
import org.robolectric.annotation.Config;
import org.robolectric.util.ReflectionHelpers;
+import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
@@ -57,6 +59,8 @@
private LinearLayoutManager mLayoutManager;
@Mock
private ConditionManager mConditionManager;
+ @Mock
+ private SummaryLoader mSummaryLoader;
private DashboardSummary mSummary;
@@ -70,12 +74,15 @@
ReflectionHelpers.setField(mSummary, "mDashboard", mDashboard);
ReflectionHelpers.setField(mSummary, "mLayoutManager", mLayoutManager);
ReflectionHelpers.setField(mSummary, "mConditionManager", mConditionManager);
+ ReflectionHelpers.setField(mSummary, "mSummaryLoader", mSummaryLoader);
}
@Test
public void updateCategoryAndSuggestion_shouldGetCategoryFromFeatureProvider() {
doReturn(mock(Activity.class)).when(mSummary).getActivity();
mSummary.updateCategoryAndSuggestion(null);
+
+ verify(mSummaryLoader).updateSummaryToCache(nullable(DashboardCategory.class));
verify(mDashboardFeatureProvider).getTilesForCategory(CategoryKey.CATEGORY_HOMEPAGE);
}
diff --git a/tests/robotests/src/com/android/settings/dashboard/SummaryLoaderTest.java b/tests/robotests/src/com/android/settings/dashboard/SummaryLoaderTest.java
index b08d62e..146be9c 100644
--- a/tests/robotests/src/com/android/settings/dashboard/SummaryLoaderTest.java
+++ b/tests/robotests/src/com/android/settings/dashboard/SummaryLoaderTest.java
@@ -17,13 +17,21 @@
package com.android.settings.dashboard;
import android.app.Activity;
-import com.android.settings.testutils.SettingsRobolectricTestRunner;
+import android.content.Context;
+import android.content.Intent;
+
import com.android.settings.TestConfig;
+import com.android.settings.testutils.FakeFeatureFactory;
+import com.android.settings.testutils.SettingsRobolectricTestRunner;
import com.android.settingslib.drawer.DashboardCategory;
import com.android.settingslib.drawer.Tile;
+
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.Answers;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
import org.robolectric.Robolectric;
import org.robolectric.annotation.Config;
@@ -31,18 +39,27 @@
import java.util.List;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.when;
@RunWith(SettingsRobolectricTestRunner.class)
@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
public class SummaryLoaderTest {
+
private static final String SUMMARY_1 = "summary1";
private static final String SUMMARY_2 = "summary2";
+
+ @Mock(answer = Answers.RETURNS_DEEP_STUBS)
+ private Context mContext;
private SummaryLoader mSummaryLoader;
private boolean mCallbackInvoked;
private Tile mTile;
+ private FakeFeatureFactory mFeatureFactory;
@Before
public void SetUp() {
+ MockitoAnnotations.initMocks(this);
+ mFeatureFactory = FakeFeatureFactory.setupForTest(mContext);
+
mTile = new Tile();
mTile.summary = SUMMARY_1;
mCallbackInvoked = false;
@@ -71,4 +88,23 @@
assertThat(mCallbackInvoked).isTrue();
}
+
+ @Test
+ public void testUpdateSummaryToCache_hasCache_shouldUpdate() {
+
+ final String testSummary = "test_summary";
+ final DashboardCategory category = new DashboardCategory();
+ final Tile tile = new Tile();
+ tile.key = "123";
+ tile.intent = new Intent();
+ category.addTile(tile);
+ when(mFeatureFactory.dashboardFeatureProvider.getDashboardKeyForTile(tile))
+ .thenReturn(tile.key);
+
+ mSummaryLoader.updateSummaryIfNeeded(tile, testSummary);
+ tile.summary = null;
+ mSummaryLoader.updateSummaryToCache(category);
+
+ assertThat(tile.summary).isEqualTo(testSummary);
+ }
}