Merge "Ensure cleanup when interrupting scroll capture" (093a2030) · Commits · e / os / android_frameworks_base

core/java/android/view/ScrollCaptureConnection.java

+38 −18

Original line number	Diff line number	Diff line
		@@ -24,9 +24,9 @@ import android.annotation.UiThread;
		import android.graphics.Point;
		import android.graphics.Rect;
		import android.os.CancellationSignal;
		import android.os.IBinder;
		import android.os.ICancellationSignal;
		import android.os.RemoteException;
		import android.os.Trace;
		import android.util.CloseGuard;
		import android.util.Log;

		@@ -44,7 +44,8 @@ import java.util.function.Consumer;
		*
		* @hide
		*/
		public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
		public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub implements
		IBinder.DeathRecipient {

		private static final String TAG = "ScrollCaptureConnection";

		@@ -54,15 +55,13 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
		private final Executor mUiThread;
		private final CloseGuard mCloseGuard = new CloseGuard();


		private ScrollCaptureCallback mLocal;
		private IScrollCaptureCallbacks mRemote;

		private ScrollCaptureSession mSession;

		private CancellationSignal mCancellation;

		private volatile boolean mActive;
		private volatile boolean mConnected;

		/**
		* Constructs a ScrollCaptureConnection.
		@@ -87,13 +86,14 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
		@Override
		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");
		mRemote.asBinder().linkToDeath(this, 0);
		mConnected = true;

		ICancellationSignal cancellation = CancellationSignal.createTransport();
		mCancellation = CancellationSignal.fromTransport(cancellation);
		@@ -115,14 +115,14 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
		Log.w(TAG, "Shutting down due to error: ", e);
		close();
		}
		mCancellation = null;
		}

		@BinderThread
		@Override
		public ICancellationSignal requestImage(Rect requestRect) throws RemoteException {
		Trace.beginSection("requestImage");
		checkActive();

		cancelPendingAction();
		ICancellationSignal cancellation = CancellationSignal.createTransport();
		mCancellation = CancellationSignal.fromTransport(cancellation);

		@@ -131,7 +131,6 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
		// -> UiThread
		mUiThread.execute(() -> mLocal.onScrollCaptureImageRequest(
		mSession, mCancellation, new Rect(requestRect), listener));
		Trace.endSection();
		return cancellation;
		}

		@@ -142,6 +141,8 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
		} catch (RemoteException e) {
		Log.w(TAG, "Shutting down due to error: ", e);
		close();
		} finally {
		mCancellation = null;
		}
		}

		@@ -149,7 +150,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
		@Override
		public ICancellationSignal endCapture() throws RemoteException {
		checkActive();

		cancelPendingAction();
		ICancellationSignal cancellation = CancellationSignal.createTransport();
		mCancellation = CancellationSignal.fromTransport(cancellation);

		@@ -170,26 +171,32 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
		} catch (RemoteException e) {
		Log.w(TAG, "Caught exception confirming capture end!", e);
		} finally {
		mCancellation = null;
		close();
		}
		}

		@Override
		public void binderDied() {
		Log.e(TAG, "Controlling process just died.");
		close();
		}

		@BinderThread
		@Override
		public void close() {
		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
		cancelPendingAction();
		final ScrollCaptureCallback callback = mLocal;
		mUiThread.execute(() -> callback.onScrollCaptureEnd(() -> { /* ignore */ }));
		mActive = false;
		}
		if (mRemote != null) {
		mRemote.asBinder().unlinkToDeath(this, 0);
		}
		mActive = false;
		mConnected = false;
		mSession = null;
		mRemote = null;
		mLocal = null;
		@@ -197,6 +204,19 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {
		Reference.reachabilityFence(this);
		}

		private void cancelPendingAction() {
		if (mCancellation != null) {
		Log.w(TAG, "cancelling pending operation.");
		mCancellation.cancel();
		mCancellation = null;
		}
		}

		@VisibleForTesting
		public boolean isConnected() {
		return mConnected;
		}

		@VisibleForTesting
		public boolean isActive() {
		return mActive;
		@@ -236,7 +256,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {

		protected SafeCallback(CancellationSignal signal, Executor executor, T value) {
		mSignal = signal;
		mValue = new AtomicReference<T>(value);
		mValue = new AtomicReference<>(value);
		mExecutor = executor;
		}

		@@ -257,7 +277,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub {

		static <T> Consumer<T> create(CancellationSignal signal, Executor executor,
		Consumer<T> target) {
		return new ConsumerCallback<T>(signal, executor, target);
		return new ConsumerCallback<>(signal, executor, target);
		}
		}

core/tests/coretests/src/android/view/ScrollCaptureConnectionTest.java

+41 −13

Original line number	Diff line number	Diff line
		@@ -21,16 +21,19 @@ import static androidx.test.InstrumentationRegistry.getTargetContext;
		import static org.junit.Assert.assertFalse;
		import static org.junit.Assert.assertTrue;
		import static org.junit.Assert.fail;
		import static org.mockito.ArgumentMatchers.any;
		import static org.mockito.ArgumentMatchers.anyInt;
		import static org.mockito.ArgumentMatchers.eq;
		import static org.mockito.Mockito.reset;
		import static org.mockito.Mockito.never;
		import static org.mockito.Mockito.times;
		import static org.mockito.Mockito.verify;
		import static org.mockito.Mockito.verifyNoMoreInteractions;
		import static org.mockito.Mockito.when;

		import android.graphics.Point;
		import android.graphics.Rect;
		import android.os.Binder;
		import android.os.Handler;
		import android.os.IBinder;
		import android.os.ICancellationSignal;
		import android.os.RemoteException;
		import android.platform.test.annotations.Presubmit;
		@@ -60,6 +63,7 @@ public class ScrollCaptureConnectionTest {

		private ScrollCaptureTarget mTarget;
		private ScrollCaptureConnection mConnection;
		private IBinder mConnectionBinder = new Binder("ScrollCaptureConnection Test");

		private Handler mHandler;

		@@ -76,6 +80,7 @@ public class ScrollCaptureConnectionTest {
		mHandler = new Handler(getTargetContext().getMainLooper());
		when(mSurface.isValid()).thenReturn(true);
		when(mView.getScrollCaptureHint()).thenReturn(View.SCROLL_CAPTURE_HINT_INCLUDE);
		when(mRemote.asBinder()).thenReturn(mConnectionBinder);

		mTarget = new ScrollCaptureTarget(mView, mLocalVisibleRect, mPositionInWindow, mCallback);
		mTarget.setScrollBounds(mScrollBounds);
		@@ -107,12 +112,13 @@ public class ScrollCaptureConnectionTest {
		@Test
		public void testStartCapture() throws Exception {
		mConnection.startCapture(mSurface, mRemote);
		assertTrue(mConnection.isConnected());
		assertFalse(mConnection.isActive());

		mCallback.completeStartRequest();
		assertTrue(mConnection.isActive());

		verify(mRemote, times(1)).onCaptureStarted();
		verifyNoMoreInteractions(mRemote);
		}

		@Test
		@@ -123,7 +129,7 @@ public class ScrollCaptureConnectionTest {
		mCallback.completeStartRequest();
		assertFalse(mConnection.isActive());

		verifyNoMoreInteractions(mRemote);
		verify(mRemote, never()).onCaptureStarted();
		}

		/** @see ScrollCaptureConnection#requestImage(Rect) */
		@@ -131,42 +137,63 @@ public class ScrollCaptureConnectionTest {
		public void testRequestImage() throws Exception {
		mConnection.startCapture(mSurface, mRemote);
		mCallback.completeStartRequest();
		reset(mRemote);

		mConnection.requestImage(new Rect(1, 2, 3, 4));
		mCallback.completeImageRequest(new Rect(1, 2, 3, 4));

		verify(mRemote, times(1))
		.onImageRequestCompleted(eq(0), eq(new Rect(1, 2, 3, 4)));
		verifyNoMoreInteractions(mRemote);
		}

		@Test
		public void testRequestImage_cancellation() throws Exception {
		mConnection.startCapture(mSurface, mRemote);
		mCallback.completeStartRequest();
		reset(mRemote);

		ICancellationSignal signal = mConnection.requestImage(new Rect(1, 2, 3, 4));
		signal.cancel();
		mCallback.completeImageRequest(new Rect(1, 2, 3, 4));

		verifyNoMoreInteractions(mRemote);
		verify(mRemote, never()).onImageRequestCompleted(anyInt(), any(Rect.class));
		}

		/** @see ScrollCaptureConnection#endCapture() */
		@Test
		public void testEndCapture() throws Exception {
		mConnection.startCapture(mSurface, mRemote);
		assertTrue(mConnection.isConnected());

		mCallback.completeStartRequest();
		reset(mRemote);
		assertTrue(mConnection.isActive());

		mConnection.endCapture();
		mCallback.completeEndRequest();
		assertFalse(mConnection.isActive());

		// And the reply is sent
		verify(mRemote, times(1)).onCaptureEnded();
		verifyNoMoreInteractions(mRemote);

		assertFalse(mConnection.isConnected());
		}

		/** @see ScrollCaptureConnection#endCapture() */
		@Test
		public void testClose_withPendingOperation() throws Exception {
		mConnection.startCapture(mSurface, mRemote);
		mCallback.completeStartRequest();

		mConnection.requestImage(new Rect(1, 2, 3, 4));
		assertFalse(mCallback.getLastCancellationSignal().isCanceled());

		mConnection.close();

		// And the reply is sent
		assertTrue(mCallback.onScrollCaptureEndCalled());
		assertTrue(mCallback.getLastCancellationSignal().isCanceled());
		assertFalse(mConnection.isActive());
		assertFalse(mConnection.isConnected());
		verify(mRemote, never()).onCaptureEnded();
		verify(mRemote, never()).onImageRequestCompleted(anyInt(), any(Rect.class));
		}

		/** @see ScrollCaptureConnection#endCapture() */
		@@ -174,20 +201,19 @@ public class ScrollCaptureConnectionTest {
		public void testEndCapture_cancellation() throws Exception {
		mConnection.startCapture(mSurface, mRemote);
		mCallback.completeStartRequest();
		reset(mRemote);

		ICancellationSignal signal = mConnection.endCapture();
		signal.cancel();
		mCallback.completeEndRequest();

		verifyNoMoreInteractions(mRemote);
		verify(mRemote, never()).onCaptureEnded();
		}

		@Test
		public void testClose() {
		mConnection.close();
		assertFalse(mConnection.isActive());
		verifyNoMoreInteractions(mRemote);
		assertFalse(mConnection.isConnected());
		}

		@Test
		@@ -196,9 +222,11 @@ public class ScrollCaptureConnectionTest {

		mCallback.completeStartRequest();
		assertTrue(mConnection.isActive());
		assertTrue(mConnection.isConnected());

		mConnection.close();
		mCallback.completeEndRequest();
		assertFalse(mConnection.isActive());
		assertFalse(mConnection.isConnected());
		}
		}

core/tests/coretests/src/android/view/TestScrollCaptureCallback.java

+22 −3

Original line number	Diff line number	Diff line
		@@ -31,10 +31,13 @@ class TestScrollCaptureCallback implements ScrollCaptureCallback {
		private Consumer<Rect> mImageOnComplete;
		private Runnable mOnEndReady;
		private volatile int mModCount;
		private boolean mOnScrollCaptureEndCalled;
		private CancellationSignal mLastCancellationSignal;

		@Override
		public void onScrollCaptureSearch(@NonNull CancellationSignal signal,
		@NonNull Consumer<Rect> onReady) {
		mLastCancellationSignal = signal;
		mSearchConsumer = onReady;
		mModCount++;
		}
		@@ -42,6 +45,7 @@ class TestScrollCaptureCallback implements ScrollCaptureCallback {
		@Override
		public void onScrollCaptureStart(@NonNull ScrollCaptureSession session,
		@NonNull CancellationSignal signal, @NonNull Runnable onReady) {
		mLastCancellationSignal = signal;
		mStartOnReady = onReady;
		mModCount++;
		}
		@@ -50,6 +54,7 @@ class TestScrollCaptureCallback implements ScrollCaptureCallback {
		public void onScrollCaptureImageRequest(@NonNull ScrollCaptureSession session,
		@NonNull CancellationSignal signal, @NonNull Rect captureArea,
		@NonNull Consumer<Rect> onComplete) {
		mLastCancellationSignal = signal;
		mImageOnComplete = onComplete;
		mModCount++;
		}
		@@ -57,8 +62,12 @@ class TestScrollCaptureCallback implements ScrollCaptureCallback {
		@Override
		public void onScrollCaptureEnd(@NonNull Runnable onReady) {
		mOnEndReady = onReady;
		mOnScrollCaptureEndCalled = true;
		}

		public boolean onScrollCaptureEndCalled() {
		return mOnScrollCaptureEndCalled;
		}
		void completeSearchRequest(Rect scrollBounds) {
		assertNotNull("Did not receive search request", mSearchConsumer);
		mSearchConsumer.accept(scrollBounds);
		@@ -71,16 +80,26 @@ class TestScrollCaptureCallback implements ScrollCaptureCallback {

		void completeStartRequest() {
		assertNotNull("Did not receive start request", mStartOnReady);
		if (mLastCancellationSignal != null && !mLastCancellationSignal.isCanceled()) {
		mStartOnReady.run();
		}
		}

		void completeImageRequest(Rect captured) {
		assertNotNull("Did not receive image request", mImageOnComplete);
		if (mLastCancellationSignal != null && !mLastCancellationSignal.isCanceled()) {
		mImageOnComplete.accept(captured);
		}
		}

		void completeEndRequest() {
		assertNotNull("Did not receive end request", mOnEndReady);
		if (mLastCancellationSignal != null && !mLastCancellationSignal.isCanceled()) {
		mOnEndReady.run();
		}
		}

		public CancellationSignal getLastCancellationSignal() {
		return mLastCancellationSignal;
		}
		}

packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java

+74 −56

Original line number	Diff line number	Diff line
		@@ -422,11 +422,6 @@ public class ScreenshotController {
		} else {
		mScreenshotView.animateDismissal();
		}

		if (mLastScrollCaptureResponse != null) {
		mLastScrollCaptureResponse.close();
		mLastScrollCaptureResponse = null;
		}
		}

		boolean isPendingSharedTransition() {
		@@ -456,6 +451,9 @@ public class ScreenshotController {
		mScreenshotView.init(mUiEventLogger, new ScreenshotView.ScreenshotViewCallback() {
		@Override
		public void onUserInteraction() {
		if (DEBUG_INPUT) {
		Log.d(TAG, "onUserInteraction");
		}
		resetTimeout();
		}

		@@ -670,19 +668,32 @@ public class ScreenshotController {
		mScreenshotView.prepareScrollingTransition(response, mScreenBitmap, newScreenshot,
		mScreenshotTakenInPortrait);
		// delay starting scroll capture to make sure the scrim is up before the app moves
		mScreenshotView.post(() -> {
		mScreenshotView.post(() -> runBatchScrollCapture(response));
		});
		} catch (CancellationException e) {
		// Ignore
		} catch (InterruptedException \| ExecutionException e) {
		Log.e(TAG, "requestScrollCapture failed", e);
		}
		}
		ListenableFuture<ScrollCaptureController.LongScreenshot> mLongScreenshotFuture;

		private void runBatchScrollCapture(ScrollCaptureResponse response) {
		// Clear the reference to prevent close() in dismissScreenshot
		mLastScrollCaptureResponse = null;
		final ListenableFuture<ScrollCaptureController.LongScreenshot> future =
		mScrollCaptureController.run(response);
		future.addListener(() -> {
		ScrollCaptureController.LongScreenshot longScreenshot;

		if (mLongScreenshotFuture != null) {
		mLongScreenshotFuture.cancel(true);
		}
		mLongScreenshotFuture = mScrollCaptureController.run(response);
		mLongScreenshotFuture.addListener(() -> {
		ScrollCaptureController.LongScreenshot longScreenshot;
		try {
		longScreenshot = future.get();
		} catch (CancellationException
		\| InterruptedException
		\| ExecutionException e) {
		longScreenshot = mLongScreenshotFuture.get();
		} catch (CancellationException e) {
		Log.e(TAG, "Long screenshot cancelled");
		return;
		} catch (InterruptedException \| ExecutionException e) {
		Log.e(TAG, "Exception", e);
		mScreenshotView.restoreNonScrollingUi();
		return;
		@@ -715,13 +726,6 @@ public class ScreenshotController {
		Log.e(TAG, "Error overriding screenshot app transition", e);
		}
		}, mMainExecutor);
		});
		});
		} catch (CancellationException e) {
		// Ignore
		} catch (InterruptedException \| ExecutionException e) {
		Log.e(TAG, "requestScrollCapture failed", e);
		}
		}

		private void withWindowAttached(Runnable action) {
		@@ -824,16 +828,27 @@ public class ScreenshotController {

		/** Reset screenshot view and then call onCompleteRunnable */
		private void finishDismiss() {
		if (DEBUG_UI) {
		if (DEBUG_DISMISS) {
		Log.d(TAG, "finishDismiss");
		}
		cancelTimeout();
		removeWindow();
		mScreenshotView.reset();
		if (mLastScrollCaptureRequest != null) {
		mLastScrollCaptureRequest.cancel(true);
		mLastScrollCaptureRequest = null;
		}
		if (mLastScrollCaptureResponse != null) {
		mLastScrollCaptureResponse.close();
		mLastScrollCaptureResponse = null;
		}
		if (mLongScreenshotFuture != null) {
		mLongScreenshotFuture.cancel(true);
		}
		if (mCurrentRequestCallback != null) {
		mCurrentRequestCallback.onFinish();
		mCurrentRequestCallback = null;
		}
		mScreenshotView.reset();
		removeWindow();
		cancelTimeout();
		}

		/**
		@@ -861,6 +876,9 @@ public class ScreenshotController {
		}

		private void cancelTimeout() {
		if (DEBUG_DISMISS) {
		Log.d(TAG, "cancel timeout");
		}
		mScreenshotHandler.removeMessages(MESSAGE_CORNER_TIMEOUT);
		}

		@@ -876,7 +894,7 @@ public class ScreenshotController {
		mScreenshotHandler.sendMessageDelayed(
		mScreenshotHandler.obtainMessage(MESSAGE_CORNER_TIMEOUT),
		timeoutMs);
		if (DEBUG_UI) {
		if (DEBUG_DISMISS) {
		Log.d(TAG, "dismiss timeout: " + timeoutMs + " ms");
		}

packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotView.java

+6 −6

Original line number	Diff line number	Diff line
		@@ -1117,9 +1117,11 @@ public class ScreenshotView extends FrameLayout implements
		mPreviousX = mStartX;
		return true;
		} else if (event.getActionMasked() == MotionEvent.ACTION_UP) {
		if (isPastDismissThreshold()
		&& (mDismissAnimation == null \|\| !mDismissAnimation.isRunning())) {
		if (DEBUG_INPUT) {
		if (mDismissAnimation != null && mDismissAnimation.isRunning()) {
		return true;
		}
		if (isPastDismissThreshold()) {
		if (DEBUG_DISMISS) {
		Log.d(TAG, "dismiss triggered via swipe gesture");
		}
		mUiEventLogger.log(ScreenshotEvent.SCREENSHOT_SWIPE_DISMISSED);
		@@ -1129,10 +1131,8 @@ public class ScreenshotView extends FrameLayout implements
		if (DEBUG_DISMISS) {
		Log.d(TAG, "swipe gesture abandoned");
		}
		if ((mDismissAnimation == null \|\| !mDismissAnimation.isRunning())) {
		createSwipeReturnAnimation().start();
		}
		}
		return true;
		}
		return gestureResult;