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

Commit 5f43140e authored by Johannes Gallmann's avatar Johannes Gallmann
Browse files

Fix app callback broken when immediately restarting back gesture after cancel 

This fixes two bugs:
1. When calling onBackStarted, we need to ensure that onBackCancelled of a potential previously cancelled gesture is called first. onBackCancelled can be delayed because the progress animates to 0 before the callback is invoked. Otherwise onBackStarted of the second gesture can be called before onBackCancelled of the first gesture is called.
2. The BackTouchTracker should be reset when the user gesture ends (instead of when BackProgressAnimator finished animating the progress back to 0). That ensures that a new gesture that starts between the gesture end and the progress reaching zero does not update the BackProgressAnimator of the previous (cancelled) gesture with new MotionEvents.

Bug: 338021694
Flag: NONE
Test: atest FrameworksCoreTests:WindowOnBackInvokedDispatcherTest
Test: Manual, i.e. verifying that onBackCancelled is dispatched before onBackStarted when immediately restarting back gesture
Change-Id: Ibe3ce291713dc5ba0512404fa47f6b17d50f7a13
parent 02aea3b1

core/java/android/view/ViewGroup.java

+4 −3
Original line number Original line Diff line number Diff line
@@ -2654,9 +2654,10 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager 
            ViewRootImpl viewRootImpl = getViewRootImpl();

            ViewRootImpl viewRootImpl = getViewRootImpl();

            if (actionMasked == MotionEvent.ACTION_DOWN || mFirstTouchTarget != null) {

            if (actionMasked == MotionEvent.ACTION_DOWN || mFirstTouchTarget != null) {

                final boolean disallowIntercept = (mGroupFlags & FLAG_DISALLOW_INTERCEPT) != 0;

                final boolean disallowIntercept = (mGroupFlags & FLAG_DISALLOW_INTERCEPT) != 0;

                final boolean isDispatchingBack = (viewRootImpl != null

                final boolean isBackGestureInProgress = (viewRootImpl != null

                        && viewRootImpl.getOnBackInvokedDispatcher().isDispatching());

                        && viewRootImpl.getOnBackInvokedDispatcher().isBackGestureInProgress());

                if (!disallowIntercept || isDispatchingBack) { // Allow back to intercept touch

                if (!disallowIntercept || isBackGestureInProgress) {

                    // Allow back to intercept touch

                    intercepted = onInterceptTouchEvent(ev);

                    intercepted = onInterceptTouchEvent(ev);

                    ev.setAction(action); // restore action in case it was changed

                    ev.setAction(action); // restore action in case it was changed

                } else {

                } else {

core/java/android/view/ViewRootImpl.java

+1 −1
Original line number Original line Diff line number Diff line
@@ -7224,7 +7224,7 @@ public final class ViewRootImpl implements ViewParent, 
        private int doOnBackKeyEvent(KeyEvent keyEvent) {

        private int doOnBackKeyEvent(KeyEvent keyEvent) {

            WindowOnBackInvokedDispatcher dispatcher = getOnBackInvokedDispatcher();

            WindowOnBackInvokedDispatcher dispatcher = getOnBackInvokedDispatcher();

            OnBackInvokedCallback topCallback = dispatcher.getTopCallback();

            OnBackInvokedCallback topCallback = dispatcher.getTopCallback();

            if (dispatcher.isDispatching()) {

            if (dispatcher.isBackGestureInProgress()) {

                return FINISH_NOT_HANDLED;

                return FINISH_NOT_HANDLED;

            }

            }

            if (topCallback instanceof OnBackAnimationCallback) {

            if (topCallback instanceof OnBackAnimationCallback) {

core/java/android/window/WindowOnBackInvokedDispatcher.java

+12 −12
Original line number Original line Diff line number Diff line
@@ -114,7 +114,7 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { 




    /** Updates the dispatcher state on a new {@link MotionEvent}. */

    /** Updates the dispatcher state on a new {@link MotionEvent}. */

    public void onMotionEvent(MotionEvent ev) {

    public void onMotionEvent(MotionEvent ev) {

        if (!isDispatching() || ev == null || ev.getAction() != MotionEvent.ACTION_MOVE) {

        if (!isBackGestureInProgress() || ev == null || ev.getAction() != MotionEvent.ACTION_MOVE) {

            return;

            return;

        }

        }

        mTouchTracker.update(ev.getX(), ev.getY(), Float.NaN, Float.NaN);

        mTouchTracker.update(ev.getX(), ev.getY(), Float.NaN, Float.NaN);
@@ -240,9 +240,9 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { 
    }

    }





    /**

    /**

     * Indicates if the dispatcher is actively dispatching to a callback.

     * Indicates if a user gesture is currently in progress.

     */

     */

    public boolean isDispatching() {

    public boolean isBackGestureInProgress() {

        synchronized (mLock) {

        synchronized (mLock) {

            return mTouchTracker.isActive() || mImeDispatchingActive;

            return mTouchTracker.isActive() || mImeDispatchingActive;

        }

        }
@@ -469,12 +469,17 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { 
        @Override

        @Override

        public void onBackStarted(BackMotionEvent backEvent) {

        public void onBackStarted(BackMotionEvent backEvent) {

            mHandler.post(() -> {

            mHandler.post(() -> {

                final OnBackAnimationCallback callback = getBackAnimationCallback();



                // reset progress animator before dispatching onBackStarted to callback. This

                // ensures that onBackCancelled (of a previous gesture) is always dispatched

                // before onBackStarted

                if (callback != null) mProgressAnimator.reset();

                mTouchTracker.setState(BackTouchTracker.TouchTrackerState.ACTIVE);

                mTouchTracker.setState(BackTouchTracker.TouchTrackerState.ACTIVE);

                mTouchTracker.setShouldUpdateStartLocation(true);

                mTouchTracker.setShouldUpdateStartLocation(true);

                mTouchTracker.setGestureStartLocation(

                mTouchTracker.setGestureStartLocation(

                        backEvent.getTouchX(), backEvent.getTouchY(), backEvent.getSwipeEdge());

                        backEvent.getTouchX(), backEvent.getTouchY(), backEvent.getSwipeEdge());





                final OnBackAnimationCallback callback = getBackAnimationCallback();

                if (callback != null) {

                if (callback != null) {

                    callback.onBackStarted(new BackEvent(

                    callback.onBackStarted(new BackEvent(

                            backEvent.getTouchX(),

                            backEvent.getTouchX(),
@@ -493,14 +498,9 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { 
        public void onBackCancelled() {

        public void onBackCancelled() {

            mHandler.post(() -> {

            mHandler.post(() -> {

                final OnBackAnimationCallback callback = getBackAnimationCallback();

                final OnBackAnimationCallback callback = getBackAnimationCallback();

                if (callback == null) {

                mTouchTracker.reset();

                mTouchTracker.reset();

                    return;

                if (callback == null) return;

                }

                mProgressAnimator.onBackCancelled(callback::onBackCancelled);

                mProgressAnimator.onBackCancelled(() -> {

                    mTouchTracker.reset();

                    callback.onBackCancelled();

                });

            });

            });

        }

        }

core/java/com/android/internal/policy/DecorView.java

+2 −2
Original line number Original line Diff line number Diff line
@@ -523,8 +523,8 @@ public class DecorView extends FrameLayout implements RootViewSurfaceTaker, Wind 
        ViewRootImpl viewRootImpl = getViewRootImpl();

        ViewRootImpl viewRootImpl = getViewRootImpl();

        if (viewRootImpl != null) {

        if (viewRootImpl != null) {

            viewRootImpl.getOnBackInvokedDispatcher().onMotionEvent(event);

            viewRootImpl.getOnBackInvokedDispatcher().onMotionEvent(event);

            // Intercept touch if back dispatching is active.

            // Intercept touch if back gesture is in progress.

            if (viewRootImpl.getOnBackInvokedDispatcher().isDispatching()) {

            if (viewRootImpl.getOnBackInvokedDispatcher().isBackGestureInProgress()) {

                return true;

                return true;

            }

            }

        }

        }

core/tests/coretests/src/android/window/WindowOnBackInvokedDispatcherTest.java

+32 −3
Original line number Original line Diff line number Diff line
@@ -50,6 +50,7 @@ import org.junit.Before; 
import org.junit.Test;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.junit.runner.RunWith;

import org.mockito.ArgumentCaptor;

import org.mockito.ArgumentCaptor;

import org.mockito.InOrder;

import org.mockito.Mock;

import org.mockito.Mock;

import org.mockito.Mockito;

import org.mockito.Mockito;

import org.mockito.MockitoAnnotations;

import org.mockito.MockitoAnnotations;
@@ -364,6 +365,34 @@ public class WindowOnBackInvokedDispatcherTest { 
        verify(mCallback1, never()).onBackCancelled();

        verify(mCallback1, never()).onBackCancelled();

    }

    }





    @Test

    public void onBackCancelled_calledBeforeOnBackStartedOfNewGesture() throws RemoteException {

        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback1);

        OnBackInvokedCallbackInfo callbackInfo = assertSetCallbackInfo();



        callbackInfo.getCallback().onBackStarted(mBackEvent);



        waitForIdle();

        verify(mCallback1).onBackStarted(any(BackEvent.class));

        clearInvocations(mCallback1);



        callbackInfo.getCallback().onBackCancelled();



        waitForIdle();

        // verify onBackCancelled not yet called (since BackProgressAnimator animates

        // progress to 0 first)

        verify(mCallback1, never()).onBackCancelled();



        // simulate start of new gesture while cancel animation is still running

        callbackInfo.getCallback().onBackStarted(mBackEvent);

        waitForIdle();



        // verify that onBackCancelled is called before onBackStarted

        InOrder orderVerifier = Mockito.inOrder(mCallback1);

        orderVerifier.verify(mCallback1).onBackCancelled();

        orderVerifier.verify(mCallback1).onBackStarted(any(BackEvent.class));

    }



    @Test

    @Test

    public void onDetachFromWindow_cancelCallbackAndIgnoreOnBackInvoked() throws RemoteException {

    public void onDetachFromWindow_cancelCallbackAndIgnoreOnBackInvoked() throws RemoteException {

        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback1);

        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback1);
@@ -392,11 +421,11 @@ public class WindowOnBackInvokedDispatcherTest { 




        callbackInfo.getCallback().onBackStarted(mBackEvent);

        callbackInfo.getCallback().onBackStarted(mBackEvent);

        waitForIdle();

        waitForIdle();

        assertTrue(mDispatcher.isDispatching());

        assertTrue(mDispatcher.isBackGestureInProgress());





        callbackInfo.getCallback().onBackInvoked();

        callbackInfo.getCallback().onBackInvoked();

        waitForIdle();

        waitForIdle();

        assertFalse(mDispatcher.isDispatching());

        assertFalse(mDispatcher.isBackGestureInProgress());

    }

    }





    @Test

    @Test
@@ -411,7 +440,7 @@ public class WindowOnBackInvokedDispatcherTest { 




        callbackInfo.getCallback().onBackStarted(mBackEvent);

        callbackInfo.getCallback().onBackStarted(mBackEvent);

        waitForIdle();

        waitForIdle();

        assertTrue(mDispatcher.isDispatching());

        assertTrue(mDispatcher.isBackGestureInProgress());

        assertTrue(mDispatcher.mTouchTracker.isActive());

        assertTrue(mDispatcher.mTouchTracker.isActive());





        main.runWithScissors(() -> mDispatcher.onMotionEvent(mMotionEvent), 100);

        main.runWithScissors(() -> mDispatcher.onMotionEvent(mMotionEvent), 100);