Ensure that a submenu is shown within screen bounds

On smaller screen it is possible that there is no room for a submenu
on either side of the parent menu. In this case instead of changing
the side just move the submenu the minimum necessary amount to fit the
screen (see screenshots in the bug).

Bug: 991227
Test: MenuControllerTest.TestSubmenuFitsOnScreen
Change-Id: I149849e9b2cf44de1b79f45854c49af3f5fe9eb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1790475Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695667}

ui/views/controls/menu/menu_controller.cc

@@ -2459,22 +2459,36 @@ gfx::Rect MenuController::CalculateBubbleMenuBounds(MenuItemView* item,
     // the parent menu item and not to the right.
     const bool layout_is_rtl = base::i18n::IsRTL();
     const bool create_on_right = prefer_leading != layout_is_rtl;
+
+    const int width_with_right_inset =
+        menu_config.touchable_menu_width + border_and_shadow_insets.right();
+    const int x_max = monitor_bounds.right() - width_with_right_inset;
+    const int x_left = item_bounds.x() - width_with_right_inset;
+    const int x_right = item_bounds.right() - border_and_shadow_insets.left();
     if (create_on_right) {
-      x = item_bounds.right() - border_and_shadow_insets.left();
-      if (monitor_bounds.width() != 0 && (x + menu_config.touchable_menu_width -
-                                              border_and_shadow_insets.right() >
-                                          monitor_bounds.right())) {
+      x = x_right;
+      if (monitor_bounds.width() == 0 || x_right <= x_max) {
+        // Enough room on the right, show normally.
+        x = x_right;
+      } else if (x_left >= monitor_bounds.x()) {
+        // Enough room on the left, show there.
         *is_leading = prefer_leading;
-        x = item_bounds.x() - menu_config.touchable_menu_width -
-            border_and_shadow_insets.right();
+        x = x_left;
+      } else {
+        // No room on either side. Flush the menu to the right edge.
+        x = x_max;
       }
     } else {
-      x = item_bounds.x() - menu_config.touchable_menu_width -
-          border_and_shadow_insets.right();
-      if (monitor_bounds.width() != 0 && x < monitor_bounds.x()) {
+      if (monitor_bounds.width() == 0 || x_left >= monitor_bounds.x()) {
+        // Enough room on the left, show normally.
+        x = x_left;
+      } else if (x_right <= x_max) {
+        // Enough room on the right, show there.
         *is_leading = !prefer_leading;
-        x = item_bounds.x() + menu_config.touchable_menu_width -
-            border_and_shadow_insets.left();
+        x = x_right;
+      } else {
+        // No room on either side. Flush the menu to the left edge.
+        x = monitor_bounds.x();
       }
     }
     y = item_bounds.y() - border_and_shadow_insets.top() -

ui/views/controls/menu/menu_controller_unittest.cc

@@ -336,7 +336,8 @@ struct MenuBoundsOptions {
       MenuItemView::MenuPosition::kBestFit;
 };
 
-class MenuControllerTest : public ViewsTestBase {
+class MenuControllerTest : public ViewsTestBase,
+                           public testing::WithParamInterface<bool> {
  public:
   MenuControllerTest() = default;
 
@@ -344,6 +345,12 @@ class MenuControllerTest : public ViewsTestBase {
 
   // ViewsTestBase:
   void SetUp() override {
+    if (testing::UnitTest::GetInstance()->current_test_info()->value_param()) {
+      // Setup right to left environment if necessary.
+      if (GetParam())
+        base::i18n::SetRTLForTesting(true);
+    }
+
     std::unique_ptr<DestructingTestViewsDelegate> views_delegate(
         new DestructingTestViewsDelegate());
     test_views_delegate_ = views_delegate.get();
@@ -378,13 +385,18 @@ class MenuControllerTest : public ViewsTestBase {
                                                  &is_leading);
   }
 
-  gfx::Rect CalculateBubbleMenuBounds(const MenuBoundsOptions& options) {
+  gfx::Rect CalculateBubbleMenuBounds(const MenuBoundsOptions& options,
+                                      MenuItemView* menu_item) {
     SetUpMenuControllerForCalculateBounds(options);
     bool is_leading;
-    return menu_controller_->CalculateBubbleMenuBounds(menu_item_.get(), true,
+    return menu_controller_->CalculateBubbleMenuBounds(menu_item, true,
                                                        &is_leading);
   }
 
+  gfx::Rect CalculateBubbleMenuBounds(const MenuBoundsOptions& options) {
+    return CalculateBubbleMenuBounds(options, menu_item_.get());
+  }
+
 #if defined(USE_AURA)
   // Verifies that a non-nested menu fully closes when receiving an escape key.
   void TestAsyncEscapeKey() {
@@ -547,6 +559,32 @@ class MenuControllerTest : public ViewsTestBase {
     EXPECT_TRUE(options.monitor_bounds.Contains(final_bounds));
   }
 
+  void TestSubmenuFitsOnScreen(MenuItemView* item,
+                               const gfx::Rect& monitor_bounds,
+                               const gfx::Rect& parent_bounds) {
+    MenuBoundsOptions options;
+    options.menu_anchor = MenuAnchorPosition::kBubbleAbove;
+    options.monitor_bounds = monitor_bounds;
+
+    // Adjust the final bounds to not include the shadow and border.
+    const gfx::Insets border_and_shadow_insets =
+        BubbleBorder::GetBorderAndShadowInsets(
+            MenuConfig::instance().touchable_menu_shadow_elevation);
+
+    MenuItemView* parent_item = item->GetParentMenuItem();
+    SubmenuView* sub_menu = parent_item->GetSubmenu();
+
+    parent_item->SetBoundsRect(parent_bounds);
+    sub_menu->ShowAt(owner(), parent_item->bounds(), false);
+    gfx::Rect final_bounds = CalculateBubbleMenuBounds(options, item);
+    final_bounds.Inset(border_and_shadow_insets);
+    sub_menu->Close();
+
+    EXPECT_TRUE(options.monitor_bounds.Contains(final_bounds))
+        << options.monitor_bounds.ToString() << " does not contain "
+        << final_bounds.ToString();
+  }
+
  protected:
   void SetPendingStateItem(MenuItemView* item) {
     menu_controller_->pending_state_.item = item;
@@ -790,6 +828,8 @@ class MenuControllerTest : public ViewsTestBase {
   DISALLOW_COPY_AND_ASSIGN(MenuControllerTest);
 };
 
+INSTANTIATE_TEST_SUITE_P(, MenuControllerTest, testing::Bool());
+
 #if defined(USE_X11)
 // Tests that an event targeter which blocks events will be honored by the menu
 // event dispatcher.
@@ -1784,7 +1824,7 @@ TEST_F(MenuControllerTest, CalculateMenuBoundsMonitorFitTest) {
 }
 
 // Test that menus show up on screen with non-zero sized anchors.
-TEST_F(MenuControllerTest, TestMenuFitsOnScreen) {
+TEST_P(MenuControllerTest, TestMenuFitsOnScreen) {
   const int display_size = 500;
   // Simulate multiple display layouts.
   for (int x = -1; x <= 1; x++)
@@ -1798,7 +1838,7 @@ TEST_F(MenuControllerTest, TestMenuFitsOnScreen) {
 }
 
 // Test that menus show up on screen with zero sized anchors.
-TEST_F(MenuControllerTest, TestMenuFitsOnScreenSmallAnchor) {
+TEST_P(MenuControllerTest, TestMenuFitsOnScreenSmallAnchor) {
   const int display_size = 500;
   // Simulate multiple display layouts.
   for (int x = -1; x <= 1; x++)
@@ -1814,6 +1854,45 @@ TEST_F(MenuControllerTest, TestMenuFitsOnScreenSmallAnchor) {
     }
 }
 
+// Test that submenus are displayed within the screen bounds on smaller screens.
+TEST_P(MenuControllerTest, TestSubmenuFitsOnScreen) {
+  menu_controller()->set_use_touchable_layout(true);
+  MenuItemView* sub_item = menu_item()->GetSubmenu()->GetMenuItemAt(0);
+  sub_item->AppendMenuItemWithLabel(11, base::ASCIIToUTF16("Subitem.One"));
+
+  const int menu_width = MenuConfig::instance().touchable_menu_width;
+  const gfx::Size parent_size(menu_width, menu_width);
+
+  const int kDisplayWidth = parent_size.width() * 2;
+  const int kDisplayHeight = parent_size.height() * 2;
+
+  // Simulate multiple display layouts.
+  for (int x = -1; x <= 1; x++)
+    for (int y = -1; y <= 1; y++) {
+      const gfx::Rect monitor_bounds(x * kDisplayWidth, y * kDisplayHeight,
+                                     kDisplayWidth, kDisplayHeight);
+
+      const int x_min = monitor_bounds.x();
+      const int x_max = monitor_bounds.right() - parent_size.width();
+      const int x_mid = (x_min + x_max) / 2;
+
+      const int y_min = monitor_bounds.y();
+      const int y_max = monitor_bounds.bottom() - parent_size.height();
+      const int y_mid = (y_min + y_max) / 2;
+
+      TestSubmenuFitsOnScreen(sub_item, monitor_bounds,
+                              gfx::Rect(gfx::Point(x_min, y_min), parent_size));
+      TestSubmenuFitsOnScreen(sub_item, monitor_bounds,
+                              gfx::Rect(gfx::Point(x_max, y_min), parent_size));
+      TestSubmenuFitsOnScreen(sub_item, monitor_bounds,
+                              gfx::Rect(gfx::Point(x_mid, y_min), parent_size));
+      TestSubmenuFitsOnScreen(sub_item, monitor_bounds,
+                              gfx::Rect(gfx::Point(x_min, y_mid), parent_size));
+      TestSubmenuFitsOnScreen(sub_item, monitor_bounds,
+                              gfx::Rect(gfx::Point(x_min, y_max), parent_size));
+    }
+}
+
 // Test that a menu that was originally drawn below the anchor does not get
 // squished or move above the anchor when it grows vertically and horizontally
 // beyond the monitor bounds.