cros: doing dbus stuff on the file thread==disaster.

libdbus-glib uses the glib main loop internally.  We were making calls on a background thread that wound up telling libdbus-glib to get a connection to the system bus on said background thread.  This led to a background thread trying to use the same glib main loop as Chrome's UI thread, and concurrency issues led to crashing.  Sadly, there can be only one main loop per process, as I understand it.

This code takes away the attempts to use the background thread.  As our interaction with dbus is semantically asynchronous, this seems ok to me. 
Review URL: http://codereview.chromium.org/339013

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

4 files changed, 23 insertions, 28 deletions

diff --git a/chrome/browser/chromeos/cros_network_library.cc b/chrome/browser/chromeos/cros_network_library.cc
index 634e81e..8ca9abc 100644
--- a/chrome/browser/chromeos/cros_network_library.cc
+++ b/chrome/browser/chromeos/cros_network_library.cc
@@ -21,18 +21,12 @@ struct RunnableMethodTraits<CrosNetworkLibrary> {
 
 CrosNetworkLibrary::CrosNetworkLibrary() {
   if (CrosLibrary::loaded()) {
-    MessageLoop* loop = ChromeThread::GetMessageLoop(ChromeThread::FILE);
-    if (loop) {
-      loop->PostTask(FROM_HERE, NewRunnableMethod(this,
-          &CrosNetworkLibrary::InitOnBackgroundThread));
-    }
+    Init();
   }
 }
 
 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_);
   }
 }
@@ -72,13 +66,12 @@ static const char* GetEncryptionString(chromeos::EncryptionType encryption) {
 void CrosNetworkLibrary::ConnectToWifiNetwork(WifiNetwork network,
                                               const string16& password) {
   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)));
+    // This call kicks off a request to connect to this network, the results of
+    // which we'll hear about through the monitoring we've set up in Init();
+    chromeos::ConnectToWifiNetwork(
+        network.ssid.c_str(),
+        password.empty() ? NULL : UTF16ToUTF8(password).c_str(),
+        GetEncryptionString(network.encryption));
   }
 }
 
@@ -122,15 +115,20 @@ void CrosNetworkLibrary::ParseNetworks(
   }
 }
 
-void CrosNetworkLibrary::InitOnBackgroundThread() {
+void CrosNetworkLibrary::Init() {
+  // First, get the currently available networks.  This data is cached
+  // on the connman side, so the call should be quick.
   chromeos::ServiceStatus* service_status = chromeos::GetAvailableNetworks();
   if (service_status) {
+    LOG(INFO) << "Getting initial CrOS network info.";
     WifiNetworkVector networks;
     bool ethernet_connected;
     ParseNetworks(*service_status, &networks, &ethernet_connected);
     UpdateNetworkStatus(networks, ethernet_connected);
     chromeos::FreeServiceStatus(service_status);
   }
+  LOG(INFO) << "Registering for network status updates.";
+  // Now, register to receive updates on network status.
   network_status_connection_ = chromeos::MonitorNetworkStatus(
       &NetworkStatusChangedHandler, this);
 }
diff --git a/chrome/browser/chromeos/cros_network_library.h b/chrome/browser/chromeos/cros_network_library.h
index 324c153..13ecb37 100644
--- a/chrome/browser/chromeos/cros_network_library.h
+++ b/chrome/browser/chromeos/cros_network_library.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/observer_list.h"
+#include "base/platform_thread.h"
 #include "base/singleton.h"
 #include "base/string16.h"
 #include "third_party/cros/chromeos_network.h"
@@ -19,7 +20,8 @@ struct WifiNetwork {
         encryption(chromeos::NONE),
         strength(0),
         connecting(false),
-        connected(false) {}
+        connected(false),
+        destroyed(false) {}
   WifiNetwork(const std::string& ssid, bool encrypted,
               chromeos::EncryptionType encryption, int strength,
               bool connecting, bool connected)
@@ -28,7 +30,8 @@ struct WifiNetwork {
         encryption(encryption),
         strength(strength),
         connecting(connecting),
-        connected(connected) {}
+        connected(connected),
+        destroyed(false) {}
 
   // WifiNetworks are sorted by ssids.
   bool operator< (const WifiNetwork& other) const {
@@ -41,6 +44,7 @@ struct WifiNetwork {
   int strength;
   bool connecting;
   bool connected;
+  bool destroyed;
 };
 typedef std::vector<WifiNetwork> WifiNetworkVector;
 
@@ -93,8 +97,7 @@ class CrosNetworkLibrary {
 
   // 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();
+  void Init();
 
   // Update the network with the a list of wifi networks and ethernet status.
   // This will notify all the Observers.
diff --git a/chrome/browser/chromeos/cros_power_library.cc b/chrome/browser/chromeos/cros_power_library.cc
index ae3092b..d3188e8 100644
--- a/chrome/browser/chromeos/cros_power_library.cc
+++ b/chrome/browser/chromeos/cros_power_library.cc
@@ -19,17 +19,12 @@ struct RunnableMethodTraits<CrosPowerLibrary> {
 
 CrosPowerLibrary::CrosPowerLibrary() : status_(chromeos::PowerStatus()) {
   if (CrosLibrary::loaded()) {
-    MessageLoop* loop = ChromeThread::GetMessageLoop(ChromeThread::FILE);
-    if (loop)
-      loop->PostTask(FROM_HERE, NewRunnableMethod(this,
-          &CrosPowerLibrary::InitOnBackgroundThread));
+    Init();
   }
 }
 
 CrosPowerLibrary::~CrosPowerLibrary() {
   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_);
   }
 }
@@ -83,7 +78,7 @@ void CrosPowerLibrary::PowerStatusChangedHandler(void* object,
   power->UpdatePowerStatus(status);
 }
 
-void CrosPowerLibrary::InitOnBackgroundThread() {
+void CrosPowerLibrary::Init() {
   power_status_connection_ = chromeos::MonitorPowerStatus(
       &PowerStatusChangedHandler, this);
 }
diff --git a/chrome/browser/chromeos/cros_power_library.h b/chrome/browser/chromeos/cros_power_library.h
index e01e76d..f53a477 100644
--- a/chrome/browser/chromeos/cros_power_library.h
+++ b/chrome/browser/chromeos/cros_power_library.h
@@ -59,8 +59,7 @@ class CrosPowerLibrary {
                                         const chromeos::PowerStatus& status);
 
   // This methods starts the monitoring of power changes.
-  // It should be called on a background thread.
-  void InitOnBackgroundThread();
+  void Init();
 
   // Called by the handler to update the power status.
   // This will notify all the Observers.