Round the corners of bookmark button ink drop effects.

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

 // Copyright (c) 2012 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/bookmarks/bookmark_bar_view.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <limits>
@@ skipping 72 matching lines @@
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/image/image_skia_operations.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/scoped_canvas.h"
 #include "ui/gfx/text_constants.h"
 #include "ui/gfx/text_elider.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_highlight.h"
 #include "ui/views/animation/ink_drop_impl.h"
+#include "ui/views/animation/ink_drop_mask.h"
 #include "ui/views/button_drag_utils.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/label_button_border.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/drag_utils.h"
 #include "ui/views/metrics.h"
 #include "ui/views/view_constants.h"
 #include "ui/views/widget/tooltip_manager.h"
 #include "ui/views/widget/widget.h"
@@ skipping 11 matching lines @@
 using views::LabelButtonBorder;
 using views::MenuButton;
 using views::View;
 
 // How inset the bookmarks bar is when displayed on the new tab page.
 static const int kNewTabHorizontalPadding = 2;
 
 // Maximum size of buttons on the bookmark bar.
 static const int kMaxButtonWidth = 150;
 
+// Corner radius for masking the ink drop effects on buttons.
+static const int kInkDropCornerRadius = 2;
+
 // Number of pixels the attached bookmark bar overlaps with the toolbar.
 static const int kToolbarAttachedBookmarkBarOverlap = 3;
 
 // Margins around the content.
 static const int kDetachedTopMargin = 1;  // When attached, we use 0 and let the
                                           // toolbar above serve as the margin.
 static const int kBottomMargin = 2;
 static const int kHorizontalMargin = 4;
 
 // Padding between buttons.
@@ skipping 101 matching lines @@
       const override {
     gfx::RectF bounds((gfx::Rect(size())));
     bounds.Inset(gfx::InsetsF(
         kInkDropVerticalInsetPx /
             GetWidget()->GetLayer()->GetCompositor()->device_scale_factor(),
         0));
     return base::MakeUnique<views::InkDropHighlight>(
         bounds.size(), 0, bounds.CenterPoint(), GetInkDropBaseColor());
   }
 
+// Ink drop masks don't work on Windows. See crbug.com/713359
+#if !defined(OS_WIN)
+  std::unique_ptr<views::InkDropMask> CreateInkDropMask() const override {
+    return base::MakeUnique<views::RoundRectInkDropMask>(size(), kInkDropInsets,
+                                                         kInkDropCornerRadius);
+  }
+#endif
+
   SkColor GetInkDropBaseColor() const override {
     return GetThemeProvider()->GetColor(
         ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON);
   }
 
  private:
   std::unique_ptr<gfx::SlideAnimation> show_animation_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkButtonBase);
 };
@@ skipping 105 matching lines @@
       const override {
     gfx::RectF bounds((gfx::Rect(size())));
     bounds.Inset(gfx::InsetsF(
         kInkDropVerticalInsetPx /
             GetWidget()->GetLayer()->GetCompositor()->device_scale_factor(),
         0));
     return base::MakeUnique<views::InkDropHighlight>(
         bounds.size(), 0, bounds.CenterPoint(), GetInkDropBaseColor());
   }
 
+// Ink drop masks don't work on Windows. See crbug.com/713359
+#if !defined(OS_WIN)

[sky, 2017/04/27 22:32:04]

Are we generally doing ifdefs for this? Is it worth a PlatformStyle value?

[Evan Stade, 2017/04/27 22:36:31]

I think this may be the first time we're doing this. The #ifdef might be better
moved to InstallInkDropMask so it applies to all masks and we won't have to add
ifdefs in the future either.

Hopefully that bug gets fixed and then we can enable them on Windows.

[sky, 2017/04/27 22:54:08]

So, would it be better to move the ifdef as you say in this patch?

+  std::unique_ptr<views::InkDropMask> CreateInkDropMask() const override {
+    return base::MakeUnique<views::RoundRectInkDropMask>(size(), kInkDropInsets,
+                                                         kInkDropCornerRadius);
+  }
+#endif
+
   SkColor GetInkDropBaseColor() const override {
     return GetThemeProvider()->GetColor(
         ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON);
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BookmarkMenuButtonBase);
 };
 
 // BookmarkFolderButton -------------------------------------------------------
@@ skipping 1771 matching lines @@
     return;
   apps_page_shortcut_->SetVisible(visible);
   UpdateBookmarksSeparatorVisibility();
   LayoutAndPaint();
 }
 
 void BookmarkBarView::OnShowManagedBookmarksPrefChanged() {
   if (UpdateOtherAndManagedButtonsVisibility())
     LayoutAndPaint();
 }