Use ash NetworkIconAnimation in NetworkMenuIcon code

chrome/browser/chromeos/status/network_menu_icon.cc

Index: chrome/browser/chromeos/status/network_menu_icon.cc
diff --git a/chrome/browser/chromeos/status/network_menu_icon.cc b/chrome/browser/chromeos/status/network_menu_icon.cc
index 800446c6ce66ab30113b93bf2a3ff56895c26d87..c6d2914be5dc879565a4b9b538273d455721a4ed 100644
--- a/chrome/browser/chromeos/status/network_menu_icon.cc
+++ b/chrome/browser/chromeos/status/network_menu_icon.cc
@@ -9,6 +9,7 @@
 #include <map>
 #include <utility>
 
+#include "ash/system/chromeos/network/network_icon_animation.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_util.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
@@ -25,6 +26,8 @@
 using std::max;
 using std::min;
 
+using ash::network_icon::NetworkIconAnimation;
+
 namespace chromeos {
 
 namespace {
@@ -32,9 +35,6 @@ namespace {
 // Amount to fade icons while connecting.
 const double kConnectingImageAlpha = 0.5;
 
-// Animation cycle length.
-const int kThrobDurationMs = 750;
-
 // Images for strength bars for wired networks.
 const int kNumBarsImages = 5;
 gfx::ImageSkia* kBarsImagesAnimatingDark[kNumBarsImages - 1];
@@ -282,6 +282,7 @@ class NetworkIcon {
 
   bool ShouldShowInTray() const;
 
+  ConnectionType type() { return type_; }
   void set_type(ConnectionType type) { type_ = type; }
   void set_state(ConnectionState state) { state_ = state; }
   void set_icon(const gfx::ImageSkia& icon) { icon_ = icon; }
@@ -608,12 +609,7 @@ NetworkMenuIcon::NetworkMenuIcon(Delegate* delegate, Mode mode)
     : mode_(mode),
       delegate_(delegate),
       resource_color_theme_(COLOR_DARK),
-      ALLOW_THIS_IN_INITIALIZER_LIST(animation_connecting_(this)),
-      connecting_network_(NULL) {
-  // Set up the connection animation throbber.
-  animation_connecting_.SetThrobDuration(kThrobDurationMs);
-  animation_connecting_.SetTweenType(ui::Tween::LINEAR);
-
+      connecting_index_(-1) {
   // Initialize the icon.
   icon_.reset(new NetworkIcon(resource_color_theme_));
 }
@@ -638,7 +634,10 @@ bool NetworkMenuIcon::ShouldShowIconInTray() {
 }
 
 const gfx::ImageSkia NetworkMenuIcon::GetIconAndText(string16* text) {
-  SetIconAndText();
+  if (SetIconAndText())
+    NetworkIconAnimation::GetInstance()->AddObserver(this);
+  else
+    NetworkIconAnimation::GetInstance()->RemoveObserver(this);
   if (text)
     *text = text_;
   icon_->GenerateImage();
@@ -646,18 +645,24 @@ const gfx::ImageSkia NetworkMenuIcon::GetIconAndText(string16* text) {
 }
 
 const gfx::ImageSkia NetworkMenuIcon::GetVpnIconAndText(string16* text) {
-  SetVpnIconAndText();
+  if (SetVpnIconAndText())
+    NetworkIconAnimation::GetInstance()->AddObserver(this);
+  else
+    NetworkIconAnimation::GetInstance()->RemoveObserver(this);
   if (text)
     *text = text_;
   icon_->GenerateImage();
   return icon_->GetImage();
 }
 
-void NetworkMenuIcon::AnimationProgressed(const ui::Animation* animation) {
-  if (animation == &animation_connecting_ && delegate_) {
-    // Only update the connecting network from here.
-    if (GetConnectingNetwork() == connecting_network_)
-      delegate_->NetworkMenuIconChanged();
+void NetworkMenuIcon::NetworkIconChanged() {
+  if (!delegate_ || !icon_.get())
+    return;
+  // Only send a message when the icon would change.
+  int connecting_index = GetConnectingIndex();
+  if (connecting_index != connecting_index_) {
+    connecting_index_ = connecting_index;
+    delegate_->NetworkMenuIconChanged();
   }
 }
 
@@ -678,20 +683,23 @@ const Network* NetworkMenuIcon::GetConnectingNetwork() {
 }
 
 double NetworkMenuIcon::GetAnimation() {
-  if (!animation_connecting_.is_animating()) {
-    animation_connecting_.Reset();
-    animation_connecting_.StartThrobbing(-1 /*throb indefinitely*/);
-    return 0;
-  }
-  return animation_connecting_.GetCurrentValue();
+  return NetworkIconAnimation::GetInstance()->GetAnimation();
 }
 
-// TODO(stevenjb): move below SetIconAndText.
-void NetworkMenuIcon::SetConnectingIconAndText() {
-  int image_count;
-  ImageType image_type;
-  gfx::ImageSkia** images;
+int NetworkMenuIcon::GetConnectingIndex() {
+  DCHECK(icon_.get());
+  double animation = GetAnimation();
+  int image_count =
+      (icon_->type() == TYPE_WIFI) ? kNumArcsImages - 1 : kNumBarsImages - 1;
+  int index = animation * nextafter(static_cast<float>(image_count), 0);
+  return std::max(std::min(index, image_count - 1), 0);
+}
 
+
+// TODO(stevenjb): move below SetIconAndText.
+void NetworkMenuIcon::SetConnectingIconAndText(
+    const Network* connecting_network) {
+  connecting_network_ = connecting_network;
   ConnectionType type;
   ConnectionState state;
   if (connecting_network_) {
@@ -714,19 +722,18 @@ void NetworkMenuIcon::SetConnectingIconAndText() {
   icon_->set_type(type);
   icon_->set_state(state);
 
+  ImageType image_type;
+  gfx::ImageSkia** images;
   if (type == TYPE_WIFI) {
-    image_count = kNumArcsImages - 1;
     image_type = ARCS;
     images = resource_color_theme_ == COLOR_DARK ? kArcsImagesAnimatingDark :
                                                    kArcsImagesAnimatingLight;
   } else {
-    image_count = kNumBarsImages - 1;
     image_type = BARS;
     images = resource_color_theme_ == COLOR_DARK ? kBarsImagesAnimatingDark :
                                                    kBarsImagesAnimatingLight;
   }
-  int index = GetAnimation() * nextafter(static_cast<float>(image_count), 0);
-  index = std::max(std::min(index, image_count - 1), 0);
+  int index = GetConnectingIndex();
 
   // Lazily cache images.
   if (!images[index]) {
@@ -741,7 +748,7 @@ void NetworkMenuIcon::SetConnectingIconAndText() {
 }
 
 // Sets up the icon and badges for GenerateBitmap().
-void NetworkMenuIcon::SetIconAndText() {
+bool NetworkMenuIcon::SetIconAndText() {
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   DCHECK(cros);
 
@@ -749,10 +756,10 @@ void NetworkMenuIcon::SetIconAndText() {
 
   // If we are connecting to a network and it was user-initiated or we are
   // not connected, display that.
-  connecting_network_ = GetConnectingNetwork();
-  if (connecting_network_) {
-    SetConnectingIconAndText();
-    return;
+  const Network* connecting_network = GetConnectingNetwork();
+  if (connecting_network) {
+    SetConnectingIconAndText(connecting_network);
+    return true;
   }
 
   // If not connecting to a network, show the active or connected network.
@@ -761,16 +768,14 @@ void NetworkMenuIcon::SetIconAndText() {
     network = cros->connected_network();
   else
     network = cros->active_nonvirtual_network();
-  if (network) {
-    SetActiveNetworkIconAndText(network);
-    return;
-  }
+  if (network)
+    return SetActiveNetworkIconAndText(network);
 
   // If no connected network, check if we are initializing Cellular.
   if (mode_ != DROPDOWN_MODE && cros->cellular_initializing()) {
     initialize_state_time_ = base::Time::Now();
-    SetConnectingIconAndText();
-    return;
+    SetConnectingIconAndText(NULL);
+    return true;
   }
   // There can be a delay between leaving the Initializing state and when a
   // Cellular device shows up, so keep showing the initializing animation
@@ -778,18 +783,16 @@ void NetworkMenuIcon::SetIconAndText() {
   const int kInitializingDelaySeconds = 1;
   base::TimeDelta dtime = base::Time::Now() - initialize_state_time_;
   if (dtime.InSeconds() < kInitializingDelaySeconds) {
-    SetConnectingIconAndText();
-    return;
+    SetConnectingIconAndText(NULL);
+    return true;
   }
 
-  // Not connecting, so stop animation.
-  animation_connecting_.Stop();
-
   // No connecting, connected, or active network.
   SetDisconnectedIconAndText();
+  return false;
 }
 
-void NetworkMenuIcon::SetVpnIconAndText() {
+bool NetworkMenuIcon::SetVpnIconAndText() {
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   DCHECK(cros);
 
@@ -798,19 +801,18 @@ void NetworkMenuIcon::SetVpnIconAndText() {
   if (!vpn) {
     NOTREACHED();

[jennyz, 2013/01/31 19:16:50]

Why should we still SetDisconnectedIconAndText if this flow should NOT be
reached?

[stevenjb, 2013/01/31 19:37:42]

Didn't you add this? :P Yes, this should be a warning.

[jennyz, 2013/01/31 19:44:46]

Really, my bad then. 

     SetDisconnectedIconAndText();
-    return;
+    return false;
   }
   if (vpn->connecting()) {
-    connecting_network_ = vpn;
-    SetConnectingIconAndText();
-    return;
+    SetConnectingIconAndText(vpn);
+    return true;
   }
 
   // If not connecting to a network, show the active/connected VPN.

[jennyz, 2013/01/31 19:16:50]

Should the comment be: "If not connecting to a vpn,"?

[stevenjb, 2013/01/31 19:37:42]

Ditto :) Changed.

[jennyz, 2013/01/31 19:44:46]

Thanks for changing, I didn't see the new patch, have you uploaded?

-  SetActiveNetworkIconAndText(vpn);
+  return SetActiveNetworkIconAndText(vpn);
 }
 
-void NetworkMenuIcon::SetActiveNetworkIconAndText(const Network* network) {
+bool NetworkMenuIcon::SetActiveNetworkIconAndText(const Network* network) {
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   bool animating = false;
@@ -833,8 +835,6 @@ void NetworkMenuIcon::SetActiveNetworkIconAndText(const Network* network) {
         GetEmptyImage(vpn_badge->size()), *vpn_badge, animation);
     icon_->set_bottom_left_badge(&vpn_connecting_badge_);
   }
-  if (!animating)
-    animation_connecting_.Stop();
 
   // Set the text to display.
   if (network->type() == TYPE_ETHERNET) {
@@ -855,6 +855,7 @@ void NetworkMenuIcon::SetActiveNetworkIconAndText(const Network* network) {
       text_ = UTF8ToUTF16(network->name());
     }
   }
+  return animating;
 }
 
 void NetworkMenuIcon::SetDisconnectedIconAndText() {