Reland "Fix context menu not dismissing when window moves with Win+Shift+Arrow"

This is a reland of commit 14045a2f04

This contains the original change, plus a fix for a crash that occurred
on Mac when invoking a menu while in immersive fullscreen.

A similar change commit c08a424435
attempted to fix this issue, but isn't a complete fix. It will be
reverted once this CL lands. It attempts to listen to the key press
events, but Win+Shift+Arrow is an OS special key combination which is
eaten by the OS. This means the application never receives the key down
for arrow when Win and Shift are already pressed. So the other CL is
relying on the key up event. If the user releases Win or Shift keys
before releasing the arrow key, then the context menu is not dismissed
even though the browser window moves. Additionally, it doesn't handle up
and down keys, it doesn't handle if the window is moved with Win+Arrow
(no shift), and on single monitor displays, Win+Shift+Arrow does not
move the browser window, but the other CL still closes the context menu.

Using browser window move to dismiss the menu matches the behavior of
existing UI like Datalist dropdown and select dropdown, which dismiss
themselves when the browser window moves.

Original change's description:
> Fix context menu not dismissing when window moves with Win+Shift+Arrow
>
> The existing Destroying and ShowStateChanged callbacks, that
> MenuController uses to cancel currently showing menus, don't handle the
> case where the user moves the browser window using the keyboard shortcut
> Win+Shift+Arrow. Adding a callback for BoundsChanged to cancel current
> menus will handle the scenario where the user moves the browser window
> with the keyboard.
>
> Bug: 394634481
> Change-Id: I8fcceb209dbd376eda8a683525dcfb891d364a40
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6244522
> Reviewed-by: Dana Fried <dfried@chromium.org>
> Commit-Queue: Brad Peters <brpeters@microsoft.com>
> Reviewed-by: Alison Gale <agale@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1427428}

Bug: 394634481
Change-Id: If1ef4efb3a6d37be121d57a098daa4a2800c3348
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6474359
Reviewed-by: Dana Fried <dfried@chromium.org>
Commit-Queue: Brad Peters <brpeters@microsoft.com>
Reviewed-by: Liang Zhao <lzhao@microsoft.com>
Reviewed-by: Alison Gale <agale@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1458197}

ash/shelf/shelf_view_unittest.cc

@@ -2803,7 +2803,9 @@ TEST_P(LockedFullscreenShelfViewTest, ContextMenuVisibilityWithPinnedWindow) {
 
   // Pin the window and verify context menu visibility.
   PinWindow(window.get(), IsLocked());
-  EXPECT_EQ(shelf_view_->IsShowingMenu(), !IsLocked());
+  // Context menus dismiss when the window moves. Pinning the window
+  // moves the window, so the context menu should always be dismissed.
+  EXPECT_FALSE(shelf_view_->IsShowingMenu());
 }
 
 INSTANTIATE_TEST_SUITE_P(LockedFullscreenShelfViewTests,

chrome/browser/ui/toasts/toast_controller_interactive_ui_test.cc

@@ -57,6 +57,8 @@
 namespace {
 DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kFirstTab);
 DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kSecondTab);
