Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b8d2cd41 authored by Doris Ling's avatar Doris Ling
Browse files

Fix ConcurrentModificationException in DashboardCategory. 

Move the category tiles sorting logic into DashboardCategory, and change
all client access to the category tiles via the proper update methods
instead of modifying the list directly.

Change-Id: I479669abd8d1d0a8ee9a4113d8ad2244da56f4d8
Fixes: 69677575
Test: make RunSettingsLibRoboTests
parent e4cf6bf4

packages/SettingsLib/src/com/android/settingslib/drawer/CategoryManager.java

+9 −32
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ package com.android.settingslib.drawer; 
import android.content.ComponentName;

import android.content.Context;

import android.support.annotation.VisibleForTesting;

import android.text.TextUtils;

import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.Log;
@@ -27,7 +26,6 @@ import android.util.Pair; 
import com.android.settingslib.applications.InterestingConfigChanges;



import java.util.ArrayList;

import java.util.Collections;

import java.util.HashMap;

import java.util.List;

import java.util.Map;
@@ -104,10 +102,10 @@ public class CategoryManager { 
        }

        for (int i = 0; i < mCategories.size(); i++) {

            DashboardCategory category = mCategories.get(i);

            for (int j = 0; j < category.tiles.size(); j++) {

                Tile tile = category.tiles.get(j);

            for (int j = 0; j < category.getTilesCount(); j++) {

                Tile tile = category.getTile(j);

                if (tileBlacklist.contains(tile.intent.getComponent())) {

                    category.tiles.remove(j--);

                    category.removeTile(j--);

                }

            }

        }
@@ -181,7 +179,7 @@ public class CategoryManager { 
                        newCategory = new DashboardCategory();

                        categoryByKeyMap.put(newCategoryKey, newCategory);

                    }

                    newCategory.tiles.add(tile);

                    newCategory.addTile(tile);

                }

            }

        }
