Fix ConcurrentModificationException in SummaryLoader.
When the dashboard summary is being initialized, it will rebuild the UI
while the summary loader tries to to go through the tiles to update the
summary. Both is being done on a separate backgroud thread, and it will
run into concurrent modification issue if the thread is being swapped
while one is looping through the list.
Instead of letting clients access the list of tiles directly, add a
getter method in DashboardCategory to get a copy of the list of tiles
for all read-only operations.
Change-Id: I479669abd8d1d0a8ee9a4113d8ad2244da56f4d8
Fixes: 69677575
Test: make RunSettingsRoboTests
diff --git a/src/com/android/settings/dashboard/DashboardAdapter.java b/src/com/android/settings/dashboard/DashboardAdapter.java
index fd2adac..54a1eeb 100644
--- a/src/com/android/settings/dashboard/DashboardAdapter.java
+++ b/src/com/android/settings/dashboard/DashboardAdapter.java
@@ -477,7 +477,7 @@
final int tintColor = a.getColor(0, mContext.getColor(R.color.fallback_tintColor));
a.recycle();
if (category != null) {
- for (Tile tile : category.tiles) {
+ for (Tile tile : category.getTiles()) {
if (tile.isIconTintable) {
// If this drawable is tintable, tint it to match the color.
tile.icon.setTint(tintColor);
diff --git a/src/com/android/settings/dashboard/DashboardData.java b/src/com/android/settings/dashboard/DashboardData.java
index a14b8c2..6407960 100644
--- a/src/com/android/settings/dashboard/DashboardData.java
+++ b/src/com/android/settings/dashboard/DashboardData.java
@@ -268,8 +268,9 @@
&& hiddenSuggestion == 0);
if (mCategory != null) {
- for (int j = 0; j < mCategory.tiles.size(); j++) {
- final Tile tile = mCategory.tiles.get(j);
+ final List<Tile> tiles = mCategory.getTiles();
+ for (int j = 0; j < tiles.size(); j++) {
+ final Tile tile = tiles.get(j);
addToItemList(tile, R.layout.dashboard_tile, Objects.hash(tile.title),
true /* add */);
}
diff --git a/src/com/android/settings/dashboard/DashboardFeatureProviderImpl.java b/src/com/android/settings/dashboard/DashboardFeatureProviderImpl.java
index bee822a..9ef38b8 100644
--- a/src/com/android/settings/dashboard/DashboardFeatureProviderImpl.java
+++ b/src/com/android/settings/dashboard/DashboardFeatureProviderImpl.java
@@ -91,7 +91,7 @@
Log.d(TAG, "NO dashboard tiles for " + TAG);
return null;
}
- final List<Tile> tiles = category.tiles;
+ final List<Tile> tiles = category.getTiles();
if (tiles == null || tiles.isEmpty()) {
Log.d(TAG, "tile list is empty, skipping category " + category.title);
return null;
diff --git a/src/com/android/settings/dashboard/DashboardFragment.java b/src/com/android/settings/dashboard/DashboardFragment.java
index 3551d23..3e1e881 100644
--- a/src/com/android/settings/dashboard/DashboardFragment.java
+++ b/src/com/android/settings/dashboard/DashboardFragment.java
@@ -307,7 +307,7 @@
Log.d(TAG, "NO dashboard tiles for " + TAG);
return;
}
- List<Tile> tiles = category.tiles;
+ final List<Tile> tiles = category.getTiles();
if (tiles == null) {
Log.d(TAG, "tile list is empty, skipping category " + category.title);
return;
diff --git a/src/com/android/settings/dashboard/SiteMapManager.java b/src/com/android/settings/dashboard/SiteMapManager.java
index 50d7a18..b54e061 100644
--- a/src/com/android/settings/dashboard/SiteMapManager.java
+++ b/src/com/android/settings/dashboard/SiteMapManager.java
@@ -154,7 +154,7 @@
continue;
}
// Build parent-child mPairs for all children listed under this key.
- for (Tile tile : category.tiles) {
+ for (Tile tile : category.getTiles()) {
final String childTitle = tile.title.toString();
String childClass = null;
if (tile.metaData != null) {
diff --git a/src/com/android/settings/dashboard/SummaryLoader.java b/src/com/android/settings/dashboard/SummaryLoader.java
index fe55be8..5af276c 100644
--- a/src/com/android/settings/dashboard/SummaryLoader.java
+++ b/src/com/android/settings/dashboard/SummaryLoader.java
@@ -216,7 +216,7 @@
if (category == null) {
return;
}
- for (Tile tile : category.tiles) {
+ for (Tile tile : category.getTiles()) {
final String key = mDashboardFeatureProvider.getDashboardKeyForTile(tile);
if (mSummaryTextMap.containsKey(key)) {
tile.summary = mSummaryTextMap.get(key);
@@ -250,12 +250,13 @@
}
private Tile getTileFromCategory(DashboardCategory category, ComponentName component) {
- if (category == null || category.tiles == null) {
+ if (category == null || category.getTilesCount() == 0) {
return null;
}
- final int tileCount = category.tiles.size();
+ final List<Tile> tiles = category.getTiles();
+ final int tileCount = tiles.size();
for (int j = 0; j < tileCount; j++) {
- final Tile tile = category.tiles.get(j);
+ final Tile tile = tiles.get(j);
if (component.equals(tile.intent.getComponent())) {
return tile;
}
@@ -291,10 +292,10 @@
case MSG_GET_CATEGORY_TILES_AND_SET_LISTENING:
final DashboardCategory category =
mDashboardFeatureProvider.getTilesForCategory(mCategoryKey);
- if (category == null || category.tiles == null) {
+ if (category == null || category.getTilesCount() == 0) {
return;
}
- final List<Tile> tiles = category.tiles;
+ final List<Tile> tiles = category.getTiles();
for (Tile tile : tiles) {
makeProviderW(tile);
}
diff --git a/src/com/android/settings/search/SettingsSearchIndexablesProvider.java b/src/com/android/settings/search/SettingsSearchIndexablesProvider.java
index 9e35082..0c98b9c 100644
--- a/src/com/android/settings/search/SettingsSearchIndexablesProvider.java
+++ b/src/com/android/settings/search/SettingsSearchIndexablesProvider.java
@@ -156,7 +156,7 @@
continue;
}
// Build parent-child class pairs for all children listed under this key.
- for (Tile tile : category.tiles) {
+ for (Tile tile : category.getTiles()) {
String childClass = null;
if (tile.metaData != null) {
childClass = tile.metaData.getString(
diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java
index c9469a8..fdb1470 100644
--- a/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java
+++ b/tests/robotests/src/com/android/settings/dashboard/DashboardAdapterTest.java
@@ -222,14 +222,12 @@
doReturn(mockTypedArray).when(mContext).obtainStyledAttributes(any(int[].class));
doReturn(0x89000000).when(mockTypedArray).getColor(anyInt(), anyInt());
- final DashboardCategory category = mock(DashboardCategory.class);
- final List<Tile> tiles = new ArrayList<>();
+ final DashboardCategory category = new DashboardCategory();
final Icon mockIcon = mock(Icon.class);
final Tile tile = new Tile();
tile.isIconTintable = true;
tile.icon = mockIcon;
- tiles.add(tile);
- category.tiles = tiles;
+ category.addTile(tile);
mDashboardAdapter.setCategory(category);
@@ -250,10 +248,8 @@
public void testBindConditionAndSuggestion_shouldSetSuggestionAdapterAndNoCrash() {
mDashboardAdapter = new DashboardAdapter(mContext, null, null, null, null, null);
final List<Tile> suggestions = makeSuggestions("pkg1");
- final DashboardCategory category = mock(DashboardCategory.class);
- final List<Tile> tiles = new ArrayList<>();
- tiles.add(mock(Tile.class));
- category.tiles = tiles;
+ final DashboardCategory category = new DashboardCategory();
+ category.addTile(mock(Tile.class));
mDashboardAdapter.setCategoriesAndSuggestions(category, suggestions);
@@ -277,10 +273,8 @@
public void testBindConditionAndSuggestion_v2_shouldSetSuggestionAdapterAndNoCrash() {
mDashboardAdapter = new DashboardAdapter(mContext, null, null, null, null, null);
final List<Suggestion> suggestions = makeSuggestionsV2("pkg1");
- final DashboardCategory category = mock(DashboardCategory.class);
- final List<Tile> tiles = new ArrayList<>();
- tiles.add(mock(Tile.class));
- category.tiles = tiles;
+ final DashboardCategory category = new DashboardCategory();
+ category.addTile(mock(Tile.class));
mDashboardAdapter.setSuggestionsV2(suggestions);
@@ -310,10 +304,8 @@
null /* SuggestionDismissController.Callback */);
final List<Tile> suggestions = new ArrayList<>();
- final DashboardCategory category = mock(DashboardCategory.class);
- final List<Tile> tiles = new ArrayList<>();
- tiles.add(mock(Tile.class));
- category.tiles = tiles;
+ final DashboardCategory category = new DashboardCategory();
+ category.addTile(mock(Tile.class));
mDashboardAdapter.setCategoriesAndSuggestions(category, suggestions);
final RecyclerView data = mock(RecyclerView.class);
diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardDataTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardDataTest.java
index a3483fb..33f379e 100644
--- a/tests/robotests/src/com/android/settings/dashboard/DashboardDataTest.java
+++ b/tests/robotests/src/com/android/settings/dashboard/DashboardDataTest.java
@@ -57,13 +57,12 @@
private DashboardData mDashboardDataWithOneConditions;
private DashboardData mDashboardDataWithTwoConditions;
private DashboardData mDashboardDataWithNoItems;
+ private DashboardCategory mDashboardCategory;
@Mock
private Tile mTestCategoryTile;
@Mock
private Tile mTestSuggestion;
@Mock
- private DashboardCategory mDashboardCategory;
- @Mock
private Condition mTestCondition;
@Mock
private Condition mSecondCondition; // condition used to test insert in DiffUtil
@@ -72,6 +71,8 @@
public void SetUp() {
MockitoAnnotations.initMocks(this);
+ mDashboardCategory = new DashboardCategory();
+
// Build suggestions
final List<Tile> suggestions = new ArrayList<>();
mTestSuggestion.title = TEST_SUGGESTION_TITLE;
@@ -91,8 +92,8 @@
// Build category
mTestCategoryTile.title = TEST_CATEGORY_TILE_TITLE;
mDashboardCategory.title = "test";
- mDashboardCategory.tiles = new ArrayList<>();
- mDashboardCategory.tiles.add(mTestCategoryTile);
+
+ mDashboardCategory.addTile(mTestCategoryTile);
// Build DashboardData
mDashboardDataWithOneConditions = new DashboardData.Builder()
diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java
index 559e989..3d36e6f 100644
--- a/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java
+++ b/tests/robotests/src/com/android/settings/dashboard/DashboardFeatureProviderImplTest.java
@@ -431,7 +431,7 @@
mImpl = new DashboardFeatureProviderImpl(mActivity);
ReflectionHelpers.setField(mImpl, "mCategoryManager", mCategoryManager);
final DashboardCategory category = new DashboardCategory();
- category.tiles.add(new Tile());
+ category.addTile(new Tile());
when(mCategoryManager
.getTilesByCategory(any(Context.class), eq(CategoryKey.CATEGORY_HOMEPAGE)))
.thenReturn(category);
diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java
index 2ee1837..14f3078 100644
--- a/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java
+++ b/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java
@@ -64,9 +64,8 @@
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
private Context mContext;
@Mock
- private DashboardCategory mDashboardCategory;
- @Mock
private FakeFeatureFactory mFakeFeatureFactory;
+ private DashboardCategory mDashboardCategory;
private TestFragment mTestFragment;
@Before
@@ -74,8 +73,8 @@
MockitoAnnotations.initMocks(this);
FakeFeatureFactory.setupForTest(mContext);
mFakeFeatureFactory = (FakeFeatureFactory) FeatureFactory.getFactory(mContext);
- mDashboardCategory.tiles = new ArrayList<>();
- mDashboardCategory.tiles.add(new Tile());
+ mDashboardCategory = new DashboardCategory();
+ mDashboardCategory.addTile(new Tile());
mTestFragment = new TestFragment(ShadowApplication.getInstance().getApplicationContext());
when(mFakeFeatureFactory.dashboardFeatureProvider
.getTilesForCategory(nullable(String.class)))
@@ -117,7 +116,7 @@
@Test
public void displayTilesAsPreference_withEmptyCategory_shouldNotAddTiles() {
- mDashboardCategory.tiles = null;
+ mDashboardCategory.removeTile(0);
mTestFragment.onCreatePreferences(new Bundle(), "rootKey");
verify(mTestFragment.mScreen, never()).addPreference(nullable(Preference.class));