CustomButton cleanup: make protected members private, create accessors

chrome/browser/ui/views/tabs/tab_strip.cc

Index: chrome/browser/ui/views/tabs/tab_strip.cc
diff --git a/chrome/browser/ui/views/tabs/tab_strip.cc b/chrome/browser/ui/views/tabs/tab_strip.cc
index 99b804179e95f6b799f31577b89dc624f6bb4b6e..8085cec7b2d41f057b82ed713339bffc842f5f3c 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.cc
+++ b/chrome/browser/ui/views/tabs/tab_strip.cc
@@ -308,7 +308,6 @@ class NewTabButton : public views::ImageButton,
   // Paints the fill region of the button into |canvas|, according to the
   // supplied values from GetImage() and the given |fill| path.
   void PaintFill(bool pressed,
-                 double hover_value,
                  float scale,
                  const SkPath& fill,
                  gfx::Canvas* canvas) const;
@@ -372,10 +371,6 @@ void NewTabButton::OnPaint(gfx::Canvas* canvas) {
   canvas->Translate(gfx::Vector2d(0, height() - kNewTabButtonHeight));
 
   const bool pressed = state() == views::CustomButton::STATE_PRESSED;
-  double hover_value =
-      (state() == views::CustomButton::STATE_HOVERED) ? 1 : 0;
-  if (hover_animation_->is_animating())
-    hover_value = hover_animation_->GetCurrentValue();

[Peter Kasting, 2015/12/19 01:22:27]

I _think_ your changes to always trust the animation state are correct now that
I've made custom_button.cc always reset the hover animation on state
transitions, but I'm nervous enough that you should make sure you try all the
different possible state transitions.

[Evan Stade, 2015/12/21 20:19:28]

I've tried every combination of hovering, clicking, unhovering, etc. that I can
think of. Seems to work.

   const float scale = canvas->image_scale();
 
   SkPath fill;
@@ -392,7 +387,7 @@ void NewTabButton::OnPaint(gfx::Canvas* canvas) {
     fill.rLineTo(5.75 * scale, 12.5 * scale);
     fill.rCubicTo(0, 0.5 * scale, -0.25 * scale, scale, -scale, scale);
     fill.close();
-    PaintFill(pressed, hover_value, scale, fill, canvas);
+    PaintFill(pressed, scale, fill, canvas);
 
     // Stroke.
     gfx::ScopedCanvas scoped_canvas(canvas);
@@ -423,7 +418,7 @@ void NewTabButton::OnPaint(gfx::Canvas* canvas) {
     const float fill_canvas_scale = mask->HasRepresentation(scale) ?
         scale : ui::GetScaleForScaleFactor(ui::SCALE_FACTOR_100P);
     gfx::Canvas fill_canvas(GetNewTabButtonSize(), fill_canvas_scale, false);
-    PaintFill(pressed, hover_value, fill_canvas_scale, fill, &fill_canvas);
+    PaintFill(pressed, fill_canvas_scale, fill, &fill_canvas);
     gfx::ImageSkia image(fill_canvas.ExtractImageRep());
     canvas->DrawImageInt(
         gfx::ImageSkiaOperations::CreateMaskedImage(image, *mask), 0, 0);
@@ -511,7 +506,6 @@ void NewTabButton::GetBorderPath(float button_y,
 }
 
 void NewTabButton::PaintFill(bool pressed,
-                             double hover_value,
                              float scale,
                              const SkPath& fill,
                              gfx::Canvas* canvas) const {
@@ -591,9 +585,9 @@ void NewTabButton::PaintFill(bool pressed,
   }
 
   // White highlight on hover.
-  if (hover_value) {
-    const int alpha =
-        gfx::Tween::LinearIntValueBetween(hover_value, 0x00, md ? 0x4D : 0x40);
+  if (hover_animation().GetCurrentValue()) {

[Peter Kasting, 2015/12/19 01:22:27]

Nit: Store this in a temp to avoid multiple calls

[Evan Stade, 2015/12/21 20:19:28]

question: you authored this code recently. Why does it use
gfx::Tween::LinearIntValueBetween instead of Animation::CurrentValueBetween? Is
the "Linear" distinction important?

I've changed the code to a version that is slightly more concise, ptal.

[Peter Kasting, 2015/12/21 20:34:18]

Using LinearIntValueBetween() matched the previous behavior (I was just
refactoring it).  The difference between this and IntValueBetween() (which is
what CurrentValueBetween() will use) should not be very perceptible to the user,
so it's fine to use the other for simplicity.

+    const int alpha = gfx::Tween::LinearIntValueBetween(
+        hover_animation().GetCurrentValue(), 0x00, md ? 0x4D : 0x40);
     canvas->FillRect(GetLocalBounds(),
                      SkColorSetA(SK_ColorWHITE, static_cast<SkAlpha>(alpha)));
   }