Refactored AppMenuAnimation

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/toolbar/app_menu_button.h"
 
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "cc/paint/paint_flags.h"
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/browser.h"
 #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"
 #include "ui/views/controls/button/label_button_border.h"
 #include "ui/views/controls/menu/menu_listener.h"
 #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
 bool AppMenuButton::g_open_app_immediately_for_testing = false;
 
 AppMenuButton::AppMenuButton(ToolbarView* toolbar_view)
     : views::MenuButton(base::string16(), toolbar_view, false),
       severity_(AppMenuIconController::Severity::NONE),
       type_(AppMenuIconController::IconType::NONE),
       toolbar_view_(toolbar_view),
@@ skipping 101 matching lines @@
   animation_->PaintAppMenu(canvas, bounds);
 }
 
 void AppMenuButton::TabInsertedAt(TabStripModel* tab_strip_model,
                                   content::WebContents* contents,
                                   int index,
                                   bool foreground) {
   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 =
       GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON);
   const ui::NativeTheme* native_theme = GetNativeTheme();
   switch (severity_) {
     case AppMenuIconController::Severity::NONE:
       severity_color = toolbar_icon_color;
       break;
     case AppMenuIconController::Severity::LOW:
@@ skipping 48 matching lines @@
 
 void AppMenuButton::AnimateIconIfPossible() {
   if (!animation_ || !should_use_new_icon_ ||
       severity_ == AppMenuIconController::Severity::NONE) {
     return;
   }
 
   animation_->StartAnimation();
 }
 
-void AppMenuButton::AppMenuAnimationStarted() {
-  SetPaintToLayer();
-  layer()->SetFillsBoundsOpaquely(false);
-}
-
-void AppMenuButton::AppMenuAnimationEnded() {
-  DestroyLayer();
-}
-
 const char* AppMenuButton::GetClassName() const {
   return "AppMenuButton";
 }
 
 std::unique_ptr<views::LabelButtonBorder> AppMenuButton::CreateDefaultBorder()
     const {
   std::unique_ptr<views::LabelButtonBorder> border =
       MenuButton::CreateDefaultBorder();
 
   // Adjust border insets to follow the margin change,
@@ skipping 44 matching lines @@
   return ui::DragDropTypes::DRAG_MOVE;
 }
 
 void AppMenuButton::OnDragExited() {
   weak_factory_.InvalidateWeakPtrs();
 }
 
 int AppMenuButton::OnPerformDrop(const ui::DropTargetEvent& event) {
   return ui::DragDropTypes::DRAG_MOVE;
 }