Merge "Add completion handler for ScreenshotHelper" into qt-qpr1-dev

am: 8a91775d3f

Change-Id: Id8a8c4ed61c269a2731d056deab96b38e3bbd583

8 files changed, 231 insertions, 18 deletions

diff --git a/core/java/com/android/internal/util/ScreenshotHelper.java b/core/java/com/android/internal/util/ScreenshotHelper.java
index 7fd94c6859fb..cac691cf7d45 100644
--- a/core/java/com/android/internal/util/ScreenshotHelper.java
+++ b/core/java/com/android/internal/util/ScreenshotHelper.java
@@ -1,6 +1,7 @@
 package com.android.internal.util;
 
 import android.annotation.NonNull;
+import android.annotation.Nullable;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
@@ -13,6 +14,8 @@ import android.os.RemoteException;
 import android.os.UserHandle;
 import android.util.Log;
 
+import java.util.function.Consumer;
+
 public class ScreenshotHelper {
     private static final String TAG = "ScreenshotHelper";
 
@@ -34,17 +37,58 @@ public class ScreenshotHelper {
     }
 
     /**
+     * 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
+     *                           {@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<Boolean> completionConsumer) {
+        takeScreenshot(screenshotType, hasStatus, hasNav, SCREENSHOT_TIMEOUT_MS, handler,
+                completionConsumer);
+    }
+
+    /**
      * Request a screenshot be taken.
      *
-     * @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
+     * 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
+     *                           {@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 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,
-            final boolean hasNav, @NonNull Handler handler) {
+            final boolean hasNav, long timeoutMs, @NonNull Handler handler,
+            @Nullable Consumer<Boolean> completionConsumer) {
         synchronized (mScreenshotLock) {
             if (mScreenshotConnection != null) {
                 return;
@@ -54,7 +98,8 @@ public class ScreenshotHelper {
             final Intent serviceIntent = new Intent();
 
             final Runnable mScreenshotTimeout = new Runnable() {
-                @Override public void run() {
+                @Override
+                public void run() {
                     synchronized (mScreenshotLock) {
                         if (mScreenshotConnection != null) {
                             mContext.unbindService(mScreenshotConnection);
@@ -62,6 +107,9 @@ public class ScreenshotHelper {
                             notifyScreenshotError();
                         }
                     }
+                    if (completionConsumer != null) {
+                        completionConsumer.accept(false);
+                    }
                 }
             };
 
@@ -86,15 +134,22 @@ public class ScreenshotHelper {
                                         handler.removeCallbacks(mScreenshotTimeout);
                                     }
                                 }
+                                if (completionConsumer != null) {
+                                    completionConsumer.accept(true);
+                                }
                             }
                         };
                         msg.replyTo = new Messenger(h);
-                        msg.arg1 = hasStatus ? 1: 0;
-                        msg.arg2 = hasNav ? 1: 0;
+                        msg.arg1 = hasStatus ? 1 : 0;
+                        msg.arg2 = hasNav ? 1 : 0;
+
                         try {
                             messenger.send(msg);
                         } catch (RemoteException e) {
                             Log.e(TAG, "Couldn't take screenshot: " + e);
+                            if (completionConsumer != null) {
+                                completionConsumer.accept(false);
+                            }
                         }
                     }
                 }
@@ -115,7 +170,7 @@ public class ScreenshotHelper {
                     Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE_WHILE_AWAKE,
                     UserHandle.CURRENT)) {
                 mScreenshotConnection = conn;
-                handler.postDelayed(mScreenshotTimeout, SCREENSHOT_TIMEOUT_MS);
+                handler.postDelayed(mScreenshotTimeout, timeoutMs);
             }
         }
     }
diff --git a/core/tests/screenshothelpertests/Android.bp b/core/tests/screenshothelpertests/Android.bp
new file mode 100644
index 000000000000..3d54b68b7ddc
--- /dev/null
+++ b/core/tests/screenshothelpertests/Android.bp
@@ -0,0 +1,28 @@
+android_test {
+    name: "ScreenshotHelperTests",
+
+    srcs: [
+        "src/**/*.java",
+    ],
+    
+    static_libs: [
+        "frameworks-base-testutils",
+        "androidx.test.runner",
+        "androidx.test.rules",
+        "androidx.test.ext.junit",
+        "mockito-target-minus-junit4",
+        "platform-test-annotations",
+    ],
+
+    libs: [
+        "android.test.runner",
+        "android.test.base",
+        "android.test.mock",
+    ],
+
+    platform_apis: true,
+    test_suites: ["device-tests"],
+
+    certificate: "platform",
+}
+
diff --git a/core/tests/screenshothelpertests/AndroidManifest.xml b/core/tests/screenshothelpertests/AndroidManifest.xml
new file mode 100644
index 000000000000..2e12ef4db358
--- /dev/null
+++ b/core/tests/screenshothelpertests/AndroidManifest.xml
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- 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.
+-->
+
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+          android:installLocation="internalOnly"
+          package="com.android.internal.util"
+          android:sharedUserId="android.uid.systemui" >
+
+    <application >
+        <uses-library android:name="android.test.runner" />
+    </application>
+
+    <instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
+            android:targetPackage="com.android.internal.util"
+            android:label="Screenshot Helper Tests" />
+
+            <protected-broadcast android:name="android.intent.action.USER_PRESENT" />
+
+</manifest>
diff --git a/core/tests/screenshothelpertests/src/com/android/internal/util/ScreenshotHelperTest.java b/core/tests/screenshothelpertests/src/com/android/internal/util/ScreenshotHelperTest.java
new file mode 100644
index 000000000000..848364584ef3
--- /dev/null
+++ b/core/tests/screenshothelpertests/src/com/android/internal/util/ScreenshotHelperTest.java
@@ -0,0 +1,98 @@
+/*
+ * 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.internal.util;
+
+import static android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN;
+import static android.view.WindowManager.TAKE_SCREENSHOT_SELECTED_REGION;
+
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.fail;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+
+import android.content.Context;
+import android.os.Handler;
+import android.os.Looper;
+
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+@RunWith(AndroidJUnit4.class)
+public final class ScreenshotHelperTest {
+    private Context mContext;
+    private ScreenshotHelper mScreenshotHelper;
+    private Handler mHandler;
+
+
+    @Before
+    public void setUp() {
+        // `ScreenshotHelper.notifyScreenshotError()` calls `Context.sendBroadcastAsUser()` and
+        // `Context.bindServiceAsUser`.
+        //
+        // This raises a `SecurityException` if the device is locked. Calling either `Context`
+        // method results in a broadcast of `android.intent.action. USER_PRESENT`. Only the system
+        // process is allowed to broadcast that `Intent`.
+        mContext = Mockito.spy(Context.class);
+        Mockito.doNothing().when(mContext).sendBroadcastAsUser(any(), any());
+        Mockito.doReturn(true).when(mContext).bindServiceAsUser(any(), any(), anyInt(), any());
+
+        mHandler = new Handler(Looper.getMainLooper());
+        mScreenshotHelper = new ScreenshotHelper(mContext);
+    }
+
+    @Test
+    public void testFullscreenScreenshot() {
+        mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_FULLSCREEN, false, false, mHandler, null);
+    }
+
+    @Test
+    public void testSelectedRegionScreenshot() {
+        mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_SELECTED_REGION, false, false, mHandler,
+                null);
+    }
+
+    @Test
+    public void testScreenshotTimesOut() {
+        long timeoutMs = 10;
+
+        CountDownLatch lock = new CountDownLatch(1);
+        mScreenshotHelper.takeScreenshot(TAKE_SCREENSHOT_FULLSCREEN, false, false, timeoutMs,
+                mHandler,
+                worked -> {
+                    assertFalse(worked);
+                    lock.countDown();
+                });
+
+        try {
+            // Add tolerance for delay to prevent flakes.
+            long awaitDurationMs = timeoutMs + 100;
+            if (!lock.await(awaitDurationMs, TimeUnit.MILLISECONDS)) {
+                fail("lock never freed");
+            }
+        } catch (InterruptedException e) {
+            fail("lock interrupted");
+        }
+    }
+}
diff --git a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java
index 0e4c15576957..ff02e71147fd 100644
--- a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java
+++ b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java
@@ -614,7 +614,7 @@ public class GlobalActionsDialog implements DialogInterface.OnDismissListener,
             mHandler.postDelayed(new Runnable() {
                 @Override
                 public void run() {
-                    mScreenshotHelper.takeScreenshot(1, true, true, mHandler);
+                    mScreenshotHelper.takeScreenshot(1, true, true, mHandler, null);
                     MetricsLogger.action(mContext,
                             MetricsEvent.ACTION_SCREENSHOT_POWER_MENU);
                 }
diff --git a/services/accessibility/java/com/android/server/accessibility/GlobalActionPerformer.java b/services/accessibility/java/com/android/server/accessibility/GlobalActionPerformer.java
index 672518cc17ed..b9b2654b93cc 100644
--- a/services/accessibility/java/com/android/server/accessibility/GlobalActionPerformer.java
+++ b/services/accessibility/java/com/android/server/accessibility/GlobalActionPerformer.java
@@ -24,10 +24,7 @@ import android.os.Binder;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.PowerManager;
-import android.os.RemoteException;
-import android.os.ServiceManager;
 import android.os.SystemClock;
-import android.view.IWindowManager;
 import android.view.InputDevice;
 import android.view.KeyCharacterMap;
 import android.view.KeyEvent;
@@ -191,7 +188,7 @@ public class GlobalActionPerformer {
         ScreenshotHelper screenshotHelper = (mScreenshotHelperSupplier != null)
                 ? mScreenshotHelperSupplier.get() : new ScreenshotHelper(mContext);
         screenshotHelper.takeScreenshot(android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN,
-                true, true, new Handler(Looper.getMainLooper()));
+                true, true, new Handler(Looper.getMainLooper()), null);
         return true;
     }
 }
diff --git a/services/core/java/com/android/server/wm/DisplayPolicy.java b/services/core/java/com/android/server/wm/DisplayPolicy.java
index ed7dbd05853a..99a9db316c63 100644
--- a/services/core/java/com/android/server/wm/DisplayPolicy.java
+++ b/services/core/java/com/android/server/wm/DisplayPolicy.java
@@ -3622,7 +3622,8 @@ public class DisplayPolicy {
         if (mScreenshotHelper != null) {
             mScreenshotHelper.takeScreenshot(screenshotType,
                     mStatusBar != null && mStatusBar.isVisibleLw(),
-                    mNavigationBar != null && mNavigationBar.isVisibleLw(), mHandler);
+                    mNavigationBar != null && mNavigationBar.isVisibleLw(),
+                    mHandler, null /* completionConsumer */);
         }
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/accessibility/GlobalActionPerformerTest.java b/services/tests/servicestests/src/com/android/server/accessibility/GlobalActionPerformerTest.java
index e72e4601bbe8..c73be6f100cd 100644
--- a/services/tests/servicestests/src/com/android/server/accessibility/GlobalActionPerformerTest.java
+++ b/services/tests/servicestests/src/com/android/server/accessibility/GlobalActionPerformerTest.java
@@ -35,6 +35,8 @@ import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import java.util.function.Consumer;
+
 /**
  * Tests for GlobalActionPerformer
  */
@@ -84,6 +86,6 @@ public class GlobalActionPerformerTest {
                 AccessibilityService.GLOBAL_ACTION_TAKE_SCREENSHOT);
         verify(mMockScreenshotHelper).takeScreenshot(
                 eq(android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN), anyBoolean(),
-                anyBoolean(), any(Handler.class));
+                anyBoolean(), any(Handler.class), any());
     }
 }