From 1f87d94e5959d63a7bd3c1678e6d6124d4a451b2 Mon Sep 17 00:00:00 2001 From: Fengjiang Li Date: Fri, 21 Jun 2024 10:31:17 -0700 Subject: [PATCH] [Launcher Jank] Avoid SimpleBroadcastReceiver making binder calls on main thread Fix: 348649441 Flag: NONE - jank fix Test: manual - presubmit Change-Id: Ie97713f3f0b1f117662d49c6d6a65651c593d424 --- .../model/QuickstepModelDelegate.java | 2 + .../launcher3/model/WellbeingModel.java | 5 +- .../launcher3/taskbar/TaskbarManager.java | 7 +- .../quickstep/OverviewComponentObserver.java | 15 ++-- .../util/AsyncClockEventDelegate.java | 8 +- .../android/launcher3/LauncherAppState.java | 4 +- src/com/android/launcher3/pm/UserCache.java | 4 +- .../launcher3/util/DisplayController.java | 5 +- .../android/launcher3/util/LockedUserState.kt | 11 +-- .../launcher3/util/ScreenOnTracker.java | 4 +- .../util/SimpleBroadcastReceiver.java | 82 +++++++++++++++---- .../util/WallpaperOffsetInterpolator.java | 5 +- .../launcher3/ui/AbstractLauncherUiTest.java | 3 +- 13 files changed, 106 insertions(+), 49 deletions(-) diff --git a/quickstep/src/com/android/launcher3/model/QuickstepModelDelegate.java b/quickstep/src/com/android/launcher3/model/QuickstepModelDelegate.java index 8b5ed7cc4b..6af5a30b07 100644 --- a/quickstep/src/com/android/launcher3/model/QuickstepModelDelegate.java +++ b/quickstep/src/com/android/launcher3/model/QuickstepModelDelegate.java @@ -205,6 +205,7 @@ public class QuickstepModelDelegate extends ModelDelegate { mActive = true; } + @WorkerThread @Override public void workspaceLoadComplete() { super.workspaceLoadComplete(); @@ -323,6 +324,7 @@ public class QuickstepModelDelegate extends ModelDelegate { } } + @WorkerThread @Override public void destroy() { super.destroy(); diff --git a/quickstep/src/com/android/launcher3/model/WellbeingModel.java b/quickstep/src/com/android/launcher3/model/WellbeingModel.java index a7c965218d..28bc01c005 100644 --- a/quickstep/src/com/android/launcher3/model/WellbeingModel.java +++ b/quickstep/src/com/android/launcher3/model/WellbeingModel.java @@ -111,6 +111,7 @@ public final class WellbeingModel implements SafeCloseable { mWorkerHandler.post(this::initializeInBackground); } + @WorkerThread private void initializeInBackground() { if (!TextUtils.isEmpty(mWellbeingProviderPkg)) { mContext.registerReceiver( @@ -134,8 +135,8 @@ public final class WellbeingModel implements SafeCloseable { public void close() { if (!TextUtils.isEmpty(mWellbeingProviderPkg)) { mWorkerHandler.post(() -> { - mWellbeingAppChangeReceiver.unregisterReceiverSafely(mContext); - mAppAddRemoveReceiver.unregisterReceiverSafely(mContext); + mWellbeingAppChangeReceiver.unregisterReceiverSafelySync(mContext); + mAppAddRemoveReceiver.unregisterReceiverSafelySync(mContext); mContext.getContentResolver().unregisterContentObserver(mContentObserver); }); } diff --git a/quickstep/src/com/android/launcher3/taskbar/TaskbarManager.java b/quickstep/src/com/android/launcher3/taskbar/TaskbarManager.java index 2a58db25df..051bdc885d 100644 --- a/quickstep/src/com/android/launcher3/taskbar/TaskbarManager.java +++ b/quickstep/src/com/android/launcher3/taskbar/TaskbarManager.java @@ -304,7 +304,7 @@ public class TaskbarManager { .register(NAV_BAR_KIDS_MODE, mOnSettingsChangeListener); Log.d(TASKBAR_NOT_DESTROYED_TAG, "registering component callbacks from constructor."); mContext.registerComponentCallbacks(mComponentCallbacks); - mShutdownReceiver.register(mContext, Intent.ACTION_SHUTDOWN); + mShutdownReceiver.registerAsync(mContext, Intent.ACTION_SHUTDOWN); UI_HELPER_EXECUTOR.execute(() -> { mSharedState.taskbarSystemActionPendingIntent = PendingIntent.getBroadcast( mContext, @@ -582,8 +582,7 @@ public class TaskbarManager { public void destroy() { debugWhyTaskbarNotDestroyed("TaskbarManager#destroy()"); removeActivityCallbacksAndListeners(); - UI_HELPER_EXECUTOR.execute( - () -> mTaskbarBroadcastReceiver.unregisterReceiverSafely(mContext)); + mTaskbarBroadcastReceiver.unregisterReceiverSafelyAsync(mContext); destroyExistingTaskbar(); removeTaskbarRootViewFromWindow(); if (mUserUnlocked) { @@ -595,7 +594,7 @@ public class TaskbarManager { .unregister(NAV_BAR_KIDS_MODE, mOnSettingsChangeListener); Log.d(TASKBAR_NOT_DESTROYED_TAG, "unregistering component callbacks from destroy()."); mContext.unregisterComponentCallbacks(mComponentCallbacks); - mContext.unregisterReceiver(mShutdownReceiver); + mShutdownReceiver.unregisterReceiverSafelyAsync(mContext); } public @Nullable TaskbarActivityContext getCurrentActivityContext() { diff --git a/quickstep/src/com/android/quickstep/OverviewComponentObserver.java b/quickstep/src/com/android/quickstep/OverviewComponentObserver.java index a71e3149ab..9c64576d45 100644 --- a/quickstep/src/com/android/quickstep/OverviewComponentObserver.java +++ b/quickstep/src/com/android/quickstep/OverviewComponentObserver.java @@ -36,6 +36,7 @@ import android.util.SparseIntArray; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.UiThread; import com.android.launcher3.R; import com.android.launcher3.util.SimpleBroadcastReceiver; @@ -101,7 +102,7 @@ public final class OverviewComponentObserver { mConfigChangesMap.append(fallbackComponent.hashCode(), fallbackInfo.configChanges); } catch (PackageManager.NameNotFoundException ignored) { /* Impossible */ } - mUserPreferenceChangeReceiver.register(mContext, ACTION_PREFERRED_ACTIVITY_CHANGED); + mUserPreferenceChangeReceiver.registerAsync(mContext, ACTION_PREFERRED_ACTIVITY_CHANGED); updateOverviewTargets(); } @@ -114,6 +115,8 @@ public final class OverviewComponentObserver { mOverviewChangeListener = overviewChangeListener; } + /** Called on {@link TouchInteractionService#onSystemUiFlagsChanged} */ + @UiThread public void onSystemUiStateChanged() { if (mDeviceState.isHomeDisabled() != mIsHomeDisabled) { updateOverviewTargets(); @@ -128,6 +131,7 @@ public final class OverviewComponentObserver { * Update overview intent and {@link BaseActivityInterface} based off the current launcher home * component. */ + @UiThread private void updateOverviewTargets() { ComponentName defaultHome = PackageManagerWrapper.getInstance() .getHomeActivities(new ArrayList<>()); @@ -187,8 +191,9 @@ public final class OverviewComponentObserver { unregisterOtherHomeAppUpdateReceiver(); mUpdateRegisteredPackage = defaultHome.getPackageName(); - mOtherHomeAppUpdateReceiver.registerPkgActions(mContext, mUpdateRegisteredPackage, - ACTION_PACKAGE_ADDED, ACTION_PACKAGE_CHANGED, ACTION_PACKAGE_REMOVED); + mOtherHomeAppUpdateReceiver.registerPkgActionsAsync( + mContext, mUpdateRegisteredPackage, ACTION_PACKAGE_ADDED, + ACTION_PACKAGE_CHANGED, ACTION_PACKAGE_REMOVED); } } mOverviewChangeListener.accept(mIsHomeAndOverviewSame); @@ -198,13 +203,13 @@ public final class OverviewComponentObserver { * Clean up any registered receivers. */ public void onDestroy() { - mContext.unregisterReceiver(mUserPreferenceChangeReceiver); + mUserPreferenceChangeReceiver.unregisterReceiverSafelyAsync(mContext); unregisterOtherHomeAppUpdateReceiver(); } private void unregisterOtherHomeAppUpdateReceiver() { if (mUpdateRegisteredPackage != null) { - mContext.unregisterReceiver(mOtherHomeAppUpdateReceiver); + mOtherHomeAppUpdateReceiver.unregisterReceiverSafelyAsync(mContext); mUpdateRegisteredPackage = null; } } diff --git a/quickstep/src/com/android/quickstep/util/AsyncClockEventDelegate.java b/quickstep/src/com/android/quickstep/util/AsyncClockEventDelegate.java index cda87c0f36..c26fc0c50d 100644 --- a/quickstep/src/com/android/quickstep/util/AsyncClockEventDelegate.java +++ b/quickstep/src/com/android/quickstep/util/AsyncClockEventDelegate.java @@ -18,8 +18,6 @@ package com.android.quickstep.util; import static android.content.Intent.ACTION_TIMEZONE_CHANGED; import static android.content.Intent.ACTION_TIME_CHANGED; -import static com.android.launcher3.util.Executors.UI_HELPER_EXECUTOR; - import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; @@ -64,9 +62,7 @@ public class AsyncClockEventDelegate extends ClockEventDelegate private AsyncClockEventDelegate(Context context) { super(context); mContext = context; - - UI_HELPER_EXECUTOR.execute(() -> - mReceiver.register(mContext, ACTION_TIME_CHANGED, ACTION_TIMEZONE_CHANGED)); + mReceiver.registerAsync(mContext, ACTION_TIME_CHANGED, ACTION_TIMEZONE_CHANGED); } @Override @@ -127,6 +123,6 @@ public class AsyncClockEventDelegate extends ClockEventDelegate public void close() { mDestroyed = true; SettingsCache.INSTANCE.get(mContext).unregister(mFormatUri, this); - UI_HELPER_EXECUTOR.execute(() -> mReceiver.unregisterReceiverSafely(mContext)); + mReceiver.unregisterReceiverSafelyAsync(mContext); } } diff --git a/src/com/android/launcher3/LauncherAppState.java b/src/com/android/launcher3/LauncherAppState.java index 3b8ff62d8a..239967dfc6 100644 --- a/src/com/android/launcher3/LauncherAppState.java +++ b/src/com/android/launcher3/LauncherAppState.java @@ -116,13 +116,13 @@ public class LauncherAppState implements SafeCloseable { SimpleBroadcastReceiver modelChangeReceiver = new SimpleBroadcastReceiver(mModel::onBroadcastIntent); - modelChangeReceiver.register(mContext, Intent.ACTION_LOCALE_CHANGED, + modelChangeReceiver.registerAsync(mContext, Intent.ACTION_LOCALE_CHANGED, ACTION_DEVICE_POLICY_RESOURCE_UPDATED); if (BuildConfig.IS_STUDIO_BUILD) { mContext.registerReceiver(modelChangeReceiver, new IntentFilter(ACTION_FORCE_ROLOAD), RECEIVER_EXPORTED); } - mOnTerminateCallback.add(() -> mContext.unregisterReceiver(modelChangeReceiver)); + mOnTerminateCallback.add(() -> modelChangeReceiver.unregisterReceiverSafelyAsync(mContext)); SafeCloseable userChangeListener = UserCache.INSTANCE.get(mContext) .addUserEventListener(mModel::onUserEvent); diff --git a/src/com/android/launcher3/pm/UserCache.java b/src/com/android/launcher3/pm/UserCache.java index ed25186da4..cf03462fd6 100644 --- a/src/com/android/launcher3/pm/UserCache.java +++ b/src/com/android/launcher3/pm/UserCache.java @@ -93,12 +93,12 @@ public class UserCache implements SafeCloseable { @Override public void close() { - MODEL_EXECUTOR.execute(() -> mUserChangeReceiver.unregisterReceiverSafely(mContext)); + MODEL_EXECUTOR.execute(() -> mUserChangeReceiver.unregisterReceiverSafelySync(mContext)); } @WorkerThread private void initAsync() { - mUserChangeReceiver.register(mContext, + mUserChangeReceiver.registerSync(mContext, Intent.ACTION_MANAGED_PROFILE_AVAILABLE, Intent.ACTION_MANAGED_PROFILE_UNAVAILABLE, Intent.ACTION_MANAGED_PROFILE_REMOVED, diff --git a/src/com/android/launcher3/util/DisplayController.java b/src/com/android/launcher3/util/DisplayController.java index 16fabe26d7..b390cb8d78 100644 --- a/src/com/android/launcher3/util/DisplayController.java +++ b/src/com/android/launcher3/util/DisplayController.java @@ -132,11 +132,11 @@ public class DisplayController implements ComponentCallbacks, SafeCloseable { mWindowContext.registerComponentCallbacks(this); } else { mWindowContext = null; - mReceiver.register(mContext, ACTION_CONFIGURATION_CHANGED); + mReceiver.registerAsync(mContext, ACTION_CONFIGURATION_CHANGED); } // Initialize navigation mode change listener - mReceiver.registerPkgActions(mContext, TARGET_OVERLAY_PACKAGE, ACTION_OVERLAY_CHANGED); + mReceiver.registerPkgActionsAsync(mContext, TARGET_OVERLAY_PACKAGE, ACTION_OVERLAY_CHANGED); WindowManagerProxy wmProxy = WindowManagerProxy.INSTANCE.get(context); Context displayInfoContext = getDisplayInfoContext(display); @@ -218,6 +218,7 @@ public class DisplayController implements ComponentCallbacks, SafeCloseable { } else { // TODO: unregister broadcast receiver } + mReceiver.unregisterReceiverSafelyAsync(mContext); } /** diff --git a/src/com/android/launcher3/util/LockedUserState.kt b/src/com/android/launcher3/util/LockedUserState.kt index 94f9e4fb69..2737249c06 100644 --- a/src/com/android/launcher3/util/LockedUserState.kt +++ b/src/com/android/launcher3/util/LockedUserState.kt @@ -25,6 +25,7 @@ class LockedUserState(private val mContext: Context) : SafeCloseable { val isUserUnlockedAtLauncherStartup: Boolean var isUserUnlocked: Boolean private set + private val mUserUnlockedActions: RunnableList = RunnableList() @VisibleForTesting @@ -50,22 +51,18 @@ class LockedUserState(private val mContext: Context) : SafeCloseable { if (isUserUnlocked) { notifyUserUnlocked() } else { - mUserUnlockedReceiver.register(mContext, Intent.ACTION_USER_UNLOCKED) + mUserUnlockedReceiver.registerAsync(mContext, Intent.ACTION_USER_UNLOCKED) } } private fun notifyUserUnlocked() { mUserUnlockedActions.executeAllAndDestroy() - Executors.THREAD_POOL_EXECUTOR.execute { - mUserUnlockedReceiver.unregisterReceiverSafely(mContext) - } + mUserUnlockedReceiver.unregisterReceiverSafelyAsync(mContext) } /** Stops the receiver from listening for ACTION_USER_UNLOCK broadcasts. */ override fun close() { - Executors.THREAD_POOL_EXECUTOR.execute { - mUserUnlockedReceiver.unregisterReceiverSafely(mContext) - } + mUserUnlockedReceiver.unregisterReceiverSafelyAsync(mContext) } /** diff --git a/src/com/android/launcher3/util/ScreenOnTracker.java b/src/com/android/launcher3/util/ScreenOnTracker.java index e16e4778e9..c1d192cb18 100644 --- a/src/com/android/launcher3/util/ScreenOnTracker.java +++ b/src/com/android/launcher3/util/ScreenOnTracker.java @@ -42,12 +42,12 @@ public class ScreenOnTracker implements SafeCloseable { // Assume that the screen is on to begin with mContext = context; mIsScreenOn = true; - mReceiver.register(context, ACTION_SCREEN_ON, ACTION_SCREEN_OFF, ACTION_USER_PRESENT); + mReceiver.registerAsync(context, ACTION_SCREEN_ON, ACTION_SCREEN_OFF, ACTION_USER_PRESENT); } @Override public void close() { - mReceiver.unregisterReceiverSafely(mContext); + mReceiver.unregisterReceiverSafelyAsync(mContext); } private void onReceive(Intent intent) { diff --git a/src/com/android/launcher3/util/SimpleBroadcastReceiver.java b/src/com/android/launcher3/util/SimpleBroadcastReceiver.java index 064bcd072f..5f39cce058 100644 --- a/src/com/android/launcher3/util/SimpleBroadcastReceiver.java +++ b/src/com/android/launcher3/util/SimpleBroadcastReceiver.java @@ -15,14 +15,21 @@ */ package com.android.launcher3.util; +import static com.android.launcher3.util.Executors.UI_HELPER_EXECUTOR; + import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.os.Looper; import android.os.PatternMatcher; import android.text.TextUtils; import androidx.annotation.Nullable; +import androidx.annotation.UiThread; +import androidx.annotation.WorkerThread; + +import com.android.launcher3.BuildConfig; import java.util.function.Consumer; @@ -39,20 +46,62 @@ public class SimpleBroadcastReceiver extends BroadcastReceiver { mIntentConsumer.accept(intent); } - /** - * Helper method to register multiple actions - */ - public void register(Context context, String... actions) { + /** Helper method to register multiple actions. Caller should be on main thread. */ + @UiThread + public void registerAsync(Context context, String... actions) { + assertOnMainThread(); + UI_HELPER_EXECUTOR.execute(() -> registerSync(context, actions)); + } + + /** Helper method to register multiple actions. Caller should be on main thread. */ + @WorkerThread + public void registerSync(Context context, String... actions) { + assertOnBgThread(); context.registerReceiver(this, getFilter(actions)); } /** - * Helper method to register multiple actions associated with a paction + * Helper method to register multiple actions associated with a action. Caller should be from + * main thread. */ - public void registerPkgActions(Context context, @Nullable String pkg, String... actions) { + @UiThread + public void registerPkgActionsAsync(Context context, @Nullable String pkg, String... actions) { + assertOnMainThread(); + UI_HELPER_EXECUTOR.execute(() -> registerPkgActionsSync(context, pkg, actions)); + } + + /** + * Helper method to register multiple actions associated with a action. Caller should be from + * bg thread. + */ + @WorkerThread + public void registerPkgActionsSync(Context context, @Nullable String pkg, String... actions) { + assertOnBgThread(); context.registerReceiver(this, getPackageFilter(pkg, actions)); } + /** + * Unregisters the receiver ignoring any errors on bg thread. Caller should be on main thread. + */ + @UiThread + public void unregisterReceiverSafelyAsync(Context context) { + assertOnMainThread(); + UI_HELPER_EXECUTOR.execute(() -> unregisterReceiverSafelySync(context)); + } + + /** + * Unregisters the receiver ignoring any errors on bg thread. Caller should be on bg thread. + */ + @WorkerThread + public void unregisterReceiverSafelySync(Context context) { + assertOnBgThread(); + try { + context.unregisterReceiver(this); + } catch (IllegalArgumentException e) { + // It was probably never registered or already unregistered. Ignore. + } + } + /** * Creates an intent filter to listen for actions with a specific package in the data field. */ @@ -73,14 +122,19 @@ public class SimpleBroadcastReceiver extends BroadcastReceiver { return filter; } - /** - * Unregisters the receiver ignoring any errors - */ - public void unregisterReceiverSafely(Context context) { - try { - context.unregisterReceiver(this); - } catch (IllegalArgumentException e) { - // It was probably never registered or already unregistered. Ignore. + private static void assertOnBgThread() { + if (BuildConfig.IS_STUDIO_BUILD && isMainThread()) { + throw new IllegalStateException("Should not be called from main thread!"); } } + + private static void assertOnMainThread() { + if (BuildConfig.IS_STUDIO_BUILD && !isMainThread()) { + throw new IllegalStateException("Should not be called from bg thread!"); + } + } + + private static boolean isMainThread() { + return Thread.currentThread() == Looper.getMainLooper().getThread(); + } } diff --git a/src/com/android/launcher3/util/WallpaperOffsetInterpolator.java b/src/com/android/launcher3/util/WallpaperOffsetInterpolator.java index b97b8894c7..a2277a047c 100644 --- a/src/com/android/launcher3/util/WallpaperOffsetInterpolator.java +++ b/src/com/android/launcher3/util/WallpaperOffsetInterpolator.java @@ -198,10 +198,11 @@ public class WallpaperOffsetInterpolator { public void setWindowToken(IBinder token) { mWindowToken = token; if (mWindowToken == null && mRegistered) { - mWallpaperChangeReceiver.unregisterReceiverSafely(mWorkspace.getContext()); + mWallpaperChangeReceiver.unregisterReceiverSafelyAsync(mWorkspace.getContext()); mRegistered = false; } else if (mWindowToken != null && !mRegistered) { - mWallpaperChangeReceiver.register(mWorkspace.getContext(), ACTION_WALLPAPER_CHANGED); + mWallpaperChangeReceiver.registerAsync( + mWorkspace.getContext(), ACTION_WALLPAPER_CHANGED); onWallpaperChanged(); mRegistered = true; } diff --git a/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java b/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java index a6f4441e2a..6e01f9e4b7 100644 --- a/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java +++ b/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java @@ -239,7 +239,8 @@ public abstract class AbstractLauncherUiTest { final CountDownLatch count = new CountDownLatch(2); final SimpleBroadcastReceiver broadcastReceiver = new SimpleBroadcastReceiver(i -> count.countDown()); - broadcastReceiver.registerPkgActions(mTargetContext, pkg, + // We OK to make binder calls on main thread in test. + broadcastReceiver.registerPkgActionsSync(mTargetContext, pkg, Intent.ACTION_PACKAGE_RESTARTED, Intent.ACTION_PACKAGE_DATA_CLEARED); mDevice.executeShellCommand("pm clear " + pkg);