+DEFINE_LOCAL_STATE_IDENTIFIER_VALUE(ui::test::PollingStateObserver<bool>,
+                                    kToastAnimation);
 
 class OmniboxInputWaiter : public OmniboxTabHelper::Observer {
  public:
@@ -144,11 +146,20 @@ class ToastControllerInteractiveTest : public InteractiveBrowserTest {
 
 
   auto ShowToast(ToastParams params) {
-    return Do(base::BindOnce(
-        [](ToastController* toast_controller, ToastParams toast_params) {
-          toast_controller->MaybeShowToast(std::move(toast_params));
-        },
-        GetToastController(), std::move(params)));
+    return Steps(
+        Do(base::BindOnce(
+            [](ToastController* toast_controller, ToastParams toast_params) {
+              toast_controller->MaybeShowToast(std::move(toast_params));
+            },
+            GetToastController(), std::move(params))),
+        PollState(kToastAnimation,
+                  [this]() {
+                    toasts::ToastView* toast_view =
+                        GetToastController()->GetToastViewForTesting();
+                    return toast_view && toast_view->is_animating_for_testing();
+                  }),
+        WaitForState(kToastAnimation, false),
+        StopObservingState(kToastAnimation));
   }
 
   auto FireToastCloseTimer() {

chrome/browser/ui/toasts/toast_view.h

@@ -104,6 +104,9 @@ class ToastView : public views::BubbleDialogDelegateView,
   views::Label* label_for_testing() { return label_; }
   views::MdTextButton* action_button_for_testing() { return action_button_; }
   views::ImageButton* close_button_for_testing() { return close_button_; }
+  bool is_animating_for_testing() const {
+    return height_animation_.is_animating();
+  }
 
   // Gets the icon/image size from the layout provider.
   static int GetIconSize();

ui/views/controls/menu/menu_controller.cc

@@ -1536,6 +1536,20 @@ void MenuController::OnWidgetDestroying(Widget* widget) {
   ExitMenu();
 }
 
+void MenuController::OnWidgetBoundsChanged(Widget* widget,
+                                           const gfx::Rect& new_bounds) {
+  DCHECK_EQ(owner_, widget);
+
+  // Ignore bounds changes while in the middle of showing a submenu.
+  if (showing_submenu_) {
+    return;
+  }
+
+  // Close all open menus when the browser window is moved or resized (e.g. due
+  // to moving the window with the keyboard).
+  Cancel(ExitType::kAll);
+}
+
 bool MenuController::IsCancelAllTimerRunningForTest() {
   return cancel_all_timer_.IsRunning();
 }

ui/views/controls/menu/menu_controller.h

@@ -254,6 +254,8 @@ class VIEWS_EXPORT MenuController final : public gfx::AnimationDelegate,
   // WidgetObserver overrides:
   void OnWidgetDestroying(Widget* widget) override;
   void OnWidgetShowStateChanged(Widget* widget) override;
+  void OnWidgetBoundsChanged(Widget* widget,
+                             const gfx::Rect& new_bounds) override;
 
   // Only used for testing.
   bool IsCancelAllTimerRunningForTest();

ui/views/controls/menu/menu_controller_unittest.cc

@@ -415,6 +415,10 @@ class MenuControllerTest : public ViewsTestBase,
     return submenu->host_;
   }
 
+  void set_showing_submenu(bool showing) {
+    menu_controller_->showing_submenu_ = showing;
+  }
+
   MenuHostRootView* CreateMenuHostRootView(MenuHost* host);
 
   void MenuHostOnDragWillStart(MenuHost* host);
@@ -2479,6 +2483,28 @@ TEST_F(MenuControllerTest, WidgetStateChangeCancelsMenu) {
   EXPECT_EQ(MenuController::ExitType::kAll, menu_controller()->exit_type());
 }
 
+TEST_F(MenuControllerTest, WidgetBoundsChangeCancelsMenu) {
+  ExitMenuRun();
+  menu_controller()->Run(owner(), nullptr, menu_item(), gfx::Rect(),
+                         MenuAnchorPosition::kTopLeft);
+  EXPECT_TRUE(showing());
+  EXPECT_EQ(MenuController::ExitType::kNone, menu_controller()->exit_type());
+
+  // Ensure menu does not dismiss if showing_submenu_ is true.
+  set_showing_submenu(true);
+  gfx::Rect bounds = owner()->GetWindowBoundsInScreen();
+  bounds.Offset(10, 10);
+  owner()->SetBounds(bounds);
+  EXPECT_TRUE(showing());
+  EXPECT_EQ(MenuController::ExitType::kNone, menu_controller()->exit_type());
+
+  set_showing_submenu(false);
+  bounds.Offset(10, 10);
+  owner()->SetBounds(bounds);
+  EXPECT_FALSE(showing());
+  EXPECT_EQ(MenuController::ExitType::kAll, menu_controller()->exit_type());
+}
+
 // TODO(pkasting): The test below fails most of the time on Wayland; not clear
 // it's important to support this case.
 #if BUILDFLAG(ENABLE_DESKTOP_AURA) && !BUILDFLAG(IS_OZONE_WAYLAND)