Use inset anchor bounds for toolbar buttons

Adds a Views::GetAnchorBoundsInScreen() method. This defaults to
GetBoundsInScreen() but can be overridden for views whose visual
bounds differ from their actual bounds. This is the case for toolbar
buttons on Touch (that provide larger hit targets than their visual
size).

This method is then used for the menus spawned by AppMenuButton and
ToolbarButton as well as BubbleDialogDelegate which allows these
specific menus and BubbleDialogDelegates in general to be better
aligned visually to their anchors.

Bug: chromium:800372, chromium:869928
Change-Id: I9c516d8a7ecb6c922e347d4db61ce1db72c71be7
Reviewed-on: https://chromium-review.googlesource.com/1152525
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579861}

chrome/browser/ui/views/toolbar/app_menu.cc

@@ -827,11 +827,9 @@ void AppMenu::Init(ui::MenuModel* model) {
 }
 
 void AppMenu::RunMenu(views::MenuButton* host) {
-  gfx::Point screen_loc;
-  views::View::ConvertPointToScreen(host, &screen_loc);
-  gfx::Rect bounds(screen_loc, host->size());
   base::RecordAction(UserMetricsAction("ShowAppMenu"));
-  menu_runner_->RunMenuAt(host->GetWidget(), host, bounds,
+  menu_runner_->RunMenuAt(host->GetWidget(), host,
+                          host->GetAnchorBoundsInScreen(),
                           views::MENU_ANCHOR_TOPRIGHT, ui::MENU_SOURCE_NONE);
 }
 

chrome/browser/ui/views/toolbar/browser_app_menu_button.cc

@@ -291,6 +291,22 @@ void BrowserAppMenuButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   AppMenuButton::OnBoundsChanged(previous_bounds);
 }
 
+gfx::Rect BrowserAppMenuButton::GetAnchorBoundsInScreen() const {
+  gfx::Rect bounds = GetBoundsInScreen();
+  gfx::Insets insets =
+      GetInkDropInsets(this, gfx::Insets(0, 0, 0, margin_trailing_));
+  // If the button is extended, don't inset the trailing edge. The anchored menu
+  // should extend to the screen edge as well so the menu is easier to hit
+  // (Fitts's law).
+  // TODO(pbos): Make sure the button is aware of that it is being extended or
+  // not (margin_trailing_ cannot be used as it can be 0 in fullscreen on
+  // Touch). When this is implemented, use 0 as a replacement for
+  // margin_trailing_ in fullscreen only. Always keep the rest.
+  insets.Set(insets.top(), 0, insets.bottom(), 0);
+  bounds.Inset(insets);
+  return bounds;
+}
+
 gfx::Rect BrowserAppMenuButton::GetThemePaintRect() const {
   gfx::Rect rect(MenuButton::GetThemePaintRect());
   rect.Inset(0, 0, margin_trailing_, 0);

chrome/browser/ui/views/toolbar/browser_app_menu_button.h

@@ -47,6 +47,7 @@ class BrowserAppMenuButton : public AppMenuButton,
 
   // views::MenuButton:
   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
+  gfx::Rect GetAnchorBoundsInScreen() const override;
   gfx::Size CalculatePreferredSize() const override;
   void Layout() override;
   void OnThemeChanged() override;

chrome/browser/ui/views/toolbar/toolbar_action_view.cc

@@ -98,6 +98,12 @@ void ToolbarActionView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   MenuButton::OnBoundsChanged(previous_bounds);
 }
 
+gfx::Rect ToolbarActionView::GetAnchorBoundsInScreen() const {
+  gfx::Rect bounds = GetBoundsInScreen();
+  bounds.Inset(GetInkDropInsets(this, gfx::Insets()));
+  return bounds;
+}
+
 void ToolbarActionView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   views::MenuButton::GetAccessibleNodeData(node_data);
   node_data->role = ax::mojom::Role::kButton;

chrome/browser/ui/views/toolbar/toolbar_action_view.h

@@ -63,6 +63,7 @@ class ToolbarActionView : public views::MenuButton,
 
   // views::MenuButton:
   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
+  gfx::Rect GetAnchorBoundsInScreen() const override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   std::unique_ptr<views::LabelButtonBorder> CreateDefaultBorder()
       const override;

chrome/browser/ui/views/toolbar/toolbar_button.cc

@@ -127,6 +127,22 @@ void ToolbarButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   LabelButton::OnBoundsChanged(previous_bounds);
 }
 
