Allow STATE_NORMAL text buttons to animate

ui/views/controls/button/text_button.cc

Index: ui/views/controls/button/text_button.cc
diff --git a/ui/views/controls/button/text_button.cc b/ui/views/controls/button/text_button.cc
index fa250ab54b9f80c6b1444d1798163d51d5da0950..7ed50f5a0f61a5970cf64ab555fec0ef8be6ad88 100644
--- a/ui/views/controls/button/text_button.cc
+++ b/ui/views/controls/button/text_button.cc
@@ -119,26 +119,26 @@ TextButtonDefaultBorder::~TextButtonDefaultBorder() {
 ////////////////////////////////////////////////////////////////////////////////
 void TextButtonDefaultBorder::Paint(const View& view, gfx::Canvas* canvas) {
   const TextButton* button = static_cast<const TextButton*>(&view);
-  int state = button->state();
+  const int state = button->state();
+  const bool animating = button->GetAnimation()->is_animating();
 
   Painter* painter = normal_painter_.get();
   if (button->show_multiple_icon_states() &&
-      ((state == TextButton::STATE_HOVERED) ||
-       (state == TextButton::STATE_PRESSED))) {
-    painter = (state == TextButton::STATE_HOVERED) ?
-        hot_painter_.get() : pushed_painter_.get();
+      (state == TextButton::STATE_HOVERED ||

[Peter Kasting, 2013/05/13 22:40:35]

Nit: Please leave in place parens around all binary subexpressions (and add more
to new code).  Among other things this will make the && vs. || order explicit.

[ckocagil, 2013/05/14 11:28:27]

Done.

+       state == TextButton::STATE_PRESSED ||
+       state == TextButton::STATE_NORMAL && animating)) {

[Peter Kasting, 2013/05/13 22:40:35]

Nit: Add a comment about why the animating flag means we should switch from the
normal painter to the hot painter.

[ckocagil, 2013/05/14 11:28:27]

Done.

+    painter = (state == TextButton::STATE_PRESSED) ?
+        pushed_painter_.get() : hot_painter_.get();
   }
-  if (painter) {
-    if (button->GetAnimation()->is_animating()) {
-      // TODO(pkasting): Really this should crossfade between states so it could
-      // handle the case of having a non-NULL |normal_set_|.
-      canvas->SaveLayerAlpha(static_cast<uint8>(
-          button->GetAnimation()->CurrentValueBetween(0, 255)));
-      painter->Paint(canvas, view.size());
-      canvas->Restore();
-    } else {
-      painter->Paint(canvas, view.size());
-    }
+  if (painter && animating) {

[Peter Kasting, 2013/05/13 22:40:35]

Nit: Don't flatten the old nested conditional this way.

[ckocagil, 2013/05/14 11:28:27]

Done.

+    // TODO(pkasting): Really this should crossfade between states so it could
+    // handle the case of having a non-NULL |normal_set_|.

[ckocagil, 2013/05/12 16:50:46]

There doesn't seem to be a |normal_set_| anymore, should I remove this TODO?

[msw, 2013/05/12 21:40:36]

The TODO is still valid, just rename |normal_set_| to |normal_painter_|.

[ckocagil, 2013/05/14 11:28:27]

Done.

+    canvas->SaveLayerAlpha(static_cast<uint8>(
+        button->GetAnimation()->CurrentValueBetween(0, 255)));
+    painter->Paint(canvas, view.size());
+    canvas->Restore();
+  } else if (painter) {
+    painter->Paint(canvas, view.size());
   }
 }