From a04de842d1ca5aa7d270ef074f8f423da0517797 Mon Sep 17 00:00:00 2001 From: Stefan Andonian Date: Thu, 17 Nov 2022 18:56:09 +0000 Subject: [PATCH] Fix ConcurrentModificationExceptions during binding. The same lists of extra items stored in BgModelData are also stored in UI components. This is causing ConcurrentModificationExceptions. The solution is to clone these lists or mark them as immutable before storing them in their respective components. Bug: 206918543 Test: Verified that crash no longer occurs after fix. Change-Id: I571a2c451af58137aa7513b372b6a8ecf9bd3ff6 --- .../HotseatPredictionController.java | 2 +- .../launcher3/model/PredictionUpdateTask.java | 9 ++++++--- .../model/QuickstepModelDelegate.java | 3 ++- .../model/WidgetsPredictionUpdateTask.java | 9 ++++++--- .../launcher3/model/BaseModelUpdateTask.java | 3 +-- .../android/launcher3/model/BgDataModel.java | 19 +------------------ 6 files changed, 17 insertions(+), 28 deletions(-) diff --git a/quickstep/src/com/android/launcher3/hybridhotseat/HotseatPredictionController.java b/quickstep/src/com/android/launcher3/hybridhotseat/HotseatPredictionController.java index 05b81671c3..0a2a9b37ba 100644 --- a/quickstep/src/com/android/launcher3/hybridhotseat/HotseatPredictionController.java +++ b/quickstep/src/com/android/launcher3/hybridhotseat/HotseatPredictionController.java @@ -305,7 +305,7 @@ public class HotseatPredictionController implements DragController.DragListener, * Sets or updates the predicted items only once the hotseat becomes hidden to the user */ private void applyPredictedItems(FixedContainerItems items) { - mPredictedItems = items.items; + mPredictedItems = new ArrayList(items.items); if (mPredictedItems.isEmpty()) { HotseatRestoreHelper.restoreBackup(mLauncher); } diff --git a/quickstep/src/com/android/launcher3/model/PredictionUpdateTask.java b/quickstep/src/com/android/launcher3/model/PredictionUpdateTask.java index bc3253fcfb..f3748ce281 100644 --- a/quickstep/src/com/android/launcher3/model/PredictionUpdateTask.java +++ b/quickstep/src/com/android/launcher3/model/PredictionUpdateTask.java @@ -34,8 +34,10 @@ import com.android.launcher3.Utilities; import com.android.launcher3.model.BgDataModel.FixedContainerItems; import com.android.launcher3.model.QuickstepModelDelegate.PredictorState; import com.android.launcher3.model.data.AppInfo; +import com.android.launcher3.model.data.ItemInfo; import com.android.launcher3.model.data.WorkspaceItemInfo; +import java.util.ArrayList; import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -68,7 +70,7 @@ public class PredictionUpdateTask extends BaseModelUpdateTask { .map(info -> info.user) .collect(Collectors.toSet()); - FixedContainerItems fci = new FixedContainerItems(mPredictorState.containerId); + List items = new ArrayList<>(mTargets.size()); for (AppTarget target : mTargets) { WorkspaceItemInfo itemInfo; ShortcutInfo si = target.getShortcutInfo(); @@ -107,10 +109,11 @@ public class PredictionUpdateTask extends BaseModelUpdateTask { } } - itemInfo.container = fci.containerId; - fci.items.add(itemInfo); + itemInfo.container = mPredictorState.containerId; + items.add(itemInfo); } + FixedContainerItems fci = new FixedContainerItems(mPredictorState.containerId, items); dataModel.extraItems.put(fci.containerId, fci); bindExtraContainerItems(fci); usersForChangedShortcuts.forEach( diff --git a/quickstep/src/com/android/launcher3/model/QuickstepModelDelegate.java b/quickstep/src/com/android/launcher3/model/QuickstepModelDelegate.java index de0b14d4fa..555be95384 100644 --- a/quickstep/src/com/android/launcher3/model/QuickstepModelDelegate.java +++ b/quickstep/src/com/android/launcher3/model/QuickstepModelDelegate.java @@ -132,7 +132,8 @@ public class QuickstepModelDelegate extends ModelDelegate { // Widgets prediction isn't used frequently. And thus, it is not persisted on disk. mDataModel.extraItems.put(mWidgetsRecommendationState.containerId, - new FixedContainerItems(mWidgetsRecommendationState.containerId)); + new FixedContainerItems(mWidgetsRecommendationState.containerId, + new ArrayList<>())); mActive = true; } diff --git a/quickstep/src/com/android/launcher3/model/WidgetsPredictionUpdateTask.java b/quickstep/src/com/android/launcher3/model/WidgetsPredictionUpdateTask.java index 1beabf1255..6160378767 100644 --- a/quickstep/src/com/android/launcher3/model/WidgetsPredictionUpdateTask.java +++ b/quickstep/src/com/android/launcher3/model/WidgetsPredictionUpdateTask.java @@ -25,6 +25,7 @@ import androidx.annotation.NonNull; import com.android.launcher3.LauncherAppState; import com.android.launcher3.model.BgDataModel.FixedContainerItems; import com.android.launcher3.model.QuickstepModelDelegate.PredictorState; +import com.android.launcher3.model.data.ItemInfo; import com.android.launcher3.util.ComponentKey; import com.android.launcher3.util.PackageUserKey; import com.android.launcher3.widget.PendingAddWidgetInfo; @@ -91,10 +92,12 @@ public final class WidgetsPredictionUpdateTask extends BaseModelUpdateTask { if (servicePredictedItems.isEmpty()) { servicePredictedItems.addAll(localFilteredWidgets); } + + List items = servicePredictedItems.stream() + .map(it -> new PendingAddWidgetInfo(it.widgetInfo, CONTAINER_WIDGETS_PREDICTION)) + .collect(Collectors.toList()); FixedContainerItems fixedContainerItems = - new FixedContainerItems(mPredictorState.containerId); - servicePredictedItems.forEach(w -> fixedContainerItems.items.add( - new PendingAddWidgetInfo(w.widgetInfo, CONTAINER_WIDGETS_PREDICTION))); + new FixedContainerItems(mPredictorState.containerId, items); dataModel.extraItems.put(mPredictorState.containerId, fixedContainerItems); bindExtraContainerItems(fixedContainerItems); diff --git a/src/com/android/launcher3/model/BaseModelUpdateTask.java b/src/com/android/launcher3/model/BaseModelUpdateTask.java index 5b6f9f62a5..74a2c5d4b5 100644 --- a/src/com/android/launcher3/model/BaseModelUpdateTask.java +++ b/src/com/android/launcher3/model/BaseModelUpdateTask.java @@ -118,8 +118,7 @@ public abstract class BaseModelUpdateTask implements ModelUpdateTask { } public void bindExtraContainerItems(@NonNull final FixedContainerItems item) { - FixedContainerItems copy = item.clone(); - scheduleCallbackTask(c -> c.bindExtraContainerItems(copy)); + scheduleCallbackTask(c -> c.bindExtraContainerItems(item)); } public void bindDeepShortcuts(@NonNull final BgDataModel dataModel) { diff --git a/src/com/android/launcher3/model/BgDataModel.java b/src/com/android/launcher3/model/BgDataModel.java index de23c4b31f..b0f6e13ae3 100644 --- a/src/com/android/launcher3/model/BgDataModel.java +++ b/src/com/android/launcher3/model/BgDataModel.java @@ -433,26 +433,9 @@ public class BgDataModel { public final int containerId; public final List items; - public FixedContainerItems(int containerId) { - this(containerId, new ArrayList<>()); - } - public FixedContainerItems(int containerId, List items) { this.containerId = containerId; - this.items = items; - } - - @Override - public FixedContainerItems clone() { - return new FixedContainerItems(containerId, new ArrayList<>(items)); - } - - public void setItems(List newItems) { - items.clear(); - newItems.forEach(item -> { - item.container = containerId; - items.add(item); - }); + this.items = Collections.unmodifiableList(items); } }