From cae66a132c1693bab948592cba0c497774a13623 Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Mon, 5 Apr 2021 11:06:47 -0700 Subject: [PATCH] Move wm component initialization to shell main thread - Previously, mWMComponent.init() would make a blocking call to WMShell to initialize on the shell main thread. We're expanding that to include the construction of the shell objects as well to provide a little more consistency and predictability on the shell side (ie. fetching some statics may seem safe, but actually aren't). We're continuing to recommending that ctors don't include any initialization logic though Bug: 181979527 Test: Verify with trace that shell init happens on the shell main thread Test: atest SystemUITests Test: atest WMShellUnitTests Change-Id: I60559a47e00e91d34be7d4e94f4f3e041fdc590d Signed-off-by: Winson Chung --- .../wm/shell/dagger/WMShellBaseModule.java | 8 ++-- .../dagger/WMShellConcurrencyModule.java | 40 ++++++++++++++----- .../com/android/systemui/SystemUIFactory.java | 39 ++++++++++++++++-- .../android/systemui/dagger/WMComponent.java | 13 +++++- 4 files changed, 81 insertions(+), 19 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellBaseModule.java b/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellBaseModule.java index 9f4ff7c8dc060..abac150b1609c 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellBaseModule.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellBaseModule.java @@ -168,8 +168,8 @@ public abstract class WMShellBaseModule { @WMSingleton @Provides - static DragAndDrop provideDragAndDrop(DragAndDropController dragAndDropController) { - return dragAndDropController.asDragAndDrop(); + static Optional provideDragAndDrop(DragAndDropController dragAndDropController) { + return Optional.of(dragAndDropController.asDragAndDrop()); } @WMSingleton @@ -184,8 +184,8 @@ public abstract class WMShellBaseModule { @WMSingleton @Provides - static CompatUI provideCompatUI(CompatUIController compatUIController) { - return compatUIController.asCompatUI(); + static Optional provideCompatUI(CompatUIController compatUIController) { + return Optional.of(compatUIController.asCompatUI()); } @WMSingleton diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellConcurrencyModule.java b/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellConcurrencyModule.java index 5c205f97beb70..8f9636c0bb306 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellConcurrencyModule.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellConcurrencyModule.java @@ -27,7 +27,10 @@ import android.os.HandlerThread; import android.os.Looper; import android.os.Trace; +import androidx.annotation.Nullable; + import com.android.internal.graphics.SfVsyncFrameCallbackProvider; +import com.android.wm.shell.R; import com.android.wm.shell.common.HandlerExecutor; import com.android.wm.shell.common.ShellExecutor; import com.android.wm.shell.common.annotations.ChoreographerSfVsync; @@ -35,7 +38,6 @@ import com.android.wm.shell.common.annotations.ExternalMainThread; import com.android.wm.shell.common.annotations.ShellAnimationThread; import com.android.wm.shell.common.annotations.ShellMainThread; import com.android.wm.shell.common.annotations.ShellSplashscreenThread; -import com.android.wm.shell.R; import dagger.Module; import dagger.Provides; @@ -53,7 +55,7 @@ public abstract class WMShellConcurrencyModule { /** * Returns whether to enable a separate shell thread for the shell features. */ - private static boolean enableShellMainThread(Context context) { + public static boolean enableShellMainThread(Context context) { return context.getResources().getBoolean(R.bool.config_enableShellMainThread); } @@ -84,24 +86,42 @@ public abstract class WMShellConcurrencyModule { return new HandlerExecutor(sysuiMainHandler); } + /** + * Creates a shell main thread to be injected into the shell components. This does not provide + * the {@param HandleThread}, but is used to create the thread prior to initializing the + * WM component, and is explicitly bound. + * + * See {@link com.android.systemui.SystemUIFactory#init(Context, boolean)}. + */ + public static HandlerThread createShellMainThread() { + HandlerThread mainThread = new HandlerThread("wmshell.main", THREAD_PRIORITY_DISPLAY); + return mainThread; + } + /** * Shell main-thread Handler, don't use this unless really necessary (ie. need to dedupe * multiple types of messages, etc.) + * + * @param mainThread If non-null, this thread is expected to be started already */ @WMSingleton @Provides @ShellMainThread public static Handler provideShellMainHandler(Context context, + @Nullable @ShellMainThread HandlerThread mainThread, @ExternalMainThread Handler sysuiMainHandler) { if (enableShellMainThread(context)) { - HandlerThread mainThread = new HandlerThread("wmshell.main", THREAD_PRIORITY_DISPLAY); - mainThread.start(); - if (Build.IS_DEBUGGABLE) { - mainThread.getLooper().setTraceTag(Trace.TRACE_TAG_WINDOW_MANAGER); - mainThread.getLooper().setSlowLogThresholdMs(MSGQ_SLOW_DISPATCH_THRESHOLD_MS, - MSGQ_SLOW_DELIVERY_THRESHOLD_MS); - } - return Handler.createAsync(mainThread.getLooper()); + if (mainThread == null) { + // If this thread wasn't pre-emptively started, then create and start it + mainThread = createShellMainThread(); + mainThread.start(); + } + if (Build.IS_DEBUGGABLE) { + mainThread.getLooper().setTraceTag(Trace.TRACE_TAG_WINDOW_MANAGER); + mainThread.getLooper().setSlowLogThresholdMs(MSGQ_SLOW_DISPATCH_THRESHOLD_MS, + MSGQ_SLOW_DELIVERY_THRESHOLD_MS); + } + return Handler.createAsync(mainThread.getLooper()); } return sysuiMainHandler; } diff --git a/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java b/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java index b3be87731fbc4..55a95ca1ccf2e 100644 --- a/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java +++ b/packages/SystemUI/src/com/android/systemui/SystemUIFactory.java @@ -21,6 +21,7 @@ import android.content.Context; import android.content.res.AssetManager; import android.content.res.Resources; import android.os.Handler; +import android.os.HandlerThread; import android.util.Log; import com.android.internal.annotations.VisibleForTesting; @@ -28,6 +29,7 @@ import com.android.systemui.dagger.DaggerGlobalRootComponent; import com.android.systemui.dagger.GlobalRootComponent; import com.android.systemui.dagger.SysUIComponent; import com.android.systemui.dagger.WMComponent; +import com.android.wm.shell.dagger.WMShellConcurrencyModule; import com.android.systemui.navigationbar.gestural.BackGestureTfClassifierProvider; import com.android.systemui.screenshot.ScreenshotNotificationSmartActionsProvider; import com.android.wm.shell.transition.ShellTransitions; @@ -93,8 +95,9 @@ public class SystemUIFactory { && android.os.Process.myUserHandle().isSystem() && ActivityThread.currentProcessName().equals(ActivityThread.currentPackageName()); mRootComponent = buildGlobalRootComponent(context); + // Stand up WMComponent - mWMComponent = mRootComponent.getWMComponentBuilder().build(); + setupWmComponent(context); if (mInitializeComponents) { // Only initialize when not starting from tests since this currently initializes some // components that shouldn't be run in the test environment @@ -121,8 +124,8 @@ public class SystemUIFactory { .setDisplayAreaHelper(mWMComponent.getDisplayAreaHelper()) .setTaskSurfaceHelper(mWMComponent.getTaskSurfaceHelper()) .setRecentTasks(mWMComponent.getRecentTasks()) - .setCompatUI(Optional.of(mWMComponent.getCompatUI())) - .setDragAndDrop(Optional.of(mWMComponent.getDragAndDrop())) + .setCompatUI(mWMComponent.getCompatUI()) + .setDragAndDrop(mWMComponent.getDragAndDrop()) .setBackAnimation(mWMComponent.getBackAnimation()); } else { // TODO: Call on prepareSysUIComponentBuilder but not with real components. Other option @@ -157,6 +160,36 @@ public class SystemUIFactory { dependency.start(); } + /** + * Sets up {@link #mWMComponent}. On devices where the Shell runs on its own main thread, + * this will pre-create the thread to ensure that the components are constructed on the + * same thread, to reduce the likelihood of side effects from running the constructors on + * a different thread than the rest of the class logic. + */ + private void setupWmComponent(Context context) { + WMComponent.Builder wmBuilder = mRootComponent.getWMComponentBuilder(); + if (!mInitializeComponents || !WMShellConcurrencyModule.enableShellMainThread(context)) { + // If running under tests or shell thread is not enabled, we don't need anything special + mWMComponent = wmBuilder.build(); + return; + } + + // If the shell main thread is enabled, initialize the component on that thread + HandlerThread shellThread = WMShellConcurrencyModule.createShellMainThread(); + shellThread.start(); + + // Use an async handler since we don't care about synchronization + Handler shellHandler = Handler.createAsync(shellThread.getLooper()); + boolean built = shellHandler.runWithScissors(() -> { + wmBuilder.setShellMainThread(shellThread); + mWMComponent = wmBuilder.build(); + }, 5000); + if (!built) { + Log.w(TAG, "Failed to initialize WMComponent"); + throw new RuntimeException(); + } + } + /** * Prepares the SysUIComponent builder before it is built. * @param sysUIBuilder the builder provided by the root component's getSysUIComponent() method diff --git a/packages/SystemUI/src/com/android/systemui/dagger/WMComponent.java b/packages/SystemUI/src/com/android/systemui/dagger/WMComponent.java index b9266923a157d..b02074a65e9ad 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/WMComponent.java +++ b/packages/SystemUI/src/com/android/systemui/dagger/WMComponent.java @@ -17,6 +17,9 @@ package com.android.systemui.dagger; import android.content.Context; +import android.os.HandlerThread; + +import androidx.annotation.Nullable; import com.android.systemui.SystemUIFactory; import com.android.systemui.tv.TvWMComponent; @@ -26,6 +29,7 @@ import com.android.wm.shell.TaskViewFactory; import com.android.wm.shell.apppairs.AppPairs; import com.android.wm.shell.back.BackAnimation; import com.android.wm.shell.bubbles.Bubbles; +import com.android.wm.shell.common.annotations.ShellMainThread; import com.android.wm.shell.compatui.CompatUI; import com.android.wm.shell.dagger.TvWMShellModule; import com.android.wm.shell.dagger.WMShellModule; @@ -44,6 +48,7 @@ import com.android.wm.shell.transition.ShellTransitions; import java.util.Optional; +import dagger.BindsInstance; import dagger.Subcomponent; /** @@ -64,6 +69,10 @@ public interface WMComponent { */ @Subcomponent.Builder interface Builder { + + @BindsInstance + Builder setShellMainThread(@Nullable @ShellMainThread HandlerThread t); + WMComponent build(); } @@ -120,10 +129,10 @@ public interface WMComponent { Optional getRecentTasks(); @WMSingleton - CompatUI getCompatUI(); + Optional getCompatUI(); @WMSingleton - DragAndDrop getDragAndDrop(); + Optional getDragAndDrop(); @WMSingleton Optional getBackAnimation();