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

Commit 3370e206 authored by Johannes Gallmann's avatar Johannes Gallmann
Browse files

Fix cancel event sent to wrong back callback 

When a callback is registered while a back gesture is in progress,
WindowOnBackInvokedDispatcher continues to dispatch the back events
to the active callback. It does so even when the newly registered
callback ends up being the new top callback in the callback stack.

A few code places in WindowOnBackInvokedDispatcher assumed that an
active gesture must always be dispatching to the top callback. However,
as the previous paragraph explains, that's not always the case. This CL
fixes this edge case problem.

Bug: 420650888
Test: WindowOnBackInvokedDispatcher
Flag: com.android.window.flags.predictive_back_callback_cancellation_fix
Change-Id: Iee98ee2222394ec4805c6e689347796771b636eb
parent 361ecf28

core/java/android/window/BackProgressAnimator.java

+29 −2
Original line number Diff line number Diff line
@@ -55,6 +55,8 @@ public class BackProgressAnimator implements DynamicAnimation.OnAnimationUpdateL 
    private static final float BUTTON_SPRING_STIFFNESS = 100;

    private final SpringAnimation mSpring;

    private ProgressCallback mCallback;

    @Nullable

    private OnBackAnimationCallback mBackCallback;

    private float mProgress = 0;

    private float mVelocity = 0;

    private BackMotionEvent mLastBackEvent;
@@ -146,8 +148,22 @@ public class BackProgressAnimator implements DynamicAnimation.OnAnimationUpdateL 
     * Starts the back progress animation.

     *

     * @param event the {@link BackMotionEvent} that started the gesture.

     * @param callback the back callback to invoke for the gesture. It will receive back progress

     *                 dispatches as the progress animation updates.

     * @param callback the progress callback to invoke for the gesture. It will receive back

     *                 progress dispatches as the progress animation updates.

     * @param backCallback the target back callback of the current back gesture

     */

    public void onBackStarted(BackMotionEvent event, ProgressCallback callback,

            @NonNull OnBackAnimationCallback backCallback) {

        mBackCallback = backCallback;

        onBackStarted(event, callback);

    }



    /**

     * Starts the back progress animation.

     *

     * @param event the {@link BackMotionEvent} that started the gesture.

     * @param callback the progress callback to invoke for the gesture. It will receive back

     *                 progress dispatches as the progress animation updates.

     */

    public void onBackStarted(BackMotionEvent event, ProgressCallback callback) {

        mLastBackEvent = event;
@@ -195,6 +211,7 @@ public class BackProgressAnimator implements DynamicAnimation.OnAnimationUpdateL 
        mBackAnimationInProgress = false;

        mLastBackEvent = null;

        mCallback = null;

        mBackCallback = null;

        mProgress = 0;

    }
@@ -260,6 +277,16 @@ public class BackProgressAnimator implements DynamicAnimation.OnAnimationUpdateL 
        return mBackAnimationInProgress;

    }



    /**

     *

     * If provided in {@link  BackProgressAnimator#onBackStarted}, returns the back callback while

     * the animation is in progress. Otherwise returns null.

     */

    @Nullable

    public OnBackAnimationCallback getActiveBackCallback() {

        return mBackCallback;

    }



    /**

     * @return The last recorded velocity. Unit: change in progress per second

     */

core/java/android/window/WindowOnBackInvokedDispatcher.java

+36 −12
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package android.window; 
import static android.window.SystemOverrideOnBackInvokedCallback.OVERRIDE_UNDEFINED;



import static com.android.window.flags.Flags.multipleSystemNavigationObserverCallbacks;

import static com.android.window.flags.Flags.predictiveBackCallbackCancellationFix;

import static com.android.window.flags.Flags.predictiveBackSystemOverrideCallback;

import static com.android.window.flags.Flags.predictiveBackPrioritySystemNavigationObserver;

import static com.android.window.flags.Flags.predictiveBackTimestampApi;
@@ -318,16 +319,27 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { 
            mOnBackInvokedCallbacks.remove(priority);

        }

        mAllCallbacks.remove(callback);

        // Re-populate the top callback to WM if the removed callback was previously the top

        // one.

        // Re-populate the top callback to WM if the removed callback was previously the top one.

        if (previousTopCallback == callback) {

            // We should call onBackCancelled() when an active callback is removed from

            setTopOnBackInvokedCallback(getTopCallback());

            if (!predictiveBackCallbackCancellationFix()) {

                // We should call onBackCancelled() when an active callback is removed from the

                // dispatcher.

                mProgressAnimator.removeOnBackCancelledFinishCallback();

                mProgressAnimator.removeOnBackInvokedFinishCallback();

                sendCancelledIfInProgress(callback);

                mHandler.post(mProgressAnimator::reset);

            }

        }



        if (predictiveBackCallbackCancellationFix() && mProgressAnimator.isBackAnimationInProgress()

                && mProgressAnimator.getActiveBackCallback() == callback) {

            // We should call onBackCancelled() when an active callback is removed from the

            // dispatcher.

            mProgressAnimator.removeOnBackCancelledFinishCallback();

            mProgressAnimator.removeOnBackInvokedFinishCallback();

            sendCancelledIfInProgress(callback);

            mHandler.post(mProgressAnimator::reset);

            setTopOnBackInvokedCallback(getTopCallback());

        }

    }
