Avoid showing the screenshot ui on external displays
When adding a window (the screenshot one in this case) to an external display that is mirroring the default one, mirroring stops.
For now we can't distinguish whether an external display is mirroring or extending the default one, so we're always passing "false" to "showUIOnExternalDisplay" ScreenshotController constructor to only show the UI on the default display
While a simple "showUi" would have worked from ScreenshotController point of view, the more verbose "showUIOnExternalDisplay" makes it explicit that we can't avoid showing the UI on the default display.
Writing a test for ScreenshotController is not currently feasible due to the many deps that can't be mocked. Making it testable would require a full risky refactor.
Note that the screenshot window is still created regardless of the value of showUIOnExternalDisplay parameter: as a follow up improvement we can avoid the creation of it, but it would involve a big refactor of the class, not easy to flag-guard.
Bug: 290910794
Bug: 304693302
Test: Tested manually on felix with simulated external display
Change-Id: Id709a7c0ff5a3b3cda665ce44c7fa25727552a69
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java
index 4b3bd0b..127a57e 100644
--- a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java
@@ -303,6 +303,13 @@
private String mPackageName = "";
private BroadcastReceiver mCopyBroadcastReceiver;
+ // When false, the screenshot is taken without showing the ui. Note that this only applies to
+ // external displays, as on the default one the UI should **always** be shown.
+ // This is needed in case of screenshot during display mirroring, as adding another window to
+ // the external display makes mirroring stop.
+ // When there is a way to distinguish between displays that are mirroring or extending, this
+ // can be removed and we can directly show the ui only in the extended case.
+ private final Boolean mShowUIOnExternalDisplay;
/** Tracks config changes that require re-creating UI */
private final InterestingConfigChanges mConfigChanges = new InterestingConfigChanges(
ActivityInfo.CONFIG_ORIENTATION
@@ -335,7 +342,8 @@
AssistContentRequester assistContentRequester,
MessageContainerController messageContainerController,
Provider<ScreenshotSoundController> screenshotSoundController,
- @Assisted int displayId
+ @Assisted int displayId,
+ @Assisted boolean showUIOnExternalDisplay
) {
mScreenshotSmartActions = screenshotSmartActions;
mNotificationsController = screenshotNotificationsController;
@@ -401,6 +409,7 @@
mContext.registerReceiver(mCopyBroadcastReceiver, new IntentFilter(
ClipboardOverlayController.COPY_OVERLAY_ACTION),
ClipboardOverlayController.SELF_PERMISSION, null, Context.RECEIVER_NOT_EXPORTED);
+ mShowUIOnExternalDisplay = showUIOnExternalDisplay;
}
void handleScreenshot(ScreenshotData screenshot, Consumer<Uri> finisher,
@@ -448,6 +457,23 @@
prepareViewForNewScreenshot(screenshot, oldPackageName);
+ if (mFlags.isEnabled(Flags.SCREENSHOT_METADATA) && screenshot.getTaskId() >= 0) {
+ mAssistContentRequester.requestAssistContent(screenshot.getTaskId(),
+ new AssistContentRequester.Callback() {
+ @Override
+ public void onAssistContentAvailable(AssistContent assistContent) {
+ screenshot.setContextUrl(assistContent.getWebUri());
+ }
+ });
+ }
+
+ if (!shouldShowUi()) {
+ saveScreenshotInWorkerThread(
+ screenshot.getUserHandle(), finisher, this::logSuccessOnActionsReady,
+ (ignored) -> {});
+ return;
+ }
+
saveScreenshotInWorkerThread(screenshot.getUserHandle(), finisher,
this::showUiOnActionsReady, this::showUiOnQuickShareActionReady);
@@ -482,22 +508,16 @@
screenshot.getUserHandle()));
mScreenshotView.setScreenshot(screenshot);
- if (mFlags.isEnabled(Flags.SCREENSHOT_METADATA) && screenshot.getTaskId() >= 0) {
- mAssistContentRequester.requestAssistContent(screenshot.getTaskId(),
- new AssistContentRequester.Callback() {
- @Override
- public void onAssistContentAvailable(AssistContent assistContent) {
- screenshot.setContextUrl(assistContent.getWebUri());
- }
- });
- }
-
// ignore system bar insets for the purpose of window layout
mWindow.getDecorView().setOnApplyWindowInsetsListener(
(v, insets) -> WindowInsets.CONSUMED);
mScreenshotHandler.cancelTimeout(); // restarted after animation
}
+ private boolean shouldShowUi() {
+ return mDisplayId == Display.DEFAULT_DISPLAY || mShowUIOnExternalDisplay;
+ }
+
void prepareViewForNewScreenshot(ScreenshotData screenshot, String oldPackageName) {
withWindowAttached(() -> {
if (mUserManager.isManagedProfile(screenshot.getUserHandle().getIdentifier())) {
@@ -1199,7 +1219,13 @@
/** Injectable factory to create screenshot controller instances for a specific display. */
@AssistedFactory
public interface Factory {
- /** Creates an instance of the controller for that specific displayId. */
- ScreenshotController create(int displayId);
+ /**
+ * Creates an instance of the controller for that specific displayId.
+ *
+ * @param displayId: display to capture
+ * @param showUIOnExternalDisplay: Whether the UI should be shown if this is an external
+ * display.
+ */
+ ScreenshotController create(int displayId, boolean showUIOnExternalDisplay);
}
}
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt
index 03c3f7a..abe40ff 100644
--- a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt
@@ -135,7 +135,9 @@
}
private fun getScreenshotController(id: Int): ScreenshotController {
- return screenshotControllers.computeIfAbsent(id) { screenshotControllerFactory.create(id) }
+ return screenshotControllers.computeIfAbsent(id) {
+ screenshotControllerFactory.create(id, /* showUIOnExternalDisplay= */ false)
+ }
}
/** For java compatibility only. see [executeScreenshots] */
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java
index 0be2265..75d52cb 100644
--- a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java
@@ -132,7 +132,8 @@
if (mFeatureFlags.isEnabled(MULTI_DISPLAY_SCREENSHOT)) {
mScreenshot = null;
} else {
- mScreenshot = screenshotControllerFactory.create(Display.DEFAULT_DISPLAY);
+ mScreenshot = screenshotControllerFactory.create(
+ Display.DEFAULT_DISPLAY, /* showUIOnExternalDisplay= */ false);
}
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt
index a105c15..d8821aa 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt
@@ -63,8 +63,8 @@
@Before
fun setUp() {
- whenever(controllerFactory.create(eq(0))).thenReturn(controller0)
- whenever(controllerFactory.create(eq(1))).thenReturn(controller1)
+ whenever(controllerFactory.create(eq(0), any())).thenReturn(controller0)
+ whenever(controllerFactory.create(eq(1), any())).thenReturn(controller1)
}
@Test
@@ -74,8 +74,8 @@
val onSaved = { _: Uri -> }
screenshotExecutor.executeScreenshots(createScreenshotRequest(), onSaved, callback)
- verify(controllerFactory).create(eq(0))
- verify(controllerFactory).create(eq(1))
+ verify(controllerFactory).create(eq(0), any())
+ verify(controllerFactory).create(eq(1), any())
val capturer = ArgumentCaptor<ScreenshotData>()
@@ -107,8 +107,8 @@
callback
)
- verify(controllerFactory).create(eq(0))
- verify(controllerFactory, never()).create(eq(1))
+ verify(controllerFactory).create(eq(0), any())
+ verify(controllerFactory, never()).create(eq(1), any())
val capturer = ArgumentCaptor<ScreenshotData>()
@@ -139,7 +139,7 @@
@Test
fun executeScreenshots_allowedTypes_allCaptured() =
testScope.runTest {
- whenever(controllerFactory.create(any())).thenReturn(controller0)
+ whenever(controllerFactory.create(any(), any())).thenReturn(controller0)
setDisplays(
display(TYPE_INTERNAL, id = 0),
diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt
index 6205d90..5091a70 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt
@@ -86,7 +86,7 @@
)
.thenReturn(false)
whenever(userManager.isUserUnlocked).thenReturn(true)
- whenever(controllerFactory.create(any())).thenReturn(controller)
+ whenever(controllerFactory.create(any(), any())).thenReturn(controller)
// Stub request processor as a synchronous no-op for tests with the flag enabled
doAnswer {