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

Commit 39a71338 authored by Mihai Popa's avatar Mihai Popa
Browse files

[Magnifier-25] Fix race condition after #dismiss 

The CL adds synchronization around the InternalPopupWindow instance
used, between the main thread and the thread that handles pixel copy
results. This comes to fix a potential null pointer exception that
might occur after a #dismiss().

Bug: 73765118
Test: atest CtsWidgetTestCases:android.widget.cts.MagnifierTest
Change-Id: I8a8feb02f3ee418597ce3eee50db77b4b67e462e
parent 3d529f76

core/java/android/widget/Magnifier.java

+26 −14
Original line number Diff line number Diff line
@@ -89,6 +89,9 @@ public final class Magnifier { 
            NONEXISTENT_PREVIOUS_CONFIG_VALUE, NONEXISTENT_PREVIOUS_CONFIG_VALUE);

    // Rectangle defining the view surface area we pixel copy content from.

    private final Rect mPixelCopyRequestRect = new Rect();

    // Lock to synchronize between the UI thread and the thread that handles pixel copy results.

    // Only sync mWindow writes from UI thread with mWindow reads from sPixelCopyHandlerThread.

    private final Object mLock = new Object();



    /**

     * Initializes a magnifier.
@@ -170,9 +173,12 @@ public final class Magnifier { 


        if (xPosInView != mPrevPosInView.x || yPosInView != mPrevPosInView.y) {

            if (mWindow == null) {

                synchronized (mLock) {

                    mWindow = new InternalPopupWindow(mView.getContext(), mView.getDisplay(),

                            getValidViewSurface(), mWindowWidth, mWindowHeight, mWindowElevation,

                        Handler.getMain() /* draw the magnifier on the UI thread */, mCallback);

                            Handler.getMain() /* draw the magnifier on the UI thread */, mLock,

                            mCallback);

                }

            }

            performPixelCopy(startX, startY, true /* update window position */);

            mPrevPosInView.x = xPosInView;
@@ -200,10 +206,12 @@ public final class Magnifier { 
     */

    public void dismiss() {

        if (mWindow != null) {

            synchronized (mLock) {

                mWindow.destroy();

                mWindow = null;

            }

        }

    }



    /**

     * Forces the magnifier to update its content. It uses the previous coordinates passed to
@@ -281,20 +289,23 @@ public final class Magnifier { 
                clampedStartYInSurface + mBitmapHeight);

        final int windowCoordsX = mWindowCoords.x;

        final int windowCoordsY = mWindowCoords.y;

        final InternalPopupWindow currentWindowInstance = mWindow;



        final Bitmap bitmap =

                Bitmap.createBitmap(mBitmapWidth, mBitmapHeight, Bitmap.Config.ARGB_8888);

        PixelCopy.request(surface, mPixelCopyRequestRect, bitmap,

                result -> {

                    synchronized (mWindow.mLock) {

                        if (mWindow != null) {

                    synchronized (mLock) {

                        if (mWindow != currentWindowInstance) {

                            // The magnifier was dismissed (and maybe shown again) in the meantime.

                            return;

                        }

                        if (updateWindowPosition) {

                            // TODO: pull the position update outside #performPixelCopy

                            mWindow.setContentPositionForNextDraw(windowCoordsX, windowCoordsY);

                        }

                        mWindow.updateContent(bitmap);

                    }

                    }

                },

                sPixelCopyHandlerThread.getThreadHandler());

        mPrevStartCoordsInSurface.x = startXInSurface;
@@ -337,7 +348,7 @@ public final class Magnifier { 
        // Members below describe the state of the magnifier. Reads/writes to them

        // have to be synchronized between the UI thread and the thread that handles

        // the pixel copy results. This is the purpose of mLock.

        private final Object mLock = new Object();

        private final Object mLock;

        // Whether a magnifier frame draw is currently pending in the UI thread queue.

        private boolean mFrameDrawScheduled;

        // The content bitmap.
@@ -353,8 +364,9 @@ public final class Magnifier { 
        InternalPopupWindow(final Context context, final Display display,

                final Surface parentSurface,

                final int width, final int height, final float elevation,

                final Handler handler, final Callback callback) {

                final Handler handler, final Object lock, final Callback callback) {

            mDisplay = display;

            mLock = lock;

            mCallback = callback;



            mContentWidth = width;