From bedb8136d358d6e3b2d002875d38fe2114c3fced Mon Sep 17 00:00:00 2001 From: Ats Jenk Date: Wed, 30 Nov 2022 14:22:59 -0800 Subject: [PATCH] Only create DesktopModeController when flag is on Ensure that DesktopModeController and DesktopModeTaskRepository are only created when they are needed, when the desktop mode flag is on. With the previous dagger configuration WMShellBaseModule relied on submodules to provide an implementation if one exists. If the submodule does have the implementation defined, it is always created and supplied to WMShellBaseModule provider method. Provider method in WMShellBaseModule then decides, based on flags, whether to return that implementation or not. But the instance is always created, regardless of that flag check. This change ensures that DesktopModeController and DesktopModeTaskRepository are only instantiated if WMShellBaseModule provider methods will provide a corresponding instance. This is achieved by relying on lazy instance creation by dagger. If the instances won't be provided due to flags being off, the instances won't be created. Bug: 260908558 Test: build and run sysui with desktop mode flag off, observe DesktopModeController object is not instatiated Change-Id: Ia155b8e079072e543c98463a58ab1907729db7b0 --- .../wm/shell/dagger/WMShellBaseModule.java | 18 ++++++++----- .../wm/shell/dagger/WMShellModule.java | 2 +- .../desktopmode/DesktopModeController.java | 4 ++- .../shell/desktopmode/DesktopModeStatus.java | 8 ++++++ .../CaptionWindowDecorViewModel.java | 21 ++++++++-------- .../DesktopModeControllerTest.java | 25 +++++++++++++++---- .../CaptionWindowDecorViewModelTests.java | 3 ++- 7 files changed, 57 insertions(+), 24 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 962be9da2111b..ad0adc6c84c4b 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 @@ -687,10 +687,13 @@ public abstract class WMShellBaseModule { @WMSingleton @Provides - static Optional providesDesktopModeController( - @DynamicOverride Optional desktopModeController) { + static Optional provideDesktopModeController( + @DynamicOverride Optional> desktopModeController) { + // Use optional-of-lazy for the dependency that this provider relies on. + // Lazy ensures that this provider will not be the cause the dependency is created + // when it will not be returned due to the condition below. if (DesktopModeStatus.IS_SUPPORTED) { - return desktopModeController; + return desktopModeController.map(Lazy::get); } return Optional.empty(); } @@ -701,10 +704,13 @@ public abstract class WMShellBaseModule { @WMSingleton @Provides - static Optional providesDesktopTaskRepository( - @DynamicOverride Optional desktopModeTaskRepository) { + static Optional provideDesktopTaskRepository( + @DynamicOverride Optional> desktopModeTaskRepository) { + // Use optional-of-lazy for the dependency that this provider relies on. + // Lazy ensures that this provider will not be the cause the dependency is created + // when it will not be returned due to the condition below. if (DesktopModeStatus.IS_SUPPORTED) { - return desktopModeTaskRepository; + return desktopModeTaskRepository.map(Lazy::get); } return Optional.empty(); } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellModule.java b/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellModule.java index f1670cd792cf8..6be83054ae597 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellModule.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellModule.java @@ -189,7 +189,7 @@ public abstract class WMShellModule { ShellTaskOrganizer taskOrganizer, DisplayController displayController, SyncTransactionQueue syncQueue, - @DynamicOverride DesktopModeController desktopModeController) { + Optional desktopModeController) { return new CaptionWindowDecorViewModel( context, mainHandler, diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DesktopModeController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DesktopModeController.java index 17ba246910d5d..5824f51a1d33d 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DesktopModeController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/desktopmode/DesktopModeController.java @@ -100,7 +100,9 @@ public class DesktopModeController implements RemoteCallable mDesktopModeController; private boolean mTransitionDragActive; private SparseArray mEventReceiversByDisplay = new SparseArray<>(); @@ -90,7 +91,7 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { ShellTaskOrganizer taskOrganizer, DisplayController displayController, SyncTransactionQueue syncQueue, - DesktopModeController desktopModeController) { + Optional desktopModeController) { this( context, mainHandler, @@ -110,7 +111,7 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { ShellTaskOrganizer taskOrganizer, DisplayController displayController, SyncTransactionQueue syncQueue, - DesktopModeController desktopModeController, + Optional desktopModeController, CaptionWindowDecoration.Factory captionWindowDecorFactory, Supplier inputManagerSupplier) { @@ -246,10 +247,10 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { } else if (id == R.id.caption_handle) { decoration.createHandleMenu(); } else if (id == R.id.desktop_button) { - mDesktopModeController.setDesktopModeActive(true); + mDesktopModeController.ifPresent(c -> c.setDesktopModeActive(true)); decoration.closeHandleMenu(); } else if (id == R.id.fullscreen_button) { - mDesktopModeController.setDesktopModeActive(false); + mDesktopModeController.ifPresent(c -> c.setDesktopModeActive(false)); decoration.closeHandleMenu(); decoration.setButtonVisibility(); } @@ -304,9 +305,9 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { */ private void handleEventForMove(MotionEvent e) { RunningTaskInfo taskInfo = mTaskOrganizer.getRunningTaskInfo(mTaskId); - int windowingMode = mDesktopModeController - .getDisplayAreaWindowingMode(taskInfo.displayId); - if (windowingMode == WINDOWING_MODE_FULLSCREEN) { + if (mDesktopModeController.isPresent() + && mDesktopModeController.get().getDisplayAreaWindowingMode(taskInfo.displayId) + == WINDOWING_MODE_FULLSCREEN) { return; } switch (e.getActionMasked()) { @@ -331,7 +332,7 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { e.getRawX(dragPointerIdx), e.getRawY(dragPointerIdx)); if (e.getRawY(dragPointerIdx) <= statusBarHeight && DesktopModeStatus.isActive(mContext)) { - mDesktopModeController.setDesktopModeActive(false); + mDesktopModeController.ifPresent(c -> c.setDesktopModeActive(false)); } break; } @@ -471,7 +472,7 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { int statusBarHeight = mDisplayController .getDisplayLayout(focusedDecor.mTaskInfo.displayId).stableInsets().top; if (ev.getY() > statusBarHeight) { - mDesktopModeController.setDesktopModeActive(true); + mDesktopModeController.ifPresent(c -> c.setDesktopModeActive(true)); return; } } diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/desktopmode/DesktopModeControllerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/desktopmode/DesktopModeControllerTest.java index 01584a067cc2a..dad9133007118 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/desktopmode/DesktopModeControllerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/desktopmode/DesktopModeControllerTest.java @@ -36,7 +36,7 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -100,15 +100,14 @@ public class DesktopModeControllerTest extends ShellTestCase { @Before public void setUp() { mMockitoSession = mockitoSession().mockStatic(DesktopModeStatus.class).startMocking(); + when(DesktopModeStatus.isSupported()).thenReturn(true); when(DesktopModeStatus.isActive(any())).thenReturn(true); mShellInit = Mockito.spy(new ShellInit(mTestExecutor)); mDesktopModeTaskRepository = new DesktopModeTaskRepository(); - mController = new DesktopModeController(mContext, mShellInit, mShellController, - mShellTaskOrganizer, mRootTaskDisplayAreaOrganizer, mTransitions, - mDesktopModeTaskRepository, mMockHandler, new TestShellExecutor()); + mController = createController(); when(mShellTaskOrganizer.getRunningTasks(anyInt())).thenReturn(new ArrayList<>()); @@ -125,7 +124,17 @@ public class DesktopModeControllerTest extends ShellTestCase { @Test public void instantiate_addInitCallback() { - verify(mShellInit, times(1)).addInitCallback(any(), any()); + verify(mShellInit).addInitCallback(any(), any()); + } + + @Test + public void instantiate_flagOff_doNotAddInitCallback() { + when(DesktopModeStatus.isSupported()).thenReturn(false); + clearInvocations(mShellInit); + + createController(); + + verify(mShellInit, never()).addInitCallback(any(), any()); } @Test @@ -353,6 +362,12 @@ public class DesktopModeControllerTest extends ShellTestCase { assertThat(wct).isNotNull(); } + private DesktopModeController createController() { + return new DesktopModeController(mContext, mShellInit, mShellController, + mShellTaskOrganizer, mRootTaskDisplayAreaOrganizer, mTransitions, + mDesktopModeTaskRepository, mMockHandler, new TestShellExecutor()); + } + private DisplayAreaInfo createMockDisplayArea() { DisplayAreaInfo displayAreaInfo = new DisplayAreaInfo(new MockToken().mToken, mContext.getDisplayId(), 0); diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModelTests.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModelTests.java index 9b37b97a4c24f..ad6fcedd3166e 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModelTests.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModelTests.java @@ -54,6 +54,7 @@ import org.mockito.Mock; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.function.Supplier; /** Tests of {@link CaptionWindowDecorViewModel} */ @@ -101,7 +102,7 @@ public class CaptionWindowDecorViewModelTests extends ShellTestCase { mTaskOrganizer, mDisplayController, mSyncQueue, - mDesktopModeController, + Optional.of(mDesktopModeController), mCaptionWindowDecorFactory, new MockObjectSupplier<>(mMockInputManagers, () -> mock(InputManager.class))); mCaptionWindowDecorViewModel.setEventReceiverFactory(mEventReceiverFactory);