Merge "Fix an issue with concurrent modification on widget holders list" into tm-qpr-dev
diff --git a/quickstep/src/com/android/launcher3/uioverrides/QuickstepWidgetHolder.java b/quickstep/src/com/android/launcher3/uioverrides/QuickstepWidgetHolder.java
index 278a45a..ff3a292 100644
--- a/quickstep/src/com/android/launcher3/uioverrides/QuickstepWidgetHolder.java
+++ b/quickstep/src/com/android/launcher3/uioverrides/QuickstepWidgetHolder.java
@@ -22,6 +22,7 @@
 import android.appwidget.AppWidgetHostView;
 import android.appwidget.AppWidgetProviderInfo;
 import android.content.Context;
+import android.util.Log;
 import android.util.SparseArray;
 import android.widget.RemoteViews;
 
@@ -33,14 +34,14 @@
 import com.android.launcher3.config.FeatureFlags;
 import com.android.launcher3.model.WidgetsModel;
 import com.android.launcher3.util.IntSet;
-import com.android.launcher3.util.Thunk;
 import com.android.launcher3.widget.LauncherAppWidgetHostView;
 import com.android.launcher3.widget.LauncherAppWidgetProviderInfo;
 import com.android.launcher3.widget.LauncherWidgetHolder;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
-import java.util.Map;
+import java.util.Set;
 import java.util.WeakHashMap;
 import java.util.function.BiConsumer;
 import java.util.function.IntConsumer;
