Respect the cached menu position when calculating bubble menu bounds

Currently the calculation of the bubble menu bounds fails to take the
cached menu position into consideration while calculating the normal
menu bounds does. This CL fixes the inconsistency.

Bug: 1126244
Change-Id: I783b435c082e766000457cf014c40e0931b2bb98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2394727
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806801}

ui/views/controls/menu/menu_controller.cc

@@ -2389,13 +2389,16 @@ gfx::Rect MenuController::CalculateBubbleMenuBounds(MenuItemView* item,
     // This is a top-level menu, position it relative to the anchor bounds.
     const gfx::Rect& anchor_bounds = state_.initial_bounds;
 
+    using MenuPosition = MenuItemView::MenuPosition;
+
     // First the size gets reduced to the possible space.
     if (!monitor_bounds.IsEmpty()) {
       int max_width = monitor_bounds.width();
       int max_height = monitor_bounds.height();
       // In case of bubbles, the maximum width is limited by the space
       // between the display corner and the target area + the tip size.
-      if (state_.anchor == MenuAnchorPosition::kBubbleAbove) {
+      if (state_.anchor == MenuAnchorPosition::kBubbleAbove ||
+          item->actual_menu_position() == MenuPosition::kAboveBounds) {
         // Don't consider |border_and_shadow_insets| because when the max size
         // is enforced, the scroll view is shown and the md shadows are not
         // applied.
@@ -2432,6 +2435,7 @@ gfx::Rect MenuController::CalculateBubbleMenuBounds(MenuItemView* item,
         y = anchor_bounds.bottom() - border_and_shadow_insets.top() +
             menu_config.touchable_anchor_offset;
       }
+      item->set_actual_menu_position(MenuPosition::kAboveBounds);
     } else if (state_.anchor == MenuAnchorPosition::kBubbleLeft ||
                state_.anchor == MenuAnchorPosition::kBubbleRight) {
       if (state_.anchor == MenuAnchorPosition::kBubbleLeft) {
@@ -2461,18 +2465,35 @@ gfx::Rect MenuController::CalculateBubbleMenuBounds(MenuItemView* item,
       const int y_below = anchor_bounds.y() - border_and_shadow_insets.top();
       const int y_above = anchor_bounds.bottom() - menu_size.height() +
                           border_and_shadow_insets.bottom();
-      if (y_below + menu_size.height() <= monitor_bounds.bottom()) {
+
+      const bool able_to_show_below =
+          (y_below + menu_size.height() <= monitor_bounds.bottom());
+      const bool able_to_show_above = (y_above >= monitor_bounds.y());
+
+      // Respect the actual menu position calculated earlier if possible, to
+      // prevent changing positions during menu size updates.
+
+      if (item->actual_menu_position() == MenuPosition::kBelowBounds &&
+          able_to_show_below) {
+        y = y_below;
+      } else if (item->actual_menu_position() == MenuPosition::kAboveBounds &&
+                 able_to_show_above) {
+        y = y_above;
+      } else if (able_to_show_below) {
         // Show below the anchor. Align the top of the menu with the top of the
         // anchor.
         y = y_below;
-      } else if (y_above >= monitor_bounds.y()) {
+        item->set_actual_menu_position(MenuPosition::kBelowBounds);
+      } else if (able_to_show_above) {
         // No room below, but there is room above. Show above the anchor. Align
         // the bottom of the menu with the bottom of the anchor.
         y = y_above;
+        item->set_actual_menu_position(MenuPosition::kAboveBounds);
       } else {
         // No room above or below. Show as low as possible. Align the bottom of
         // the menu with the bottom of the screen.
         y = monitor_bounds.bottom() - menu_size.height();
+        item->set_actual_menu_position(MenuPosition::kBestFit);
       }
     }
     // The above adjustments may have shifted a large menu off the screen.

ui/views/controls/menu/menu_controller_unittest.cc

@@ -988,6 +988,70 @@ TEST_F(MenuControllerTest, InitialSelectedItem) {
   ResetSelection();
 }
 
+// Verifies that the context menu bubble should prioritize its cached menu
+// position (above or below the anchor) after its size updates
+// (https://crbug.com/1126244).
+TEST_F(MenuControllerTest, VerifyMenuBubblePositionAfterSizeChanges) {
+  constexpr gfx::Rect monitor_bounds(0, 0, 500, 500);
+  constexpr gfx::Size menu_size(100, 200);
+  const gfx::Insets border_and_shadow_insets =
+      BubbleBorder::GetBorderAndShadowInsets(
+          MenuConfig::instance().touchable_menu_shadow_elevation);
+
+  // Calculate the suitable anchor point to ensure that if the menu shows below
+  // the anchor point, the bottom of the menu should be one pixel off the
+  // bottom of the display. It means that there is insufficient space for the
+  // menu below the anchor.
+  const gfx::Point anchor_point(monitor_bounds.width() / 2,
+                                monitor_bounds.bottom() + 1 -
+                                    menu_size.height() +
+                                    border_and_shadow_insets.top());
+
+  MenuBoundsOptions options;
+  options.menu_anchor = MenuAnchorPosition::kBubbleRight;
+  options.monitor_bounds = monitor_bounds;
+  options.anchor_bounds = gfx::Rect(anchor_point, gfx::Size());
+
+  // Case 1: There is insufficient space for the menu below `anchor_point` and
+  // there is no cached menu position. The menu should show above the anchor.
+  {
+    options.menu_size = menu_size;
+    ASSERT_GT(options.anchor_bounds.y() - border_and_shadow_insets.top() +
+                  menu_size.height(),
+              monitor_bounds.bottom());
+    CalculateBubbleMenuBounds(options);
+    EXPECT_EQ(MenuItemView::MenuPosition::kAboveBounds,
+              menu_item()->ActualMenuPosition());
+  }
+
+  // Case 2: There is insufficient space for the menu below `anchor_point`. The
+  // cached position is below the anchor. The menu should show above the anchor.
+  {
+    options.menu_size = menu_size;
+    options.menu_position = MenuItemView::MenuPosition::kBelowBounds;
+    CalculateBubbleMenuBounds(options);
+    EXPECT_EQ(MenuItemView::MenuPosition::kAboveBounds,
+              menu_item()->ActualMenuPosition());
+  }
+
+  // Case 3: There is enough space for the menu below `anchor_point`. The cached
+  // menu position is above the anchor. The menu should show above the anchor.
+  {
+    // Shrink the menu size. Verify that there is enough space below the anchor
+    // point now.
+    constexpr gfx::Size updated_size(menu_size.width(), menu_size.height() / 2);
+    options.menu_size = updated_size;
+    EXPECT_LE(options.anchor_bounds.y() - border_and_shadow_insets.top() +
+                  updated_size.height(),
+              monitor_bounds.bottom());
+
+    options.menu_position = MenuItemView::MenuPosition::kAboveBounds;
+    CalculateBubbleMenuBounds(options);
+    EXPECT_EQ(MenuItemView::MenuPosition::kAboveBounds,
+              menu_item()->ActualMenuPosition());
+  }
+}
+
 // Tests that opening the menu and pressing 'Home' selects the first menu item.
 TEST_F(MenuControllerTest, FirstSelectedItem) {
   SetPendingStateItem(menu_item()->GetSubmenu()->GetMenuItemAt(0));