From 4a0fedc2e3f8eaa5ae0e5830c3b0f2136eceed5a Mon Sep 17 00:00:00 2001
From: Miranda Kephart <mkephart@google.com>
Date: Mon, 12 Oct 2020 12:08:02 -0400
Subject: [PATCH] Add logs for screenshot errors

Currently, we post a notification but do not log anything for some
classes of screenshot error (specifically, if the bitmap is null
or if the service connection is disrupted). This means that even
if a user reports that an error ocurred, we have no way to tell
which error it was or what the cause might have been.

Bug: 170310033
Fix: 170310033
Test: manual (deliberately caused the relevant errors to verify
that the logs were observed)

Change-Id: I18a32655e0f353e8befa55e030434dda0c4a2071
---
 .../internal/util/ScreenshotHelper.java       |  2 ++
 .../systemui/screenshot/GlobalScreenshot.java | 31 +++++++++++++------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/core/java/com/android/internal/util/ScreenshotHelper.java b/core/java/com/android/internal/util/ScreenshotHelper.java
index a23fc4b57b457..0bafb2f6ff57b 100644
--- a/core/java/com/android/internal/util/ScreenshotHelper.java
+++ b/core/java/com/android/internal/util/ScreenshotHelper.java
@@ -279,6 +279,7 @@ public class ScreenshotHelper {
             final Runnable mScreenshotTimeout = () -> {
                 synchronized (mScreenshotLock) {
                     if (mScreenshotConnection != null) {
+                        Log.e(TAG, "Timed out before getting screenshot capture response");
                         mContext.unbindService(mScreenshotConnection);
                         mScreenshotConnection = null;
                         mScreenshotService = null;
@@ -353,6 +354,7 @@ public class ScreenshotHelper {
                                 mScreenshotService = null;
                                 // only log an error if we're still within the timeout period
                                 if (handler.hasCallbacks(mScreenshotTimeout)) {
+                                    Log.e(TAG, "Screenshot service disconnected");
                                     handler.removeCallbacks(mScreenshotTimeout);
                                     notifyScreenshotError();
                                 }
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/GlobalScreenshot.java b/packages/SystemUI/src/com/android/systemui/screenshot/GlobalScreenshot.java
index fe322276d220d..aaa335c25d5d6 100644
--- a/packages/SystemUI/src/com/android/systemui/screenshot/GlobalScreenshot.java
+++ b/packages/SystemUI/src/com/android/systemui/screenshot/GlobalScreenshot.java
@@ -321,8 +321,17 @@ public class GlobalScreenshot implements ViewTreeObserver.OnComputeInternalInset
             Insets visibleInsets, int taskId, int userId, ComponentName topComponent,
             Consumer<Uri> finisher, Runnable onComplete) {
         // TODO: use task Id, userId, topComponent for smart handler
-
         mOnCompleteRunnable = onComplete;
+
+        if (screenshot == null) {
+            Log.e(TAG, "Got null bitmap from screenshot message");
+            mNotificationsController.notifyScreenshotError(
+                    R.string.screenshot_failed_to_capture_text);
+            finisher.accept(null);
+            mOnCompleteRunnable.run();
+            return;
+        }
+
         if (aspectRatiosMatch(screenshot, visibleInsets, screenshotScreenBounds)) {
             saveScreenshot(screenshot, finisher, screenshotScreenBounds, visibleInsets, false);
         } else {
@@ -569,7 +578,17 @@ public class GlobalScreenshot implements ViewTreeObserver.OnComputeInternalInset
                         .build();
         final SurfaceControl.ScreenshotHardwareBuffer screenshotBuffer =
                 SurfaceControl.captureDisplay(captureArgs);
-        final Bitmap screenshot = screenshotBuffer == null ? null : screenshotBuffer.asBitmap();
+        Bitmap screenshot = screenshotBuffer == null ? null : screenshotBuffer.asBitmap();
+
+        if (screenshot == null) {
+            Log.e(TAG, "Screenshot bitmap was null");
+            mNotificationsController.notifyScreenshotError(
+                    R.string.screenshot_failed_to_capture_text);
+            finisher.accept(null);
+            mOnCompleteRunnable.run();
+            return;
+        }
+
         saveScreenshot(screenshot, finisher, screenRect, Insets.NONE, true);
     }
 
@@ -593,14 +612,6 @@ public class GlobalScreenshot implements ViewTreeObserver.OnComputeInternalInset
 
         mScreenBitmap = screenshot;
 
-        if (mScreenBitmap == null) {
-            mNotificationsController.notifyScreenshotError(
-                    R.string.screenshot_failed_to_capture_text);
-            finisher.accept(null);
-            mOnCompleteRunnable.run();
-            return;
-        }
-
         if (!isUserSetupComplete()) {
             // User setup isn't complete, so we don't want to show any UI beyond a toast, as editing
             // and sharing shouldn't be exposed to the user.