ash: Implement keyboard shortcut to resize the PiP Window

Use "Search + X" to resize the PiP Window.
This shortcut behaves the same as double-tapping the PiP window.

- Design doc: https://docs.google.com/document/d/1wTcLc9LEvDHQgi6K1FOiOxAYoUAf-yvwfLWCWiEq6pU/edit?usp=sharing&resourcekey=0-BZSEliNNgSFAnhKEJ69B5Q

- UXW review doc: https://docs.google.com/document/d/1E1tU2Tnv7urIcKeGjqjaHUTMz383l4YVSBfk5vq4L8U/edit?tab=t.0#heading=h.p6ytmfqg0fql
- Discussions we had we added a shortcut for PiP resizing: https://docs.google.com/document/d/1ZHLOd55rDCKDUSUeNbKEqOkwvskMQH1HepfreLL64KQ/edit?usp=sharing

    Push "Search + X" to resize the pip window properly.
    Show a description on the Key Shortcuts setting.

Bug: b:333656468
Test: ash_unittests --gtest_filter="*PipToggleResizeFeatureTest*"
Change-Id: I15f7919376abf327b9eab243c511ed277085906d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5874953
Commit-Queue: Kokoa Matsuda <komatsud@google.com>
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: Nergi Rahardi <nergi@google.com>
Cr-Commit-Position: refs/heads/main@{#1362270}

ash/accelerators/accelerator_commands.cc

@@ -68,6 +68,7 @@
 #include "ash/wm/overview/overview_controller.h"
 #include "ash/wm/overview/overview_session.h"
 #include "ash/wm/overview/overview_utils.h"
+#include "ash/wm/pip/pip_controller.h"
 #include "ash/wm/screen_pinning_controller.h"
 #include "ash/wm/snap_group/snap_group.h"
 #include "ash/wm/snap_group/snap_group_controller.h"
@@ -652,6 +653,10 @@ bool CanPerformMagnifierZoom() {
          Shell::Get()->docked_magnifier_controller()->GetEnabled();
 }
 
+bool CanResizePipWindow() {
+  return Shell::Get()->pip_controller()->CanResizePip();
+}
+
 bool CanScreenshot(bool take_screenshot) {
   // |AcceleratorAction::kTakeScreenshot| is allowed when user session is
   // blocked.
@@ -1155,6 +1160,10 @@ void ResetDisplayZoom() {
   display_manager->ResetDisplayZoom(display.id());
 }
 
+void ResizePipWindow() {
+  Shell::Get()->pip_controller()->HandleKeyboardShortcut();
+}
+
 void RestoreTab() {
   NewWindowDelegate::GetPrimary()->RestoreTab();
 }

ash/accelerators/accelerator_commands.h

@@ -111,6 +111,8 @@ ASH_EXPORT bool CanUnpinWindow();
 
 ASH_EXPORT bool CanWindowSnap();
 
+ASH_EXPORT bool CanResizePipWindow();
+
 //////////////////////////////////////////////////////////////////////////////
 // Accelerator commands.
 // Note: These functions should be independent and not depend on ui::Accelerator
@@ -449,6 +451,9 @@ ASH_EXPORT void WindowSnap(AcceleratorAction action);
 // Changes the display zooming up or down.
 ASH_EXPORT bool ZoomDisplay(bool up);
 
+// Resize the pip window.
+ASH_EXPORT void ResizePipWindow();
+
 }  // namespace accelerators
 }  // namespace ash
 

ash/accelerators/accelerator_controller_impl.cc

