Merge "Null and thread safety fixes" into main (7fc93439) · Commits · e / os / android_frameworks_base

core/java/android/view/ScrollCaptureConnection.java

+34 −10

Original line number	Diff line number	Diff line
		@@ -20,8 +20,9 @@ import static android.os.Trace.TRACE_TAG_GRAPHICS;

		import static java.util.Objects.requireNonNull;

		import android.annotation.BinderThread;
		import android.annotation.AnyThread;
		import android.annotation.NonNull;
		import android.annotation.Nullable;
		import android.annotation.UiThread;
		import android.graphics.Point;
		import android.graphics.Rect;
		@@ -64,9 +65,13 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		private final Executor mUiThread;
		private final CloseGuard mCloseGuard = new CloseGuard();

		@Nullable
		private ScrollCaptureCallback mLocal;
		@Nullable
		private IScrollCaptureCallbacks mRemote;
		@Nullable
		private ScrollCaptureSession mSession;
		@Nullable
		private CancellationSignal mCancellation;

		private volatile boolean mActive;
		@@ -92,7 +97,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		mPositionInWindow = new Point(selectedTarget.getPositionInWindow());
		}

		@BinderThread
		@AnyThread
		@Override
		public ICancellationSignal startCapture(@NonNull Surface surface,
		@NonNull IScrollCaptureCallbacks remote) throws RemoteException {
		@@ -115,7 +120,11 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		Runnable listener =
		SafeCallback.create(mCancellation, mUiThread, this::onStartCaptureCompleted);
		// -> UiThread
		mUiThread.execute(() -> mLocal.onScrollCaptureStart(mSession, mCancellation, listener));
		mUiThread.execute(() -> {
		if (mLocal != null && mCancellation != null) {
		mLocal.onScrollCaptureStart(mSession, mCancellation, listener);
		}
		});
		return cancellation;
		}

		@@ -123,7 +132,11 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		private void onStartCaptureCompleted() {
		mActive = true;
		try {
		if (mRemote != null) {
		mRemote.onCaptureStarted();
		} else {
		close();
		}
		} catch (RemoteException e) {
		Log.w(TAG, "Shutting down due to error: ", e);
		close();
		@@ -132,7 +145,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		Trace.asyncTraceForTrackEnd(TRACE_TAG_GRAPHICS, TRACE_TRACK, mTraceId);
		}

		@BinderThread
		@AnyThread
		@Override
		public ICancellationSignal requestImage(Rect requestRect) throws RemoteException {
		Trace.asyncTraceForTrackBegin(TRACE_TAG_GRAPHICS, TRACE_TRACK, REQUEST_IMAGE, mTraceId);
		@@ -145,7 +158,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		SafeCallback.create(mCancellation, mUiThread, this::onImageRequestCompleted);
		// -> UiThread
		mUiThread.execute(() -> {
		if (mLocal != null) {
		if (mLocal != null && mSession != null && mCancellation != null) {
		mLocal.onScrollCaptureImageRequest(
		mSession, mCancellation, new Rect(requestRect), listener);
		}
		@@ -157,7 +170,11 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		@UiThread
		void onImageRequestCompleted(Rect capturedArea) {
		try {
		if (mRemote != null) {
		mRemote.onImageRequestCompleted(0, capturedArea);
		} else {
		close();
		}
		} catch (RemoteException e) {
		Log.w(TAG, "Shutting down due to error: ", e);
		close();
		@@ -167,7 +184,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		Trace.asyncTraceForTrackEnd(TRACE_TAG_GRAPHICS, TRACE_TRACK, mTraceId);
		}

		@BinderThread
		@AnyThread
		@Override
		public ICancellationSignal endCapture() throws RemoteException {
		Trace.asyncTraceForTrackBegin(TRACE_TAG_GRAPHICS, TRACE_TRACK, END_CAPTURE, mTraceId);
		@@ -212,7 +229,7 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple

		}

		@BinderThread
		@AnyThread
		@Override
		public synchronized void close() {
		Trace.instantForTrack(TRACE_TAG_GRAPHICS, TRACE_TRACK, "close");
		@@ -220,7 +237,11 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		Log.w(TAG, "close(): capture session still active! Ending now.");
		cancelPendingAction();
		final ScrollCaptureCallback callback = mLocal;
		mUiThread.execute(() -> callback.onScrollCaptureEnd(() -> { /* ignore */ }));
		mUiThread.execute(() -> {
		if (callback != null) {
		callback.onScrollCaptureEnd(() -> { /* ignore */ });
		}
		});
		mActive = false;
		}
		if (mRemote != null) {
		@@ -297,10 +318,13 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple
		protected final void maybeAccept(Consumer<T> consumer) {
		T value = mValue.getAndSet(null);
		if (mSignal.isCanceled()) {
		Log.w(TAG, "callback ignored, operation already cancelled");
		return;
		}
		if (value != null) {
		mExecutor.execute(() -> consumer.accept(value));
		} else {
		Log.w(TAG, "callback ignored, value already delivered");
		}
		}

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

+62 −8

Original line number	Diff line number	Diff line
		@@ -16,8 +16,6 @@

		package android.view;

		import static androidx.test.InstrumentationRegistry.getTargetContext;

		import static org.junit.Assert.assertFalse;
		import static org.junit.Assert.assertTrue;
		import static org.junit.Assert.fail;
		@@ -32,7 +30,6 @@ 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;
		@@ -54,7 +51,6 @@ import java.util.concurrent.Executor;
		/**
		* Tests of {@link ScrollCaptureConnection}.
		*/
		@SuppressWarnings("UnnecessaryLocalVariable")
		@Presubmit
		@SmallTest
		@RunWith(AndroidJUnit4.class)
		@@ -68,9 +64,8 @@ public class ScrollCaptureConnectionTest {

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

		private Handler mHandler;

		@Mock
		private Surface mSurface;
		@@ -85,7 +80,6 @@ public class ScrollCaptureConnectionTest {
		@Before
		public void setUp() {
		MockitoAnnotations.initMocks(this);
		mHandler = new Handler(getTargetContext().getMainLooper());
		when(mSurface.isValid()).thenReturn(true);
		when(mView.getScrollCaptureHint()).thenReturn(View.SCROLL_CAPTURE_HINT_INCLUDE);
		when(mRemote.asBinder()).thenReturn(mConnectionBinder);
		@@ -269,8 +263,68 @@ public class ScrollCaptureConnectionTest {
		assertFalse(mConnection.isConnected());
		}

		@Test(expected = RemoteException.class)
		public void testRequestImage_beforeStarted() throws RemoteException {
		mConnection.requestImage(new Rect(0, 1, 2, 3));
		}


		@Test(expected = RemoteException.class)
		public void testRequestImage_beforeStartCompleted() throws RemoteException {
		mFakeUiThread.setImmediate(false);
		mConnection.startCapture(mSurface, mRemote);
		mConnection.requestImage(new Rect(0, 1, 2, 3));
		mFakeUiThread.runAll();
		}

		@Test
		public void testCompleteStart_afterClosing() throws RemoteException {
		mConnection.startCapture(mSurface, mRemote);
		mConnection.close();
		mFakeUiThread.setImmediate(false);
		mCallback.completeStartRequest();
		mFakeUiThread.runAll();
		}

		@Test
		public void testLateCallbacks() throws RemoteException {
		mConnection.startCapture(mSurface, mRemote);
		mCallback.completeStartRequest();
		mConnection.requestImage(new Rect(1, 2, 3, 4));
		mConnection.endCapture();
		mFakeUiThread.setImmediate(false);
		mCallback.completeImageRequest(new Rect(1, 2, 3, 4));
		mCallback.completeEndRequest();
		mFakeUiThread.runAll();
		}

		@Test
		public void testDelayedClose() throws RemoteException {
		mConnection.startCapture(mSurface, mRemote);
		mCallback.completeStartRequest();
		mFakeUiThread.setImmediate(false);
		mConnection.endCapture();
		mFakeUiThread.runAll();
		mConnection.close();
		mCallback.completeEndRequest();
		mFakeUiThread.runAll();
		}

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

		ICancellationSignal signal = mConnection.requestImage(new Rect(1, 2, 3, 4));
		mFakeUiThread.setImmediate(false);

		signal.cancel();
		mCallback.completeImageRequest(new Rect(1, 2, 3, 4));
		}


		static class FakeExecutor implements Executor {
		private Queue<Runnable> mQueue = new ArrayDeque<>();
		private final Queue<Runnable> mQueue = new ArrayDeque<>();
		private boolean mImmediate;

		@Override

packages/SystemUI/multivalentTests/src/com/android/systemui/screenshot/scroll/ScrollCaptureControllerTest.java

+15 −0

Original line number	Diff line number	Diff line
		@@ -204,6 +204,21 @@ public class ScrollCaptureControllerTest extends SysuiTestCase {
		assertEquals("bottom", 200, screenshot.getBottom());
		}

		@Test
		public void testCancellation() {
		ScrollCaptureController controller = new TestScenario()
		.withPageHeight(100)
		.withMaxPages(2.5f)
		.withTileHeight(10)
		.withAvailableRange(-10, Integer.MAX_VALUE)
		.createController(mContext);

		ScrollCaptureController.LongScreenshot screenshot =
		getUnchecked(controller.run(EMPTY_RESPONSE));

		assertEquals("top", -10, screenshot.getTop());
		assertEquals("bottom", 240, screenshot.getBottom());
		}
		/**
		* Build and configure a stubbed controller for each test case.
		*/

packages/SystemUI/src/com/android/systemui/screenshot/scroll/ScrollCaptureController.java

+18 −5

Original line number	Diff line number	Diff line
		@@ -24,6 +24,8 @@ import android.provider.Settings;
		import android.util.Log;
		import android.view.ScrollCaptureResponse;

		import androidx.annotation.NonNull;
		import androidx.annotation.Nullable;
		import androidx.concurrent.futures.CallbackToFutureAdapter;
		import androidx.concurrent.futures.CallbackToFutureAdapter.Completer;

		@@ -68,11 +70,15 @@ public class ScrollCaptureController {
		private final UiEventLogger mEventLogger;
		private final ScrollCaptureClient mClient;

		@Nullable
		private Completer<LongScreenshot> mCaptureCompleter;

		@Nullable
		private ListenableFuture<Session> mSessionFuture;
		private Session mSession;
		@Nullable
		private ListenableFuture<CaptureResult> mTileFuture;
		@Nullable
		private ListenableFuture<Void> mEndFuture;
		private String mWindowOwner;
		private volatile boolean mCancelled;
		@@ -148,8 +154,9 @@ public class ScrollCaptureController {
		}

		@Inject
		ScrollCaptureController(Context context, @Background Executor bgExecutor,
		ScrollCaptureClient client, ImageTileSet imageTileSet, UiEventLogger logger) {
		ScrollCaptureController(@NonNull Context context, @Background Executor bgExecutor,
		@NonNull ScrollCaptureClient client, @NonNull ImageTileSet imageTileSet,
		@NonNull UiEventLogger logger) {
		mContext = context;
		mBgExecutor = bgExecutor;
		mClient = client;
		@@ -214,7 +221,9 @@ public class ScrollCaptureController {
		} catch (InterruptedException \| ExecutionException e) {
		// Failure to start, propagate to caller
		Log.e(TAG, "session start failed!");
		if (mCaptureCompleter != null) {
		mCaptureCompleter.setException(e);
		}
		mEventLogger.log(ScreenshotEvent.SCREENSHOT_LONG_SCREENSHOT_FAILURE, 0, mWindowOwner);
		}
		}
		@@ -235,8 +244,10 @@ public class ScrollCaptureController {
		Log.e(TAG, "requestTile cancelled");
		} catch (InterruptedException \| ExecutionException e) {
		Log.e(TAG, "requestTile failed!", e);
		if (mCaptureCompleter != null) {
		mCaptureCompleter.setException(e);
		}
		}
		}, mBgExecutor);
		}

		@@ -350,7 +361,9 @@ public class ScrollCaptureController {
		}
		// Provide result to caller and complete the top-level future
		// Caller is responsible for releasing this resource (ImageReader/HardwareBuffers)
		if (mCaptureCompleter != null) {
		mCaptureCompleter.set(new LongScreenshot(mSession, mImageTileSet));
		}
		}, mContext.getMainExecutor());
		}
		}