Adjust TvPipMenuView.Listener interface wrt TvPipMenuModes

The tv pip menu modes allow to merge the onBackPress and
onExitMoveMenu callbacks into a single callback - onExitMenuMode.
This CL cleans up the menu mode exit path using the tv pip menu
modes.

The CL also simplifies the onPipMovement callback since the
TvPipMenuView already knows the menu mode.

Bug: 220108601
Test: atest TvPipMenuControllerTest
Change-Id: I9c26332a69ca2f1717662693a32403d0cd82b459

3 files changed, 98 insertions, 104 deletions

diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuController.java
index b2a189b45d6c..a343d2ddf991 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuController.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuController.java
@@ -414,17 +414,20 @@ public class TvPipMenuController implements PipMenuController, TvPipMenuView.Lis
     }
 
     private void switchToMenuMode(@TvPipMenuMode int menuMode, boolean resetMenu) {
-        // Note: we intentionally don't return early here, because the TvPipMenuView needs to
-        // refresh the Ui even if there is no menu mode change.
-        mPrevMenuMode = mCurrentMenuMode;
-        mCurrentMenuMode = menuMode;
-
         ProtoLog.i(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE,
-                "%s: switchToMenuMode: setting mCurrentMenuMode=%s, mPrevMenuMode=%s", TAG,
-                getMenuModeString(), getMenuModeString(mPrevMenuMode));
-
-        updateUiOnNewMenuModeRequest(resetMenu);
-        updateDelegateOnNewMenuModeRequest();
+                "%s: switchToMenuMode: from=%s, to=%s", TAG, getMenuModeString(),
+                getMenuModeString(menuMode));
+
+        if (mCurrentMenuMode != menuMode) {
+            mPrevMenuMode = mCurrentMenuMode;
+            mCurrentMenuMode = menuMode;
+            updateUiOnNewMenuModeRequest(resetMenu);
+            updateDelegateOnNewMenuModeRequest();
+        } else if (resetMenu) {
+            // Note: we intentionally update the Ui even if the menu mode hasn't changed, because
+            // the Ui may have to be updated when resetting the menu.
+            updateUiOnNewMenuModeRequest(resetMenu);
+        }
     }
 
     private void updateUiOnNewMenuModeRequest(boolean resetMenu) {
@@ -476,32 +479,19 @@ public class TvPipMenuController implements PipMenuController, TvPipMenuView.Lis
     }
 
     @Override
-    public void onBackPress() {
-        if (!onExitMoveMode()) {
-            closeMenu();
-        }
-    }
-
-    @Override
-    public boolean onExitMoveMode() {
+    public void onExitCurrentMenuMode() {
         ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE,
-                "%s: onExitMoveMode - mCurrentMenuMode=%s", TAG, getMenuModeString());
-
-        final int saveMenuMode = mCurrentMenuMode;
-        if (isInMoveMode()) {
-            switchToMenuMode(mPrevMenuMode);
-        }
-        return saveMenuMode == MODE_MOVE_MENU;
+                "%s: onExitCurrentMenuMode - mCurrentMenuMode=%s", TAG, getMenuModeString());
+        switchToMenuMode(isInMoveMode() ? mPrevMenuMode : MODE_NO_MENU);
     }
 
     @Override
