Make cros library thread-safe.

Adds unmonitoring of network status changes.
Fixed a few edge case bugs.
BUG=23923
TEST=none
Review URL: http://codereview.chromium.org/272019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@28918 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 196 insertions, 86 deletions

diff --git a/chrome/browser/chromeos/clock_menu_button.cc b/chrome/browser/chromeos/clock_menu_button.cc
index a628cc8..99da88a 100644
--- a/chrome/browser/chromeos/clock_menu_button.cc
+++ b/chrome/browser/chromeos/clock_menu_button.cc
@@ -24,6 +24,11 @@ ClockMenuButton::ClockMenuButton()
       ResourceBundle::BaseFont).DeriveFont(0, gfx::Font::BOLD));
   SetEnabledColor(SK_ColorWHITE);
   SetShowHighlighted(false);
+  // Set initial text to make sure that the text button wide enough.
+  std::wstring zero = ASCIIToWide("00");
+  SetText(l10n_util::GetStringF(IDS_STATUSBAR_CLOCK_SHORT_TIME_AM, zero, zero));
+  SetText(l10n_util::GetStringF(IDS_STATUSBAR_CLOCK_SHORT_TIME_PM, zero, zero));
+  set_alignment(TextButton::ALIGN_RIGHT);
   UpdateText();
 }
 
diff --git a/chrome/browser/chromeos/cros_network_library.cc b/chrome/browser/chromeos/cros_network_library.cc
index 4b37aaf..634e81e 100644
--- a/chrome/browser/chromeos/cros_network_library.cc
+++ b/chrome/browser/chromeos/cros_network_library.cc
@@ -6,18 +6,34 @@
 
 #include <algorithm>
 
+#include "base/message_loop.h"
 #include "base/string_util.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/chromeos/cros_library.h"
 
