Merge "Code cleanup of ScreenshotRequest and usages" into tm-qpr-dev am: bbbfa95637

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/19377517

Change-Id: I48b3e476f874bd81b5f1af53ed5f1757f4640bd4
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

core/java/com/android/internal/util/ScreenshotHelper.java

@@ -1,7 +1,6 @@
 package com.android.internal.util;
 
 import static android.content.Intent.ACTION_USER_SWITCHED;
-import static android.view.WindowManager.ScreenshotSource.SCREENSHOT_OTHER;
 
 import android.annotation.NonNull;
 import android.annotation.Nullable;
@@ -29,6 +28,10 @@ import android.os.RemoteException;
 import android.os.UserHandle;
 import android.util.Log;
 import android.view.WindowManager;
+import android.view.WindowManager.ScreenshotSource;
+import android.view.WindowManager.ScreenshotType;
+
+import com.android.internal.annotations.VisibleForTesting;
 
 import java.util.Objects;
 import java.util.function.Consumer;
@@ -42,24 +45,28 @@ public class ScreenshotHelper {
      * Describes a screenshot request (to make it easier to pass data through to the handler).
      */
     public static class ScreenshotRequest implements Parcelable {
-        private int mSource;
+        private final int mSource;
-        private boolean mHasStatusBar;
+        private final Bundle mBitmapBundle;
-        private boolean mHasNavBar;
+        private final Rect mBoundsInScreen;
-        private Bundle mBitmapBundle;
+        private final Insets mInsets;
-        private Rect mBoundsInScreen;
+        private final int mTaskId;
-        private Insets mInsets;
+        private final int mUserId;
-        private int mTaskId;
+        private final ComponentName mTopComponent;
-        private int mUserId;
-        private ComponentName mTopComponent;
 
-        ScreenshotRequest(int source, boolean hasStatus, boolean hasNav) {
+        @VisibleForTesting
+        public ScreenshotRequest(int source) {
             mSource = source;
-            mHasStatusBar = hasStatus;
+            mBitmapBundle = null;
-            mHasNavBar = hasNav;
+            mBoundsInScreen = null;
+            mInsets = null;
+            mTaskId = -1;
+            mUserId = -1;
+            mTopComponent = null;
         }
 
-        ScreenshotRequest(int source, Bundle bitmapBundle, Rect boundsInScreen, Insets insets,
+        @VisibleForTesting
-                int taskId, int userId, ComponentName topComponent) {
+        public ScreenshotRequest(int source, Bundle bitmapBundle, Rect boundsInScreen,
+                Insets insets, int taskId, int userId, ComponentName topComponent) {
             mSource = source;
             mBitmapBundle = bitmapBundle;
             mBoundsInScreen = boundsInScreen;
@@ -71,16 +78,21 @@ public class ScreenshotHelper {
 
         ScreenshotRequest(Parcel in) {
             mSource = in.readInt();
-            mHasStatusBar = in.readBoolean();
-            mHasNavBar = in.readBoolean();
-
             if (in.readInt() == 1) {
                 mBitmapBundle = in.readBundle(getClass().getClassLoader());
-                mBoundsInScreen = in.readParcelable(Rect.class.getClassLoader(), android.graphics.Rect.class);
+                mBoundsInScreen = in.readParcelable(Rect.class.getClassLoader(), Rect.class);
-                mInsets = in.readParcelable(Insets.class.getClassLoader(), android.graphics.Insets.class);
+                mInsets = in.readParcelable(Insets.class.getClassLoader(), Insets.class);
                 mTaskId = in.readInt();
                 mUserId = in.readInt();
-                mTopComponent = in.readParcelable(ComponentName.class.getClassLoader(), android.content.ComponentName.class);
+                mTopComponent = in.readParcelable(ComponentName.class.getClassLoader(),
+                        ComponentName.class);
+            } else {
+                mBitmapBundle = null;
+                mBoundsInScreen = null;
+                mInsets = null;
+                mTaskId = -1;
+                mUserId = -1;
+                mTopComponent = null;
             }
         }
 
@@ -88,14 +100,6 @@ public class ScreenshotHelper {
             return mSource;
         }
 
-        public boolean getHasStatusBar() {
-            return mHasStatusBar;
-        }
-
-        public boolean getHasNavBar() {
-            return mHasNavBar;
-        }
-
         public Bundle getBitmapBundle() {
             return mBitmapBundle;
         }
@@ -112,7 +116,6 @@ public class ScreenshotHelper {
             return mTaskId;
         }
 
-
         public int getUserId() {
             return mUserId;
         }
@@ -129,8 +132,6 @@ public class ScreenshotHelper {
         @Override
         public void writeToParcel(Parcel dest, int flags) {
             dest.writeInt(mSource);
-            dest.writeBoolean(mHasStatusBar);
-            dest.writeBoolean(mHasNavBar);
             if (mBitmapBundle == null) {
                 dest.writeInt(0);
             } else {
@@ -144,7 +145,8 @@ public class ScreenshotHelper {
             }
         }
 
-        public static final @NonNull Parcelable.Creator<ScreenshotRequest> CREATOR =
+        @NonNull
+        public static final Parcelable.Creator<ScreenshotRequest> CREATOR =
                 new Parcelable.Creator<ScreenshotRequest>() {
 
                     @Override
@@ -254,113 +256,71 @@ public class ScreenshotHelper {
 
     /**
      * Request a screenshot be taken.
-     *
+     * <p>
      * Added to support reducing unit test duration; the method variant without a timeout argument
      * is recommended for general use.
      *
-     * @param screenshotType     The type of screenshot, for example either
+     * @param screenshotType The type of screenshot, defined by {@link ScreenshotType}
-     *                           {@link android.view.WindowManager#TAKE_SCREENSHOT_FULLSCREEN}
+     * @param source The source of the screenshot request, defined by {@link ScreenshotSource}
-     *                           or
+     * @param handler used to process messages received from the screenshot service
-     *                           {@link android.view.WindowManager#TAKE_SCREENSHOT_SELECTED_REGION}
+     * @param completionConsumer receives the URI of the captured screenshot, once saved or
-     * @param hasStatus          {@code true} if the status bar is currently showing. {@code false}
+     *         null if no screenshot was saved
-     *                           if not.
-     * @param hasNav             {@code true} if the navigation bar is currently showing. {@code
-     *                           false} if not.
-     * @param source             The source of the screenshot request. One of
-     *                           {SCREENSHOT_GLOBAL_ACTIONS, SCREENSHOT_KEY_CHORD,
-     *                           SCREENSHOT_OVERVIEW, SCREENSHOT_OTHER}
-     * @param handler            A handler used in case the screenshot times out
-     * @param completionConsumer Consumes `false` if a screenshot was not taken, and `true` if the
-     *                           screenshot was taken.
      */
-    public void takeScreenshot(final int screenshotType, final boolean hasStatus,
+    public void takeScreenshot(@ScreenshotType int screenshotType, @ScreenshotSource int source,
-            final boolean hasNav, int source, @NonNull Handler handler,
+            @NonNull Handler handler, @Nullable Consumer<Uri> completionConsumer) {
-            @Nullable Consumer<Uri> completionConsumer) {
+        ScreenshotRequest screenshotRequest = new ScreenshotRequest(source);
-        ScreenshotRequest screenshotRequest = new ScreenshotRequest(source, hasStatus, hasNav);
+        takeScreenshot(screenshotType, handler, screenshotRequest, SCREENSHOT_TIMEOUT_MS,
-        takeScreenshot(screenshotType, SCREENSHOT_TIMEOUT_MS, handler, screenshotRequest,
                 completionConsumer);
     }
 
     /**
-     * Request a screenshot be taken, with provided reason.
+     * Request a screenshot be taken.
-     *
+     * <p>
-     * @param screenshotType     The type of screenshot, for example either
-     *                           {@link android.view.WindowManager#TAKE_SCREENSHOT_FULLSCREEN}
-     *                           or
-     *                           {@link android.view.WindowManager#TAKE_SCREENSHOT_SELECTED_REGION}
-     * @param hasStatus          {@code true} if the status bar is currently showing. {@code false}
-     *                           if
-     *                           not.
-     * @param hasNav             {@code true} if the navigation bar is currently showing. {@code
-     *                           false}
-     *                           if not.
-     * @param handler            A handler used in case the screenshot times out
-     * @param completionConsumer Consumes `false` if a screenshot was not taken, and `true` if the
-     *                           screenshot was taken.
-     */
-    public void takeScreenshot(final int screenshotType, final boolean hasStatus,
-            final boolean hasNav, @NonNull Handler handler,
-            @Nullable Consumer<Uri> completionConsumer) {
-        takeScreenshot(screenshotType, hasStatus, hasNav, SCREENSHOT_TIMEOUT_MS, handler,
-                completionConsumer);
-    }
-
-    /**
-     * Request a screenshot be taken with a specific timeout.
-     *
      * Added to support reducing unit test duration; the method variant without a timeout argument
      * is recommended for general use.
      *
-     * @param screenshotType     The type of screenshot, for example either
+     * @param screenshotType The type of screenshot, defined by {@link ScreenshotType}
-     *                           {@link android.view.WindowManager#TAKE_SCREENSHOT_FULLSCREEN}
+     * @param source The source of the screenshot request, defined by {@link ScreenshotSource}
-     *                           or
+     * @param handler used to process messages received from the screenshot service
-     *                           {@link android.view.WindowManager#TAKE_SCREENSHOT_SELECTED_REGION}
+     * @param timeoutMs time limit for processing, intended only for testing
-     * @param hasStatus          {@code true} if the status bar is currently showing. {@code false}
+     * @param completionConsumer receives the URI of the captured screenshot, once saved or
-     *                           if
+     *         null if no screenshot was saved
-     *                           not.
-     * @param hasNav             {@code true} if the navigation bar is currently showing. {@code
-     *                           false}
-     *                           if not.
-     * @param timeoutMs          If the screenshot hasn't been completed within this time period,
-     *                           the screenshot attempt will be cancelled and `completionConsumer`
-     *                           will be run.
-     * @param handler            A handler used in case the screenshot times out
-     * @param completionConsumer Consumes `false` if a screenshot was not taken, and `true` if the
-     *                           screenshot was taken.
      */
-    public void takeScreenshot(final int screenshotType, final boolean hasStatus,
+    @VisibleForTesting
-            final boolean hasNav, long timeoutMs, @NonNull Handler handler,
+    public void takeScreenshot(@ScreenshotType int screenshotType, @ScreenshotSource int source,
-            @Nullable Consumer<Uri> completionConsumer) {
+            @NonNull Handler handler, long timeoutMs, @Nullable Consumer<Uri> completionConsumer) {
-        ScreenshotRequest screenshotRequest = new ScreenshotRequest(SCREENSHOT_OTHER, hasStatus,
+        ScreenshotRequest screenshotRequest = new ScreenshotRequest(source);
-                hasNav);
+        takeScreenshot(screenshotType, handler, screenshotRequest, timeoutMs, completionConsumer);
-        takeScreenshot(screenshotType, timeoutMs, handler, screenshotRequest, completionConsumer);
     }
 
     /**
      * Request that provided image be handled as if it was a screenshot.
      *
-     * @param screenshotBundle   Bundle containing the buffer and color space of the screenshot.
+     * @param screenshotBundle Bundle containing the buffer and color space of the screenshot.
-     * @param boundsInScreen     The bounds in screen coordinates that the bitmap orginated from.
+     * @param boundsInScreen The bounds in screen coordinates that the bitmap originated from.
-     * @param insets             The insets that the image was shown with, inside the screenbounds.
+     * @param insets The insets that the image was shown with, inside the screen bounds.
-     * @param taskId             The taskId of the task that the screen shot was taken of.
+     * @param taskId The taskId of the task that the screen shot was taken of.
-     * @param userId             The userId of user running the task provided in taskId.
+     * @param userId The userId of user running the task provided in taskId.
-     * @param topComponent       The component name of the top component running in the task.
+     * @param topComponent The component name of the top component running in the task.
-     * @param handler            A handler used in case the screenshot times out
+     * @param source The source of the screenshot request, defined by {@link ScreenshotSource}
-     * @param completionConsumer Consumes `false` if a screenshot was not taken, and `true` if the
+     * @param handler A handler used in case the screenshot times out
-     *                           screenshot was taken.
+     * @param completionConsumer receives the URI of the captured screenshot, once saved or
+     *         null if no screenshot was saved
      */
     public void provideScreenshot(@NonNull Bundle screenshotBundle, @NonNull Rect boundsInScreen,
-            @NonNull Insets insets, int taskId, int userId, ComponentName topComponent, int source,
+            @NonNull Insets insets, int taskId, int userId, ComponentName topComponent,
-            @NonNull Handler handler, @Nullable Consumer<Uri> completionConsumer) {
+            @ScreenshotSource int source, @NonNull Handler handler,
-        ScreenshotRequest screenshotRequest =
+            @Nullable Consumer<Uri> completionConsumer) {
-                new ScreenshotRequest(source, screenshotBundle, boundsInScreen, insets, taskId,
+        ScreenshotRequest screenshotRequest = new ScreenshotRequest(source, screenshotBundle,
-                        userId, topComponent);
+                boundsInScreen, insets, taskId, userId, topComponent);
-        takeScreenshot(WindowManager.TAKE_SCREENSHOT_PROVIDED_IMAGE, SCREENSHOT_TIMEOUT_MS,
+        takeScreenshot(WindowManager.TAKE_SCREENSHOT_PROVIDED_IMAGE, handler, screenshotRequest,
-                handler, screenshotRequest, completionConsumer);
+                SCREENSHOT_TIMEOUT_MS,
+                completionConsumer);
     }
 
-    private void takeScreenshot(final int screenshotType, long timeoutMs, @NonNull Handler handler,
+    private void takeScreenshot(@ScreenshotType int screenshotType, @NonNull Handler handler,
-            ScreenshotRequest screenshotRequest, @Nullable Consumer<Uri> completionConsumer) {
+            ScreenshotRequest screenshotRequest, long timeoutMs,
+            @Nullable Consumer<Uri> completionConsumer) {
         synchronized (mScreenshotLock) {
 
             final Runnable mScreenshotTimeout = () -> {

core/tests/screenshothelpertests/src/com/android/internal/util/ScreenshotHelperTest.java

@@ -80,13 +80,14 @@ public final class ScreenshotHelperTest {
 
     @Test
     public void testFullscreenScreenshot() {
-        mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_FULLSCREEN, false, false, mHandler, null);
+        mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_FULLSCREEN,
+                WindowManager.ScreenshotSource.SCREENSHOT_OTHER, mHandler, null);
     }
 
     @Test
     public void testSelectedRegionScreenshot() {
-        mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_SELECTED_REGION, false, false, mHandler,
+        mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_SELECTED_REGION,
-                null);
+                WindowManager.ScreenshotSource.SCREENSHOT_OTHER, mHandler, null);
     }
 
     @Test
@@ -101,8 +102,10 @@ public final class ScreenshotHelperTest {
         long timeoutMs = 10;
 
         CountDownLatch lock = new CountDownLatch(1);
-        mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_FULLSCREEN, false, false, timeoutMs,
+        mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_FULLSCREEN,
+                WindowManager.ScreenshotSource.SCREENSHOT_OTHER,
                 mHandler,
+                timeoutMs,
                 uri -> {
                     assertNull(uri);
                     lock.countDown();

packages/SystemUI/src/com/android/systemui/accessibility/SystemActions.java

@@ -500,7 +500,7 @@ public class SystemActions extends CoreStartable {
 
     private void handleTakeScreenshot() {
         ScreenshotHelper screenshotHelper = new ScreenshotHelper(mContext);
-        screenshotHelper.takeScreenshot(WindowManager.TAKE_SCREENSHOT_FULLSCREEN, true, true,
+        screenshotHelper.takeScreenshot(WindowManager.TAKE_SCREENSHOT_FULLSCREEN,
                 SCREENSHOT_ACCESSIBILITY_ACTIONS, new Handler(Looper.getMainLooper()), null);
     }
 

packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialogLite.java

@@ -947,7 +947,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene
             mHandler.postDelayed(new Runnable() {
                 @Override
                 public void run() {
-                    mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_FULLSCREEN, true, true,
+                    mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_FULLSCREEN,
                             SCREENSHOT_GLOBAL_ACTIONS, mHandler, null);
                     mMetricsLogger.action(MetricsEvent.ACTION_SCREENSHOT_POWER_MENU);
                     mUiEventLogger.log(GlobalActionsEvent.GA_SCREENSHOT_PRESS);

services/accessibility/java/com/android/server/accessibility/SystemActionPerformer.java

@@ -16,8 +16,6 @@
 
 package com.android.server.accessibility;
 
-import static android.view.WindowManager.ScreenshotSource.SCREENSHOT_ACCESSIBILITY_ACTIONS;
-
 import android.accessibilityservice.AccessibilityService;
 import android.app.PendingIntent;
 import android.app.RemoteAction;
@@ -34,6 +32,7 @@ import android.util.Slog;
 import android.view.InputDevice;
 import android.view.KeyCharacterMap;
 import android.view.KeyEvent;
+import android.view.WindowManager;
 import android.view.accessibility.AccessibilityNodeInfo.AccessibilityAction;
 
 import com.android.internal.R;
@@ -392,8 +391,8 @@ public class SystemActionPerformer {
     private boolean takeScreenshot() {
         ScreenshotHelper screenshotHelper = (mScreenshotHelperSupplier != null)
                 ? mScreenshotHelperSupplier.get() : new ScreenshotHelper(mContext);
-        screenshotHelper.takeScreenshot(android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN,
+        screenshotHelper.takeScreenshot(WindowManager.TAKE_SCREENSHOT_FULLSCREEN,
-                true, true, SCREENSHOT_ACCESSIBILITY_ACTIONS,
+                WindowManager.ScreenshotSource.SCREENSHOT_ACCESSIBILITY_ACTIONS,
                 new Handler(Looper.getMainLooper()), null);
         return true;
     }

services/core/java/com/android/server/wm/DisplayPolicy.java

@@ -2748,8 +2748,6 @@ public class DisplayPolicy {
     public void takeScreenshot(int screenshotType, int source) {
         if (mScreenshotHelper != null) {
             mScreenshotHelper.takeScreenshot(screenshotType,
-                    getStatusBar() != null && getStatusBar().isVisible(),
-                    getNavigationBar() != null && getNavigationBar().isVisible(),
                     source, mHandler, null /* completionConsumer */);
         }
     }

services/tests/servicestests/src/com/android/server/accessibility/SystemActionPerformerTest.java

@@ -17,6 +17,7 @@
 package com.android.server.accessibility;
 
 import static android.view.WindowManager.ScreenshotSource.SCREENSHOT_ACCESSIBILITY_ACTIONS;
+import static android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN;
 
 import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.is;
@@ -27,7 +28,6 @@ import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.never;
@@ -301,8 +301,9 @@ public class SystemActionPerformerTest {
         mSystemActionPerformer.performSystemAction(
                 AccessibilityService.GLOBAL_ACTION_TAKE_SCREENSHOT);
         verify(mMockScreenshotHelper).takeScreenshot(
-                eq(android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN), anyBoolean(),
+                eq(TAKE_SCREENSHOT_FULLSCREEN),
-                anyBoolean(), eq(SCREENSHOT_ACCESSIBILITY_ACTIONS), any(Handler.class), any());
+                eq(SCREENSHOT_ACCESSIBILITY_ACTIONS),
+                any(Handler.class), any());
     }
 
     // PendingIntent is a final class and cannot be mocked. So we are using this