@@ -1030,6 +1030,8 @@ bool AcceleratorControllerImpl::CanPerformAction(
     case AcceleratorAction::kLockPressed:
     case AcceleratorAction::kLockReleased:
       return CanHandleLockButton(accelerator);
+    case AcceleratorAction::kResizePipWindow:
+      return accelerators::CanResizePipWindow();
 
     // The following are always enabled.
     case AcceleratorAction::kBrightnessDown:
@@ -1676,6 +1678,9 @@ void AcceleratorControllerImpl::PerformAction(
     case kTouchFingerprintSensor3:
       accelerators::TouchFingerprintSensor(3);
       break;
+    case AcceleratorAction::kResizePipWindow:
+      accelerators::ResizePipWindow();
+      break;
   }
 
   RecordActionUmaHistogram(action, accelerator);

ash/ash_strings.grd

@@ -4216,6 +4216,9 @@ No devices connected.
       <message name="IDS_ASH_ACCELERATOR_DESCRIPTION_FOCUS_PIP" desc="Label for accelerator action - Focus on the picture-in-picture window.">
         Focus on the picture-in-picture window
       </message>
+      <message name="IDS_ASH_ACCELERATOR_DESCRIPTION_RESIZE_PIP_WINDOW" desc="Label for accelerator action - Resize the picture-in-picture window.">
+        Switch between the maximum or current size of the picture-in-picture window
+      </message>
       <message name="IDS_ASH_ACCELERATOR_DESCRIPTION_FOCUS_PREVIOUS_PANE" desc="Label for accelerator action - Move focus to previous pane.">
         Move focus to previous pane
       </message>

ash/ash_strings_grd/IDS_ASH_ACCELERATOR_DESCRIPTION_RESIZE_PIP_WINDOW.png.sha1

@@ -0,0 +1 @@
+a286b6299d3a608deb52a7990aadafa537e06711

ash/public/cpp/accelerator_actions.h

@@ -170,6 +170,7 @@ namespace ash {
   ACCELERATOR_ACTION_ENTRY(TilingWindowResizeUp)                       \
   ACCELERATOR_ACTION_ENTRY(TilingWindowResizeDown)                     \
   ACCELERATOR_ACTION_ENTRY(ToggleMouseKeys)                            \
+  ACCELERATOR_ACTION_ENTRY(ResizePipWindow)                            \
   /* Debug actions are kept at an offset.*/                            \
   /* This offset should be kept consistent with the enum*/             \
   /* `AcceleratorAction` in*/                                          \

ash/public/cpp/accelerator_actions_unittest.cc

@@ -18,12 +18,12 @@ namespace ash {
 namespace {
 
 // The total number of accelerator actions.
-constexpr int kAcceleratorActionsTotalNum = 166;
+constexpr int kAcceleratorActionsTotalNum = 167;
 // The toal number of debug accelerators, these will not be used for hashing.
 constexpr int kDebugAcceleratorActionsNum = 28;
 // The hash of accelerator actions. Please update this when adding a new
 // accelerator action.
-constexpr char kAcceleratorActionsHash[] = "af26f45ccfb2450d29171e3b3697a8dd";
+constexpr char kAcceleratorActionsHash[] = "19f19f0e593d97ece036a1e5a9905135";
 
 // Define the mapping between an AcceleratorAction and its string name.
 // Example:

ash/public/cpp/accelerators.cc

@@ -313,6 +313,10 @@ const AcceleratorData kAcceleratorData[] = {
 
     // TODO(yusukes): Handle VKEY_MEDIA_STOP, and VKEY_MEDIA_LAUNCH_MAIL.
 
+    // PIP-resize shortcut.
+    {true, ui::VKEY_X, ui::EF_COMMAND_DOWN,
+     AcceleratorAction::kResizePipWindow},
+
     // ARC-specific shortcut.
     {true, ui::VKEY_C, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN,
      AcceleratorAction::kToggleResizeLockMenu},

ash/public/mojom/accelerator_actions.mojom

@@ -144,6 +144,7 @@ enum AcceleratorAction {
   kTilingWindowResizeUp,
   kTilingWindowResizeDown,
   kToggleMouseKeys,
+  kResizePipWindow,
   // The following are DEBUG actions with an offset. This is to keep the enum
   // in sync with `AcceleratorActions` in ash/public/cpp/accelerator_actions.h.
   kDebugClearUseKMeansPref = 9000,

ash/public/mojom/accelerator_actions_mojom_traits.cc

@@ -291,6 +291,8 @@ EnumTraits<mojom_accelerator_action, ash::AcceleratorAction>::ToMojom(
       return mojom_accelerator_action::kWindowMinimize;
     case ash::AcceleratorAction::kMinimizeTopWindowOnBack:
       return mojom_accelerator_action::kMinimizeTopWindowOnBack;
+    case ash::AcceleratorAction::kResizePipWindow:
+      return mojom_accelerator_action::kResizePipWindow;
     case ash::AcceleratorAction::kDebugClearUseKMeansPref:
       return mojom_accelerator_action::kDebugClearUseKMeansPref;
     case ash::AcceleratorAction::kDebugKeyboardBacklightToggle:
@@ -770,6 +772,9 @@ bool EnumTraits<mojom_accelerator_action, ash::AcceleratorAction>::FromMojom(
     case mojom_accelerator_action::kMinimizeTopWindowOnBack:
       *out = ash::AcceleratorAction::kMinimizeTopWindowOnBack;
       return true;
+    case mojom_accelerator_action::kResizePipWindow:
+      *out = ash::AcceleratorAction::kResizePipWindow;
+      return true;
     case mojom_accelerator_action::kDebugClearUseKMeansPref:
       *out = ash::AcceleratorAction::kDebugClearUseKMeansPref;
       return true;

ash/webui/shortcut_customization_ui/backend/accelerator_layout_table.cc

@@ -754,6 +754,14 @@ const AcceleratorLayoutMap& GetAcceleratorLayoutMap() {
                                  /*locked=*/false,
                                  mojom::AcceleratorLayoutStyle::kDefault,
                                  mojom::AcceleratorSource::kAsh)},
+       {AcceleratorAction::kResizePipWindow,
+        AcceleratorLayoutDetails(
+            AcceleratorAction::kResizePipWindow,
+            IDS_ASH_ACCELERATOR_DESCRIPTION_RESIZE_PIP_WINDOW,
+            mojom::AcceleratorCategory::kDevice,
+            mojom::AcceleratorSubcategory::kMedia,
+            /*locked=*/false, mojom::AcceleratorLayoutStyle::kDefault,
+            mojom::AcceleratorSource::kAsh)},
        {AcceleratorAction::kKeyboardBacklightToggle,
         AcceleratorLayoutDetails(
             AcceleratorAction::kKeyboardBacklightToggle,

ash/webui/shortcut_customization_ui/backend/accelerator_layout_table.h

@@ -322,6 +322,7 @@ inline constexpr uint32_t kAcceleratorLayouts[] = {
     AcceleratorAction::kMediaPrevTrack,
     AcceleratorAction::kMediaFastForward,
     AcceleratorAction::kFocusPip,
+    AcceleratorAction::kResizePipWindow,
 
     // Device > Input
     AcceleratorAction::kKeyboardBacklightToggle,

ash/webui/shortcut_customization_ui/backend/accelerator_layout_table_unittest.cc

@@ -27,9 +27,9 @@ namespace ash {
 namespace {
 
 // The total number of Ash accelerators.
-constexpr int kAshAcceleratorsTotalNum = 156;
+constexpr int kAshAcceleratorsTotalNum = 157;
 // The hash of Ash accelerators.
-constexpr char kAshAcceleratorsHash[] = "a51c3a9d4e052db8deba9a46a9ef48ce";
+constexpr char kAshAcceleratorsHash[] = "00d64e050abba1b0354ad946f608d420";
 
 std::string ToActionName(ash::AcceleratorAction action) {
   return base::StrCat(

ash/wm/pip/pip_controller_unittest.cc

@@ -220,17 +220,17 @@ TEST_F(PipControllerTest, TuckHandleIsShownAtCorrectPosition) {
   EXPECT_FALSE(test_api.scoped_window_tucker());
 }
 
-
-class PipDoubleTapHandlerTest : public AshTestBase,
-                                public testing::WithParamInterface<bool> {
+class PipToggleResizeFeatureTest : public AshTestBase,
+                                   public testing::WithParamInterface<bool> {
  public:
-  PipDoubleTapHandlerTest()
+  PipToggleResizeFeatureTest()
       : scoped_feature_list_(features::kPipDoubleTapToResize) {}
 
-  PipDoubleTapHandlerTest(const PipDoubleTapHandlerTest&) = delete;
-  PipDoubleTapHandlerTest& operator=(const PipDoubleTapHandlerTest&) = delete;
+  PipToggleResizeFeatureTest(const PipToggleResizeFeatureTest&) = delete;
+  PipToggleResizeFeatureTest& operator=(const PipToggleResizeFeatureTest&) =
+      delete;
 
-  ~PipDoubleTapHandlerTest() override = default;
+  ~PipToggleResizeFeatureTest() override = default;
 
   std::unique_ptr<aura::Window> CreateAppWindow(
       const gfx::Rect& bounds,
@@ -272,11 +272,18 @@ class PipDoubleTapHandlerTest : public AshTestBase,
     }
   }
 
+  void PressHotKey(aura::Window* window) {
+    auto* event_generator = GetEventGenerator();
+
+    event_generator->PressKeyAndModifierKeys(ui::VKEY_X, ui::EF_COMMAND_DOWN);
+    event_generator->ReleaseKeyAndModifierKeys(ui::VKEY_X, ui::EF_COMMAND_DOWN);
+  }
+
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
 };
 
-TEST_P(PipDoubleTapHandlerTest, TestSingleClick) {
+TEST_P(PipToggleResizeFeatureTest, TestSingleClick) {
   // Tests that single clicking a normal window or a PiP window does not change
   // the bounds.
   for (WindowStateType state_type :
@@ -302,7 +309,7 @@ TEST_P(PipDoubleTapHandlerTest, TestSingleClick) {
   }
 }
 
-TEST_P(PipDoubleTapHandlerTest, TestDoubleClickWithPip) {
+TEST_P(PipToggleResizeFeatureTest, TestDoubleClickWithPip) {
   auto window = CreateAppWindow(gfx::Rect(kExpectedPipDefaultSize),
                                 WindowStateType::kPip);
 
@@ -311,7 +318,7 @@ TEST_P(PipDoubleTapHandlerTest, TestDoubleClickWithPip) {
   EXPECT_TRUE(WindowState::Get(window.get())->bounds_changed_by_user());
 }
 
-TEST_P(PipDoubleTapHandlerTest, TestDoubleClickAtMaxSize) {
+TEST_P(PipToggleResizeFeatureTest, TestDoubleClickAtMaxSize) {
   auto window =
       CreateAppWindow(gfx::Rect(kExpectedPipMaxSize), WindowStateType::kPip);
 
@@ -324,7 +331,7 @@ TEST_P(PipDoubleTapHandlerTest, TestDoubleClickAtMaxSize) {
             gfx::Rect(kExpectedPipDefaultSize));
 }
 
-TEST_P(PipDoubleTapHandlerTest, TestDoubleClickAtNonMaxSize) {
+TEST_P(PipToggleResizeFeatureTest, TestDoubleClickAtNonMaxSize) {
   auto window = CreateAppWindow(gfx::Rect(300, 300), WindowStateType::kPip);
 
   DoubleTapWindowCenter(window.get());
@@ -332,7 +339,7 @@ TEST_P(PipDoubleTapHandlerTest, TestDoubleClickAtNonMaxSize) {
   auto* window_state = WindowState::Get(window.get());
 
   EXPECT_TRUE(window_state->bounds_changed_by_user());
-  // Expect it to go to max size on the first double-click/tap.
+  // Expect it to go to max size on the first hotkey press.
   EXPECT_EQ(GetFakeWindowStateLastBounds(window_state),
             gfx::Rect(kExpectedPipMaxSize));
 
@@ -347,7 +354,40 @@ TEST_P(PipDoubleTapHandlerTest, TestDoubleClickAtNonMaxSize) {
   EXPECT_EQ(GetFakeWindowStateLastBounds(window_state), gfx::Rect(300, 300));
 }
 
+TEST_P(PipToggleResizeFeatureTest, TestHotkeyWithPipWindow) {
+  auto window = CreateAppWindow(gfx::Rect(300, 300), WindowStateType::kPip);
+
+  PressHotKey(window.get());
+
+  auto* window_state = WindowState::Get(window.get());
+
+  EXPECT_TRUE(window_state->bounds_changed_by_user());
+}
+
+TEST_P(PipToggleResizeFeatureTest, TestHotKeyToggleTheSizeOfThePipWindow) {
+  auto window = CreateAppWindow(gfx::Rect(300, 300), WindowStateType::kPip);
+
+  PressHotKey(window.get());
+
+  auto* window_state = WindowState::Get(window.get());
+
+  EXPECT_TRUE(window_state->bounds_changed_by_user());
+  // Expect it to go to max size on the first double-click/tap.
+  EXPECT_EQ(GetFakeWindowStateLastBounds(window_state),
+            gfx::Rect(kExpectedPipMaxSize));
+
+  window =
+      CreateAppWindow(gfx::Rect(kExpectedPipMaxSize), WindowStateType::kPip);
+  window_state = WindowState::Get(window.get());
+
+  // Check when re-pressing the hotkey, if the window goes back to the last
+  // user-defined size.
+  PressHotKey(window.get());
+  EXPECT_TRUE(window_state->bounds_changed_by_user());
+  EXPECT_EQ(GetFakeWindowStateLastBounds(window_state), gfx::Rect(300, 300));
+}
+
 // Assume true is for mouse clicks and false is for gesture taps.
-INSTANTIATE_TEST_SUITE_P(All, PipDoubleTapHandlerTest, testing::Bool());
+INSTANTIATE_TEST_SUITE_P(All, PipToggleResizeFeatureTest, testing::Bool());
 
 }  // namespace ash

tools/metrics/histograms/metadata/ash/histograms.xml

@@ -123,6 +123,7 @@ chromium-metrics-reviews@google.com.
   <variant name="PowerReleased"/>
   <variant name="PrintUiHierarchies"/>
   <variant name="PrivacyScreenToggle"/>
+  <variant name="ResizePipWindow"/>
   <variant name="RestoreTab"/>
   <variant name="RotateScreen"/>
   <variant name="RotateWindow"/>

tools/metrics/histograms/metadata/chromeos/enums.xml

@@ -188,6 +188,7 @@ chromium-metrics-reviews@google.com.
   <int value="134" label="TilingWindowResizeUp"/>
   <int value="135" label="TilingWindowResizeDown"/>
   <int value="136" label="ToggleMouseKeys"/>
+  <int value="137" label="ResizePipWindow"/>
   <int value="9000" label="DebugClearUseKMeansPref"/>
   <int value="9001" label="DebugKeyboardBacklightToggle"/>
   <int value="9002" label="DebugMicrophoneMuteToggle"/>