From 8f1b73e1839803c9d8665846e50a3010697fe8b9 Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Thu, 28 Jan 2021 15:41:18 -0500 Subject: Long screenshots framework update and API unhide This change includes a series of API review changes. The most significant update is the addition of cancellation signals to API methods where needed. Renames some internal classes and interfaces to support the API changes and to simplify tests and improve quality. Test: numerous presubmits Bug: 175830670 CTS-Coverage-Bug: 180419562 Change-Id: I9b1b950a2779fc902ecf4d6394e3c35171926700 --- .../java/android/view/ScrollCaptureConnection.java | 373 +++++++++++---------- 1 file changed, 192 insertions(+), 181 deletions(-) (limited to 'core/java/android/view/ScrollCaptureConnection.java') diff --git a/core/java/android/view/ScrollCaptureConnection.java b/core/java/android/view/ScrollCaptureConnection.java index 0e6cdd1dbec5..3456e016c42c 100644 --- a/core/java/android/view/ScrollCaptureConnection.java +++ b/core/java/android/view/ScrollCaptureConnection.java @@ -18,18 +18,23 @@ package android.view; import static java.util.Objects.requireNonNull; +import android.annotation.BinderThread; import android.annotation.NonNull; import android.annotation.UiThread; -import android.annotation.WorkerThread; import android.graphics.Point; import android.graphics.Rect; -import android.os.Handler; +import android.os.CancellationSignal; +import android.os.ICancellationSignal; import android.os.RemoteException; +import android.os.Trace; import android.util.CloseGuard; +import android.util.Log; import com.android.internal.annotations.VisibleForTesting; -import java.util.concurrent.atomic.AtomicBoolean; +import java.lang.ref.WeakReference; +import java.util.concurrent.Executor; +import java.util.function.Consumer; /** * Mediator between a selected scroll capture target view and a remote process. @@ -41,270 +46,276 @@ import java.util.concurrent.atomic.AtomicBoolean; public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub { private static final String TAG = "ScrollCaptureConnection"; - private static final int DEFAULT_TIMEOUT = 1000; - - private final Handler mHandler; - private ScrollCaptureTarget mSelectedTarget; - private int mTimeoutMillis = DEFAULT_TIMEOUT; - - protected Surface mSurface; - private IScrollCaptureCallbacks mCallbacks; + private final Object mLock = new Object(); private final Rect mScrollBounds; private final Point mPositionInWindow; private final CloseGuard mCloseGuard; + private final Executor mUiThread; + + private ScrollCaptureCallback mLocal; + private IScrollCaptureCallbacks mRemote; - // The current session instance in use by the callback. private ScrollCaptureSession mSession; - // Helps manage timeout callbacks registered to handler and aids testing. - private DelayedAction mTimeoutAction; + private CancellationSignal mCancellation; + + private volatile boolean mStarted; + private volatile boolean mConnected; /** * Constructs a ScrollCaptureConnection. * * @param selectedTarget the target the client is controlling - * @param callbacks the callbacks to reply to system requests + * @param remote the callbacks to reply to system requests * * @hide */ public ScrollCaptureConnection( + @NonNull Executor uiThread, @NonNull ScrollCaptureTarget selectedTarget, - @NonNull IScrollCaptureCallbacks callbacks) { + @NonNull IScrollCaptureCallbacks remote) { + mUiThread = requireNonNull(uiThread, " must non-null"); requireNonNull(selectedTarget, " must non-null"); - requireNonNull(callbacks, " must non-null"); - final Rect scrollBounds = requireNonNull(selectedTarget.getScrollBounds(), + mRemote = requireNonNull(remote, " must non-null"); + mScrollBounds = requireNonNull(Rect.copyOrNull(selectedTarget.getScrollBounds()), "target.getScrollBounds() must be non-null to construct a client"); - mSelectedTarget = selectedTarget; - mHandler = selectedTarget.getContainingView().getHandler(); - mScrollBounds = new Rect(scrollBounds); + mLocal = selectedTarget.getCallback(); mPositionInWindow = new Point(selectedTarget.getPositionInWindow()); - mCallbacks = callbacks; mCloseGuard = new CloseGuard(); mCloseGuard.open("close"); - - selectedTarget.getContainingView().addOnAttachStateChangeListener( - new View.OnAttachStateChangeListener() { - @Override - public void onViewAttachedToWindow(View v) { - - } - - @Override - public void onViewDetachedFromWindow(View v) { - selectedTarget.getContainingView().removeOnAttachStateChangeListener(this); - endCapture(); - } - }); - } - - @VisibleForTesting - public void setTimeoutMillis(int timeoutMillis) { - mTimeoutMillis = timeoutMillis; + mConnected = true; } - @VisibleForTesting - public DelayedAction getTimeoutAction() { - return mTimeoutAction; - } - - private void checkConnected() { - if (mSelectedTarget == null || mCallbacks == null) { - throw new IllegalStateException("This client has been disconnected."); + @BinderThread + @Override + public ICancellationSignal startCapture(Surface surface) throws RemoteException { + checkConnected(); + if (!surface.isValid()) { + throw new RemoteException(new IllegalArgumentException("surface must be valid")); } - } - private void checkStarted() { - if (mSession == null) { - throw new IllegalStateException("Capture session has not been started!"); - } - } + ICancellationSignal cancellation = CancellationSignal.createTransport(); + mCancellation = CancellationSignal.fromTransport(cancellation); + mSession = new ScrollCaptureSession(surface, mScrollBounds, mPositionInWindow); - @WorkerThread // IScrollCaptureConnection - @Override - public void startCapture(Surface surface) throws RemoteException { - checkConnected(); - mSurface = surface; - scheduleTimeout(mTimeoutMillis, this::onStartCaptureTimeout); - mSession = new ScrollCaptureSession(mSurface, mScrollBounds, mPositionInWindow, this); - mHandler.post(() -> mSelectedTarget.getCallback().onScrollCaptureStart(mSession, - this::onStartCaptureCompleted)); + Runnable listener = + SafeCallback.create(mCancellation, mUiThread, this::onStartCaptureCompleted); + // -> UiThread + mUiThread.execute(() -> mLocal.onScrollCaptureStart(mSession, mCancellation, listener)); + return cancellation; } @UiThread private void onStartCaptureCompleted() { - if (cancelTimeout()) { - mHandler.post(() -> { - try { - mCallbacks.onCaptureStarted(); - } catch (RemoteException e) { - doShutdown(); - } - }); + mStarted = true; + try { + mRemote.onCaptureStarted(); + } catch (RemoteException e) { + Log.w(TAG, "Shutting down due to error: ", e); + close(); } } - @UiThread - private void onStartCaptureTimeout() { - endCapture(); - } - @WorkerThread // IScrollCaptureConnection + @BinderThread @Override - public void requestImage(Rect requestRect) { + public ICancellationSignal requestImage(Rect requestRect) throws RemoteException { + Trace.beginSection("requestImage"); checkConnected(); checkStarted(); - scheduleTimeout(mTimeoutMillis, this::onRequestImageTimeout); - // Response is dispatched via ScrollCaptureSession, to onRequestImageCompleted - mHandler.post(() -> mSelectedTarget.getCallback().onScrollCaptureImageRequest( - mSession, new Rect(requestRect))); - } - @UiThread - void onRequestImageCompleted(long frameNumber, Rect capturedArea) { - final Rect finalCapturedArea = new Rect(capturedArea); - if (cancelTimeout()) { - mHandler.post(() -> { - try { - mCallbacks.onCaptureBufferSent(frameNumber, finalCapturedArea); - } catch (RemoteException e) { - doShutdown(); - } - }); - } + ICancellationSignal cancellation = CancellationSignal.createTransport(); + mCancellation = CancellationSignal.fromTransport(cancellation); + + Consumer listener = + SafeCallback.create(mCancellation, mUiThread, this::onImageRequestCompleted); + // -> UiThread + mUiThread.execute(() -> mLocal.onScrollCaptureImageRequest( + mSession, mCancellation, new Rect(requestRect), listener)); + Trace.endSection(); + return cancellation; } @UiThread - private void onRequestImageTimeout() { - endCapture(); + void onImageRequestCompleted(Rect capturedArea) { + try { + mRemote.onImageRequestCompleted(0, capturedArea); + } catch (RemoteException e) { + Log.w(TAG, "Shutting down due to error: ", e); + close(); + } } - @WorkerThread // IScrollCaptureConnection + @BinderThread @Override - public void endCapture() { - if (isStarted()) { - scheduleTimeout(mTimeoutMillis, this::onEndCaptureTimeout); - mHandler.post(() -> - mSelectedTarget.getCallback().onScrollCaptureEnd(this::onEndCaptureCompleted)); - } else { - disconnect(); - } - } + public ICancellationSignal endCapture() throws RemoteException { + checkConnected(); + checkStarted(); - private boolean isStarted() { - return mCallbacks != null && mSelectedTarget != null; - } + ICancellationSignal cancellation = CancellationSignal.createTransport(); + mCancellation = CancellationSignal.fromTransport(cancellation); - @UiThread - private void onEndCaptureCompleted() { // onEndCaptureCompleted - if (cancelTimeout()) { - doShutdown(); - } + Runnable listener = + SafeCallback.create(mCancellation, mUiThread, this::onEndCaptureCompleted); + // -> UiThread + mUiThread.execute(() -> mLocal.onScrollCaptureEnd(listener)); + return cancellation; } @UiThread - private void onEndCaptureTimeout() { - doShutdown(); + private void onEndCaptureCompleted() { + synchronized (mLock) { + mStarted = false; + try { + mRemote.onCaptureEnded(); + } catch (RemoteException e) { + Log.w(TAG, "Shutting down due to error: ", e); + close(); + } + } } + @BinderThread + @Override + public void close() { + if (mStarted) { + Log.w(TAG, "close(): capture is still started?! Ending now."); - private void doShutdown() { - try { - if (mCallbacks != null) { - mCallbacks.onConnectionClosed(); - } - } catch (RemoteException e) { - // Ignore - } finally { - disconnect(); + // -> UiThread + mUiThread.execute(() -> mLocal.onScrollCaptureEnd(() -> { /* ignore */ })); + mStarted = false; } + disconnect(); } /** * Shuts down this client and releases references to dependent objects. No attempt is made * to notify the controller, use with caution! */ - public void disconnect() { - if (mSession != null) { - mSession.disconnect(); + private void disconnect() { + synchronized (mLock) { mSession = null; + mConnected = false; + mStarted = false; + mRemote = null; + mLocal = null; + mCloseGuard.close(); } + } + + public boolean isConnected() { + return mConnected; + } - mSelectedTarget = null; - mCallbacks = null; + public boolean isStarted() { + return mStarted; + } + + private synchronized void checkConnected() throws RemoteException { + synchronized (mLock) { + if (!mConnected) { + throw new RemoteException(new IllegalStateException("Not connected")); + } + } + } + + private void checkStarted() throws RemoteException { + synchronized (mLock) { + if (!mStarted) { + throw new RemoteException(new IllegalStateException("Not started!")); + } + } } /** @return a string representation of the state of this client */ public String toString() { return "ScrollCaptureConnection{" + + "connected=" + mConnected + + ", started=" + mStarted + ", session=" + mSession - + ", selectedTarget=" + mSelectedTarget - + ", clientCallbacks=" + mCallbacks + + ", remote=" + mRemote + + ", local=" + mLocal + "}"; } - private boolean cancelTimeout() { - if (mTimeoutAction != null) { - return mTimeoutAction.cancel(); - } - return false; + @VisibleForTesting + public CancellationSignal getCancellation() { + return mCancellation; } - private void scheduleTimeout(long timeoutMillis, Runnable action) { - if (mTimeoutAction != null) { - mTimeoutAction.cancel(); + protected void finalize() throws Throwable { + try { + if (mCloseGuard != null) { + mCloseGuard.warnIfOpen(); + } + close(); + } finally { + super.finalize(); } - mTimeoutAction = new DelayedAction(mHandler, timeoutMillis, action); } - /** @hide */ - @VisibleForTesting - public static class DelayedAction { - private final AtomicBoolean mCompleted = new AtomicBoolean(); - private final Object mToken = new Object(); - private final Handler mHandler; - private final Runnable mAction; - - @VisibleForTesting - public DelayedAction(Handler handler, long timeoutMillis, Runnable action) { - mHandler = handler; - mAction = action; - mHandler.postDelayed(this::onTimeout, mToken, timeoutMillis); + private static class SafeCallback { + private final CancellationSignal mSignal; + private final WeakReference mTargetRef; + private final Executor mExecutor; + private boolean mExecuted; + + protected SafeCallback(CancellationSignal signal, Executor executor, T target) { + mSignal = signal; + mTargetRef = new WeakReference<>(target); + mExecutor = executor; } - private boolean onTimeout() { - if (mCompleted.compareAndSet(false, true)) { - mAction.run(); - return true; + // Provide the target to the consumer to invoke, forward on handler thread ONCE, + // and only if noy cancelled, and the target is still available (not collected) + protected final void maybeAccept(Consumer targetConsumer) { + if (mExecuted) { + return; + } + mExecuted = true; + if (mSignal.isCanceled()) { + return; + } + T target = mTargetRef.get(); + if (target == null) { + return; } - return false; + mExecutor.execute(() -> targetConsumer.accept(target)); } - /** - * Cause the timeout action to run immediately and mark as timed out. - * - * @return true if the timeout was run, false if the timeout had already been canceled - */ - @VisibleForTesting - public boolean timeoutNow() { - return onTimeout(); + static Runnable create(CancellationSignal signal, Executor executor, Runnable target) { + return new RunnableCallback(signal, executor, target); } - /** - * Attempt to cancel the timeout action (such as after a callback is made) - * - * @return true if the timeout was canceled and will not run, false if time has expired and - * the timeout action has or will run momentarily - */ - public boolean cancel() { - if (!mCompleted.compareAndSet(false, true)) { - // Whoops, too late! - return false; - } - mHandler.removeCallbacksAndMessages(mToken); - return true; + static Consumer create(CancellationSignal signal, Executor executor, + Consumer target) { + return new ConsumerCallback(signal, executor, target); + } + } + + private static final class RunnableCallback extends SafeCallback implements Runnable { + RunnableCallback(CancellationSignal signal, Executor executor, Runnable target) { + super(signal, executor, target); + } + + @Override + public void run() { + maybeAccept(Runnable::run); + } + } + + private static final class ConsumerCallback extends SafeCallback> + implements Consumer { + ConsumerCallback(CancellationSignal signal, Executor executor, Consumer target) { + super(signal, executor, target); + } + + @Override + public void accept(T value) { + maybeAccept((target) -> target.accept(value)); } } } -- cgit v1.2.3