Splits up internal binder api for scroll capture

Splits the connection callback out from the session
callbacks.

Use ListenableFuture at the ScrollCaptureClient layer
to safely chain together futures, provide a better
exception handling/reporting path, and allow for
cancellation of background tasks.

Bug: 180671218
Test: atest ScrollCaptureClientTest ScrollCaptureConnectionTest
Test: atest ScrollCaptureFrameworkSmokeTest
Change-Id: Ibc8536d12a5554f62b3d7e1f4f717d3589ec45e0

9 files changed, 120 insertions, 107 deletions

diff --git a/core/java/android/view/IScrollCaptureCallbacks.aidl b/core/java/android/view/IScrollCaptureCallbacks.aidl
index 26eaac0a2bf5..9b3561400efc 100644
--- a/core/java/android/view/IScrollCaptureCallbacks.aidl
+++ b/core/java/android/view/IScrollCaptureCallbacks.aidl
@@ -27,13 +27,6 @@ import android.view.Surface;
  */
 interface IScrollCaptureCallbacks {
     /**
-     * Provides the result of WindowManagerService#requestScrollCapture
-     *
-     * @param response the response which describes the result
-     */
-    oneway void onScrollCaptureResponse(in ScrollCaptureResponse response);
-
-    /**
      * Called in reply to IScrollCaptureConnection#startCapture, when the remote end has confirmed
      * the request and is ready to begin capturing images.
      */
diff --git a/core/java/android/view/IScrollCaptureConnection.aidl b/core/java/android/view/IScrollCaptureConnection.aidl
index c55e88800393..3a6b69397919 100644
--- a/core/java/android/view/IScrollCaptureConnection.aidl
+++ b/core/java/android/view/IScrollCaptureConnection.aidl
@@ -18,6 +18,7 @@ package android.view;
 
 import android.graphics.Rect;
 import android.os.ICancellationSignal;
+import android.view.IScrollCaptureCallbacks;
 import android.view.Surface;
 
 
@@ -31,11 +32,12 @@ interface IScrollCaptureConnection {
     /**
      * Informs the target that it has been selected for scroll capture.
      *
-     * @param surface a return channel for image buffers
+     * @param surface used to shuttle image buffers between processes
+     * @param callbacks a return channel for requests
      *
-     * @return a cancallation signal which is used cancel the request
+     * @return a cancallation signal which is used cancel the start request
      */
-    ICancellationSignal startCapture(in Surface surface);
+    ICancellationSignal startCapture(in Surface surface, IScrollCaptureCallbacks callbacks);
 
     /**
      * Request the target capture an image within the provided rectangle.
diff --git a/core/java/android/view/IScrollCaptureResponseListener.aidl b/core/java/android/view/IScrollCaptureResponseListener.aidl
new file mode 100644
index 000000000000..7220f6ca5661
--- /dev/null
+++ b/core/java/android/view/IScrollCaptureResponseListener.aidl
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2021 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 android.view;
+
+import android.graphics.Rect;
+import android.view.ScrollCaptureResponse;
+import android.view.Surface;
+
+/**
+ * Asynchronous callback channel for the initial response to a scroll capture request.
+ *
+ * {@hide}
+ */
+interface IScrollCaptureResponseListener {
+    /**
+     * Provides the initial response to a scroll capture request.
+     *
+     * @param response the response which describes the result
+     */
+    oneway void onScrollCaptureResponse(in ScrollCaptureResponse response);
+}
diff --git a/core/java/android/view/IWindow.aidl b/core/java/android/view/IWindow.aidl
index fb012ebbc3db..8d59ba0b1f76 100644
--- a/core/java/android/view/IWindow.aidl
+++ b/core/java/android/view/IWindow.aidl
@@ -26,7 +26,7 @@ import android.view.DisplayCutout;
 import android.view.DragEvent;
 import android.view.InsetsSourceControl;
 import android.view.InsetsState;
-import android.view.IScrollCaptureCallbacks;
+import android.view.IScrollCaptureResponseListener;
 import android.view.KeyEvent;
 import android.view.MotionEvent;
 import android.window.ClientWindowFrames;
@@ -134,5 +134,5 @@ oneway interface IWindow {
      *
      * @param callbacks to receive responses
      */
-    void requestScrollCapture(in IScrollCaptureCallbacks callbacks);
+    void requestScrollCapture(in IScrollCaptureResponseListener callbacks);
 }
diff --git a/core/java/android/view/IWindowManager.aidl b/core/java/android/view/IWindowManager.aidl
index b345b2e58252..a42126f18357 100644
--- a/core/java/android/view/IWindowManager.aidl
+++ b/core/java/android/view/IWindowManager.aidl
@@ -42,7 +42,7 @@ import android.view.IDisplayFoldListener;
 import android.view.IDisplayWindowRotationController;
 import android.view.IOnKeyguardExitResult;
 import android.view.IPinnedTaskListener;
-import android.view.IScrollCaptureCallbacks;
+import android.view.IScrollCaptureResponseListener;
 import android.view.RemoteAnimationAdapter;
 import android.view.IRotationWatcher;
 import android.view.ISystemGestureExclusionListener;
@@ -744,10 +744,10 @@ interface IWindowManager
      * @param behindClient token for a window, used to filter the search to windows behind it, or
      *                     {@code null} to accept a window at any zOrder
      * @param taskId specifies the id of a task the result must belong to, or -1 to ignore task ids
-     * @param callbacks the object to receive replies
+     * @param listener the object to receive the response
      */
     void requestScrollCapture(int displayId, IBinder behindClient, int taskId,
-            IScrollCaptureCallbacks callbacks);
+            IScrollCaptureResponseListener listener);
 
     /**
      * Holds the WM lock for the specified amount of milliseconds.
diff --git a/core/java/android/view/ScrollCaptureConnection.java b/core/java/android/view/ScrollCaptureConnection.java
index 3456e016c42c..a6d786e1db21 100644
--- a/core/java/android/view/ScrollCaptureConnection.java
+++ b/core/java/android/view/ScrollCaptureConnection.java
@@ -32,6 +32,7 @@ import android.util.Log;
 
 import com.android.internal.annotations.VisibleForTesting;
 
+import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
 import java.util.concurrent.Executor;
 import java.util.function.Consumer;
@@ -50,8 +51,9 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
     private final Object mLock = new Object();
     private final Rect mScrollBounds;
     private final Point mPositionInWindow;
-    private final CloseGuard mCloseGuard;
     private final Executor mUiThread;
+    private final CloseGuard mCloseGuard = new CloseGuard();
+
 
     private ScrollCaptureCallback mLocal;
     private IScrollCaptureCallbacks mRemote;
@@ -60,42 +62,38 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
 
     private CancellationSignal mCancellation;
 
-    private volatile boolean mStarted;
-    private volatile boolean mConnected;
+    private volatile boolean mActive;
 
     /**
      * Constructs a ScrollCaptureConnection.
      *
+     * @param uiThread an executor for the UI thread of the containing View
      * @param selectedTarget  the target the client is controlling
-     * @param remote the callbacks to reply to system requests
      *
      * @hide
      */
     public ScrollCaptureConnection(
             @NonNull Executor uiThread,
-            @NonNull ScrollCaptureTarget selectedTarget,
-            @NonNull IScrollCaptureCallbacks remote) {
+            @NonNull ScrollCaptureTarget selectedTarget) {
         mUiThread = requireNonNull(uiThread, "<uiThread> must non-null");
         requireNonNull(selectedTarget, "<selectedTarget> must non-null");
-        mRemote = requireNonNull(remote, "<callbacks> must non-null");
         mScrollBounds = requireNonNull(Rect.copyOrNull(selectedTarget.getScrollBounds()),
                 "target.getScrollBounds() must be non-null to construct a client");
-
         mLocal = selectedTarget.getCallback();
         mPositionInWindow = new Point(selectedTarget.getPositionInWindow());
-
-        mCloseGuard = new CloseGuard();
-        mCloseGuard.open("close");
-        mConnected = true;
     }
 
     @BinderThread
     @Override
-    public ICancellationSignal startCapture(Surface surface) throws RemoteException {
-        checkConnected();
+    public ICancellationSignal startCapture(@NonNull Surface surface,
+            @NonNull IScrollCaptureCallbacks remote) throws RemoteException {
+
+        mCloseGuard.open("close");
+
         if (!surface.isValid()) {
             throw new RemoteException(new IllegalArgumentException("surface must be valid"));
         }
+        mRemote = requireNonNull(remote, "<callbacks> must non-null");
 
         ICancellationSignal cancellation = CancellationSignal.createTransport();
         mCancellation = CancellationSignal.fromTransport(cancellation);
@@ -110,7 +108,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
 
     @UiThread
     private void onStartCaptureCompleted() {
-        mStarted = true;
+        mActive = true;
         try {
             mRemote.onCaptureStarted();
         } catch (RemoteException e) {
@@ -119,13 +117,11 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
         }
     }
 
-
     @BinderThread
     @Override
     public ICancellationSignal requestImage(Rect requestRect) throws RemoteException {
         Trace.beginSection("requestImage");
-        checkConnected();
-        checkStarted();
+        checkActive();
 
         ICancellationSignal cancellation = CancellationSignal.createTransport();
         mCancellation = CancellationSignal.fromTransport(cancellation);
@@ -152,8 +148,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
     @BinderThread
     @Override
     public ICancellationSignal endCapture() throws RemoteException {
-        checkConnected();
-        checkStarted();
+        checkActive();
 
         ICancellationSignal cancellation = CancellationSignal.createTransport();
         mCancellation = CancellationSignal.fromTransport(cancellation);
@@ -167,64 +162,48 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
 
     @UiThread
     private void onEndCaptureCompleted() {
-        synchronized (mLock) {
-            mStarted = false;
-            try {
+        mActive = false;
+        try {
+            if (mRemote != null) {
                 mRemote.onCaptureEnded();
-            } catch (RemoteException e) {
-                Log.w(TAG, "Shutting down due to error: ", e);
-                close();
             }
+        } catch (RemoteException e) {
+            Log.w(TAG, "Caught exception confirming capture end!", e);
+        } finally {
+            close();
         }
     }
 
     @BinderThread
     @Override
     public void close() {
-        if (mStarted) {
-            Log.w(TAG, "close(): capture is still started?! Ending now.");
-
+        if (mActive) {
+            if (mCancellation != null) {
+                Log.w(TAG, "close(): cancelling pending operation.");
+                mCancellation.cancel();
+                mCancellation = null;
+            }
+            Log.w(TAG, "close(): capture session still active! Ending now.");
             // -> UiThread
             mUiThread.execute(() -> mLocal.onScrollCaptureEnd(() -> { /* ignore */ }));
-            mStarted = false;
+            mActive = false;
         }
-        disconnect();
+        mActive = false;
+        mSession = null;
+        mRemote = null;
+        mLocal = null;
+        mCloseGuard.close();
+        Reference.reachabilityFence(this);
     }
 
-    /**
-     * Shuts down this client and releases references to dependent objects. No attempt is made
-     * to notify the controller, use with caution!
-     */
-    private void disconnect() {
-        synchronized (mLock) {
-            mSession = null;
-            mConnected = false;
-            mStarted = false;
-            mRemote = null;
-            mLocal = null;
-            mCloseGuard.close();
-        }
-    }
-
-    public boolean isConnected() {
-        return mConnected;
-    }
-
-    public boolean isStarted() {
-        return mStarted;
-    }
-
-    private synchronized void checkConnected() throws RemoteException {
-        synchronized (mLock) {
-            if (!mConnected) {
-                throw new RemoteException(new IllegalStateException("Not connected"));
-            }
-        }
+    @VisibleForTesting
+    public boolean isActive() {
+        return mActive;
     }
 
-    private void checkStarted() throws RemoteException {
+    private void checkActive() throws RemoteException {
         synchronized (mLock) {
-            if (!mStarted) {
+            if (!mActive) {
                 throw new RemoteException(new IllegalStateException("Not started!"));
             }
         }
@@ -233,24 +212,16 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
     /** @return a string representation of the state of this client */
     public String toString() {
         return "ScrollCaptureConnection{"
-                + "connected=" + mConnected
-                + ", started=" + mStarted
+                + "active=" + mActive
                 + ", session=" + mSession
                 + ", remote=" + mRemote
                 + ", local=" + mLocal
                 + "}";
     }
 
-    @VisibleForTesting
-    public CancellationSignal getCancellation() {
-        return mCancellation;
-    }
-
     protected void finalize() throws Throwable {
         try {
-            if (mCloseGuard != null) {
-                mCloseGuard.warnIfOpen();
-            }
+            mCloseGuard.warnIfOpen();
             close();
         } finally {
             super.finalize();
diff --git a/core/java/android/view/ScrollCaptureResponse.java b/core/java/android/view/ScrollCaptureResponse.java
index 564113edb3c7..8808827b248a 100644
--- a/core/java/android/view/ScrollCaptureResponse.java
+++ b/core/java/android/view/ScrollCaptureResponse.java
@@ -20,6 +20,7 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.graphics.Rect;
 import android.os.Parcelable;
+import android.os.RemoteException;
 
 import com.android.internal.util.DataClass;
 
@@ -57,11 +58,22 @@ public class ScrollCaptureResponse implements Parcelable {
     @DataClass.PluralOf("message")
     private ArrayList<String> mMessages = new ArrayList<>();
 
-    /** Whether a connection has been returned. */
+    /** Whether an active connection is present. */
     public boolean isConnected() {
-        return mConnection != null;
+        return mConnection != null && mConnection.asBinder().isBinderAlive();
     }
 
+    /** Closes a connection returned with this response. */
+    public void close() {
+        if (mConnection != null) {
+            try {
+                mConnection.close();
+            } catch (RemoteException e) {
+                // Ignore
+            }
+            mConnection = null;
+        }
+    }
 
 
 
@@ -367,10 +379,10 @@ public class ScrollCaptureResponse implements Parcelable {
     }
 
     @DataClass.Generated(
-            time = 1612282689462L,
+            time = 1614833185795L,
             codegenVersion = "1.0.22",
             sourceFile = "frameworks/base/core/java/android/view/ScrollCaptureResponse.java",
-            inputSignatures = "private @android.annotation.NonNull java.lang.String mDescription\nprivate @android.annotation.Nullable @com.android.internal.util.DataClass.MaySetToNull android.view.IScrollCaptureConnection mConnection\nprivate @android.annotation.Nullable android.graphics.Rect mWindowBounds\nprivate @android.annotation.Nullable android.graphics.Rect mBoundsInWindow\nprivate @android.annotation.Nullable java.lang.String mWindowTitle\nprivate @android.annotation.NonNull @com.android.internal.util.DataClass.PluralOf(\"message\") java.util.ArrayList<java.lang.String> mMessages\npublic  boolean isConnected()\nclass ScrollCaptureResponse extends java.lang.Object implements [android.os.Parcelable]\n@com.android.internal.util.DataClass(genToString=true, genGetters=true)")
+            inputSignatures = "private @android.annotation.NonNull java.lang.String mDescription\nprivate @android.annotation.Nullable @com.android.internal.util.DataClass.MaySetToNull android.view.IScrollCaptureConnection mConnection\nprivate @android.annotation.Nullable android.graphics.Rect mWindowBounds\nprivate @android.annotation.Nullable android.graphics.Rect mBoundsInWindow\nprivate @android.annotation.Nullable java.lang.String mWindowTitle\nprivate @android.annotation.NonNull @com.android.internal.util.DataClass.PluralOf(\"message\") java.util.ArrayList<java.lang.String> mMessages\npublic  boolean isConnected()\npublic  void close()\nclass ScrollCaptureResponse extends java.lang.Object implements [android.os.Parcelable]\n@com.android.internal.util.DataClass(genToString=true, genGetters=true)")
     @Deprecated
     private void __metadata() {}
 
diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index 35726c0c1f04..d462f5844a70 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -5356,7 +5356,7 @@ public final class ViewRootImpl implements ViewParent,
                     updateLocationInParentDisplay(msg.arg1, msg.arg2);
                 } break;
                 case MSG_REQUEST_SCROLL_CAPTURE:
-                    handleScrollCaptureRequest((IScrollCaptureCallbacks) msg.obj);
+                    handleScrollCaptureRequest((IScrollCaptureResponseListener) msg.obj);
                     break;
             }
         }
@@ -9267,10 +9267,10 @@ public final class ViewRootImpl implements ViewParent,
     /**
      * Dispatches a scroll capture request to the view hierarchy on the ui thread.
      *
-     * @param callbacks for replies
+     * @param listener for the response
      */
-    public void dispatchScrollCaptureRequest(@NonNull IScrollCaptureCallbacks callbacks) {
-        mHandler.obtainMessage(MSG_REQUEST_SCROLL_CAPTURE, callbacks).sendToTarget();
+    public void dispatchScrollCaptureRequest(@NonNull IScrollCaptureResponseListener listener) {
+        mHandler.obtainMessage(MSG_REQUEST_SCROLL_CAPTURE, listener).sendToTarget();
     }
 
     /**
@@ -9317,10 +9317,10 @@ public final class ViewRootImpl implements ViewParent,
      * A call to {@link IScrollCaptureCallbacks#onScrollCaptureResponse(ScrollCaptureResponse)}
      * will follow.
      *
-     * @param callbacks to receive responses
+     * @param listener to receive responses
      * @see ScrollCaptureTargetSelector
      */
-    public void handleScrollCaptureRequest(@NonNull IScrollCaptureCallbacks callbacks) {
+    public void handleScrollCaptureRequest(@NonNull IScrollCaptureResponseListener listener) {
         ScrollCaptureSearchResults results =
                 new ScrollCaptureSearchResults(mContext.getMainExecutor());
 
@@ -9335,7 +9335,7 @@ public final class ViewRootImpl implements ViewParent,
             getChildVisibleRect(rootView, rect, point);
             rootView.dispatchScrollCaptureSearch(rect, point, results::addTarget);
         }
-        Runnable onComplete = () -> dispatchScrollCaptureSearchResult(callbacks, results);
+        Runnable onComplete = () -> dispatchScrollCaptureSearchResponse(listener, results);
         results.setOnCompleteListener(onComplete);
         if (!results.isComplete()) {
             mHandler.postDelayed(results::finish, getScrollCaptureRequestTimeout());
@@ -9343,8 +9343,8 @@ public final class ViewRootImpl implements ViewParent,
     }
 
     /** Called by {@link #handleScrollCaptureRequest} when a result is returned */
-    private void dispatchScrollCaptureSearchResult(
-            @NonNull IScrollCaptureCallbacks callbacks,
+    private void dispatchScrollCaptureSearchResponse(
+            @NonNull IScrollCaptureResponseListener listener,
             @NonNull ScrollCaptureSearchResults results) {
 
         ScrollCaptureTarget selectedTarget = results.getTopResult();
@@ -9361,7 +9361,7 @@ public final class ViewRootImpl implements ViewParent,
         if (selectedTarget == null) {
             response.setDescription("No scrollable targets found in window");
             try {
-                callbacks.onScrollCaptureResponse(response.build());
+                listener.onScrollCaptureResponse(response.build());
             } catch (RemoteException e) {
                 Log.e(TAG, "Failed to send scroll capture search result", e);
             }
@@ -9387,11 +9387,11 @@ public final class ViewRootImpl implements ViewParent,
 
         // Create a connection and return it to the caller
         ScrollCaptureConnection connection = new ScrollCaptureConnection(
-                mView.getContext().getMainExecutor(), selectedTarget, callbacks);
+                mView.getContext().getMainExecutor(), selectedTarget);
         response.setConnection(connection);
 
         try {
-            callbacks.onScrollCaptureResponse(response.build());
+            listener.onScrollCaptureResponse(response.build());
         } catch (RemoteException e) {
             if (DEBUG_SCROLL_CAPTURE) {
                 Log.w(TAG, "Failed to send scroll capture search response.", e);
@@ -9691,10 +9691,10 @@ public final class ViewRootImpl implements ViewParent,
         }
 
         @Override
-        public void requestScrollCapture(IScrollCaptureCallbacks callbacks) {
+        public void requestScrollCapture(IScrollCaptureResponseListener listener) {
             final ViewRootImpl viewAncestor = mViewAncestor.get();
             if (viewAncestor != null) {
-                viewAncestor.dispatchScrollCaptureRequest(callbacks);
+                viewAncestor.dispatchScrollCaptureRequest(listener);
             }
         }
     }
diff --git a/core/java/android/view/Window.java b/core/java/android/view/Window.java
index cf5ec8de0362..c814e5add1b7 100644
--- a/core/java/android/view/Window.java
+++ b/core/java/android/view/Window.java
@@ -2619,10 +2619,10 @@ public abstract class Window {
     /**
      * System request to begin scroll capture.
      *
-     * @param callbacks to receive responses
+     * @param listener to receive the response
      * @hide
      */
-    public void requestScrollCapture(IScrollCaptureCallbacks callbacks) {
+    public void requestScrollCapture(IScrollCaptureResponseListener listener) {
     }
 
     /**