Refactor screenshot logging in case of external display
This makes a few changes to make error logging more explicit in case of
connected displays.
- RequestProcessor now throws a typed exception, so we can avoid
catching unrelated errors in TakeScreenshotExecutor
- ScreenshotNotificationsController can be created only with a factory,
providing a displayId. The display ID is used to append "(External
Display)" string for error notifications happening on any display that
is not the default one.
- There can be a maximum of 2 error notifications: one for the default
display (as before), and another that is shown in case of any error in
any external display connected.
- Error reporting for ScreenshotData processing or ScreenshotController
issues has been completely moved to TakeScreenshotExecutor and scoped to
display ID. TakeScreenshotService still reports some early failures
unrelated to displays.
- As we can't specify the displayId in UiEventLogger: if the failure is
generic and we reach it from TakescreenshotService, only one
SCREENSHOT_REQUESTED_... is reported. However, if the screenshot request
reaches TakeScreenshotExecutor, then one event per display will be
reported.
Test: RequestProcessorTest, TakeScreenshotExecutorTest, TakeScreenshotServiceTest
Bug: 296575569
Bug: 290910794
Change-Id: If187e9713b344605466a2dcb78267ededccfcc85
diff --git a/packages/SystemUI/res/values/strings.xml b/packages/SystemUI/res/values/strings.xml
index 321594f..9f2d01e 100644
--- a/packages/SystemUI/res/values/strings.xml
+++ b/packages/SystemUI/res/values/strings.xml
@@ -209,6 +209,8 @@
<string name="screenshot_saved_title">Screenshot saved</string>
<!-- Notification title displayed when we fail to take a screenshot. [CHAR LIMIT=50] -->
<string name="screenshot_failed_title">Couldn\'t save screenshot</string>
+ <!-- Appended to the notification content when a screenshot failure happens on an external display. [CHAR LIMIT=50] -->
+ <string name="screenshot_failed_external_display_indication">External Display</string>
<!-- Notification text displayed when we fail to save a screenshot due to locked storage. [CHAR LIMIT=100] -->
<string name="screenshot_failed_to_save_user_locked_text">Device must be unlocked before screenshot can be saved</string>
<!-- Notification text displayed when we fail to save a screenshot for unknown reasons. [CHAR LIMIT=100] -->
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/RequestProcessor.kt b/packages/SystemUI/src/com/android/systemui/screenshot/RequestProcessor.kt
index 5154067..c34fd42 100644
--- a/packages/SystemUI/src/com/android/systemui/screenshot/RequestProcessor.kt
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/RequestProcessor.kt
@@ -20,11 +20,10 @@
import android.view.WindowManager.TAKE_SCREENSHOT_PROVIDED_IMAGE
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Application
-import com.android.systemui.flags.FeatureFlags
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch
import java.util.function.Consumer
import javax.inject.Inject
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.launch
/** Processes a screenshot request sent from [ScreenshotHelper]. */
interface ScreenshotRequestProcessor {
@@ -36,16 +35,15 @@
suspend fun process(screenshot: ScreenshotData): ScreenshotData
}
-/**
- * Implementation of [ScreenshotRequestProcessor]
- */
+/** Implementation of [ScreenshotRequestProcessor] */
@SysUISingleton
-class RequestProcessor @Inject constructor(
- private val capture: ImageCapture,
- private val policy: ScreenshotPolicy,
- private val flags: FeatureFlags,
- /** For the Java Async version, to invoke the callback. */
- @Application private val mainScope: CoroutineScope
+class RequestProcessor
+@Inject
+constructor(
+ private val capture: ImageCapture,
+ private val policy: ScreenshotPolicy,
+ /** For the Java Async version, to invoke the callback. */
+ @Application private val mainScope: CoroutineScope
) : ScreenshotRequestProcessor {
override suspend fun process(screenshot: ScreenshotData): ScreenshotData {
@@ -67,8 +65,9 @@
result.userHandle = info.user
if (policy.isManagedProfile(info.user.identifier)) {
- val image = capture.captureTask(info.taskId)
- ?: error("Task snapshot returned a null Bitmap!")
+ val image =
+ capture.captureTask(info.taskId)
+ ?: throw RequestProcessorException("Task snapshot returned a null Bitmap!")
// Provide the task snapshot as the screenshot
result.type = TAKE_SCREENSHOT_PROVIDED_IMAGE
@@ -97,3 +96,6 @@
}
private const val TAG = "RequestProcessor"
+
+/** Exception thrown by [RequestProcessor] if something goes wrong. */
+class RequestProcessorException(message: String) : IllegalStateException(message)
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java
index 127a57e..21a08a9 100644
--- a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java
@@ -325,7 +325,7 @@
Context context,
FeatureFlags flags,
ScreenshotSmartActions screenshotSmartActions,
- ScreenshotNotificationsController screenshotNotificationsController,
+ ScreenshotNotificationsController.Factory screenshotNotificationsControllerFactory,
ScrollCaptureClient scrollCaptureClient,
UiEventLogger uiEventLogger,
ImageExporter imageExporter,
@@ -346,7 +346,7 @@
@Assisted boolean showUIOnExternalDisplay
) {
mScreenshotSmartActions = screenshotSmartActions;
- mNotificationsController = screenshotNotificationsController;
+ mNotificationsController = screenshotNotificationsControllerFactory.create(displayId);
mScrollCaptureClient = scrollCaptureClient;
mUiEventLogger = uiEventLogger;
mImageExporter = imageExporter;
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationsController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationsController.java
deleted file mode 100644
index 4344fd1..0000000
--- a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationsController.java
+++ /dev/null
@@ -1,96 +0,0 @@
-/*
- * Copyright (C) 2019 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.systemui.screenshot;
-
-import static android.content.Context.NOTIFICATION_SERVICE;
-
-import android.app.Notification;
-import android.app.NotificationManager;
-import android.app.PendingIntent;
-import android.app.admin.DevicePolicyManager;
-import android.content.Context;
-import android.content.Intent;
-import android.content.res.Resources;
-import android.os.UserHandle;
-import android.util.DisplayMetrics;
-import android.view.WindowManager;
-
-import com.android.internal.messages.nano.SystemMessageProto;
-import com.android.systemui.res.R;
-import com.android.systemui.SystemUIApplication;
-import com.android.systemui.util.NotificationChannels;
-
-import javax.inject.Inject;
-
-/**
- * Convenience class to handle showing and hiding notifications while taking a screenshot.
- */
-public class ScreenshotNotificationsController {
- private static final String TAG = "ScreenshotNotificationManager";
-
- private final Context mContext;
- private final Resources mResources;
- private final NotificationManager mNotificationManager;
-
- @Inject
- ScreenshotNotificationsController(Context context, WindowManager windowManager) {
- mContext = context;
- mResources = context.getResources();
- mNotificationManager =
- (NotificationManager) context.getSystemService(NOTIFICATION_SERVICE);
-
- DisplayMetrics displayMetrics = new DisplayMetrics();
- windowManager.getDefaultDisplay().getRealMetrics(displayMetrics);
- }
-
- /**
- * Sends a notification that the screenshot capture has failed.
- */
- public void notifyScreenshotError(int msgResId) {
- Resources res = mContext.getResources();
- String errorMsg = res.getString(msgResId);
-
- // Repurpose the existing notification to notify the user of the error
- Notification.Builder b = new Notification.Builder(mContext, NotificationChannels.ALERTS)
- .setTicker(res.getString(R.string.screenshot_failed_title))
- .setContentTitle(res.getString(R.string.screenshot_failed_title))
- .setContentText(errorMsg)
- .setSmallIcon(R.drawable.stat_notify_image_error)
- .setWhen(System.currentTimeMillis())
- .setVisibility(Notification.VISIBILITY_PUBLIC) // ok to show outside lockscreen
- .setCategory(Notification.CATEGORY_ERROR)
- .setAutoCancel(true)
- .setColor(mContext.getColor(
- com.android.internal.R.color.system_notification_accent_color));
- final DevicePolicyManager dpm =
- (DevicePolicyManager) mContext.getSystemService(Context.DEVICE_POLICY_SERVICE);
- final Intent intent =
- dpm.createAdminSupportIntent(DevicePolicyManager.POLICY_DISABLE_SCREEN_CAPTURE);
- if (intent != null) {
- final PendingIntent pendingIntent = PendingIntent.getActivityAsUser(
- mContext, 0, intent, PendingIntent.FLAG_IMMUTABLE, null, UserHandle.CURRENT);
- b.setContentIntent(pendingIntent);
- }
-
- SystemUIApplication.overrideNotificationAppName(mContext, b, true);
-
- Notification n = new Notification.BigTextStyle(b)
- .bigText(errorMsg)
- .build();
- mNotificationManager.notify(SystemMessageProto.SystemMessage.NOTE_GLOBAL_SCREENSHOT, n);
- }
-}
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationsController.kt b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationsController.kt
new file mode 100644
index 0000000..d874eb6
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotNotificationsController.kt
@@ -0,0 +1,111 @@
+/*
+ * Copyright (C) 2023 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.systemui.screenshot
+
+import android.app.Notification
+import android.app.NotificationManager
+import android.app.PendingIntent
+import android.app.admin.DevicePolicyManager
+import android.content.Context
+import android.os.UserHandle
+import android.view.Display
+import com.android.internal.R
+import com.android.internal.messages.nano.SystemMessageProto
+import com.android.systemui.SystemUIApplication
+import com.android.systemui.util.NotificationChannels
+import dagger.assisted.Assisted
+import dagger.assisted.AssistedFactory
+import dagger.assisted.AssistedInject
+
+/** Convenience class to handle showing and hiding notifications while taking a screenshot. */
+class ScreenshotNotificationsController
+@AssistedInject
+internal constructor(
+ @Assisted private val displayId: Int,
+ private val context: Context,
+ private val notificationManager: NotificationManager,
+ private val devicePolicyManager: DevicePolicyManager,
+) {
+ private val res = context.resources
+
+ /**
+ * Sends a notification that the screenshot capture has failed.
+ *
+ * Errors for the non-default display are shown in a unique separate notification.
+ */
+ fun notifyScreenshotError(msgResId: Int) {
+ val displayErrorString =
+ if (displayId != Display.DEFAULT_DISPLAY) {
+ " ($externalDisplayString)"
+ } else {
+ ""
+ }
+ val errorMsg = res.getString(msgResId) + displayErrorString
+
+ // Repurpose the existing notification or create a new one
+ val builder =
+ Notification.Builder(context, NotificationChannels.ALERTS)
+ .setTicker(res.getString(com.android.systemui.res.R.string.screenshot_failed_title))
+ .setContentTitle(
+ res.getString(com.android.systemui.res.R.string.screenshot_failed_title)
+ )
+ .setContentText(errorMsg)
+ .setSmallIcon(com.android.systemui.res.R.drawable.stat_notify_image_error)
+ .setWhen(System.currentTimeMillis())
+ .setVisibility(Notification.VISIBILITY_PUBLIC) // ok to show outside lockscreen
+ .setCategory(Notification.CATEGORY_ERROR)
+ .setAutoCancel(true)
+ .setColor(context.getColor(R.color.system_notification_accent_color))
+ val intent =
+ devicePolicyManager.createAdminSupportIntent(
+ DevicePolicyManager.POLICY_DISABLE_SCREEN_CAPTURE
+ )
+ if (intent != null) {
+ val pendingIntent =
+ PendingIntent.getActivityAsUser(
+ context,
+ 0,
+ intent,
+ PendingIntent.FLAG_IMMUTABLE,
+ null,
+ UserHandle.CURRENT
+ )
+ builder.setContentIntent(pendingIntent)
+ }
+ SystemUIApplication.overrideNotificationAppName(context, builder, true)
+ val notification = Notification.BigTextStyle(builder).bigText(errorMsg).build()
+ // A different id for external displays to keep the 2 error notifications separated.
+ val id =
+ if (displayId == Display.DEFAULT_DISPLAY) {
+ SystemMessageProto.SystemMessage.NOTE_GLOBAL_SCREENSHOT
+ } else {
+ SystemMessageProto.SystemMessage.NOTE_GLOBAL_SCREENSHOT_EXTERNAL_DISPLAY
+ }
+ notificationManager.notify(id, notification)
+ }
+
+ private val externalDisplayString: String
+ get() =
+ res.getString(
+ com.android.systemui.res.R.string.screenshot_failed_external_display_indication
+ )
+
+ /** Factory for [ScreenshotNotificationsController]. */
+ @AssistedFactory
+ interface Factory {
+ fun create(displayId: Int = Display.DEFAULT_DISPLAY): ScreenshotNotificationsController
+ }
+}
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotServiceErrorReceiver.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotServiceErrorReceiver.java
index 070fb1e..049799e 100644
--- a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotServiceErrorReceiver.java
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotServiceErrorReceiver.java
@@ -16,25 +16,29 @@
package com.android.systemui.screenshot;
+import android.app.NotificationManager;
+import android.app.admin.DevicePolicyManager;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
-import android.view.WindowManager;
+import android.view.Display;
import com.android.systemui.res.R;
/**
- * Performs a number of miscellaneous, non-system-critical actions
- * after the system has finished booting.
+ * Receives errors related to screenshot.
*/
public class ScreenshotServiceErrorReceiver extends BroadcastReceiver {
@Override
public void onReceive(final Context context, Intent intent) {
// Show a message that we've failed to save the image to disk
- WindowManager wm = (WindowManager) context.getSystemService(Context.WINDOW_SERVICE);
- ScreenshotNotificationsController controller =
- new ScreenshotNotificationsController(context, wm);
+ NotificationManager notificationManager = context.getSystemService(
+ NotificationManager.class);
+ DevicePolicyManager devicePolicyManager = context.getSystemService(
+ DevicePolicyManager.class);
+ ScreenshotNotificationsController controller = new ScreenshotNotificationsController(
+ Display.DEFAULT_DISPLAY, context, notificationManager, devicePolicyManager);
controller.notifyScreenshotError(R.string.screenshot_failed_to_save_unknown_text);
}
}
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt
index abe40ff..0158284 100644
--- a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt
@@ -10,6 +10,8 @@
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Application
import com.android.systemui.display.data.repository.DisplayRepository
+import com.android.systemui.res.R
+import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_CAPTURE_FAILED
import com.android.systemui.screenshot.TakeScreenshotService.RequestCallback
import java.util.function.Consumer
import javax.inject.Inject
@@ -34,7 +36,8 @@
displayRepository: DisplayRepository,
@Application private val mainScope: CoroutineScope,
private val screenshotRequestProcessor: ScreenshotRequestProcessor,
- private val uiEventLogger: UiEventLogger
+ private val uiEventLogger: UiEventLogger,
+ private val screenshotNotificationControllerFactory: ScreenshotNotificationsController.Factory,
) {
private lateinit var displays: StateFlow<Set<Display>>
@@ -44,6 +47,7 @@
}
private val screenshotControllers = mutableMapOf<Int, ScreenshotController>()
+ private val notificationControllers = mutableMapOf<Int, ScreenshotNotificationsController>()
/**
* Executes the [ScreenshotRequest].
@@ -58,40 +62,68 @@
) {
val displayIds = getDisplaysToScreenshot(screenshotRequest.type)
val resultCallbackWrapper = MultiResultCallbackWrapper(requestCallback)
- screenshotRequest.oneForEachDisplay(displayIds).forEach { screenshotData: ScreenshotData ->
+ displayIds.forEach { displayId: Int ->
dispatchToController(
- screenshotData = screenshotData,
+ rawScreenshotData = ScreenshotData.fromRequest(screenshotRequest, displayId),
onSaved =
- if (screenshotData.displayId == Display.DEFAULT_DISPLAY) onSaved else { _ -> },
- callback = resultCallbackWrapper.createCallbackForId(screenshotData.displayId)
+ if (displayId == Display.DEFAULT_DISPLAY) {
+ onSaved
+ } else { _ -> },
+ callback = resultCallbackWrapper.createCallbackForId(displayId)
)
}
}
- /** Creates a [ScreenshotData] for each display. */
- private suspend fun ScreenshotRequest.oneForEachDisplay(
- displayIds: List<Int>
- ): List<ScreenshotData> {
- return displayIds
- .map { displayId -> ScreenshotData.fromRequest(this, displayId) }
- .map { screenshotData: ScreenshotData ->
- screenshotRequestProcessor.process(screenshotData)
- }
- }
-
- private fun dispatchToController(
- screenshotData: ScreenshotData,
+ /** All logging should be triggered only by this method. */
+ private suspend fun dispatchToController(
+ rawScreenshotData: ScreenshotData,
onSaved: (Uri) -> Unit,
callback: RequestCallback
) {
+ // Let's wait before logging "screenshot requested", as we should log the processed
+ // ScreenshotData.
+ val screenshotData =
+ try {
+ screenshotRequestProcessor.process(rawScreenshotData)
+ } catch (e: RequestProcessorException) {
+ Log.e(TAG, "Failed to process screenshot request!", e)
+ logScreenshotRequested(rawScreenshotData)
+ onFailedScreenshotRequest(rawScreenshotData, callback)
+ return
+ }
+
+ logScreenshotRequested(screenshotData)
+ Log.d(TAG, "Screenshot request: $screenshotData")
+ try {
+ getScreenshotController(screenshotData.displayId)
+ .handleScreenshot(screenshotData, onSaved, callback)
+ } catch (e: IllegalStateException) {
+ Log.e(TAG, "Error while ScreenshotController was handling ScreenshotData!", e)
+ onFailedScreenshotRequest(screenshotData, callback)
+ return // After a failure log, nothing else should run.
+ }
+ }
+
+ /**
+ * This should be logged also in case of failed requests, before the [SCREENSHOT_CAPTURE_FAILED]
+ * event.
+ */
+ private fun logScreenshotRequested(screenshotData: ScreenshotData) {
uiEventLogger.log(
ScreenshotEvent.getScreenshotSource(screenshotData.source),
0,
screenshotData.packageNameString
)
- Log.d(TAG, "Screenshot request: $screenshotData")
- getScreenshotController(screenshotData.displayId)
- .handleScreenshot(screenshotData, onSaved, callback)
+ }
+
+ private fun onFailedScreenshotRequest(
+ screenshotData: ScreenshotData,
+ callback: RequestCallback
+ ) {
+ uiEventLogger.log(SCREENSHOT_CAPTURE_FAILED, 0, screenshotData.packageNameString)
+ getNotificationController(screenshotData.displayId)
+ .notifyScreenshotError(R.string.screenshot_failed_to_capture_text)
+ callback.reportError()
}
private fun getDisplaysToScreenshot(requestType: Int): List<Int> {
@@ -140,6 +172,12 @@
}
}
+ private fun getNotificationController(id: Int): ScreenshotNotificationsController {
+ return notificationControllers.computeIfAbsent(id) {
+ screenshotNotificationControllerFactory.create(id)
+ }
+ }
+
/** For java compatibility only. see [executeScreenshots] */
fun executeScreenshotsAsync(
screenshotRequest: ScreenshotRequest,
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java
index 75d52cb..0991c9a 100644
--- a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java
@@ -113,7 +113,8 @@
@Inject
public TakeScreenshotService(ScreenshotController.Factory screenshotControllerFactory,
UserManager userManager, DevicePolicyManager devicePolicyManager,
- UiEventLogger uiEventLogger, ScreenshotNotificationsController notificationsController,
+ UiEventLogger uiEventLogger,
+ ScreenshotNotificationsController.Factory notificationsControllerFactory,
Context context, @Background Executor bgExecutor, FeatureFlags featureFlags,
RequestProcessor processor, Provider<TakeScreenshotExecutor> takeScreenshotExecutor) {
if (DEBUG_SERVICE) {
@@ -123,7 +124,7 @@
mUserManager = userManager;
mDevicePolicyManager = devicePolicyManager;
mUiEventLogger = uiEventLogger;
- mNotificationsController = notificationsController;
+ mNotificationsController = notificationsControllerFactory.create(Display.DEFAULT_DISPLAY);
mContext = context;
mBgExecutor = bgExecutor;
mFeatureFlags = featureFlags;
@@ -246,15 +247,17 @@
Log.d(TAG, "Processing screenshot data");
- ScreenshotData screenshotData = ScreenshotData.fromRequest(
- request, Display.DEFAULT_DISPLAY);
+ if (mFeatureFlags.isEnabled(MULTI_DISPLAY_SCREENSHOT)) {
+ mTakeScreenshotExecutor.get().executeScreenshotsAsync(request, onSaved, callback);
+ return;
+ }
+ // TODO(b/295143676): Delete the following after the flag is released.
try {
- if (mFeatureFlags.isEnabled(MULTI_DISPLAY_SCREENSHOT)) {
- mTakeScreenshotExecutor.get().executeScreenshotsAsync(request, onSaved, callback);
- } else {
- mProcessor.processAsync(screenshotData, (data) ->
- dispatchToController(data, onSaved, callback));
- }
+ ScreenshotData screenshotData = ScreenshotData.fromRequest(
+ request, Display.DEFAULT_DISPLAY);
+ mProcessor.processAsync(screenshotData, (data) ->
+ dispatchToController(data, onSaved, callback));
+
} catch (IllegalStateException e) {
Log.e(TAG, "Failed to process screenshot request!", e);
logFailedRequest(request);
@@ -264,6 +267,7 @@
}
}
+ // TODO(b/295143676): Delete this.
private void dispatchToController(ScreenshotData screenshot,
Consumer<Uri> uriConsumer, RequestCallback callback) {
mUiEventLogger.log(ScreenshotEvent.getScreenshotSource(screenshot.getSource()), 0,
diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/RequestProcessorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/screenshot/RequestProcessorTest.kt
index 0d694ee..7e41745 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/RequestProcessorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/RequestProcessorTest.kt
@@ -28,7 +28,6 @@
import android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN
import android.view.WindowManager.TAKE_SCREENSHOT_PROVIDED_IMAGE
import com.android.internal.util.ScreenshotRequest
-import com.android.systemui.flags.FakeFeatureFlags
import com.android.systemui.screenshot.ScreenshotPolicy.DisplayContentInfo
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.CoroutineScope
@@ -47,7 +46,6 @@
private val scope = CoroutineScope(Dispatchers.Unconfined)
private val policy = FakeScreenshotPolicy()
- private val flags = FakeFeatureFlags()
/** Tests the Java-compatible function wrapper, ensures callback is invoked. */
@Test
@@ -58,7 +56,7 @@
.setBitmap(Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888))
.build()
)
- val processor = RequestProcessor(imageCapture, policy, flags, scope)
+ val processor = RequestProcessor(imageCapture, policy, scope)
var result: ScreenshotData? = null
var callbackCount = 0
@@ -86,7 +84,7 @@
val request =
ScreenshotRequest.Builder(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_OTHER).build()
- val processor = RequestProcessor(imageCapture, policy, flags, scope)
+ val processor = RequestProcessor(imageCapture, policy, scope)
val processedData = processor.process(ScreenshotData.fromRequest(request))
@@ -111,7 +109,7 @@
val request =
ScreenshotRequest.Builder(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_KEY_OTHER).build()
- val processor = RequestProcessor(imageCapture, policy, flags, scope)
+ val processor = RequestProcessor(imageCapture, policy, scope)
val processedData = processor.process(ScreenshotData.fromRequest(request))
@@ -138,7 +136,7 @@
val request =
ScreenshotRequest.Builder(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_KEY_OTHER).build()
- val processor = RequestProcessor(imageCapture, policy, flags, scope)
+ val processor = RequestProcessor(imageCapture, policy, scope)
Assert.assertThrows(IllegalStateException::class.java) {
runBlocking { processor.process(ScreenshotData.fromRequest(request)) }
@@ -148,7 +146,7 @@
@Test
fun testProvidedImageScreenshot() = runBlocking {
val bounds = Rect(50, 50, 150, 150)
- val processor = RequestProcessor(imageCapture, policy, flags, scope)
+ val processor = RequestProcessor(imageCapture, policy, scope)
policy.setManagedProfile(USER_ID, false)
@@ -173,7 +171,7 @@
@Test
fun testProvidedImageScreenshot_managedProfile() = runBlocking {
val bounds = Rect(50, 50, 150, 150)
- val processor = RequestProcessor(imageCapture, policy, flags, scope)
+ val processor = RequestProcessor(imageCapture, policy, scope)
// Indicate that the screenshot belongs to a manged profile
policy.setManagedProfile(USER_ID, true)
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 d8821aa..3dc9037 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt
@@ -25,6 +25,7 @@
import com.android.systemui.util.mockito.nullable
import com.android.systemui.util.mockito.whenever
import com.google.common.truth.Truth.assertThat
+import java.lang.IllegalStateException
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runCurrent
@@ -43,8 +44,11 @@
private val controller0 = mock<ScreenshotController>()
private val controller1 = mock<ScreenshotController>()
+ private val notificationsController0 = mock<ScreenshotNotificationsController>()
+ private val notificationsController1 = mock<ScreenshotNotificationsController>()
private val controllerFactory = mock<ScreenshotController.Factory>()
private val callback = mock<TakeScreenshotService.RequestCallback>()
+ private val notificationControllerFactory = mock<ScreenshotNotificationsController.Factory>()
private val fakeDisplayRepository = FakeDisplayRepository()
private val requestProcessor = FakeRequestProcessor()
@@ -59,12 +63,15 @@
testScope,
requestProcessor,
eventLogger,
+ notificationControllerFactory
)
@Before
fun setUp() {
whenever(controllerFactory.create(eq(0), any())).thenReturn(controller0)
whenever(controllerFactory.create(eq(1), any())).thenReturn(controller1)
+ whenever(notificationControllerFactory.create(eq(0))).thenReturn(notificationsController0)
+ whenever(notificationControllerFactory.create(eq(1))).thenReturn(notificationsController1)
}
@Test
@@ -310,6 +317,123 @@
screenshotExecutor.onDestroy()
}
+ @Test
+ fun executeScreenshots_errorFromProcessor_logsScreenshotRequested() =
+ testScope.runTest {
+ setDisplays(display(TYPE_INTERNAL, id = 0), display(TYPE_EXTERNAL, id = 1))
+ val onSaved = { _: Uri -> }
+ requestProcessor.shouldThrowException = true
+
+ screenshotExecutor.executeScreenshots(createScreenshotRequest(), onSaved, callback)
+
+ val screenshotRequested =
+ eventLogger.logs.filter {
+ it.eventId == ScreenshotEvent.SCREENSHOT_REQUESTED_KEY_OTHER.id
+ }
+ assertThat(screenshotRequested).hasSize(2)
+ screenshotExecutor.onDestroy()
+ }
+
+ @Test
+ fun executeScreenshots_errorFromProcessor_logsUiError() =
+ testScope.runTest {
+ setDisplays(display(TYPE_INTERNAL, id = 0), display(TYPE_EXTERNAL, id = 1))
+ val onSaved = { _: Uri -> }
+ requestProcessor.shouldThrowException = true
+
+ screenshotExecutor.executeScreenshots(createScreenshotRequest(), onSaved, callback)
+
+ val screenshotRequested =
+ eventLogger.logs.filter {
+ it.eventId == ScreenshotEvent.SCREENSHOT_CAPTURE_FAILED.id
+ }
+ assertThat(screenshotRequested).hasSize(2)
+ screenshotExecutor.onDestroy()
+ }
+
+ @Test
+ fun executeScreenshots_errorFromProcessorOnDefaultDisplay_showsErrorNotification() =
+ testScope.runTest {
+ setDisplays(display(TYPE_INTERNAL, id = 0), display(TYPE_EXTERNAL, id = 1))
+ val onSaved = { _: Uri -> }
+ requestProcessor.shouldThrowException = true
+
+ screenshotExecutor.executeScreenshots(createScreenshotRequest(), onSaved, callback)
+
+ verify(notificationsController0).notifyScreenshotError(any())
+ screenshotExecutor.onDestroy()
+ }
+
+ @Test
+ fun executeScreenshots_errorFromProcessorOnSecondaryDisplay_showsErrorNotification() =
+ testScope.runTest {
+ setDisplays(display(TYPE_INTERNAL, id = 0))
+ val onSaved = { _: Uri -> }
+ requestProcessor.shouldThrowException = true
+
+ screenshotExecutor.executeScreenshots(createScreenshotRequest(), onSaved, callback)
+
+ verify(notificationsController0).notifyScreenshotError(any())
+ screenshotExecutor.onDestroy()
+ }
+
+ @Test
+ fun executeScreenshots_errorFromScreenshotController_reportsRequested() =
+ testScope.runTest {
+ setDisplays(display(TYPE_INTERNAL, id = 0), display(TYPE_EXTERNAL, id = 1))
+ val onSaved = { _: Uri -> }
+ whenever(controller0.handleScreenshot(any(), any(), any()))
+ .thenThrow(IllegalStateException::class.java)
+ whenever(controller1.handleScreenshot(any(), any(), any()))
+ .thenThrow(IllegalStateException::class.java)
+
+ screenshotExecutor.executeScreenshots(createScreenshotRequest(), onSaved, callback)
+
+ val screenshotRequested =
+ eventLogger.logs.filter {
+ it.eventId == ScreenshotEvent.SCREENSHOT_REQUESTED_KEY_OTHER.id
+ }
+ assertThat(screenshotRequested).hasSize(2)
+ screenshotExecutor.onDestroy()
+ }
+
+ @Test
+ fun executeScreenshots_errorFromScreenshotController_reportsError() =
+ testScope.runTest {
+ setDisplays(display(TYPE_INTERNAL, id = 0), display(TYPE_EXTERNAL, id = 1))
+ val onSaved = { _: Uri -> }
+ whenever(controller0.handleScreenshot(any(), any(), any()))
+ .thenThrow(IllegalStateException::class.java)
+ whenever(controller1.handleScreenshot(any(), any(), any()))
+ .thenThrow(IllegalStateException::class.java)
+
+ screenshotExecutor.executeScreenshots(createScreenshotRequest(), onSaved, callback)
+
+ val screenshotRequested =
+ eventLogger.logs.filter {
+ it.eventId == ScreenshotEvent.SCREENSHOT_CAPTURE_FAILED.id
+ }
+ assertThat(screenshotRequested).hasSize(2)
+ screenshotExecutor.onDestroy()
+ }
+
+ @Test
+ fun executeScreenshots_errorFromScreenshotController_showsErrorNotification() =
+ testScope.runTest {
+ setDisplays(display(TYPE_INTERNAL, id = 0), display(TYPE_EXTERNAL, id = 1))
+ val onSaved = { _: Uri -> }
+ whenever(controller0.handleScreenshot(any(), any(), any()))
+ .thenThrow(IllegalStateException::class.java)
+ whenever(controller1.handleScreenshot(any(), any(), any()))
+ .thenThrow(IllegalStateException::class.java)
+
+ screenshotExecutor.executeScreenshots(createScreenshotRequest(), onSaved, callback)
+
+ verify(notificationsController0).notifyScreenshotError(any())
+ verify(notificationsController1).notifyScreenshotError(any())
+ screenshotExecutor.onDestroy()
+ }
+
private suspend fun TestScope.setDisplays(vararg displays: Display) {
fakeDisplayRepository.emit(displays.toSet())
runCurrent()
@@ -328,8 +452,9 @@
private class FakeRequestProcessor : ScreenshotRequestProcessor {
var processed: ScreenshotData? = null
var toReturn: ScreenshotData? = null
-
+ var shouldThrowException = false
override suspend fun process(screenshot: ScreenshotData): ScreenshotData {
+ if (shouldThrowException) throw RequestProcessorException("")
processed = screenshot
return toReturn ?: screenshot
}
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 5091a70..f3809aa 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt
@@ -65,6 +65,7 @@
private val requestProcessor = mock<RequestProcessor>()
private val devicePolicyManager = mock<DevicePolicyManager>()
private val devicePolicyResourcesManager = mock<DevicePolicyResourcesManager>()
+ private val notificationsControllerFactory = mock<ScreenshotNotificationsController.Factory>()
private val notificationsController = mock<ScreenshotNotificationsController>()
private val callback = mock<RequestCallback>()
@@ -87,6 +88,7 @@
.thenReturn(false)
whenever(userManager.isUserUnlocked).thenReturn(true)
whenever(controllerFactory.create(any(), any())).thenReturn(controller)
+ whenever(notificationsControllerFactory.create(any())).thenReturn(notificationsController)
// Stub request processor as a synchronous no-op for tests with the flag enabled
doAnswer {
@@ -323,7 +325,7 @@
userManager,
devicePolicyManager,
eventLogger,
- notificationsController,
+ notificationsControllerFactory,
mContext,
Runnable::run,
flags,
diff --git a/proto/src/system_messages.proto b/proto/src/system_messages.proto
index 21d0979..b403a7f 100644
--- a/proto/src/system_messages.proto
+++ b/proto/src/system_messages.proto
@@ -404,5 +404,9 @@
// Notify the user that audio was lowered based on Calculated Sound Dose (CSD)
NOTE_CSD_LOWER_AUDIO = 1007;
+
+ // Notify the user about external display events related to screenshot.
+ // Package: com.android.systemui
+ NOTE_GLOBAL_SCREENSHOT_EXTERNAL_DISPLAY = 1008;
}
}