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

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 148 matching lines @@
 namespace {
 
 // To enable/disable BookmarkBar animations during testing. In production
 // animations are enabled by default.
 bool animations_enabled = true;
 
 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 -----------------------------------------------
 
 // Base class for non-menu hosting buttons used on the bookmark bar.
 
 class BookmarkButtonBase : public views::LabelButton {
  public:
   BookmarkButtonBase(views::ButtonListener* listener,
                      const base::string16& title)
       : LabelButton(listener, title) {
@@ skipping 41 matching lines @@
   }
 
   std::unique_ptr<views::InkDropRipple> CreateInkDropRipple() const override {
     return base::MakeUnique<views::FloodFillInkDropRipple>(
         size(), kInkDropInsets, GetInkDropCenterBasedOnLastEvent(),
         GetInkDropBaseColor(), ink_drop_visible_opacity());
   }
 
   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 {
     return GetThemeProvider()->GetColor(
         ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON);
   }
 
  private:
   std::unique_ptr<gfx::SlideAnimation> show_animation_;
 
@@ skipping 98 matching lines @@
   }
 
   std::unique_ptr<views::InkDropRipple> CreateInkDropRipple() const override {
     return base::MakeUnique<views::FloodFillInkDropRipple>(
         size(), kInkDropInsets, GetInkDropCenterBasedOnLastEvent(),
         GetInkDropBaseColor(), ink_drop_visible_opacity());
   }
 
   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 {
     return GetThemeProvider()->GetColor(
         ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON);
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BookmarkMenuButtonBase);
 };
@@ skipping 1772 matching lines @@
     return;
   apps_page_shortcut_->SetVisible(visible);
   UpdateBookmarksSeparatorVisibility();
   LayoutAndPaint();
 }
 
 void BookmarkBarView::OnShowManagedBookmarksPrefChanged() {
   if (UpdateOtherAndManagedButtonsVisibility())
     LayoutAndPaint();
 }