Merge "Cancel the back animation if the topCallback is removed or the WindowOnBackInvokedDispatcher is detached from the window." into udc-dev

core/java/android/window/WindowOnBackInvokedDispatcher.java

@@ -163,19 +163,25 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher {
         // 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 dispatcher.
-            if (mProgressAnimator.isBackAnimationInProgress()
+            sendCancelledIfInProgress(callback);
-                    && callback instanceof OnBackAnimationCallback) {
-                // The ProgressAnimator will handle the new topCallback, so we don't want to call
-                // onBackCancelled() on it. We call immediately the callback instead.
-                OnBackAnimationCallback animatedCallback = (OnBackAnimationCallback) callback;
-                animatedCallback.onBackCancelled();
-                Log.d(TAG, "The callback was removed while a back animation was in progress, "
-                        + "an onBackCancelled() was dispatched.");
-            }
             setTopOnBackInvokedCallback(getTopCallback());
         }
     }
 
+    private void sendCancelledIfInProgress(@NonNull OnBackInvokedCallback callback) {
+        boolean isInProgress = mProgressAnimator.isBackAnimationInProgress();
+        if (isInProgress && callback instanceof OnBackAnimationCallback) {
+            OnBackAnimationCallback animatedCallback = (OnBackAnimationCallback) callback;
+            animatedCallback.onBackCancelled();
+            if (DEBUG) {
+                Log.d(TAG, "sendCancelIfRunning: callback canceled");
+            }
+        } else {
+            Log.w(TAG, "sendCancelIfRunning: isInProgress=" + isInProgress
+                    + "callback=" + callback);
+        }
+    }
+
     @Override
     public void registerSystemOnBackInvokedCallback(@NonNull OnBackInvokedCallback callback) {
         registerOnBackInvokedCallbackUnchecked(callback, OnBackInvokedDispatcher.PRIORITY_SYSTEM);
@@ -188,9 +194,20 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher {
             mImeDispatcher = null;
         }
         if (!mAllCallbacks.isEmpty()) {
+            OnBackInvokedCallback topCallback = getTopCallback();
+            if (topCallback != null) {
+                sendCancelledIfInProgress(topCallback);
+            } else {
+                // Should not be possible
+                Log.e(TAG, "There is no topCallback, even if mAllCallbacks is not empty");
+            }
             // Clear binder references in WM.
             setTopOnBackInvokedCallback(null);
         }
+
+        // We should also stop running animations since all callbacks have been removed.
+        // note: mSpring.skipToEnd(), in ProgressAnimator.reset(), requires the main handler.
+        Handler.getMain().post(mProgressAnimator::reset);
         mAllCallbacks.clear();
         mOnBackInvokedCallbacks.clear();
     }
@@ -342,12 +359,17 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher {
         @Override
         public void onBackInvoked() throws RemoteException {
             Handler.getMain().post(() -> {
+                boolean isInProgress = mProgressAnimator.isBackAnimationInProgress();
                 mProgressAnimator.reset();
                 final OnBackInvokedCallback callback = mCallbackRef.get();
                 if (callback == null) {
                     Log.d(TAG, "Trying to call onBackInvoked() on a null callback reference.");
                     return;
                 }
+                if (callback instanceof OnBackAnimationCallback && !isInProgress) {
+                    Log.w(TAG, "ProgressAnimator was not in progress, skip onBackInvoked().");
+                    return;
+                }
                 callback.onBackInvoked();
             });
         }

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

@@ -25,6 +25,7 @@ import static org.mockito.ArgumentMatchers.isNull;
 import static org.mockito.Mockito.atLeast;
 import static org.mockito.Mockito.atMost;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.verifyZeroInteractions;
@@ -340,4 +341,42 @@ public class WindowOnBackInvokedDispatcherTest {
         verify(mCallback1).onBackCancelled();
         verify(mWindowSession).setOnBackInvokedCallbackInfo(Mockito.eq(mWindow), isNull());
     }
+
+    @Test
+    public void onBackInvoked_calledAfterOnBackStarted() throws RemoteException {
+        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback1);
+        OnBackInvokedCallbackInfo callbackInfo = assertSetCallbackInfo();
+
+        callbackInfo.getCallback().onBackStarted(mBackEvent);
+
+        waitForIdle();
+        verify(mCallback1).onBackStarted(any(BackEvent.class));
+
+        callbackInfo.getCallback().onBackInvoked();
+
+        waitForIdle();
+        verify(mCallback1).onBackInvoked();
+        verify(mCallback1, never()).onBackCancelled();
+    }
+
+    @Test
+    public void onDetachFromWindow_cancelCallbackAndIgnoreOnBackInvoked() throws RemoteException {
+        mDispatcher.registerOnBackInvokedCallback(PRIORITY_DEFAULT, mCallback1);
+
+        OnBackInvokedCallbackInfo callbackInfo = assertSetCallbackInfo();
+
+        callbackInfo.getCallback().onBackStarted(mBackEvent);
+
+        waitForIdle();
+        verify(mCallback1).onBackStarted(any(BackEvent.class));
+
+        // This should trigger mCallback1.onBackCancelled()
+        mDispatcher.detachFromWindow();
+        // This should be ignored by mCallback1
+        callbackInfo.getCallback().onBackInvoked();
+
+        waitForIdle();
+        verify(mCallback1, never()).onBackInvoked();
+        verify(mCallback1).onBackCancelled();
+    }
 }