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

Commit b3477a65 authored by Johannes Gallmann's avatar Johannes Gallmann Committed by Android (Google) Code Review
Browse files

Merge "Fix cancel event sent to wrong back callback" into main

parents 95eab5b8 3370e206
Loading
Loading
Loading
Loading
+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
     */
+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);

                    }
                }
            });
        }
+10 −0
Original line number Diff line number Diff line
@@ -465,3 +465,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
+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);