Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ac9cef96 authored by Mark Renouf's avatar Mark Renouf
Browse files

Avoid NPE on ScrollCaptureConnection callbacks 

State is checked at the front half of some calls, but is
dispatched onto the UI thread. If the connection is closed
in this time, the queued call will NPE when the field it
uses is cleared.

This adds a simple null check in two places, and test for both.

Bug: 232375183
Test: atest ScrollCaptureConnectionTest
Change-Id: Ida7fbf0a21c206db8d774dc247b6ab5257dacabe
parent 5843318c

core/java/android/view/ScrollCaptureConnection.java

+12 −3
Original line number Diff line number Diff line
@@ -144,8 +144,13 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple 
        Consumer<Rect> listener =

                SafeCallback.create(mCancellation, mUiThread, this::onImageRequestCompleted);

        // -> UiThread

        mUiThread.execute(() -> mLocal.onScrollCaptureImageRequest(

                mSession, mCancellation, new Rect(requestRect), listener));

        mUiThread.execute(() -> {

            if (mLocal != null) {

                mLocal.onScrollCaptureImageRequest(

                        mSession, mCancellation, new Rect(requestRect), listener);

            }

        });



        return cancellation;

    }
@@ -174,7 +179,11 @@ public class ScrollCaptureConnection extends IScrollCaptureConnection.Stub imple 
        Runnable listener =

                SafeCallback.create(mCancellation, mUiThread, this::onEndCaptureCompleted);

        // -> UiThread

        mUiThread.execute(() -> mLocal.onScrollCaptureEnd(listener));

        mUiThread.execute(() -> {

            if (mLocal != null) {

                mLocal.onScrollCaptureEnd(listener);

            }

        });

        return cancellation;

    }

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

+64 −1
Original line number Diff line number Diff line
@@ -47,6 +47,10 @@ import org.junit.runner.RunWith; 
import org.mockito.Mock;

import org.mockito.MockitoAnnotations;



import java.util.ArrayDeque;

import java.util.Queue;

import java.util.concurrent.Executor;



/**

 * Tests of {@link ScrollCaptureConnection}.

 */
@@ -56,6 +60,7 @@ import org.mockito.MockitoAnnotations; 
@RunWith(AndroidJUnit4.class)

public class ScrollCaptureConnectionTest {





    private final Point mPositionInWindow = new Point(1, 2);

    private final Rect mLocalVisibleRect = new Rect(2, 3, 4, 5);

    private final Rect mScrollBounds = new Rect(3, 4, 5, 6);
@@ -73,6 +78,9 @@ public class ScrollCaptureConnectionTest { 
    private IScrollCaptureCallbacks mRemote;

    @Mock

    private View mView;

    private FakeExecutor mFakeUiThread;



    private final Executor mImmediateExecutor = Runnable::run;



    @Before

    public void setUp() {
@@ -84,7 +92,9 @@ public class ScrollCaptureConnectionTest { 


        mTarget = new ScrollCaptureTarget(mView, mLocalVisibleRect, mPositionInWindow, mCallback);

        mTarget.setScrollBounds(mScrollBounds);

        mConnection = new ScrollCaptureConnection(Runnable::run, mTarget);

        mFakeUiThread = new FakeExecutor();

        mFakeUiThread.setImmediate(true);

        mConnection = new ScrollCaptureConnection(mFakeUiThread, mTarget);

    }



    /** Test creating a client with valid info */
@@ -145,6 +155,21 @@ public class ScrollCaptureConnectionTest { 
                .onImageRequestCompleted(eq(0), eq(new Rect(1, 2, 3, 4)));

    }



    /** Test closing while callbacks are in-flight: b/232375183 */

    @Test

    public void testRequestImage_afterClose() throws Exception {

        mConnection = new ScrollCaptureConnection(mFakeUiThread, mTarget);

        mConnection.startCapture(mSurface, mRemote);

        mCallback.completeStartRequest();



        mFakeUiThread.setImmediate(false);

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



        // Now close connection, before the UI thread executes

        mConnection.close();

        mFakeUiThread.runAll();

    }



    @Test

    public void testRequestImage_cancellation() throws Exception {

        mConnection.startCapture(mSurface, mRemote);
@@ -176,6 +201,20 @@ public class ScrollCaptureConnectionTest { 
        assertFalse(mConnection.isConnected());

    }



    /** Test closing while callbacks are in-flight: b/232375183 */

    @Test

    public void testEndCapture_afterClose() throws Exception {

        mConnection.startCapture(mSurface, mRemote);

        mCallback.completeStartRequest();



        mFakeUiThread.setImmediate(false);

        mConnection.endCapture();



        // Now close connection, before the UI thread executes

        mConnection.close();

        mFakeUiThread.runAll();

    }



    /** @see ScrollCaptureConnection#endCapture() */

    @Test

    public void testClose_withPendingOperation() throws Exception {
@@ -229,4 +268,28 @@ public class ScrollCaptureConnectionTest { 
        assertFalse(mConnection.isActive());

        assertFalse(mConnection.isConnected());

    }



    static class FakeExecutor implements Executor {

        private Queue<Runnable> mQueue = new ArrayDeque<>();

        private boolean mImmediate;



        @Override

        public void execute(Runnable command) {

            if (mImmediate) {

                command.run();

            } else {

                mQueue.add(command);

            }

        }



        void setImmediate(boolean immediate) {

            mImmediate = immediate;

        }



        public void runAll() {

            while (!mQueue.isEmpty()) {

                mQueue.remove().run();

            }

        }

    }

}