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
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 b2a189b..a343d2d 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 @@
}
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));
+ "%s: switchToMenuMode: from=%s, to=%s", TAG, getMenuModeString(),
+ getMenuModeString(menuMode));
- updateUiOnNewMenuModeRequest(resetMenu);
- updateDelegateOnNewMenuModeRequest();
+ 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 @@
}
@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 613791c..3d44140 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 @@
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 @@
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 @@
if (a11yEnabled) {
mA11yDoneButton.setVisibility(VISIBLE);
mA11yDoneButton.setOnClickListener(v -> {
- mListener.onExitMoveMode();
+ mListener.onExitCurrentMenuMode();
});
mA11yDoneButton.requestFocus();
mA11yDoneButton.requestAccessibilityFocus();
@@ -626,20 +626,16 @@
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 3a08d32..77b1865 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 void testSwitch_FromMoveMode_ToAllActionsMode() {
showAndAssertMoveMenu();
showAndAssertAllActionsMenu();
+ verify(mMockDelegate, times(2)).onInMoveModeChanged();
}
@Test
@@ -124,6 +125,15 @@
}
@Test
+ public void testCloseMenu_MoveModeFollowedByMoveMode() {
+ showAndAssertMoveMenu();
+ showAndAssertMoveMenu();
+
+ closeMenuAndAssertMenuClosed();
+ verify(mMockDelegate, times(2)).onInMoveModeChanged();
+ }
+
+ @Test
public void testCloseMenu_MoveModeFollowedByAllActionsMode() {
showAndAssertMoveMenu();
showAndAssertAllActionsMenu();
@@ -142,103 +152,100 @@
}
@Test
- public void testExitMoveMode_NoMenuMode() {
- mTvPipMenuController.onExitMoveMode();
- assertMenuIsOpen(false);
- verify(mMockDelegate, never()).onMenuClosed();
+ public void testCloseMenu_AllActionsModeFollowedByAllActionsMode() {
+ showAndAssertAllActionsMenu();
+ showAndAssertAllActionsMenu(2);
+
+ closeMenuAndAssertMenuClosed();
+ verify(mMockDelegate, never()).onInMoveModeChanged();
}
@Test
- public void testExitMoveMode_MoveMode() {
+ public void testExitMenuMode_NoMenuMode() {
+ mTvPipMenuController.onExitCurrentMenuMode();
+ assertMenuIsOpen(false);
+ verify(mMockDelegate, never()).onMenuClosed();
+ verify(mMockDelegate, never()).onInMoveModeChanged();
+ }
+
+ @Test
+ 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();
- 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();
- }
-
- @Test
- public void testOnBackPress_AllActionsMode() {
- showAndAssertAllActionsMenu();
-
- pressBackAndAssertMenuClosed();
- }
-
- @Test
- public void testOnBackPress_MoveModeFollowedByAllActionsMode() {
- showAndAssertMoveMenu();
- showAndAssertAllActionsMenu();
- verify(mMockDelegate, times(2)).onInMoveModeChanged();
-
- pressBackAndAssertMenuClosed();
- }
-
- @Test
- public void testOnBackPress_AllActionsModeFollowedByMoveMode() {
- showAndAssertAllActionsMenu();
- showAndAssertMoveMenu();
-
- mTvPipMenuController.onBackPress();
+ 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));
- pressBackAndAssertMenuClosed();
+ mTvPipMenuController.onExitCurrentMenuMode();
+ assertMenuClosed();
+ }
+
+ @Test
+ public void testExitMenuMode_AllActionsModeFollowedByAllActionsMode() {
+ showAndAssertAllActionsMenu();
+ showAndAssertAllActionsMenu(2);
+
+ mTvPipMenuController.onExitCurrentMenuMode();
+ assertMenuClosed();
+ verify(mMockDelegate, never()).onInMoveModeChanged();
+ }
+
+ @Test
+ public void testExitMenuMode_MoveModeFollowedByAllActionsMode() {
+ showAndAssertMoveMenu();
+
+ showAndAssertAllActionsMenu();
+ verify(mMockDelegate, times(2)).onInMoveModeChanged();
+
+ mTvPipMenuController.onExitCurrentMenuMode();
+ assertMenuClosed();
+ }
+
+ @Test
+ public void testExitMenuMode_MoveModeFollowedByMoveMode() {
+ showAndAssertMoveMenu();
+ showAndAssertMoveMenu();
+
+ mTvPipMenuController.onExitCurrentMenuMode();
+ assertMenuClosed();
+ verify(mMockDelegate, times(2)).onInMoveModeChanged();
}
@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 @@
}
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 @@
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 @@
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() {