Refactored AppMenuAnimation

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

Index: chrome/browser/ui/views/toolbar/app_menu_button.cc
diff --git a/chrome/browser/ui/views/toolbar/app_menu_button.cc b/chrome/browser/ui/views/toolbar/app_menu_button.cc
index 272cc0d040ab95740e921be83559b1e7ada2e5b5..b41effd363e8e177d93441902b8afeee682bc9d1 100644
--- a/chrome/browser/ui/views/toolbar/app_menu_button.cc
+++ b/chrome/browser/ui/views/toolbar/app_menu_button.cc
@@ -18,16 +18,17 @@
 #include "chrome/browser/ui/browser_otr_state.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/toolbar/app_menu_animation.h"
 #include "chrome/browser/ui/toolbar/app_menu_model.h"
 #include "chrome/browser/ui/views/extensions/browser_action_drag_data.h"
 #include "chrome/browser/ui/views/toolbar/app_menu.h"
-#include "chrome/browser/ui/views/toolbar/app_menu_animation.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_button.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/grit/theme_resources.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
+#include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/keyboard/keyboard_controller.h"
@@ -36,7 +37,12 @@
 #include "ui/views/metrics.h"
 
 namespace {
-const float kIconSize = 16;
+
+constexpr float kIconSize = 16;
+
+// The radius of each dot in the icon.
+constexpr float kDotRadius = 2.0f;

[msw, 2017/05/05 18:47:38]

nit: maybe this should be owned by app_menu_animation.cc and passed as an arg to
the PaintDot function (so we don't need to duplicate this in the non-views UI?)
That said, I'm not sure why it's 2.0f here and 8.0f in
https://codereview.chromium.org/2859903003/diff/100001/chrome/browser/ui/coco...
(ignore this if the radii should really be different per-platform)

[spqchan, 2017/05/05 21:20:53]

Removed the delegate's PaintDot function so this is no longer applicable.

+
 }  // namespace
 
 // static
@@ -158,6 +164,32 @@ void AppMenuButton::TabInsertedAt(TabStripModel* tab_strip_model,
   AnimateIconIfPossible();
 }
 
+void AppMenuButton::PaintDot(const gfx::PointF point,
+                             SkColor color,
+                             gfx::SizeF size,
+                             gfx::Canvas* canvas) {
+  cc::PaintFlags flags;
+  flags.setColor(color);
+  flags.setStrokeWidth(size.height());
+  flags.setStrokeCap(cc::PaintFlags::kRound_Cap);
+  flags.setStyle(cc::PaintFlags::kFill_Style);
+  flags.setAntiAlias(true);

[msw, 2017/05/05 18:47:38]

aside: would avoiding anti aliasing avoid the blur mentioned in the bug?

[spqchan, 2017/05/05 21:20:53]

That's a really good point. I tried to remove it and the dots look choppy
without it. It looks like I'll need to explore this option a little more.

+  canvas->DrawRoundRect(gfx::RectF(point, size), kDotRadius, flags);
+}
+
+void AppMenuButton::AppMenuAnimationStarted() {
+  SetPaintToLayer();
+  layer()->SetFillsBoundsOpaquely(false);
+}
+
+void AppMenuButton::AppMenuAnimationEnded() {
+  DestroyLayer();
+}
+
+void AppMenuButton::InvalidateIcon() {
+  SchedulePaint();
+}
+
 void AppMenuButton::UpdateIcon(bool should_animate) {
   SkColor severity_color = gfx::kPlaceholderColor;
   SkColor toolbar_icon_color =
@@ -226,15 +258,6 @@ void AppMenuButton::AnimateIconIfPossible() {
   animation_->StartAnimation();
 }
 
-void AppMenuButton::AppMenuAnimationStarted() {
-  SetPaintToLayer();
-  layer()->SetFillsBoundsOpaquely(false);
-}
-
-void AppMenuButton::AppMenuAnimationEnded() {
-  DestroyLayer();
-}
-
 const char* AppMenuButton::GetClassName() const {
   return "AppMenuButton";
 }