@@ -50,6 +51,8 @@
  */
 public final class QuickstepWidgetHolder extends LauncherWidgetHolder {
 
+    private static final String TAG = "QuickstepWidgetHolder";
+
     private static final UpdateKey<AppWidgetProviderInfo> KEY_PROVIDER_UPDATE =
             AppWidgetHostView::onUpdateProviderInfo;
     private static final UpdateKey<RemoteViews> KEY_VIEWS_UPDATE =
@@ -63,6 +66,8 @@
 
     private static AppWidgetHost sWidgetHost = null;
 
+    private final SparseArray<AppWidgetHostView> mViews = new SparseArray<>();
+
     private final @Nullable RemoteViews.InteractionHandler mInteractionHandler;
 
     private final @NonNull IntConsumer mAppWidgetRemovedCallback;
@@ -71,15 +76,14 @@
     // Map to all pending updated keyed with appWidgetId;
     private final SparseArray<PendingUpdate> mPendingUpdateMap = new SparseArray<>();
 
-    @Thunk
-    QuickstepWidgetHolder(@NonNull Context context,
+    private QuickstepWidgetHolder(@NonNull Context context,
             @Nullable IntConsumer appWidgetRemovedCallback,
             @Nullable RemoteViews.InteractionHandler interactionHandler) {
         super(context, appWidgetRemovedCallback);
         mAppWidgetRemovedCallback = appWidgetRemovedCallback != null ? appWidgetRemovedCallback
                 : i -> {};
         mInteractionHandler = interactionHandler;
-        sHolders.add(this);
+        MAIN_EXECUTOR.execute(() -> sHolders.add(this));
     }
 
     @Override
@@ -92,7 +96,7 @@
                             sHolders.forEach(h -> h.mAppWidgetRemovedCallback.accept(i))),
                     () -> MAIN_EXECUTOR.execute(() ->
                             sHolders.forEach(h -> h.mProviderChangedListeners.forEach(
-                            ProviderChangedListener::notifyWidgetProvidersChanged))),
+                                    ProviderChangedListener::notifyWidgetProvidersChanged))),
                     UI_HELPER_EXECUTOR.getLooper());
             if (!WidgetsModel.GO_DISABLE_WIDGETS) {
                 sWidgetHost.startListening();
@@ -107,11 +111,7 @@
         int count = mPendingUpdateMap.size();
         for (int i = 0; i < count; i++) {
             int widgetId = mPendingUpdateMap.keyAt(i);
-            QuickstepWidgetHolderListener listener = sListeners.get(widgetId);
-            if (listener == null) {
-                continue;
-            }
-            AppWidgetHostView view = listener.mView.get(this);
+            AppWidgetHostView view = mViews.get(widgetId);
             if (view == null) {
                 continue;
             }
@@ -131,7 +131,16 @@
         mPendingUpdateMap.clear();
     }
 
-    private <T> void addPendingAction(int widgetId, UpdateKey<T> key, T data) {
+    private <T> void onWidgetUpdate(int widgetId, UpdateKey<T> key, T data) {
+        if (isListening()) {
+            AppWidgetHostView view = mViews.get(widgetId);
+            if (view == null) {
+                return;
+            }
+            key.accept(view, data);
+            return;
+        }
+
         PendingUpdate pendingUpdate = mPendingUpdateMap.get(widgetId);
         if (pendingUpdate == null) {
             pendingUpdate = new PendingUpdate();
@@ -167,7 +176,11 @@
      */
     @Override
     public void destroy() {
-        sHolders.remove(this);
+        try {
+            MAIN_EXECUTOR.submit(() -> sHolders.remove(this)).get();
+        } catch (Exception e) {
+            Log.e(TAG, "Failed to remove self from holder list", e);
+        }
     }
 
     @Override
@@ -228,15 +241,16 @@
         }
         widgetView.setInteractionHandler(mInteractionHandler);
         widgetView.setAppWidget(appWidgetId, appWidget);
+        mViews.put(appWidgetId, widgetView);
 
         QuickstepWidgetHolderListener listener = sListeners.get(appWidgetId);
         if (listener == null) {
-            listener = new QuickstepWidgetHolderListener(appWidgetId, this, widgetView);
+            listener = new QuickstepWidgetHolderListener(appWidgetId);
             sWidgetHost.setListener(appWidgetId, listener);
             sListeners.put(appWidgetId, listener);
-        } else {
-            listener.resetView(this, widgetView);
         }
+        RemoteViews remoteViews = listener.addHolder(this);
+        widgetView.updateAppWidget(remoteViews);
 
         return widgetView;
     }
@@ -247,31 +261,30 @@
     @Override
     public void clearViews() {
         for (int i = sListeners.size() - 1; i >= 0; i--) {
-            sListeners.valueAt(i).mView.remove(this);
+            sListeners.valueAt(i).mListeningHolders.remove(this);
         }
     }
 
     private static class QuickstepWidgetHolderListener
             implements AppWidgetHost.AppWidgetHostListener {
 
-        @NonNull
-        private final Map<QuickstepWidgetHolder, AppWidgetHostView> mView = new WeakHashMap<>();
+        // Static listeners should use a set that is backed by WeakHashMap to avoid memory leak
+        private final Set<QuickstepWidgetHolder> mListeningHolders = Collections.newSetFromMap(
+                new WeakHashMap<>());
 
         private final int mWidgetId;
 
-        @Nullable private RemoteViews mRemoteViews = null;
+        private @Nullable RemoteViews mRemoteViews;
 
-        QuickstepWidgetHolderListener(int widgetId, @NonNull QuickstepWidgetHolder holder,
-                @NonNull LauncherAppWidgetHostView view) {
+        QuickstepWidgetHolderListener(int widgetId) {
             mWidgetId = widgetId;
-            mView.put(holder, view);
         }
 
         @UiThread
-        public void resetView(@NonNull QuickstepWidgetHolder holder,
-                @NonNull AppWidgetHostView view) {
-            mView.put(holder, view);
-            view.updateAppWidget(mRemoteViews);
+        @Nullable
+        public RemoteViews addHolder(@NonNull QuickstepWidgetHolder holder) {
+            mListeningHolders.add(holder);
+            return mRemoteViews;
         }
 
         @Override
@@ -295,13 +308,8 @@
         }
 
         private <T> void executeOnMainExecutor(UpdateKey<T> key, T data) {
-            MAIN_EXECUTOR.execute(() -> mView.forEach((holder, view) -> {
-                if (holder.isListening()) {
-                    key.accept(view, data);
-                } else {
-                    holder.addPendingAction(mWidgetId, key, data);
-                }
-            }));
+            MAIN_EXECUTOR.execute(() -> mListeningHolders.forEach(holder ->
+                    holder.onWidgetUpdate(mWidgetId, key, data)));
         }
     }