-    public boolean onPipMovement(int keycode) {
+    public void onPipMovement(int keycode) {
         ProtoLog.d(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE,
                 "%s: onPipMovement - mCurrentMenuMode=%s", TAG, getMenuModeString());
         if (isInMoveMode()) {
             mDelegate.movePip(keycode);
         }
-        return isInMoveMode();
     }
 
     @Override
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuView.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuView.java
index 613791ccc062..3d441406e7ea 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuView.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuView.java
@@ -378,7 +378,7 @@ public class TvPipMenuView extends FrameLayout implements TvPipActionsProvider.L
         animateAlphaTo(1f, mDimLayer);
         mEduTextDrawer.closeIfNeeded();
 
-        if (mCurrentMenuMode == MODE_MOVE_MENU) {
+        if (mCurrentMenuMode == MODE_MOVE_MENU && !resetMenu) {
             refocusButton(mTvPipActionsProvider.getFirstIndexOfAction(ACTION_MOVE));
         }
 
@@ -483,28 +483,28 @@ public class TvPipMenuView extends FrameLayout implements TvPipActionsProvider.L
         if (event.getAction() == ACTION_UP) {
 
             if (event.getKeyCode() == KEYCODE_BACK) {
-                mListener.onBackPress();
+                mListener.onExitCurrentMenuMode();
                 return true;
             }
 
-            if (mA11yManager.isEnabled()) {
-                return super.dispatchKeyEvent(event);
-            }
-
-            switch (event.getKeyCode()) {
-                case KEYCODE_DPAD_UP:
-                case KEYCODE_DPAD_DOWN:
-                case KEYCODE_DPAD_LEFT:
-                case KEYCODE_DPAD_RIGHT:
-                    return mListener.onPipMovement(event.getKeyCode()) || super.dispatchKeyEvent(
-                            event);
-                case KEYCODE_ENTER:
-                case KEYCODE_DPAD_CENTER:
-                    return mListener.onExitMoveMode() || super.dispatchKeyEvent(event);
-                default:
-                    break;
+            if (mCurrentMenuMode == MODE_MOVE_MENU && !mA11yManager.isEnabled()) {
+                switch (event.getKeyCode()) {
+                    case KEYCODE_DPAD_UP:
+                    case KEYCODE_DPAD_DOWN:
+                    case KEYCODE_DPAD_LEFT:
+                    case KEYCODE_DPAD_RIGHT:
+                        mListener.onPipMovement(event.getKeyCode());
+                        return true;
+                    case KEYCODE_ENTER:
+                    case KEYCODE_DPAD_CENTER:
+                        mListener.onExitCurrentMenuMode();
+                        return true;
+                    default:
+                        // Dispatch key event as normal below
+                }
             }
         }
+
         return super.dispatchKeyEvent(event);
     }
 
@@ -529,7 +529,7 @@ public class TvPipMenuView extends FrameLayout implements TvPipActionsProvider.L
         if (a11yEnabled) {
             mA11yDoneButton.setVisibility(VISIBLE);
             mA11yDoneButton.setOnClickListener(v -> {
-                mListener.onExitMoveMode();
+                mListener.onExitCurrentMenuMode();
             });
             mA11yDoneButton.requestFocus();
             mA11yDoneButton.requestAccessibilityFocus();
@@ -626,20 +626,16 @@ public class TvPipMenuView extends FrameLayout implements TvPipActionsProvider.L
 
     interface Listener {
 
-        void onBackPress();
-
         /**
-         * Called when a button for exiting move mode was pressed.
+         * Called when a button for exiting the current menu mode was pressed.
          *
-         * @return true if the event was handled or false if the key event should be handled by the
-         * next receiver.
          */
-        boolean onExitMoveMode();
+        void onExitCurrentMenuMode();
 
         /**
-         * @return whether pip movement was handled.
+         * Called when a button to move the PiP in a certain direction, indicated by keycode.
          */
-        boolean onPipMovement(int keycode);
+        void onPipMovement(int keycode);
 
         /**
          * Called when the TvPipMenuView loses focus. This also means that the TV PiP menu window
diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/tv/TvPipMenuControllerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/tv/TvPipMenuControllerTest.java
index 3a08d32bc430..77b186590056 100644
--- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/tv/TvPipMenuControllerTest.java
+++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/tv/TvPipMenuControllerTest.java
@@ -93,6 +93,7 @@ public class TvPipMenuControllerTest extends ShellTestCase {
     public void testSwitch_FromMoveMode_ToAllActionsMode() {
         showAndAssertMoveMenu();
         showAndAssertAllActionsMenu();
+        verify(mMockDelegate, times(2)).onInMoveModeChanged();
     }
 
     @Test
@@ -124,6 +125,15 @@ public class TvPipMenuControllerTest extends ShellTestCase {
     }
 
     @Test
+    public void testCloseMenu_MoveModeFollowedByMoveMode() {
+        showAndAssertMoveMenu();
+        showAndAssertMoveMenu();
+
+        closeMenuAndAssertMenuClosed();
+        verify(mMockDelegate, times(2)).onInMoveModeChanged();
+    }
+
+    @Test
     public void testCloseMenu_MoveModeFollowedByAllActionsMode() {
         showAndAssertMoveMenu();
         showAndAssertAllActionsMenu();
@@ -142,103 +152,100 @@ public class TvPipMenuControllerTest extends ShellTestCase {
     }
 
     @Test
-    public void testExitMoveMode_NoMenuMode() {
-        mTvPipMenuController.onExitMoveMode();
+    public void testCloseMenu_AllActionsModeFollowedByAllActionsMode() {
+        showAndAssertAllActionsMenu();
+        showAndAssertAllActionsMenu(2);
+
+        closeMenuAndAssertMenuClosed();
+        verify(mMockDelegate, never()).onInMoveModeChanged();
+    }
+
+    @Test
+    public void testExitMenuMode_NoMenuMode() {
+        mTvPipMenuController.onExitCurrentMenuMode();
         assertMenuIsOpen(false);
         verify(mMockDelegate, never()).onMenuClosed();
+        verify(mMockDelegate, never()).onInMoveModeChanged();
     }
 
     @Test
-    public void testExitMoveMode_MoveMode() {
+    public void testExitMenuMode_MoveMode() {
         showAndAssertMoveMenu();
 
-        mTvPipMenuController.onExitMoveMode();
+        mTvPipMenuController.onExitCurrentMenuMode();
         assertMenuClosed();
         verify(mMockDelegate, times(2)).onInMoveModeChanged();
     }
 
     @Test
-    public void testExitMoveMode_AllActionsMode() {
+    public void testExitMenuMode_AllActionsMode() {
         showAndAssertAllActionsMenu();
 
-        mTvPipMenuController.onExitMoveMode();
-        assertMenuIsInAllActionsMode();
-
+        mTvPipMenuController.onExitCurrentMenuMode();
+        assertMenuClosed();
     }
 
     @Test
-    public void testExitMoveMode_AllActionsModeFollowedByMoveMode() {
+    public void testExitMenuMode_AllActionsModeFollowedByMoveMode() {
         showAndAssertAllActionsMenu();
         showAndAssertMoveMenu();
 
-        mTvPipMenuController.onExitMoveMode();
+        mTvPipMenuController.onExitCurrentMenuMode();
         assertMenuIsInAllActionsMode();
         verify(mMockDelegate, times(2)).onInMoveModeChanged();
         verify(mMockTvPipMenuView).transitionToMenuMode(eq(MODE_ALL_ACTIONS_MENU), eq(false));
         verify(mMockTvPipBackgroundView, times(2)).transitionToMenuMode(eq(MODE_ALL_ACTIONS_MENU));
-    }
-
-    @Test
-    public void testOnBackPress_NoMenuMode() {
-        mTvPipMenuController.onBackPress();
-        assertMenuIsOpen(false);
-        verify(mMockDelegate, never()).onMenuClosed();
-    }
-
-    @Test
-    public void testOnBackPress_MoveMode() {
-        showAndAssertMoveMenu();
 
-        pressBackAndAssertMenuClosed();
-        verify(mMockDelegate, times(2)).onInMoveModeChanged();
+        mTvPipMenuController.onExitCurrentMenuMode();
+        assertMenuClosed();
     }
 
     @Test
-    public void testOnBackPress_AllActionsMode() {
+    public void testExitMenuMode_AllActionsModeFollowedByAllActionsMode() {
         showAndAssertAllActionsMenu();
+        showAndAssertAllActionsMenu(2);
 
-        pressBackAndAssertMenuClosed();
+        mTvPipMenuController.onExitCurrentMenuMode();
+        assertMenuClosed();
+        verify(mMockDelegate, never()).onInMoveModeChanged();
     }
 
     @Test
-    public void testOnBackPress_MoveModeFollowedByAllActionsMode() {
+    public void testExitMenuMode_MoveModeFollowedByAllActionsMode() {
         showAndAssertMoveMenu();
+
         showAndAssertAllActionsMenu();
         verify(mMockDelegate, times(2)).onInMoveModeChanged();
 
-        pressBackAndAssertMenuClosed();
+        mTvPipMenuController.onExitCurrentMenuMode();
+        assertMenuClosed();
     }
 
     @Test
-    public void testOnBackPress_AllActionsModeFollowedByMoveMode() {
-        showAndAssertAllActionsMenu();
+    public void testExitMenuMode_MoveModeFollowedByMoveMode() {
+        showAndAssertMoveMenu();
         showAndAssertMoveMenu();
 
-        mTvPipMenuController.onBackPress();
-        assertMenuIsInAllActionsMode();
+        mTvPipMenuController.onExitCurrentMenuMode();
+        assertMenuClosed();
         verify(mMockDelegate, times(2)).onInMoveModeChanged();
-        verify(mMockTvPipMenuView).transitionToMenuMode(eq(MODE_ALL_ACTIONS_MENU), eq(false));
-        verify(mMockTvPipBackgroundView, times(2)).transitionToMenuMode(eq(MODE_ALL_ACTIONS_MENU));
-
-        pressBackAndAssertMenuClosed();
     }
 
     @Test
     public void testOnPipMovement_NoMenuMode() {
-        assertPipMoveSuccessful(false, mTvPipMenuController.onPipMovement(TEST_MOVE_KEYCODE));
+        moveAndAssertMoveSuccessful(false);
     }
 
     @Test
     public void testOnPipMovement_MoveMode() {
         showAndAssertMoveMenu();
-        assertPipMoveSuccessful(true, mTvPipMenuController.onPipMovement(TEST_MOVE_KEYCODE));
-        verify(mMockDelegate).movePip(eq(TEST_MOVE_KEYCODE));
+        moveAndAssertMoveSuccessful(true);
     }
 
     @Test
     public void testOnPipMovement_AllActionsMode() {
         showAndAssertAllActionsMenu();
-        assertPipMoveSuccessful(false, mTvPipMenuController.onPipMovement(TEST_MOVE_KEYCODE));
+        moveAndAssertMoveSuccessful(false);
     }
 
     @Test
@@ -270,10 +277,16 @@ public class TvPipMenuControllerTest extends ShellTestCase {
     }
 
     private void showAndAssertAllActionsMenu() {
+        showAndAssertAllActionsMenu(1);
+    }
+
+    private void showAndAssertAllActionsMenu(int times) {
         mTvPipMenuController.showMenu();
         assertMenuIsInAllActionsMode();
-        verify(mMockTvPipMenuView).transitionToMenuMode(eq(MODE_ALL_ACTIONS_MENU), eq(true));
-        verify(mMockTvPipBackgroundView).transitionToMenuMode(eq(MODE_ALL_ACTIONS_MENU));
+        verify(mMockTvPipMenuView, times(times))
+                .transitionToMenuMode(eq(MODE_ALL_ACTIONS_MENU), eq(true));
+        verify(mMockTvPipBackgroundView, times(times))
+                .transitionToMenuMode(eq(MODE_ALL_ACTIONS_MENU));
     }
 
     private void closeMenuAndAssertMenuClosed() {
@@ -281,9 +294,9 @@ public class TvPipMenuControllerTest extends ShellTestCase {
         assertMenuClosed();
     }
 
-    private void pressBackAndAssertMenuClosed() {
-        mTvPipMenuController.onBackPress();
-        assertMenuClosed();
+    private void moveAndAssertMoveSuccessful(boolean expectedSuccess) {
+        mTvPipMenuController.onPipMovement(TEST_MOVE_KEYCODE);
+        verify(mMockDelegate, times(expectedSuccess ? 1 : 0)).movePip(eq(TEST_MOVE_KEYCODE));
     }
 
     private void assertMenuClosed() {
@@ -312,11 +325,6 @@ public class TvPipMenuControllerTest extends ShellTestCase {
         assertMenuIsOpen(true);
     }
 
-    private void assertPipMoveSuccessful(boolean expected, boolean actual) {
-        assertTrue("Should " + (expected ? "" : "not ") + "move PiP when the menu is in mode "
-                + mTvPipMenuController.getMenuModeString(), expected == actual);
-    }
-
     private class TestTvPipMenuController extends TvPipMenuController {
 
         TestTvPipMenuController() {