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
This commit is contained in:
@@ -477,7 +477,7 @@ public class DashboardAdapter extends RecyclerView.Adapter<DashboardAdapter.Dash
|
|||||||
final int tintColor = a.getColor(0, mContext.getColor(R.color.fallback_tintColor));
|
final int tintColor = a.getColor(0, mContext.getColor(R.color.fallback_tintColor));
|
||||||
a.recycle();
|
a.recycle();
|
||||||
if (category != null) {
|
if (category != null) {
|
||||||
for (Tile tile : category.tiles) {
|
for (Tile tile : category.getTiles()) {
|
||||||
if (tile.isIconTintable) {
|
if (tile.isIconTintable) {
|
||||||
// If this drawable is tintable, tint it to match the color.
|
// If this drawable is tintable, tint it to match the color.
|
||||||
tile.icon.setTint(tintColor);
|
tile.icon.setTint(tintColor);
|
||||||
|
@@ -268,8 +268,9 @@ public class DashboardData {
|
|||||||
&& hiddenSuggestion == 0);
|
&& hiddenSuggestion == 0);
|
||||||
|
|
||||||
if (mCategory != null) {
|
if (mCategory != null) {
|
||||||
for (int j = 0; j < mCategory.tiles.size(); j++) {
|
final List<Tile> tiles = mCategory.getTiles();
|
||||||
final Tile tile = mCategory.tiles.get(j);
|
for (int j = 0; j < tiles.size(); j++) {
|
||||||
|
final Tile tile = tiles.get(j);
|
||||||
addToItemList(tile, R.layout.dashboard_tile, Objects.hash(tile.title),
|
addToItemList(tile, R.layout.dashboard_tile, Objects.hash(tile.title),
|
||||||
true /* add */);
|
true /* add */);
|
||||||
}
|
}
|
||||||
|
@@ -91,7 +91,7 @@ public class DashboardFeatureProviderImpl implements DashboardFeatureProvider {
|
|||||||
Log.d(TAG, "NO dashboard tiles for " + TAG);
|
Log.d(TAG, "NO dashboard tiles for " + TAG);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
final List<Tile> tiles = category.tiles;
|
final List<Tile> tiles = category.getTiles();
|
||||||
if (tiles == null || tiles.isEmpty()) {
|
if (tiles == null || tiles.isEmpty()) {
|
||||||
Log.d(TAG, "tile list is empty, skipping category " + category.title);
|
Log.d(TAG, "tile list is empty, skipping category " + category.title);
|
||||||
return null;
|
return null;
|
||||||
|
@@ -307,7 +307,7 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment
|
|||||||
Log.d(TAG, "NO dashboard tiles for " + TAG);
|
Log.d(TAG, "NO dashboard tiles for " + TAG);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
List<Tile> tiles = category.tiles;
|
final List<Tile> tiles = category.getTiles();
|
||||||
if (tiles == null) {
|
if (tiles == null) {
|
||||||
Log.d(TAG, "tile list is empty, skipping category " + category.title);
|
Log.d(TAG, "tile list is empty, skipping category " + category.title);
|
||||||
return;
|
return;
|
||||||
|
@@ -154,7 +154,7 @@ public class SiteMapManager {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
// Build parent-child mPairs for all children listed under this key.
|
// 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();
|
final String childTitle = tile.title.toString();
|
||||||
String childClass = null;
|
String childClass = null;
|
||||||
if (tile.metaData != null) {
|
if (tile.metaData != null) {
|
||||||
|
@@ -216,7 +216,7 @@ public class SummaryLoader {
|
|||||||
if (category == null) {
|
if (category == null) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
for (Tile tile : category.tiles) {
|
for (Tile tile : category.getTiles()) {
|
||||||
final String key = mDashboardFeatureProvider.getDashboardKeyForTile(tile);
|
final String key = mDashboardFeatureProvider.getDashboardKeyForTile(tile);
|
||||||
if (mSummaryTextMap.containsKey(key)) {
|
if (mSummaryTextMap.containsKey(key)) {
|
||||||
tile.summary = mSummaryTextMap.get(key);
|
tile.summary = mSummaryTextMap.get(key);
|
||||||
@@ -250,12 +250,13 @@ public class SummaryLoader {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private Tile getTileFromCategory(DashboardCategory category, ComponentName component) {
|
private Tile getTileFromCategory(DashboardCategory category, ComponentName component) {
|
||||||
if (category == null || category.tiles == null) {
|
if (category == null || category.getTilesCount() == 0) {
|
||||||
return null;
|
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++) {
|
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())) {
|
if (component.equals(tile.intent.getComponent())) {
|
||||||
return tile;
|
return tile;
|
||||||
}
|
}
|
||||||
@@ -291,10 +292,10 @@ public class SummaryLoader {
|
|||||||
case MSG_GET_CATEGORY_TILES_AND_SET_LISTENING:
|
case MSG_GET_CATEGORY_TILES_AND_SET_LISTENING:
|
||||||
final DashboardCategory category =
|
final DashboardCategory category =
|
||||||
mDashboardFeatureProvider.getTilesForCategory(mCategoryKey);
|
mDashboardFeatureProvider.getTilesForCategory(mCategoryKey);
|
||||||
if (category == null || category.tiles == null) {
|
if (category == null || category.getTilesCount() == 0) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
final List<Tile> tiles = category.tiles;
|
final List<Tile> tiles = category.getTiles();
|
||||||
for (Tile tile : tiles) {
|
for (Tile tile : tiles) {
|
||||||
makeProviderW(tile);
|
makeProviderW(tile);
|
||||||
}
|
}
|
||||||
|
@@ -156,7 +156,7 @@ public class SettingsSearchIndexablesProvider extends SearchIndexablesProvider {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
// Build parent-child class pairs for all children listed under this key.
|
// 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;
|
String childClass = null;
|
||||||
if (tile.metaData != null) {
|
if (tile.metaData != null) {
|
||||||
childClass = tile.metaData.getString(
|
childClass = tile.metaData.getString(
|
||||||
|
@@ -222,14 +222,12 @@ public class DashboardAdapterTest {
|
|||||||
doReturn(mockTypedArray).when(mContext).obtainStyledAttributes(any(int[].class));
|
doReturn(mockTypedArray).when(mContext).obtainStyledAttributes(any(int[].class));
|
||||||
doReturn(0x89000000).when(mockTypedArray).getColor(anyInt(), anyInt());
|
doReturn(0x89000000).when(mockTypedArray).getColor(anyInt(), anyInt());
|
||||||
|
|
||||||
final DashboardCategory category = mock(DashboardCategory.class);
|
final DashboardCategory category = new DashboardCategory();
|
||||||
final List<Tile> tiles = new ArrayList<>();
|
|
||||||
final Icon mockIcon = mock(Icon.class);
|
final Icon mockIcon = mock(Icon.class);
|
||||||
final Tile tile = new Tile();
|
final Tile tile = new Tile();
|
||||||
tile.isIconTintable = true;
|
tile.isIconTintable = true;
|
||||||
tile.icon = mockIcon;
|
tile.icon = mockIcon;
|
||||||
tiles.add(tile);
|
category.addTile(tile);
|
||||||
category.tiles = tiles;
|
|
||||||
|
|
||||||
mDashboardAdapter.setCategory(category);
|
mDashboardAdapter.setCategory(category);
|
||||||
|
|
||||||
@@ -250,10 +248,8 @@ public class DashboardAdapterTest {
|
|||||||
public void testBindConditionAndSuggestion_shouldSetSuggestionAdapterAndNoCrash() {
|
public void testBindConditionAndSuggestion_shouldSetSuggestionAdapterAndNoCrash() {
|
||||||
mDashboardAdapter = new DashboardAdapter(mContext, null, null, null, null, null);
|
mDashboardAdapter = new DashboardAdapter(mContext, null, null, null, null, null);
|
||||||
final List<Tile> suggestions = makeSuggestions("pkg1");
|
final List<Tile> suggestions = makeSuggestions("pkg1");
|
||||||
final DashboardCategory category = mock(DashboardCategory.class);
|
final DashboardCategory category = new DashboardCategory();
|
||||||
final List<Tile> tiles = new ArrayList<>();
|
category.addTile(mock(Tile.class));
|
||||||
tiles.add(mock(Tile.class));
|
|
||||||
category.tiles = tiles;
|
|
||||||
|
|
||||||
mDashboardAdapter.setCategoriesAndSuggestions(category, suggestions);
|
mDashboardAdapter.setCategoriesAndSuggestions(category, suggestions);
|
||||||
|
|
||||||
@@ -277,10 +273,8 @@ public class DashboardAdapterTest {
|
|||||||
public void testBindConditionAndSuggestion_v2_shouldSetSuggestionAdapterAndNoCrash() {
|
public void testBindConditionAndSuggestion_v2_shouldSetSuggestionAdapterAndNoCrash() {
|
||||||
mDashboardAdapter = new DashboardAdapter(mContext, null, null, null, null, null);
|
mDashboardAdapter = new DashboardAdapter(mContext, null, null, null, null, null);
|
||||||
final List<Suggestion> suggestions = makeSuggestionsV2("pkg1");
|
final List<Suggestion> suggestions = makeSuggestionsV2("pkg1");
|
||||||
final DashboardCategory category = mock(DashboardCategory.class);
|
final DashboardCategory category = new DashboardCategory();
|
||||||
final List<Tile> tiles = new ArrayList<>();
|
category.addTile(mock(Tile.class));
|
||||||
tiles.add(mock(Tile.class));
|
|
||||||
category.tiles = tiles;
|
|
||||||
|
|
||||||
mDashboardAdapter.setSuggestionsV2(suggestions);
|
mDashboardAdapter.setSuggestionsV2(suggestions);
|
||||||
|
|
||||||
@@ -310,10 +304,8 @@ public class DashboardAdapterTest {
|
|||||||
null /* SuggestionDismissController.Callback */);
|
null /* SuggestionDismissController.Callback */);
|
||||||
|
|
||||||
final List<Tile> suggestions = new ArrayList<>();
|
final List<Tile> suggestions = new ArrayList<>();
|
||||||
final DashboardCategory category = mock(DashboardCategory.class);
|
final DashboardCategory category = new DashboardCategory();
|
||||||
final List<Tile> tiles = new ArrayList<>();
|
category.addTile(mock(Tile.class));
|
||||||
tiles.add(mock(Tile.class));
|
|
||||||
category.tiles = tiles;
|
|
||||||
mDashboardAdapter.setCategoriesAndSuggestions(category, suggestions);
|
mDashboardAdapter.setCategoriesAndSuggestions(category, suggestions);
|
||||||
|
|
||||||
final RecyclerView data = mock(RecyclerView.class);
|
final RecyclerView data = mock(RecyclerView.class);
|
||||||
|
@@ -57,13 +57,12 @@ public class DashboardDataTest {
|
|||||||
private DashboardData mDashboardDataWithOneConditions;
|
private DashboardData mDashboardDataWithOneConditions;
|
||||||
private DashboardData mDashboardDataWithTwoConditions;
|
private DashboardData mDashboardDataWithTwoConditions;
|
||||||
private DashboardData mDashboardDataWithNoItems;
|
private DashboardData mDashboardDataWithNoItems;
|
||||||
|
private DashboardCategory mDashboardCategory;
|
||||||
@Mock
|
@Mock
|
||||||
private Tile mTestCategoryTile;
|
private Tile mTestCategoryTile;
|
||||||
@Mock
|
@Mock
|
||||||
private Tile mTestSuggestion;
|
private Tile mTestSuggestion;
|
||||||
@Mock
|
@Mock
|
||||||
private DashboardCategory mDashboardCategory;
|
|
||||||
@Mock
|
|
||||||
private Condition mTestCondition;
|
private Condition mTestCondition;
|
||||||
@Mock
|
@Mock
|
||||||
private Condition mSecondCondition; // condition used to test insert in DiffUtil
|
private Condition mSecondCondition; // condition used to test insert in DiffUtil
|
||||||
@@ -72,6 +71,8 @@ public class DashboardDataTest {
|
|||||||
public void SetUp() {
|
public void SetUp() {
|
||||||
MockitoAnnotations.initMocks(this);
|
MockitoAnnotations.initMocks(this);
|
||||||
|
|
||||||
|
mDashboardCategory = new DashboardCategory();
|
||||||
|
|
||||||
// Build suggestions
|
// Build suggestions
|
||||||
final List<Tile> suggestions = new ArrayList<>();
|
final List<Tile> suggestions = new ArrayList<>();
|
||||||
mTestSuggestion.title = TEST_SUGGESTION_TITLE;
|
mTestSuggestion.title = TEST_SUGGESTION_TITLE;
|
||||||
@@ -91,8 +92,8 @@ public class DashboardDataTest {
|
|||||||
// Build category
|
// Build category
|
||||||
mTestCategoryTile.title = TEST_CATEGORY_TILE_TITLE;
|
mTestCategoryTile.title = TEST_CATEGORY_TILE_TITLE;
|
||||||
mDashboardCategory.title = "test";
|
mDashboardCategory.title = "test";
|
||||||
mDashboardCategory.tiles = new ArrayList<>();
|
|
||||||
mDashboardCategory.tiles.add(mTestCategoryTile);
|
mDashboardCategory.addTile(mTestCategoryTile);
|
||||||
|
|
||||||
// Build DashboardData
|
// Build DashboardData
|
||||||
mDashboardDataWithOneConditions = new DashboardData.Builder()
|
mDashboardDataWithOneConditions = new DashboardData.Builder()
|
||||||
|
@@ -431,7 +431,7 @@ public class DashboardFeatureProviderImplTest {
|
|||||||
mImpl = new DashboardFeatureProviderImpl(mActivity);
|
mImpl = new DashboardFeatureProviderImpl(mActivity);
|
||||||
ReflectionHelpers.setField(mImpl, "mCategoryManager", mCategoryManager);
|
ReflectionHelpers.setField(mImpl, "mCategoryManager", mCategoryManager);
|
||||||
final DashboardCategory category = new DashboardCategory();
|
final DashboardCategory category = new DashboardCategory();
|
||||||
category.tiles.add(new Tile());
|
category.addTile(new Tile());
|
||||||
when(mCategoryManager
|
when(mCategoryManager
|
||||||
.getTilesByCategory(any(Context.class), eq(CategoryKey.CATEGORY_HOMEPAGE)))
|
.getTilesByCategory(any(Context.class), eq(CategoryKey.CATEGORY_HOMEPAGE)))
|
||||||
.thenReturn(category);
|
.thenReturn(category);
|
||||||
|
@@ -64,9 +64,8 @@ public class DashboardFragmentTest {
|
|||||||
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
|
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
|
||||||
private Context mContext;
|
private Context mContext;
|
||||||
@Mock
|
@Mock
|
||||||
private DashboardCategory mDashboardCategory;
|
|
||||||
@Mock
|
|
||||||
private FakeFeatureFactory mFakeFeatureFactory;
|
private FakeFeatureFactory mFakeFeatureFactory;
|
||||||
|
private DashboardCategory mDashboardCategory;
|
||||||
private TestFragment mTestFragment;
|
private TestFragment mTestFragment;
|
||||||
|
|
||||||
@Before
|
@Before
|
||||||
@@ -74,8 +73,8 @@ public class DashboardFragmentTest {
|
|||||||
MockitoAnnotations.initMocks(this);
|
MockitoAnnotations.initMocks(this);
|
||||||
FakeFeatureFactory.setupForTest(mContext);
|
FakeFeatureFactory.setupForTest(mContext);
|
||||||
mFakeFeatureFactory = (FakeFeatureFactory) FeatureFactory.getFactory(mContext);
|
mFakeFeatureFactory = (FakeFeatureFactory) FeatureFactory.getFactory(mContext);
|
||||||
mDashboardCategory.tiles = new ArrayList<>();
|
mDashboardCategory = new DashboardCategory();
|
||||||
mDashboardCategory.tiles.add(new Tile());
|
mDashboardCategory.addTile(new Tile());
|
||||||
mTestFragment = new TestFragment(ShadowApplication.getInstance().getApplicationContext());
|
mTestFragment = new TestFragment(ShadowApplication.getInstance().getApplicationContext());
|
||||||
when(mFakeFeatureFactory.dashboardFeatureProvider
|
when(mFakeFeatureFactory.dashboardFeatureProvider
|
||||||
.getTilesForCategory(nullable(String.class)))
|
.getTilesForCategory(nullable(String.class)))
|
||||||
@@ -117,7 +116,7 @@ public class DashboardFragmentTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void displayTilesAsPreference_withEmptyCategory_shouldNotAddTiles() {
|
public void displayTilesAsPreference_withEmptyCategory_shouldNotAddTiles() {
|
||||||
mDashboardCategory.tiles = null;
|
mDashboardCategory.removeTile(0);
|
||||||
mTestFragment.onCreatePreferences(new Bundle(), "rootKey");
|
mTestFragment.onCreatePreferences(new Bundle(), "rootKey");
|
||||||
|
|
||||||
verify(mTestFragment.mScreen, never()).addPreference(nullable(Preference.class));
|
verify(mTestFragment.mScreen, never()).addPreference(nullable(Preference.class));
|
||||||
|
Reference in New Issue
Block a user