Reland: Fix extensions badges and decorations positions

The extensions browser actions badges and decorations should
be positioned at the corners of the icon area, rather than the
corners of the full button's area (i.e. ignoring the paddings).

Screenshots: https://bugs.chromium.org/p/chromium/issues/detail?id=831946#c17

BUG=831946

Change-Id: Ib7f4123aa2349dcba5c10e34927d5ef787c53764
Reviewed-on: https://chromium-review.googlesource.com/1012868
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551570}

chrome/browser/ui/extensions/icon_with_badge_image_source.cc

@@ -12,6 +12,7 @@
 #include "build/build_config.h"
 #include "cc/paint/paint_flags.h"
 #include "chrome/browser/extensions/extension_action.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
 #include "chrome/grit/theme_resources.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
@@ -23,13 +24,6 @@
 
 namespace {
 
-const int kPadding = 2;
-const int kBadgeHeight = 11;
-const int kMaxTextWidth = 23;
-
-// The minimum width for center-aligning the badge.
-const int kCenterAlignThreshold = 20;
-
 gfx::ImageSkiaRep ScaleImageSkiaRep(const gfx::ImageSkiaRep& rep,
                                     int target_width_dp,
                                     float target_scale) {
@@ -110,6 +104,7 @@ void IconWithBadgeImageSource::PaintBadge(gfx::Canvas* canvas) {
           ? gfx::kGoogleBlue500
           : SkColorSetA(badge_->background_color, SK_AlphaOPAQUE);
 
+  constexpr int kBadgeHeight = 12;
   ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
   gfx::FontList base_font = rb->GetFontList(ui::ResourceBundle::BaseFont)
                                 .DeriveWithHeightUpperBound(kBadgeHeight);
@@ -117,7 +112,7 @@ void IconWithBadgeImageSource::PaintBadge(gfx::Canvas* canvas) {
 
   // See if we can squeeze a slightly larger font into the badge given the
   // actual string that is to be displayed.
-  const int kMaxIncrementAttempts = 5;
+  constexpr int kMaxIncrementAttempts = 5;
   for (size_t i = 0; i < kMaxIncrementAttempts; ++i) {
     int w = 0;
     int h = 0;
@@ -130,23 +125,32 @@ void IconWithBadgeImageSource::PaintBadge(gfx::Canvas* canvas) {
     base_font = bigger_font;
   }
 
+  constexpr int kMaxTextWidth = 23;
   const int text_width =
         std::min(kMaxTextWidth, canvas->GetStringWidth(utf16_text, base_font));
   // Calculate badge size. It is clamped to a min width just because it looks
   // silly if it is too skinny.
+  constexpr int kPadding = 2;
   int badge_width = text_width + kPadding * 2;
+
+  const gfx::Rect icon_area = GetIconAreaRect();
+
   // Force the pixel width of badge to be either odd (if the icon width is odd)
   // or even otherwise. If there is a mismatch you get http://crbug.com/26400.
-  if (size().width() != 0 && (badge_width % 2 != size().width() % 2))
+  if (icon_area.width() != 0 && (badge_width % 2 != icon_area.width() % 2))
     badge_width += 1;
   badge_width = std::max(kBadgeHeight, badge_width);
 
+  // The minimum width for center-aligning the badge.
+  constexpr int kCenterAlignThreshold = 20;
   // Calculate the badge background rect. It is usually right-aligned, but it
   // can also be center-aligned if it is large.
-  gfx::Rect rect(badge_width >= kCenterAlignThreshold
-                     ? (size().width() - badge_width) / 2
-                     : size().width() - badge_width,
-                 size().height() - kBadgeHeight, badge_width, kBadgeHeight);
+  const int badge_offset_x = badge_width >= kCenterAlignThreshold
+                                 ? (icon_area.width() - badge_width) / 2
+                                 : icon_area.width() - badge_width;
+  const int badge_offset_y = icon_area.height() - kBadgeHeight;
+  gfx::Rect rect(icon_area.x() + badge_offset_x, icon_area.y() + badge_offset_y,
+                 badge_width, kBadgeHeight);
   cc::PaintFlags rect_flags;
   rect_flags.setStyle(cc::PaintFlags::kFill_Style);
   rect_flags.setAntiAlias(true);
@@ -157,10 +161,11 @@ void IconWithBadgeImageSource::PaintBadge(gfx::Canvas* canvas) {
   cutout_rect.Inset(-1, -1);
   cc::PaintFlags cutout_flags = rect_flags;
   cutout_flags.setBlendMode(SkBlendMode::kClear);
-  canvas->DrawRoundRect(cutout_rect, 2, cutout_flags);
+  constexpr int kOuterCornerRadius = 3;
+  canvas->DrawRoundRect(cutout_rect, kOuterCornerRadius, cutout_flags);
 
   // Paint the backdrop.
-  canvas->DrawRoundRect(rect, 1, rect_flags);
+  canvas->DrawRoundRect(rect, kOuterCornerRadius - 1, rect_flags);
 
   // Paint the text.
   rect.Inset(std::max(kPadding, (rect.width() - text_width) / 2),
@@ -169,19 +174,21 @@ void IconWithBadgeImageSource::PaintBadge(gfx::Canvas* canvas) {
 }
 
 void IconWithBadgeImageSource::PaintPageActionDecoration(gfx::Canvas* canvas) {
-  static const SkColor decoration_color = SkColorSetARGB(255, 70, 142, 226);
-
-  int major_radius = std::ceil(size().width() / 5.0);
-  int minor_radius = std::ceil(major_radius / 2.0);
-  gfx::Point center_point(major_radius + 1, size().height() - (major_radius)-1);
+  const gfx::Rect icon_area = GetIconAreaRect();
+  constexpr float kMajorRadius = 4.5;
+  constexpr float kMinorRadius = 3;
+  // This decoration is positioned at the bottom left corner of the icon area.
+  gfx::PointF center_point = gfx::PointF(icon_area.bottom_left());
+  center_point.Offset(kMajorRadius + 1, -kMajorRadius - 1);
   cc::PaintFlags flags;
   flags.setAntiAlias(true);
   flags.setStyle(cc::PaintFlags::kFill_Style);
   flags.setColor(SK_ColorTRANSPARENT);
   flags.setBlendMode(SkBlendMode::kSrc);
-  canvas->DrawCircle(center_point, major_radius, flags);
+  canvas->DrawCircle(center_point, kMajorRadius, flags);
+  constexpr SkColor decoration_color = SkColorSetARGB(255, 70, 142, 226);
   flags.setColor(decoration_color);
-  canvas->DrawCircle(center_point, minor_radius, flags);
+  canvas->DrawCircle(center_point, kMinorRadius, flags);
 }
 
 void IconWithBadgeImageSource::PaintBlockedActionDecoration(
@@ -190,6 +197,14 @@ void IconWithBadgeImageSource::PaintBlockedActionDecoration(
   gfx::ImageSkia img =
       *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
           IDR_BLOCKED_EXTENSION_SCRIPT);
-  canvas->DrawImageInt(img, size().width() - img.width(), 0);
+  // This decoration is positioned at the top right corner of the icon area.
+  const gfx::Point top_right = GetIconAreaRect().top_right();
+  canvas->DrawImageInt(img, top_right.x() - img.width(), top_right.y());
   canvas->Restore();
 }
+
+gfx::Rect IconWithBadgeImageSource::GetIconAreaRect() const {
+  gfx::Rect icon_area(size());
+  icon_area.ClampToCenteredSize(ToolbarActionsBar::GetIconAreaSize());
+  return icon_area;
+}

chrome/browser/ui/extensions/icon_with_badge_image_source.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/image/canvas_image_source.h"
 #include "ui/gfx/image/image.h"
 
@@ -70,6 +71,13 @@ class IconWithBadgeImageSource : public gfx::CanvasImageSource {
   // a blocked action that wants to run.
   void PaintBlockedActionDecoration(gfx::Canvas* canvas);
 
+  // The toolbar action view may have different values of paddings depending on
+  // the current material design mode (See ToolbarActionsBar::GetViewSize()). In
+  // all cases, our badges and decorations should be positions at the corners of
+  // the area where the icon exists (ignoring all the paddings).
+  // https://crbug.com/831946.
+  gfx::Rect GetIconAreaRect() const;
+
   // The base icon to draw.
   gfx::Image icon_;
 

chrome/browser/ui/toolbar/toolbar_actions_bar.cc

@@ -134,15 +134,20 @@ void ToolbarActionsBar::RegisterProfilePrefs(
                               0);
 }
 
-gfx::Size ToolbarActionsBar::GetViewSize() const {
+// static
+gfx::Size ToolbarActionsBar::GetIconAreaSize() {
 #if defined(OS_MACOSX)
   // On the Mac, the spec is a 24x24 button in a 28x28 space.
-  constexpr gfx::Size kSize(24, 24);
+  constexpr gfx::Size kIconAreaSize(24, 24);
 #else
-  constexpr gfx::Size kSize(28, 28);
+  constexpr gfx::Size kIconAreaSize(28, 28);
 #endif
-  gfx::Rect rect(kSize);
-  rect.Inset(-GetLayoutInsets(TOOLBAR_ACTION_VIEW));
+  return kIconAreaSize;
+}
+
+gfx::Size ToolbarActionsBar::GetViewSize() const {
+  gfx::Rect rect(GetIconAreaSize());
+  rect.Inset(-GetIconAreaInsets());
   return rect.size();
 }
 
@@ -580,6 +585,10 @@ void ToolbarActionsBar::set_extension_bubble_appearance_wait_time_for_testing(
   g_extension_bubble_appearance_wait_time_in_seconds = time_in_seconds;
 }
 
+gfx::Insets ToolbarActionsBar::GetIconAreaInsets() const {
+  return GetLayoutInsets(TOOLBAR_ACTION_VIEW);
+}
+
 void ToolbarActionsBar::OnToolbarActionAdded(
     const ToolbarActionsModel::ToolbarItem& item,
     int index) {

chrome/browser/ui/toolbar/toolbar_actions_bar.h

@@ -85,8 +85,11 @@ class ToolbarActionsBar : public ToolbarActionsModel::Observer,
   // Registers profile preferences.
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
+  // Returns the size of the area where the action icon resides.
+  static gfx::Size GetIconAreaSize();
+
   // Returns the size of ToolbarActionView.
-  virtual gfx::Size GetViewSize() const;
+  gfx::Size GetViewSize() const;
 
   // Returns the default/full size for the toolbar; this does *not* reflect any
   // animations that may be running.
@@ -251,6 +254,10 @@ class ToolbarActionsBar : public ToolbarActionsModel::Observer,
       int time_in_seconds);
 
  private:
+  // Returns the insets by which the icon area bounds (See GetIconAreaRect())
+  // are insetted. This defines the amount of paddings around the icon area.
+  virtual gfx::Insets GetIconAreaInsets() const;
+
   // ToolbarActionsModel::Observer:
   void OnToolbarActionAdded(const ToolbarActionsModel::ToolbarItem& item,
                             int index) override;

chrome/browser/ui/views/frame/hosted_app_button_container.cc

@@ -27,8 +27,6 @@ namespace {
 
 bool g_animation_disabled_for_testing = false;
 
-constexpr int kBrowserActionSize = 32;
-
 constexpr base::TimeDelta kContentSettingsFadeInDuration =
     base::TimeDelta::FromMilliseconds(500);
 
@@ -36,10 +34,10 @@ class HostedAppToolbarActionsBar : public ToolbarActionsBar {
  public:
   using ToolbarActionsBar::ToolbarActionsBar;
 
-  gfx::Size GetViewSize() const override {
-    // TODO(calamity): Unify this toolbar action size with other clients once
+  gfx::Insets GetIconAreaInsets() const override {
+    // TODO(calamity): Unify these toolbar action insets with other clients once
     // all toolbar button sizings are consolidated. https://crbug.com/822967.
-    return gfx::Size(kBrowserActionSize, kBrowserActionSize);
+    return gfx::Insets(2);
   }
 
   size_t GetIconCount() const override {