@@ -198,7 +196,7 @@ public class CategoryManager { 
    synchronized void sortCategories(Context context,

            Map<String, DashboardCategory> categoryByKeyMap) {

        for (Entry<String, DashboardCategory> categoryEntry : categoryByKeyMap.entrySet()) {

            sortCategoriesForExternalTiles(context, categoryEntry.getValue());

            categoryEntry.getValue().sortTiles(context.getPackageName());

        }

    }
@@ -210,16 +208,16 @@ public class CategoryManager { 
    synchronized void filterDuplicateTiles(Map<String, DashboardCategory> categoryByKeyMap) {

        for (Entry<String, DashboardCategory> categoryEntry : categoryByKeyMap.entrySet()) {

            final DashboardCategory category = categoryEntry.getValue();

            final int count = category.tiles.size();

            final int count = category.getTilesCount();

            final Set<ComponentName> components = new ArraySet<>();

            for (int i = count - 1; i >= 0; i--) {

                final Tile tile = category.tiles.get(i);

                final Tile tile = category.getTile(i);

                if (tile.intent == null) {

                    continue;

                }

                final ComponentName tileComponent = tile.intent.getComponent();

                if (components.contains(tileComponent)) {

                    category.tiles.remove(i);

                    category.removeTile(i);

                } else {

                    components.add(tileComponent);

                }
@@ -234,28 +232,7 @@ public class CategoryManager { 
     */

    private synchronized void sortCategoriesForExternalTiles(Context context,

            DashboardCategory dashboardCategory) {

        final String skipPackageName = context.getPackageName();

        dashboardCategory.sortTiles(context.getPackageName());



        // Sort tiles based on [priority, package within priority]

        Collections.sort(dashboardCategory.tiles, (tile1, tile2) -> {

            final String package1 = tile1.intent.getComponent().getPackageName();

            final String package2 = tile2.intent.getComponent().getPackageName();

            final int packageCompare = CASE_INSENSITIVE_ORDER.compare(package1, package2);

            // First sort by priority

            final int priorityCompare = tile2.priority - tile1.priority;

            if (priorityCompare != 0) {

                return priorityCompare;

            }

            // Then sort by package name, skip package take precedence

            if (packageCompare != 0) {

                if (TextUtils.equals(package1, skipPackageName)) {

                    return -1;

                }

                if (TextUtils.equals(package2, skipPackageName)) {

                    return 1;

                }

            }

            return packageCompare;

        });

    }

}

packages/SettingsLib/src/com/android/settingslib/drawer/DashboardCategory.java

+79 −12
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


package com.android.settingslib.drawer;



import static java.lang.String.CASE_INSENSITIVE_ORDER;



import android.content.ComponentName;

import android.os.Parcel;

import android.os.Parcelable;
@@ -23,6 +25,8 @@ import android.text.TextUtils; 
import android.util.Log;



import java.util.ArrayList;

import java.util.Collections;

import java.util.Comparator;

import java.util.List;



public class DashboardCategory implements Parcelable {
@@ -48,39 +52,59 @@ public class DashboardCategory implements Parcelable { 
    /**

     * List of the category's children

     */

    public List<Tile> tiles = new ArrayList<>();

    private List<Tile> mTiles = new ArrayList<>();



    DashboardCategory(DashboardCategory in) {

        if (in != null) {

            title = in.title;

            key = in.key;

            priority = in.priority;

            for (Tile tile : in.mTiles) {

                mTiles.add(tile);

            }

        }

    }



    public DashboardCategory() {

        // Empty

    }



    /**

     * Get a copy of the list of the category's children.

     *

     * Note: the returned list serves as a read-only list. If tiles needs to be added or removed

     * from the actual tiles list, it should be done through {@link #addTile}, {@link #removeTile}.

     */

    public List<Tile> getTiles() {

        return Collections.unmodifiableList(mTiles);

    }



    public void addTile(Tile tile) {

        tiles.add(tile);

        mTiles.add(tile);

    }



    public void addTile(int n, Tile tile) {

        tiles.add(n, tile);

        mTiles.add(n, tile);

    }



    public void removeTile(Tile tile) {

        tiles.remove(tile);

        mTiles.remove(tile);

    }



    public void removeTile(int n) {

        tiles.remove(n);

        mTiles.remove(n);

    }



    public int getTilesCount() {

        return tiles.size();

        return mTiles.size();

    }



    public Tile getTile(int n) {

        return tiles.get(n);

        return mTiles.get(n);

    }



    public boolean containsComponent(ComponentName component) {

        for (Tile tile : tiles) {

        for (Tile tile : mTiles) {

            if (TextUtils.equals(tile.intent.getComponent().getClassName(),

                    component.getClassName())) {

                if (DEBUG) {
@@ -95,6 +119,40 @@ public class DashboardCategory implements Parcelable { 
        return false;

    }



    /**

     * Sort priority value for tiles in this category.

     */

    public void sortTiles() {

        Collections.sort(mTiles, TILE_COMPARATOR);

    }



    /**

     * Sort priority value and package name for tiles in this category.

     */

    public void sortTiles(String skipPackageName) {

        // Sort mTiles based on [priority, package within priority]

        Collections.sort(mTiles, (tile1, tile2) -> {

            final String package1 = tile1.intent.getComponent().getPackageName();

            final String package2 = tile2.intent.getComponent().getPackageName();

            final int packageCompare = CASE_INSENSITIVE_ORDER.compare(package1, package2);

            // First sort by priority

            final int priorityCompare = tile2.priority - tile1.priority;

            if (priorityCompare != 0) {

                return priorityCompare;

            }

            // Then sort by package name, skip package take precedence

            if (packageCompare != 0) {

                if (TextUtils.equals(package1, skipPackageName)) {

                    return -1;

                }

                if (TextUtils.equals(package2, skipPackageName)) {

                    return 1;

                }

            }

            return packageCompare;

        });

    }



    @Override

    public int describeContents() {

        return 0;
@@ -106,11 +164,11 @@ public class DashboardCategory implements Parcelable { 
        dest.writeString(key);

        dest.writeInt(priority);



        final int count = tiles.size();

        final int count = mTiles.size();

        dest.writeInt(count);



        for (int n = 0; n < count; n++) {

            Tile tile = tiles.get(n);

            Tile tile = mTiles.get(n);

            tile.writeToParcel(dest, flags);

        }

    }
@@ -124,7 +182,7 @@ public class DashboardCategory implements Parcelable { 


        for (int n = 0; n < count; n++) {

            Tile tile = Tile.CREATOR.createFromParcel(in);

            tiles.add(tile);

            mTiles.add(tile);

        }

    }
@@ -141,4 +199,13 @@ public class DashboardCategory implements Parcelable { 
            return new DashboardCategory[size];

        }

    };



    public static final Comparator<Tile> TILE_COMPARATOR =

            new Comparator<Tile>() {

                @Override

                public int compare(Tile lhs, Tile rhs) {

                    return rhs.priority - lhs.priority;

                }

            };



}

packages/SettingsLib/src/com/android/settingslib/drawer/TileUtils.java

+1 −9
Original line number Diff line number Diff line
@@ -253,7 +253,7 @@ public class TileUtils { 
        }

        ArrayList<DashboardCategory> categories = new ArrayList<>(categoryMap.values());

        for (DashboardCategory category : categories) {

            Collections.sort(category.tiles, TILE_COMPARATOR);

            category.sortTiles();

        }

        Collections.sort(categories, CATEGORY_COMPARATOR);

        if (DEBUG_TIMING) Log.d(LOG_TAG, "getCategories took "
@@ -595,14 +595,6 @@ public class TileUtils { 
        return pathSegments.get(0);

    }



    public static final Comparator<Tile> TILE_COMPARATOR =

            new Comparator<Tile>() {

        @Override

        public int compare(Tile lhs, Tile rhs) {

            return rhs.priority - lhs.priority;

        }

    };



    private static final Comparator<DashboardCategory> CATEGORY_COMPARATOR =

            new Comparator<DashboardCategory>() {

        @Override

packages/SettingsLib/tests/robotests/src/com/android/settingslib/drawer/CategoryManagerTest.java

+46 −44
Original line number Diff line number Diff line
@@ -95,8 +95,9 @@ public class CategoryManagerTest { 
        mCategoryManager.backwardCompatCleanupForCategory(mTileByComponentCache, mCategoryByKeyMap);



        assertThat(mCategoryByKeyMap.size()).isEqualTo(2);

        assertThat(mCategoryByKeyMap.get(CategoryKey.CATEGORY_ACCOUNT).tiles.size()).isEqualTo(1);

        assertThat(mCategoryByKeyMap.get(oldCategory).tiles.size()).isEqualTo(1);

        assertThat(

                mCategoryByKeyMap.get(CategoryKey.CATEGORY_ACCOUNT).getTilesCount()).isEqualTo(1);

        assertThat(mCategoryByKeyMap.get(oldCategory).getTilesCount()).isEqualTo(1);

    }



    @Test
@@ -114,9 +115,10 @@ public class CategoryManagerTest { 
        // Added 1 more category to category map.

        assertThat(mCategoryByKeyMap.size()).isEqualTo(2);

        // The new category map has CATEGORY_NETWORK type now, which contains 1 tile.

        assertThat(mCategoryByKeyMap.get(CategoryKey.CATEGORY_NETWORK).tiles.size()).isEqualTo(1);

        assertThat(

                mCategoryByKeyMap.get(CategoryKey.CATEGORY_NETWORK).getTilesCount()).isEqualTo(1);

        // Old category still exists.

        assertThat(mCategoryByKeyMap.get(oldCategory).tiles.size()).isEqualTo(1);

        assertThat(mCategoryByKeyMap.get(oldCategory).getTilesCount()).isEqualTo(1);

    }



    @Test
@@ -136,9 +138,9 @@ public class CategoryManagerTest { 
        tile3.intent =

                new Intent().setComponent(new ComponentName(testPackage, "class3"));

        tile3.priority = 200;

        category.tiles.add(tile1);

        category.tiles.add(tile2);

        category.tiles.add(tile3);

        category.addTile(tile1);

        category.addTile(tile2);

        category.addTile(tile3);

        mCategoryByKeyMap.put(CategoryKey.CATEGORY_HOMEPAGE, category);



        // Sort their priorities
@@ -146,9 +148,9 @@ public class CategoryManagerTest { 
                mCategoryByKeyMap);



        // Verify they are now sorted.

        assertThat(category.tiles.get(0)).isSameAs(tile3);

        assertThat(category.tiles.get(1)).isSameAs(tile1);

        assertThat(category.tiles.get(2)).isSameAs(tile2);

        assertThat(category.getTile(0)).isSameAs(tile3);

        assertThat(category.getTile(1)).isSameAs(tile1);

        assertThat(category.getTile(2)).isSameAs(tile2);

    }



    @Test
@@ -169,9 +171,9 @@ public class CategoryManagerTest { 
        tile3.intent =

                new Intent().setComponent(new ComponentName(testPackage1, "class3"));

        tile3.priority = 50;

        category.tiles.add(tile1);

        category.tiles.add(tile2);

        category.tiles.add(tile3);

        category.addTile(tile1);

        category.addTile(tile2);

        category.addTile(tile3);

        mCategoryByKeyMap.put(CategoryKey.CATEGORY_HOMEPAGE, category);



        // Sort their priorities
@@ -179,9 +181,9 @@ public class CategoryManagerTest { 
                mCategoryByKeyMap);



        // Verify they are now sorted.

        assertThat(category.tiles.get(0)).isSameAs(tile2);

        assertThat(category.tiles.get(1)).isSameAs(tile1);

        assertThat(category.tiles.get(2)).isSameAs(tile3);

        assertThat(category.getTile(0)).isSameAs(tile2);

        assertThat(category.getTile(1)).isSameAs(tile1);

        assertThat(category.getTile(2)).isSameAs(tile3);

    }



    @Test
@@ -202,9 +204,9 @@ public class CategoryManagerTest { 
        tile3.intent =

                new Intent().setComponent(new ComponentName(testPackage, "class3"));

        tile3.priority = 50;

        category.tiles.add(tile1);

        category.tiles.add(tile2);

        category.tiles.add(tile3);

        category.addTile(tile1);

        category.addTile(tile2);

        category.addTile(tile3);

        mCategoryByKeyMap.put(CategoryKey.CATEGORY_HOMEPAGE, category);



        // Sort their priorities
@@ -212,9 +214,9 @@ public class CategoryManagerTest { 
                mCategoryByKeyMap);



        // Verify the sorting order is not changed

        assertThat(category.tiles.get(0)).isSameAs(tile1);

        assertThat(category.tiles.get(1)).isSameAs(tile2);

        assertThat(category.tiles.get(2)).isSameAs(tile3);

        assertThat(category.getTile(0)).isSameAs(tile1);

        assertThat(category.getTile(1)).isSameAs(tile2);

        assertThat(category.getTile(2)).isSameAs(tile3);

    }



    @Test
@@ -236,10 +238,10 @@ public class CategoryManagerTest { 
        final Tile tile4 = new Tile();

        tile4.intent = new Intent().setComponent(new ComponentName(testPackage, "class3"));

        tile4.priority = -1;

        category.tiles.add(tile1);

        category.tiles.add(tile2);

        category.tiles.add(tile3);

        category.tiles.add(tile4);

        category.addTile(tile1);

        category.addTile(tile2);

        category.addTile(tile3);

        category.addTile(tile4);

        mCategoryByKeyMap.put(CategoryKey.CATEGORY_HOMEPAGE, category);



        // Sort their priorities
@@ -247,10 +249,10 @@ public class CategoryManagerTest { 
            mCategoryByKeyMap);



        // Verify the sorting order is not changed

        assertThat(category.tiles.get(0)).isSameAs(tile1);

        assertThat(category.tiles.get(1)).isSameAs(tile2);

        assertThat(category.tiles.get(2)).isSameAs(tile3);

        assertThat(category.tiles.get(3)).isSameAs(tile4);

        assertThat(category.getTile(0)).isSameAs(tile1);

        assertThat(category.getTile(1)).isSameAs(tile2);

        assertThat(category.getTile(2)).isSameAs(tile3);

        assertThat(category.getTile(3)).isSameAs(tile4);

    }



    @Test
@@ -270,9 +272,9 @@ public class CategoryManagerTest { 
        final Tile tile3 = new Tile();

        tile3.intent = new Intent().setComponent(new ComponentName(testPackage3, "class3"));

        tile3.priority = 1;

        category.tiles.add(tile1);

        category.tiles.add(tile2);

        category.tiles.add(tile3);

        category.addTile(tile1);

        category.addTile(tile2);

        category.addTile(tile3);

        mCategoryByKeyMap.put(CategoryKey.CATEGORY_HOMEPAGE, category);



        // Sort their priorities
@@ -280,9 +282,9 @@ public class CategoryManagerTest { 
            mCategoryByKeyMap);



        // Verify the sorting order is internal first, follow by package name ordering

        assertThat(category.tiles.get(0)).isSameAs(tile2);

        assertThat(category.tiles.get(1)).isSameAs(tile3);

        assertThat(category.tiles.get(2)).isSameAs(tile1);

        assertThat(category.getTile(0)).isSameAs(tile2);

        assertThat(category.getTile(1)).isSameAs(tile3);

        assertThat(category.getTile(2)).isSameAs(tile1);

    }



    @Test
@@ -303,14 +305,14 @@ public class CategoryManagerTest { 
        tile3.intent =

                new Intent().setComponent(new ComponentName(testPackage, "class3"));

        tile3.priority = 50;

        category.tiles.add(tile1);

        category.tiles.add(tile2);

        category.tiles.add(tile3);

        category.addTile(tile1);

        category.addTile(tile2);

        category.addTile(tile3);

        mCategoryByKeyMap.put(CategoryKey.CATEGORY_HOMEPAGE, category);



        mCategoryManager.filterDuplicateTiles(mCategoryByKeyMap);



        assertThat(category.tiles.size()).isEqualTo(3);

        assertThat(category.getTilesCount()).isEqualTo(3);

    }



    @Test
@@ -331,13 +333,13 @@ public class CategoryManagerTest { 
        tile3.intent =

                new Intent().setComponent(new ComponentName(testPackage, "class1"));

        tile3.priority = 50;

        category.tiles.add(tile1);

        category.tiles.add(tile2);

        category.tiles.add(tile3);

        category.addTile(tile1);

        category.addTile(tile2);

        category.addTile(tile3);

        mCategoryByKeyMap.put(CategoryKey.CATEGORY_HOMEPAGE, category);



        mCategoryManager.filterDuplicateTiles(mCategoryByKeyMap);



        assertThat(category.tiles.size()).isEqualTo(1);

        assertThat(category.getTilesCount()).isEqualTo(1);

    }

}

packages/SettingsLib/tests/robotests/src/com/android/settingslib/drawer/TileUtilsTest.java

+1 −1
Original line number Diff line number Diff line
@@ -216,7 +216,7 @@ public class TileUtilsTest { 
        List<DashboardCategory> categoryList = TileUtils.getCategories(

                mContext, cache, false /* categoryDefinedInManifest */, testAction,

                TileUtils.SETTING_PKG);

        assertThat(categoryList.get(0).tiles.get(0).category).isEqualTo(testCategory);

        assertThat(categoryList.get(0).getTile(0).category).isEqualTo(testCategory);

    }



    @Test