-CrosNetworkLibrary::CrosNetworkLibrary()
-  : ethernet_connected_(false),
-    wifi_connecting_(false),
-    wifi_strength_(0) {
+// Allows InvokeLater without adding refcounting. This class is a Singleton and
+// won't be deleted until it's last InvokeLater is run.
+template <>
+struct RunnableMethodTraits<CrosNetworkLibrary> {
+  void RetainCallee(CrosNetworkLibrary* obj) {}
+  void ReleaseCallee(CrosNetworkLibrary* obj) {}
+};
+
+CrosNetworkLibrary::CrosNetworkLibrary() {
   if (CrosLibrary::loaded()) {
-    chromeos::ServiceStatus* service_status = chromeos::GetAvailableNetworks();
-    UpdateNetworkServiceStatus(*service_status);
-    chromeos::FreeServiceStatus(service_status);
-    chromeos::MonitorNetworkStatus(&NetworkStatusChangedHandler, this);
+    MessageLoop* loop = ChromeThread::GetMessageLoop(ChromeThread::FILE);
+    if (loop) {
+      loop->PostTask(FROM_HERE, NewRunnableMethod(this,
+          &CrosNetworkLibrary::InitOnBackgroundThread));
+    }
+  }
+}
+
+CrosNetworkLibrary::~CrosNetworkLibrary() {
+  if (CrosLibrary::loaded()) {
+    // FILE thread is already gone by the time we get to this destructor.
+    // So it's ok to just make the disconnect call on the main thread.
+    chromeos::DisconnectNetworkStatus(network_status_connection_);
   }
 }
 
@@ -39,28 +55,6 @@ void CrosNetworkLibrary::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-WifiNetworkVector CrosNetworkLibrary::GetWifiNetworks() {
-  WifiNetworkVector networks;
-  if (CrosLibrary::loaded()) {
-    chromeos::ServiceStatus* service_status = chromeos::GetAvailableNetworks();
-    for (int i = 0; i < service_status->size; i++) {
-      const chromeos::ServiceInfo& service = service_status->services[i];
-      if (service.type == chromeos::TYPE_WIFI) {
-        // Found a wifi network, add it to the list.
-        networks.push_back(WifiNetwork(service.ssid, service.needs_passphrase,
-                                       service.encryption,
-                                       service.signal_strength));
-      }
-    }
-    chromeos::FreeServiceStatus(service_status);
-  }
-
-  // Sort the list of wifi networks by ssid.
-  std::sort(networks.begin(), networks.end());
-
-  return networks;
-}
-
 static const char* GetEncryptionString(chromeos::EncryptionType encryption) {
   switch (encryption) {
     case chromeos::NONE:
@@ -77,25 +71,32 @@ static const char* GetEncryptionString(chromeos::EncryptionType encryption) {
 
 void CrosNetworkLibrary::ConnectToWifiNetwork(WifiNetwork network,
                                               const string16& password) {
-  if (CrosLibrary::loaded())
-    chromeos::ConnectToWifiNetwork(network.ssid.c_str(),
-        password.empty() ? NULL : UTF16ToUTF8(password).c_str(),
-        GetEncryptionString(network.encryption));
+  if (CrosLibrary::loaded()) {
+    MessageLoop* loop = ChromeThread::GetMessageLoop(ChromeThread::FILE);
+    if (loop)
+      loop->PostTask(FROM_HERE, NewRunnableFunction(
+          &chromeos::ConnectToWifiNetwork,
+          network.ssid.c_str(),
+          password.empty() ? NULL : UTF16ToUTF8(password).c_str(),
+          GetEncryptionString(network.encryption)));
+  }
 }
 
 // static
 void CrosNetworkLibrary::NetworkStatusChangedHandler(void* object,
     const chromeos::ServiceStatus& service_status) {
   CrosNetworkLibrary* network = static_cast<CrosNetworkLibrary*>(object);
-  network->UpdateNetworkServiceStatus(service_status);
-  FOR_EACH_OBSERVER(Observer, network->observers_, NetworkChanged(network));
+  WifiNetworkVector networks;
+  bool ethernet_connected;
+  ParseNetworks(service_status, &networks, &ethernet_connected);
+  network->UpdateNetworkStatus(networks, ethernet_connected);
 }
 
-void CrosNetworkLibrary::UpdateNetworkServiceStatus(
-    const chromeos::ServiceStatus& service_status) {
-  // Loop through the services and figure out what the current state is.
-  bool found_ethernet = false;
-  bool found_wifi = false;
+// static
+void CrosNetworkLibrary::ParseNetworks(
+    const chromeos::ServiceStatus& service_status, WifiNetworkVector* networks,
+    bool* ethernet_connected) {
+  *ethernet_connected = false;
   for (int i = 0; i < service_status.size; i++) {
     const chromeos::ServiceInfo& service = service_status.services[i];
     DLOG(INFO) << "Parse " << service.ssid <<
@@ -105,26 +106,57 @@ void CrosNetworkLibrary::UpdateNetworkServiceStatus(
                   " enc=" << service.encryption <<
                   " sig=" << service.signal_strength;
     if (service.type == chromeos::TYPE_ETHERNET) {
-      found_ethernet = true;
       // Get the ethernet status.
-      ethernet_connected_ = service.state == chromeos::STATE_READY;
+      *ethernet_connected = service.state == chromeos::STATE_READY;
     } else if (service.type == chromeos::TYPE_WIFI) {
-      if (service.state == chromeos::STATE_READY ||
-          service.state == chromeos::STATE_ASSOCIATION ||
-          service.state == chromeos::STATE_CONFIGURATION) {
-        found_wifi = true;
-        // Record the wifi network that is connected.
-        wifi_ssid_ = service.ssid;
-        wifi_connecting_ = service.state != chromeos::STATE_READY;
-        wifi_strength_ = service.signal_strength;
-      }
+      bool connecting = service.state == chromeos::STATE_ASSOCIATION ||
+                        service.state == chromeos::STATE_CONFIGURATION;
+      bool connected = service.state == chromeos::STATE_READY;
+      networks->push_back(WifiNetwork(service.ssid,
+                                      service.needs_passphrase,
+                                      service.encryption,
+                                      service.signal_strength,
+                                      connecting,
+                                      connected));
     }
   }
-  if (!found_ethernet)
-    ethernet_connected_ = false;
-  if (!found_wifi) {
-    wifi_ssid_.clear();
-    wifi_connecting_ = false;
-    wifi_strength_ = 0;
+}
+
+void CrosNetworkLibrary::InitOnBackgroundThread() {
+  chromeos::ServiceStatus* service_status = chromeos::GetAvailableNetworks();
+  if (service_status) {
+    WifiNetworkVector networks;
+    bool ethernet_connected;
+    ParseNetworks(*service_status, &networks, &ethernet_connected);
+    UpdateNetworkStatus(networks, ethernet_connected);
+    chromeos::FreeServiceStatus(service_status);
+  }
+  network_status_connection_ = chromeos::MonitorNetworkStatus(
+      &NetworkStatusChangedHandler, this);
+}
+
+void CrosNetworkLibrary::UpdateNetworkStatus(
+    const WifiNetworkVector& networks, bool ethernet_connected) {
+  // Make sure we run on UI thread.
+  if (!ChromeThread::CurrentlyOn(ChromeThread::UI)) {
+    MessageLoop* loop = ChromeThread::GetMessageLoop(ChromeThread::UI);
+    if (loop)
+      loop->PostTask(FROM_HERE, NewRunnableMethod(this,
+          &CrosNetworkLibrary::UpdateNetworkStatus, networks,
+          ethernet_connected));
+    return;
+  }
+
+  ethernet_connected_ = ethernet_connected;
+  wifi_networks_ = networks;
+  // Sort the list of wifi networks by ssid.
+  std::sort(wifi_networks_.begin(), wifi_networks_.end());
+  wifi_ = WifiNetwork();
+  for (size_t i = 0; i < wifi_networks_.size(); i++) {
+    if (wifi_networks_[i].connecting || wifi_networks_[i].connected) {
+      wifi_ = wifi_networks_[i];
+      break;  // There is only one connected or connecting wifi network.
+    }
   }
+  FOR_EACH_OBSERVER(Observer, observers_, NetworkChanged(this));
 }
diff --git a/chrome/browser/chromeos/cros_network_library.h b/chrome/browser/chromeos/cros_network_library.h
index a400a25..324c153 100644
--- a/chrome/browser/chromeos/cros_network_library.h
+++ b/chrome/browser/chromeos/cros_network_library.h
@@ -14,13 +14,21 @@
 #include "third_party/cros/chromeos_network.h"
 
 struct WifiNetwork {
-  WifiNetwork() : encrypted(false), encryption(chromeos::NONE), strength(0) {}
+  WifiNetwork()
+      : encrypted(false),
+        encryption(chromeos::NONE),
+        strength(0),
+        connecting(false),
+        connected(false) {}
   WifiNetwork(const std::string& ssid, bool encrypted,
-              chromeos::EncryptionType encryption, int strength)
+              chromeos::EncryptionType encryption, int strength,
+              bool connecting, bool connected)
       : ssid(ssid),
         encrypted(encrypted),
         encryption(encryption),
-        strength(strength) { }
+        strength(strength),
+        connecting(connecting),
+        connected(connected) {}
 
   // WifiNetworks are sorted by ssids.
   bool operator< (const WifiNetwork& other) const {
@@ -31,6 +39,8 @@ struct WifiNetwork {
   bool encrypted;
   chromeos::EncryptionType encryption;
   int strength;
+  bool connecting;
+  bool connected;
 };
 typedef std::vector<WifiNetwork> WifiNetworkVector;
 
@@ -54,12 +64,12 @@ class CrosNetworkLibrary {
   void RemoveObserver(Observer* observer);
 
   bool ethernet_connected() const { return ethernet_connected_; }
-  const std::string& wifi_ssid() const { return wifi_ssid_; }
-  bool wifi_connecting() const { return wifi_connecting_; }
-  int wifi_strength() const { return wifi_strength_; }
+  const std::string& wifi_ssid() const { return wifi_.ssid; }
+  bool wifi_connecting() const { return wifi_.connecting; }
+  int wifi_strength() const { return wifi_.strength; }
 
   // Returns the current list of wifi networks.
-  WifiNetworkVector GetWifiNetworks();
+  const WifiNetworkVector& wifi_networks() const { return wifi_networks_; }
 
   // Connect to the specified wireless network with password.
   void ConnectToWifiNetwork(WifiNetwork network, const string16& password);
@@ -68,30 +78,42 @@ class CrosNetworkLibrary {
   friend struct DefaultSingletonTraits<CrosNetworkLibrary>;
 
   CrosNetworkLibrary();
-  ~CrosNetworkLibrary() {}
+  ~CrosNetworkLibrary();
 
   // This method is called when there's a change in network status.
-  // This will notify all the Observers.
+  // This method is called on a background thread.
   static void NetworkStatusChangedHandler(void* object,
       const chromeos::ServiceStatus& service_status);
 
-  // Update the network with the ServiceStatus.
-  void UpdateNetworkServiceStatus(
-      const chromeos::ServiceStatus& service_status);
+  // This parses ServiceStatus and creates a WifiNetworkVector of wifi networks.
+  // It also sets ethernet_connected depending on if we have ethernet or not.
+  static void ParseNetworks(const chromeos::ServiceStatus& service_status,
+                            WifiNetworkVector* networks,
+                            bool* ethernet_connected);
+
+  // This methods loads the initial list of networks on startup and starts the
+  // monitoring of network changes.
+  // It should be called on a background thread.
+  void InitOnBackgroundThread();
+
+  // Update the network with the a list of wifi networks and ethernet status.
+  // This will notify all the Observers.
+  void UpdateNetworkStatus(const WifiNetworkVector& networks,
+                           bool ethernet_connected);
 
   ObserverList<Observer> observers_;
 
+  // The network status connection for monitoring network status changes.
+  chromeos::NetworkStatusConnection network_status_connection_;
+
   // Whether or not we are connected to the ethernet line.
   bool ethernet_connected_;
 
-  // The current connected (or connecting) wireless ssid. Empty if none.
-  std::string wifi_ssid_;
-
-  // Whether or not we are connecting right now.
-  bool wifi_connecting_;
+  // The list of available wifi networks.
+  WifiNetworkVector wifi_networks_;
 
-  // The strength of the currently connected ssid.
-  int wifi_strength_;
+  // The current connected (or connecting) wifi network.
+  WifiNetwork wifi_;
 
   DISALLOW_COPY_AND_ASSIGN(CrosNetworkLibrary);
 };
diff --git a/chrome/browser/chromeos/cros_power_library.cc b/chrome/browser/chromeos/cros_power_library.cc
index ed17264..f3d8abf 100644
--- a/chrome/browser/chromeos/cros_power_library.cc
+++ b/chrome/browser/chromeos/cros_power_library.cc
@@ -4,19 +4,34 @@
 
 #include "chrome/browser/chromeos/cros_power_library.h"
 
+#include "base/message_loop.h"
 #include "base/string_util.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/chromeos/cros_library.h"
 
-CrosPowerLibrary::CrosPowerLibrary() {
+// Allows InvokeLater without adding refcounting. This class is a Singleton and
+// won't be deleted until it's last InvokeLater is run.
+template <>
+struct RunnableMethodTraits<CrosPowerLibrary> {
+  void RetainCallee(CrosPowerLibrary* obj) {}
+  void ReleaseCallee(CrosPowerLibrary* obj) {}
+};
+
+CrosPowerLibrary::CrosPowerLibrary() : status_(chromeos::PowerStatus()) {
   if (CrosLibrary::loaded()) {
-    power_status_connection_ = chromeos::MonitorPowerStatus(
-        &PowerStatusChangedHandler, this);
+    MessageLoop* loop = ChromeThread::GetMessageLoop(ChromeThread::FILE);
+    if (loop)
+      loop->PostTask(FROM_HERE, NewRunnableMethod(this,
+          &CrosPowerLibrary::InitOnBackgroundThread));
   }
 }
 
 CrosPowerLibrary::~CrosPowerLibrary() {
-  if (CrosLibrary::loaded())
+  if (CrosLibrary::loaded()) {
+    // FILE thread is already gone by the time we get to this destructor.
+    // So it's ok to just make the disconnect call on the main thread.
     chromeos::DisconnectPowerStatus(power_status_connection_);
+  }
 }
 
 // static
@@ -62,12 +77,30 @@ base::TimeDelta CrosPowerLibrary::battery_time_to_full() const {
 void CrosPowerLibrary::PowerStatusChangedHandler(void* object,
     const chromeos::PowerStatus& status) {
   CrosPowerLibrary* power = static_cast<CrosPowerLibrary*>(object);
+  power->UpdatePowerStatus(status);
+}
+
+void CrosPowerLibrary::InitOnBackgroundThread() {
+  power_status_connection_ = chromeos::MonitorPowerStatus(
+      &PowerStatusChangedHandler, this);
+}
+
+void CrosPowerLibrary::UpdatePowerStatus(const chromeos::PowerStatus& status) {
+  // Make sure we run on UI thread.
+  if (!ChromeThread::CurrentlyOn(ChromeThread::UI)) {
+    MessageLoop* loop = ChromeThread::GetMessageLoop(ChromeThread::UI);
+    if (loop)
+      loop->PostTask(FROM_HERE, NewRunnableMethod(this,
+          &CrosPowerLibrary::UpdatePowerStatus, status));
+    return;
+  }
+
   DLOG(INFO) << "Power" <<
                 " lpo=" << status.line_power_on <<
                 " sta=" << status.battery_state <<
                 " per=" << status.battery_percentage <<
                 " tte=" << status.battery_time_to_empty <<
                 " ttf=" << status.battery_time_to_full;
-  power->status_ = status;
-  FOR_EACH_OBSERVER(Observer, power->observers_, PowerChanged(power));
+  status_ = status;
+  FOR_EACH_OBSERVER(Observer, observers_, PowerChanged(this));
 }
diff --git a/chrome/browser/chromeos/cros_power_library.h b/chrome/browser/chromeos/cros_power_library.h
index fdd931a..3184627 100644
--- a/chrome/browser/chromeos/cros_power_library.h
+++ b/chrome/browser/chromeos/cros_power_library.h
@@ -51,10 +51,18 @@ class CrosPowerLibrary {
   ~CrosPowerLibrary();
 
   // This method is called when there's a change in power status.
-  // This will notify all the Observers.
+  // This method is called on a background thread.
   static void PowerStatusChangedHandler(void* object,
                                         const chromeos::PowerStatus& status);
 
+  // This methods starts the monitoring of power changes.
+  // It should be called on a background thread.
+  void InitOnBackgroundThread();
+
+  // Called by the handler to update the power status.
+  // This will notify all the Observers.
+  void UpdatePowerStatus(const chromeos::PowerStatus& status);
+
   ObserverList<Observer> observers_;
 
   // A reference to the battery power api, to allow callbacks when the battery
diff --git a/chrome/browser/chromeos/network_menu_button.cc b/chrome/browser/chromeos/network_menu_button.cc
index 9ee7de9..242a9d9 100644
--- a/chrome/browser/chromeos/network_menu_button.cc
+++ b/chrome/browser/chromeos/network_menu_button.cc
@@ -128,7 +128,7 @@ void NetworkMenuButton::AnimationProgressed(const Animation* animation) {
 // NetworkMenuButton, views::ViewMenuDelegate implementation:
 
 void NetworkMenuButton::RunMenu(views::View* source, const gfx::Point& pt) {
-  wifi_networks_ = CrosNetworkLibrary::Get()->GetWifiNetworks();
+  wifi_networks_ = CrosNetworkLibrary::Get()->wifi_networks();
   refreshing_menu_ = true;
   network_menu_.Rebuild();
   network_menu_.UpdateStates();
@@ -168,6 +168,11 @@ void NetworkMenuButton::UpdateIcon() {
       // strength is from 0 to 100, so we need to convert that to 0 to 7.
       int index = static_cast<int>(cros->wifi_strength() / 100.0 *
                   nextafter(static_cast<float>(kNumWifiImages), 0));
+      // Make sure that index is between 0 and kNumWifiImages - 1
+      if (index < 0)
+        index = 0;
+      if (index >= kNumWifiImages)
+        index = kNumWifiImages - 1;
       id = IDR_STATUSBAR_WIFI_1 + index;
     }
   }
diff --git a/chrome/browser/chromeos/power_menu_button.cc b/chrome/browser/chromeos/power_menu_button.cc
index b15c559..192893b 100644
--- a/chrome/browser/chromeos/power_menu_button.cc
+++ b/chrome/browser/chromeos/power_menu_button.cc
@@ -112,6 +112,11 @@ void PowerMenuButton::UpdateIcon() {
       // from 0 to 100, so we need to convert that to 0 to kNumPowerImages - 1.
       int index = static_cast<int>(percent / 100.0 *
                   nextafter(static_cast<float>(kNumPowerImages), 0));
+      // Make sure that index is between 0 and kNumWifiImages - 1
+      if (index < 0)
+        index = 0;
+      if (index >= kNumPowerImages)
+        index = kNumPowerImages - 1;
       if (cros->line_power_on())
         id = IDR_STATUSBAR_BATTERY_CHARGING_1 + index;
       else
diff --git a/chrome/browser/chromeos/settings_contents_view.cc b/chrome/browser/chromeos/settings_contents_view.cc
index d0e5922..fc828c2 100644
--- a/chrome/browser/chromeos/settings_contents_view.cc
+++ b/chrome/browser/chromeos/settings_contents_view.cc
@@ -53,7 +53,7 @@ class WifiNetworkComboModel : public ComboboxModel {
 };
 
 WifiNetworkComboModel::WifiNetworkComboModel() {
-  wifi_networks_ = CrosNetworkLibrary::Get()->GetWifiNetworks();
+  wifi_networks_ = CrosNetworkLibrary::Get()->wifi_networks();
 }
 
 int WifiNetworkComboModel::GetItemCount() {