Use FocusRing RoundRects to match toolbar inkdrops

Creates a SkPath that matches the inkdrop shape and use that as the
focus ring.

In the future, hopefully the inkdrop shape can be generated from the
same path so that they are forced to stay in sync. For now though they
are less separate as the focus ring shape is generated using the same
insets as the inkdrop.

This focus ring is installed for ToolbarButton, BrowserAppMenuButton and
ToolbarActionView (all are visibly toolbar buttons).

Bug: chromium:862925
Change-Id: I898d2d9c6a9e058e9f84aff37615ebf3ab41cee7
Reviewed-on: https://chromium-review.googlesource.com/1139221Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575653}

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

@@ -280,6 +280,15 @@ BrowserAppMenuButton::CreateDefaultBorder() const {
   return border;
 }
 
+void BrowserAppMenuButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
+  // TODO(pbos): Consolidate with ToolbarButton::OnBoundsChanged.
+  if (focus_ring()) {
+    focus_ring()->SetPath(CreateToolbarFocusRingPath(
+        this, gfx::Insets(0, 0, 0, margin_trailing_)));
+  }
+  AppMenuButton::OnBoundsChanged(previous_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

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

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

@@ -91,6 +91,14 @@ ToolbarActionView::~ToolbarActionView() {
   view_controller_->SetDelegate(nullptr);
 }
 
+void ToolbarActionView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
+  // TODO(pbos): Consolidate with ToolbarButton::OnBoundsChanged.
+  if (focus_ring()) {
+    focus_ring()->SetPath(CreateToolbarFocusRingPath(this, gfx::Insets()));
+  }
+  MenuButton::OnBoundsChanged(previous_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

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

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

@@ -119,9 +119,8 @@ bool ToolbarButton::IsMenuShowing() const {
 
 void ToolbarButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   if (focus_ring()) {
-    SkPath path;
-    path.addOval(gfx::RectToSkRect(GetLocalBounds()));
-    focus_ring()->SetPath(path);
+    focus_ring()->SetPath(CreateToolbarFocusRingPath(
+        this, gfx::Insets(0, leading_margin_, 0, 0)));
   }
   UpdateHighlightBackgroundAndInsets();
   LabelButton::OnBoundsChanged(previous_bounds);

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

@@ -54,6 +54,24 @@ gfx::Insets GetInkDropInsets(BaseInkDropHostView* host_view,
   return inkdrop_insets;
 }
 
+// Create a SkPath matching the toolbar inkdrops to be used for the focus ring.
+// TODO(pbos): Consolidate inkdrop effects, highlights and ripples along with
+// focus rings so that they are derived  from the same actual SkPath or other
+// shared primitive. That way they would be significantly easier to keep in
+// sync. This method at least reuses GetInkDropInsets.
+template <class BaseInkDropHostView>
+SkPath CreateToolbarFocusRingPath(BaseInkDropHostView* host_view,
+                                  const gfx::Insets& margin_insets) {
+  gfx::Rect rect(host_view->size());
+  rect.Inset(GetInkDropInsets(host_view, margin_insets));
+
+  SkPath path;
+  path.addRoundRect(gfx::RectToSkRect(rect),
+                    host_view->ink_drop_large_corner_radius(),
+                    host_view->ink_drop_large_corner_radius());
+  return path;
+}
+
 // Creates the appropriate ink drop for the calling button. When the newer
 // material UI is not enabled, it uses the default implementation of the
 // calling button's base class (the template argument BaseInkDropHostView).

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

@@ -600,9 +600,11 @@ void ToolbarView::Layout() {
   if (maximized)
     app_menu_width += end_padding;
 
+  // Set trailing margin before updating the bounds so OnBoundsChange can use
+  // the trailing margin.
+  app_menu_button_->SetTrailingMargin(maximized ? end_padding : 0);
   app_menu_button_->SetBounds(next_element_x, toolbar_button_y, app_menu_width,
                               toolbar_button_height);
-  app_menu_button_->SetTrailingMargin(maximized ? end_padding : 0);
 }
 
 void ToolbarView::OnPaintBackground(gfx::Canvas* canvas) {