From 49fc3f7aa8b0da180f6cbc61da4df942ba398c2c Mon Sep 17 00:00:00 2001 From: Sunny Goyal Date: Fri, 18 Aug 2017 10:17:06 -0700 Subject: [PATCH] Fixing bindAllApplications scheduled with null list. When deferring bindAllApplications, the same global state was being used to store the pending state. This global state was being cleared in one deferred execution after a second execution was scheduled, causing the second execution to get null/wrong values. Instead using independent local/anonymous class to maintian states, so that it does not get cleared by other methods. Bug: 64789383 Change-Id: I78477a110fe663603b4bdeb7d1fac7c4ce0831a2 --- src/com/android/launcher3/Launcher.java | 58 ++++++++----------- .../launcher3/util/RunnableWithId.java | 36 ++++++++++++ 2 files changed, 59 insertions(+), 35 deletions(-) create mode 100644 src/com/android/launcher3/util/RunnableWithId.java diff --git a/src/com/android/launcher3/Launcher.java b/src/com/android/launcher3/Launcher.java index 28b44825db..e229d6c65a 100644 --- a/src/com/android/launcher3/Launcher.java +++ b/src/com/android/launcher3/Launcher.java @@ -119,6 +119,7 @@ import com.android.launcher3.userevent.nano.LauncherLogProto.Action; import com.android.launcher3.userevent.nano.LauncherLogProto.ContainerType; import com.android.launcher3.userevent.nano.LauncherLogProto.ControlType; import com.android.launcher3.util.ActivityResultInfo; +import com.android.launcher3.util.RunnableWithId; import com.android.launcher3.util.ComponentKey; import com.android.launcher3.util.ItemInfoMatcher; import com.android.launcher3.util.MultiHashMap; @@ -146,6 +147,9 @@ import java.util.List; import java.util.Set; import java.util.concurrent.Executor; +import static com.android.launcher3.util.RunnableWithId.RUNNABLE_ID_BIND_APPS; +import static com.android.launcher3.util.RunnableWithId.RUNNABLE_ID_BIND_WIDGETS; + /** * Default launcher application. */ @@ -246,7 +250,6 @@ public class Launcher extends BaseActivity // Main container view and the model for the widget tray screen. @Thunk WidgetsContainerView mWidgetsView; - @Thunk MultiHashMap mAllWidgets; // We set the state in both onCreate and then onNewIntent in some cases, which causes both // scroll issues (because the workspace may not have been measured yet) and extra work. @@ -3113,12 +3116,12 @@ public class Launcher extends BaseActivity * * @return {@code true} if we are currently paused. The caller might be able to skip some work */ - @Thunk boolean waitUntilResume(Runnable run, boolean deletePreviousRunnables) { + @Thunk boolean waitUntilResume(Runnable run) { if (mPaused) { if (LOGD) Log.d(TAG, "Deferring update until onResume"); - if (deletePreviousRunnables) { - while (mBindOnResumeCallbacks.remove(run)) { - } + if (run instanceof RunnableWithId) { + // Remove any runnables which have the same id + while (mBindOnResumeCallbacks.remove(run)) { } } mBindOnResumeCallbacks.add(run); return true; @@ -3127,10 +3130,6 @@ public class Launcher extends BaseActivity } } - private boolean waitUntilResume(Runnable run) { - return waitUntilResume(run, false); - } - public void addOnResumeCallback(Runnable run) { mOnResumeCallbacks.add(run); } @@ -3663,26 +3662,18 @@ public class Launcher extends BaseActivity return LauncherCallbacks.SEARCH_BAR_HEIGHT_NORMAL; } - /** - * A runnable that we can dequeue and re-enqueue when all applications are bound (to prevent - * multiple calls to bind the same list.) - */ - @Thunk ArrayList mTmpAppsList; - private final Runnable mBindAllApplicationsRunnable = new Runnable() { - public void run() { - bindAllApplications(mTmpAppsList); - mTmpAppsList = null; - } - }; - /** * Add the icons for all apps. * * Implementation of the method from LauncherModel.Callbacks. */ public void bindAllApplications(final ArrayList apps) { - if (waitUntilResume(mBindAllApplicationsRunnable, true)) { - mTmpAppsList = apps; + Runnable r = new RunnableWithId(RUNNABLE_ID_BIND_APPS) { + public void run() { + bindAllApplications(apps); + } + }; + if (waitUntilResume(r)) { return; } @@ -3690,10 +3681,10 @@ public class Launcher extends BaseActivity Executor pendingExecutor = getPendingExecutor(); if (pendingExecutor != null && mState != State.APPS) { // Wait until the fade in animation has finished before setting all apps list. - mTmpAppsList = apps; - pendingExecutor.execute(mBindAllApplicationsRunnable); + pendingExecutor.execute(r); return; } + mAppsView.setApps(apps); } if (mLauncherCallbacks != null) { @@ -3887,28 +3878,25 @@ public class Launcher extends BaseActivity } } - private final Runnable mBindAllWidgetsRunnable = new Runnable() { + @Override + public void bindAllWidgets(final MultiHashMap allWidgets) { + Runnable r = new RunnableWithId(RUNNABLE_ID_BIND_WIDGETS) { + @Override public void run() { - bindAllWidgets(mAllWidgets); + bindAllWidgets(allWidgets); } }; - - @Override - public void bindAllWidgets(MultiHashMap allWidgets) { - if (waitUntilResume(mBindAllWidgetsRunnable, true)) { - mAllWidgets = allWidgets; + if (waitUntilResume(r)) { return; } if (mWidgetsView != null && allWidgets != null) { Executor pendingExecutor = getPendingExecutor(); if (pendingExecutor != null && mState != State.WIDGETS) { - mAllWidgets = allWidgets; - pendingExecutor.execute(mBindAllWidgetsRunnable); + pendingExecutor.execute(r); return; } mWidgetsView.setWidgets(allWidgets); - mAllWidgets = null; } AbstractFloatingView topView = AbstractFloatingView.getTopOpenView(this); diff --git a/src/com/android/launcher3/util/RunnableWithId.java b/src/com/android/launcher3/util/RunnableWithId.java new file mode 100644 index 0000000000..030eb09ff3 --- /dev/null +++ b/src/com/android/launcher3/util/RunnableWithId.java @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.launcher3.util; + +/** + * A runnable with an id associated which is used for equality check. + */ +public abstract class RunnableWithId implements Runnable { + + public static final int RUNNABLE_ID_BIND_APPS = 1; + public static final int RUNNABLE_ID_BIND_WIDGETS = 2; + + public final int id; + + public RunnableWithId(int id) { + this.id = id; + } + + @Override + public boolean equals(Object obj) { + return obj instanceof RunnableWithId && ((RunnableWithId) obj).id == id; + } +}