@@ -358,8 +370,10 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { 
        }

    }



    private void sendCancelledIfInProgress(@NonNull OnBackInvokedCallback callback) {

        boolean isInProgress = mProgressAnimator.isBackAnimationInProgress();

    private void sendCancelledIfInProgress(@Nullable OnBackInvokedCallback callback) {

        boolean isInProgress = callback != null && mProgressAnimator.isBackAnimationInProgress()

                && (!predictiveBackCallbackCancellationFix()

                        || mProgressAnimator.getActiveBackCallback() == callback);

        if (isInProgress && callback instanceof OnBackAnimationCallback) {

            OnBackAnimationCallback animatedCallback = (OnBackAnimationCallback) callback;

            animatedCallback.onBackCancelled();
@@ -382,6 +396,9 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { 
                mImeDispatcher = null;

            }

            if (!mAllCallbacks.isEmpty()) {

                if (predictiveBackCallbackCancellationFix()) {

                    sendCancelledIfInProgress(mProgressAnimator.getActiveBackCallback());

                } else {

                    OnBackInvokedCallback topCallback = getTopCallback();

                    if (topCallback != null) {

                        sendCancelledIfInProgress(topCallback);
@@ -389,6 +406,7 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { 
                        // Should not be possible

                        Log.e(TAG, "There is no topCallback, even if mAllCallbacks is not empty");

                    }

                }

                // Clear binder references in WM.

                setTopOnBackInvokedCallback(null);

            }
@@ -598,7 +616,13 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { 


                if (callback != null) {

                    callback.onBackStarted(BackEvent.fromBackMotionEvent(backEvent));

                    if (predictiveBackCallbackCancellationFix()) {

                        mProgressAnimator.onBackStarted(backEvent, callback::onBackProgressed,

                                callback);

                    } else {

                        mProgressAnimator.onBackStarted(backEvent, callback::onBackProgressed);



                    }

                }

            });

        }

core/java/android/window/flags/windowing_frontend.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -488,3 +488,13 @@ flag { 
        purpose: PURPOSE_BUGFIX

    }

}



flag {

    name: "predictive_back_callback_cancellation_fix"

    namespace: "windowing_frontend"

    description: "Fixes a bug that caused the wrong callback to receive onBackCancelled sometimes"

    bug: "420650888"

    metadata {

        purpose: PURPOSE_BUGFIX

    }

}
 
 No newline at end of file

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

+59 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import static android.window.OnBackInvokedDispatcher.PRIORITY_OVERLAY; 
import static android.window.OnBackInvokedDispatcher.PRIORITY_SYSTEM_NAVIGATION_OBSERVER;



import static com.android.window.flags.Flags.FLAG_MULTIPLE_SYSTEM_NAVIGATION_OBSERVER_CALLBACKS;

import static com.android.window.flags.Flags.FLAG_PREDICTIVE_BACK_CALLBACK_CANCELLATION_FIX;

import static com.android.window.flags.Flags.FLAG_PREDICTIVE_BACK_PRIORITY_SYSTEM_NAVIGATION_OBSERVER;

import static com.android.window.flags.Flags.FLAG_PREDICTIVE_BACK_TIMESTAMP_API;
@@ -442,6 +443,64 @@ public class WindowOnBackInvokedDispatcherTest { 
        verify(mCallback1).onBackCancelled();

    }



    @Test

    @RequiresFlagsEnabled(FLAG_PREDICTIVE_BACK_CALLBACK_CANCELLATION_FIX)

    public void onDetachFromWindow_cancelsInProgressNonTopCallback() throws RemoteException {

        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback1);



        OnBackInvokedCallbackInfo callbackInfo = assertSetCallbackInfo();



        callbackInfo.getCallback().onBackStarted(mBackEvent);



        waitForIdle();

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



        // Register another callback. Back event dispatching continues to be routed to mCallback1

        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback2);



        // This should trigger mCallback1.onBackCancelled()

        mDispatcher.detachFromWindow();

        // This should be ignored by mCallback1

        callbackInfo.getCallback().onBackInvoked();



        waitForIdle();

        verify(mCallback1).onBackCancelled();

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

        verify(mCallback2, never()).onBackStarted(any());

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

        verify(mCallback2, never()).onBackInvoked();

    }



    @Test

    @RequiresFlagsEnabled(FLAG_PREDICTIVE_BACK_CALLBACK_CANCELLATION_FIX)

    public void callbackEvents_continueAfterNewRegistration_andUnregistration()

            throws RemoteException {

        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback1);



        OnBackInvokedCallbackInfo callbackInfo = assertSetCallbackInfo();



        callbackInfo.getCallback().onBackStarted(mBackEvent);



        waitForIdle();

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



        // Register another callback. Back event dispatching continues to be routed to mCallback1

        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback2);

        waitForIdle();



        // Unregister the second callback again

        mDispatcher.unregisterOnBackInvokedCallback(mCallback2);



        callbackInfo.getCallback().onBackInvoked();



        waitForIdle();

        verify(mCallback1).onBackInvoked();

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

        verify(mCallback2, never()).onBackStarted(any());

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

        verify(mCallback2, never()).onBackInvoked();

    }



    @Test

    public void updatesDispatchingState() throws RemoteException {

        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback1);