+gfx::Rect ToolbarButton::GetAnchorBoundsInScreen() const {
+  gfx::Rect bounds = GetBoundsInScreen();
+  gfx::Insets insets =
+      GetInkDropInsets(this, gfx::Insets(0, leading_margin_, 0, 0));
+  // If the button is extended, don't inset the leading edge. The anchored menu
+  // should extend to the screen edge as well so the menu is easier to hit
+  // (Fitts's law).
+  // TODO(pbos): Make sure the button is aware of that it is being extended or
+  // not (leading_margin_ cannot be used as it can be 0 in fullscreen on Touch).
+  // When this is implemented, use 0 as a replacement for leading_margin_ in
+  // fullscreen only. Always keep the rest.
+  insets.Set(insets.top(), 0, insets.bottom(), 0);
+  bounds.Inset(insets);
+  return bounds;
+}
+
 bool ToolbarButton::OnMousePressed(const ui::MouseEvent& event) {
   if (IsTriggerableEvent(event) && enabled() && ShouldShowMenu() &&
       HitTestPoint(event.location())) {
@@ -260,16 +276,7 @@ void ToolbarButton::ShowDropDownMenu(ui::MenuSourceType source_type) {
   if (!ShouldShowMenu())
     return;
 
-  gfx::Rect lb = GetLocalBounds();
-
-  // Both the menu position and the menu anchor type change if the UI layout
-  // is right-to-left.
-  gfx::Point menu_position(lb.origin());
-  menu_position.Offset(0, lb.height() - 1);
-  if (base::i18n::IsRTL())
-    menu_position.Offset(lb.width() - 1, 0);
-
-  View::ConvertPointToScreen(this, &menu_position);
+  gfx::Rect menu_anchor_bounds = GetAnchorBoundsInScreen();
 
 #if defined(OS_CHROMEOS)
   // A window won't overlap between displays on ChromeOS.
@@ -287,8 +294,8 @@ void ToolbarButton::ShowDropDownMenu(ui::MenuSourceType source_type) {
       screen->GetDisplayNearestPoint(screen->GetCursorScreenPoint());
   int left_bound = display.bounds().x();
 #endif
-  if (menu_position.x() < left_bound)
-    menu_position.set_x(left_bound);
+  if (menu_anchor_bounds.x() < left_bound)
+    menu_anchor_bounds.set_x(left_bound);
 
   // Make the button look depressed while the menu is open.
   SetState(STATE_PRESSED);
@@ -312,8 +319,7 @@ void ToolbarButton::ShowDropDownMenu(ui::MenuSourceType source_type) {
   menu_model_adapter_->set_triggerable_event_flags(triggerable_event_flags());
   menu_runner_ = std::make_unique<views::MenuRunner>(
       menu_model_adapter_->CreateMenu(), views::MenuRunner::HAS_MNEMONICS);
-  menu_runner_->RunMenuAt(GetWidget(), nullptr,
-                          gfx::Rect(menu_position, gfx::Size(0, 0)),
+  menu_runner_->RunMenuAt(GetWidget(), nullptr, menu_anchor_bounds,
                           views::MENU_ANCHOR_TOPLEFT, source_type);
 }
 

chrome/browser/ui/views/toolbar/toolbar_button.h

@@ -66,6 +66,7 @@ class ToolbarButton : public views::LabelButton,
 
   // views::LabelButton:
   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
+  gfx::Rect GetAnchorBoundsInScreen() const override;
   bool OnMousePressed(const ui::MouseEvent& event) override;
   bool OnMouseDragged(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;

ui/views/bubble/bubble_dialog_delegate.cc

@@ -206,7 +206,7 @@ gfx::Rect BubbleDialogDelegateView::GetAnchorRect() const {
   if (!GetAnchorView())
     return anchor_rect_;
 
-  anchor_rect_ = GetAnchorView()->GetBoundsInScreen();
+  anchor_rect_ = GetAnchorView()->GetAnchorBoundsInScreen();
   anchor_rect_.Inset(anchor_view_insets_);
   return anchor_rect_;
 }

ui/views/bubble/bubble_dialog_delegate.h

@@ -89,6 +89,8 @@ class VIEWS_EXPORT BubbleDialogDelegateView : public DialogDelegateView,
     title_margins_ = title_margins;
   }
 
+  // TODO(pbos): Remove by overriding Views::GetAnchorBoundsInScreen() instead.
+  // See https://crbug.com/869928.
   const gfx::Insets& anchor_view_insets() const { return anchor_view_insets_; }
   void set_anchor_view_insets(const gfx::Insets& i) { anchor_view_insets_ = i; }
 

ui/views/view.cc

@@ -419,6 +419,10 @@ gfx::Rect View::GetBoundsInScreen() const {
   return gfx::Rect(origin, size());
 }
 
+gfx::Rect View::GetAnchorBoundsInScreen() const {
+  return GetBoundsInScreen();
+}
+
 gfx::Size View::GetPreferredSize() const {
   if (preferred_size_)
     return *preferred_size_;

ui/views/view.h

@@ -363,6 +363,11 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
   // Return the bounds of the View in screen coordinate system.
   gfx::Rect GetBoundsInScreen() const;
 
+  // Return the bounds that an anchored widget should anchor to. These can be
+  // different from |GetBoundsInScreen()| when a view is larger than its visible
+  // size, for instance to provide a larger hittable area.
+  virtual gfx::Rect GetAnchorBoundsInScreen() const;
+
   // Returns the baseline of this view, or -1 if this view has no baseline. The
   // return value is relative to the preferred height.
   virtual int GetBaseline() const;