Make updates to InkDropHighlight to pave the way for improved ink drops

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

Index: chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
index 95f9331f36bfb7b04c087abc8b34ec9e94b77f46..6fb0b37ff913cb41463f3fef3de4aa7d0d385934 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
@@ -166,15 +166,13 @@ gfx::ImageSkia* GetImageSkiaNamed(int id) {
   return ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(id);
 }
 
+constexpr int kInkDropVerticalInsetPx = 1;
+
 // Ink drop ripple/highlight for bookmark buttons should be inset 1px vertically
 // so that they do not touch the bookmark bar borders.
-constexpr gfx::Insets kInkDropInsets(1, 0);
-
-gfx::Rect CalculateInkDropBounds(const gfx::Size& size) {
-  gfx::Rect ink_drop_bounds(size);
-  ink_drop_bounds.Inset(kInkDropInsets);
-  return ink_drop_bounds;
-}
+// TODO(estade): currently this is used as DIP rather than pixels. This should
+// be fixed: see crbug.com/706228
+constexpr gfx::Insets kInkDropInsets(kInkDropVerticalInsetPx, 0);
 
 // BookmarkButtonBase -----------------------------------------------
 
@@ -236,10 +234,13 @@ class BookmarkButtonBase : public views::LabelButton {
 
   std::unique_ptr<views::InkDropHighlight> CreateInkDropHighlight()
       const override {
-    const gfx::Rect bounds = CalculateInkDropBounds(size());
+    gfx::RectF bounds((gfx::Rect(size())));

[bruthig, 2017/03/30 16:37:08]

nit: One too many '()' set

[Evan Stade, 2017/03/30 17:49:38]

that is actually necessary or the compiler complains.

[sky, 2017/03/30 20:37:54]

optional: creating a rect just to create a rectf is overkill. how about (0, 0,
size().width(), size().height())?

Maybe a bit more characters.

[Evan Stade, 2017/03/31 00:04:12]

Perhaps there should be a RectF ctor that takes a Size?

+    bounds.Inset(gfx::InsetsF(
+        kInkDropVerticalInsetPx /
+            GetWidget()->GetLayer()->GetCompositor()->device_scale_factor(),

[bruthig, 2017/03/30 16:37:08]

Will we be doing this enough to warrant a util function like 'float
PxToDip(Layer*, int px)' ?

[Evan Stade, 2017/03/30 17:49:38]

maybe, ui/compositor/dip_util.h has similar things but not quite what we want
here.

+        0));
     return base::MakeUnique<views::InkDropHighlight>(
-        bounds.size(), 0, gfx::RectF(bounds).CenterPoint(),
-        GetInkDropBaseColor());
+        bounds.size(), 0, bounds.CenterPoint(), GetInkDropBaseColor());
   }
 
   SkColor GetInkDropBaseColor() const override {
@@ -358,10 +359,13 @@ class BookmarkMenuButtonBase : public views::MenuButton {
 
   std::unique_ptr<views::InkDropHighlight> CreateInkDropHighlight()
       const override {
-    const gfx::Rect bounds = CalculateInkDropBounds(size());
+    gfx::RectF bounds((gfx::Rect(size())));

[bruthig, 2017/03/30 16:37:08]

nit: Too many '()'

+    bounds.Inset(gfx::InsetsF(
+        kInkDropVerticalInsetPx /
+            GetWidget()->GetLayer()->GetCompositor()->device_scale_factor(),
+        0));
     return base::MakeUnique<views::InkDropHighlight>(
-        bounds.size(), 0, gfx::RectF(bounds).CenterPoint(),
-        GetInkDropBaseColor());
+        bounds.size(), 0, bounds.CenterPoint(), GetInkDropBaseColor());
   }
 
   SkColor GetInkDropBaseColor() const override {