Fix touchable insets for Avatar/Cast toolbar icons

An earlier refactoring set zero-insets for touchable AvatarToolbarButton
instead of insets for TOOLBAR_BUTTON. This API was confusing to use so
SetLayoutInsets was changed and renamed to SetLayoutInsetDelta to make
it easier to use. Callers no longer need to include the call to
GetLayoutInsets(TOOLBAR_BUTTON).

CastToolbarButton having incorrect ink-drops was not a regression but
has always used a 16dp icon in both touchable and non-touchable modes.
This change also calls SetInsetDelta to make up for the missing 8dp.

Bug: chromium:895519
Change-Id: I30595913dfb8a19358d198d9f4e0dbcb37993117
Reviewed-on: https://chromium-review.googlesource.com/c/1281903
Reviewed-by: Bret Sepulveda <[email protected]>
Commit-Queue: Peter Boström <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#599765}(cherry picked from commit 019423b)
Reviewed-on: https://chromium-review.googlesource.com/c/1285321
Reviewed-by: Peter Boström <[email protected]>
Cr-Commit-Position: refs/branch-heads/3578@{#74}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}

Author: pbos

chrome/browser/ui/views/media_router/cast_toolbar_button.cc

@@ -9,10 +9,12 @@
 #include "chrome/browser/media/router/media_router_factory.h"
 #include "chrome/browser/media/router/media_router_metrics.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/media_router/media_router_ui_service.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/vector_icons/vector_icons.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/models/menu_model.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
@@ -83,6 +85,10 @@ void CastToolbarButton::UpdateIcon() {
   const gfx::VectorIcon& icon = GetCurrentIcon();
   SetImage(views::Button::STATE_NORMAL,
            gfx::CreateVectorIcon(icon, GetIconColor(&icon)));
+  // This icon is smaller than the touchable-UI expected 24dp, so we need to pad
+  // the insets to match.
+  SetLayoutInsetDelta(gfx::Insets(
+      ui::MaterialDesignController::IsTouchOptimizedUiEnabled() ? 4 : 0));
 }
 
 const gfx::VectorIcon& CastToolbarButton::GetCurrentIcon() const {

chrome/browser/ui/views/profiles/avatar_toolbar_button.cc

@@ -340,9 +340,7 @@ AvatarToolbarButton::SyncState AvatarToolbarButton::GetSyncState() const {
 
 void AvatarToolbarButton::SetInsets() {
   // In non-touch mode we use a larger-than-normal icon size for avatars as 16dp
-  // is hard to read for user avatars. This constant is correspondingly smaller
-  // than GetLayoutInsets(TOOLBAR_BUTTON).
-  SetLayoutInsets(ui::MaterialDesignController::IsTouchOptimizedUiEnabled()
-                      ? gfx::Insets()
-                      : GetLayoutInsets(TOOLBAR_BUTTON) - gfx::Insets(2));
+  // is hard to read for user avatars, so we need to set corresponding insets.
+  SetLayoutInsetDelta(gfx::Insets(
+      ui::MaterialDesignController::IsTouchOptimizedUiEnabled() ? 0 : -2));
 }

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

@@ -88,17 +88,18 @@ void ToolbarButton::UpdateHighlightBackgroundAndInsets() {
     SetEnabledTextColors(*highlight_color_);
   }
 
-  gfx::Insets insets = (layout_insets_ ? layout_insets_.value()
-                                       : GetLayoutInsets(TOOLBAR_BUTTON)) +
+  gfx::Insets insets = GetLayoutInsets(TOOLBAR_BUTTON) + layout_inset_delta_ +
                        gfx::Insets(0, leading_margin_, 0, 0);
   if (highlight_color_)
     insets += gfx::Insets(0, highlight_radius / 2, 0, 0);
 
   SetBorder(views::CreateEmptyBorder(insets));
 }
 
-void ToolbarButton::SetLayoutInsets(const gfx::Insets& insets) {
-  layout_insets_ = insets;
+void ToolbarButton::SetLayoutInsetDelta(const gfx::Insets& inset_delta) {
+  if (layout_inset_delta_ == inset_delta)
+    return;
+  layout_inset_delta_ = inset_delta;
   UpdateHighlightBackgroundAndInsets();
 }
 

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

@@ -97,8 +97,8 @@ class ToolbarButton : public views::LabelButton,
   // Function to show the dropdown menu.
   virtual void ShowDropDownMenu(ui::MenuSourceType source_type);
 
-  // Sets |layout_insets_|, see comment there.
-  void SetLayoutInsets(const gfx::Insets& insets);
+  // Sets |layout_inset_delta_|, see comment there.
+  void SetLayoutInsetDelta(const gfx::Insets& insets);
 
  private:
   friend test::ToolbarButtonTestApi;
@@ -136,11 +136,11 @@ class ToolbarButton : public views::LabelButton,
   // to extend to the full window width.
   int leading_margin_ = 0;
 
-  // Base layout insets that are used for the button. This is overridable as
-  // AvatarToolbarButton uses smaller insets to accommodate for a larger avatar
-  // avatar icon. |leading_margin_| and |ink_drop_large_corner_radius()| are
-  // also used to calculate final insets.
-  base::Optional<gfx::Insets> layout_insets_;
+  // Delta from regular toolbar-button insets. This is necessary for buttons
+  // that use smaller or larger icons than regular ToolbarButton instances.
+  // AvatarToolbarButton for instance uses smaller insets to accommodate for a
+  // larger-than-16dp avatar avatar icon outside of touchable mode.
+  gfx::Insets layout_inset_delta_;
 
   // A highlight color is used to signal error states. When set this color is
   // used as a base for background, text and ink drops. When not set, uses the