Add chromeos::NetworkStateManager

chromeos/dbus/shill_manager_client.cc

Index: chromeos/dbus/shill_manager_client.cc
diff --git a/chromeos/dbus/shill_manager_client.cc b/chromeos/dbus/shill_manager_client.cc
index caffa301c7f37cc542d5b165084e2f354873575a..c63efc00875c1d0caac8e8ca2e2453ca1eda3a0b 100644
--- a/chromeos/dbus/shill_manager_client.cc
+++ b/chromeos/dbus/shill_manager_client.cc
@@ -172,14 +172,29 @@ class ShillManagerClientImpl : public ShillManagerClient {
   DISALLOW_COPY_AND_ASSIGN(ShillManagerClientImpl);
 };
 
+namespace {

[hashimoto, 2012/11/05 04:03:03]

We are already in the unnamed namespace, no need to go deeper here.

[stevenjb, 2012/11/05 22:42:35]

Oops. Done.

+
+struct ValueEquals {

[hashimoto, 2012/11/05 04:03:03]

Could you add some comment about how this struct is used?

[stevenjb, 2012/11/05 22:42:35]

Done.

+  ValueEquals(const Value* first) : first_(first) {}
+  bool operator ()(const Value* second) const {
+    return first_->Equals(second);
+  }
+  const Value* first_;
+};
+
+}  // namespace
+
 // A stub implementation of ShillManagerClient.
 // Implemented: Stub devices and services for NetworkStateManager tests.
 // Implemented: Stub cellular device entry for SMS tests.
 class ShillManagerClientStubImpl : public ShillManagerClient,
                                    public ShillManagerClient::TestInterface {
  public:
-  ShillManagerClientStubImpl() : weak_ptr_factory_(this) {
+  ShillManagerClientStubImpl()
+      : initialized_(false),
+        weak_ptr_factory_(this) {
     SetDefaultProperties();
+    initialized_ = true;
   }
 
   virtual ~ShillManagerClientStubImpl() {}
@@ -221,12 +236,8 @@ class ShillManagerClientStubImpl : public ShillManagerClient,
                            const ErrorCallback& error_callback) OVERRIDE {
     MessageLoop::current()->PostTask(FROM_HERE, callback);
     const int kScanDelaySeconds = 3;
-    MessageLoop::current()->PostDelayedTask(
-        FROM_HERE,
-        base::Bind(&ShillManagerClientStubImpl::NotifyObserversPropertyChanged,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   flimflam::kServicesProperty),
-        base::TimeDelta::FromSeconds(kScanDelaySeconds));
+    CallNotifyObserversPropertyChanged(
+        flimflam::kServicesProperty, kScanDelaySeconds);
   }
 
   virtual void EnableTechnology(
@@ -243,11 +254,8 @@ class ShillManagerClientStubImpl : public ShillManagerClient,
     }
     MessageLoop::current()->PostTask(FROM_HERE, callback);
     enabled_list->AppendIfNotPresent(new base::StringValue(type));
-    MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&ShillManagerClientStubImpl::NotifyObserversPropertyChanged,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   flimflam::kEnabledTechnologiesProperty));
+    CallNotifyObserversPropertyChanged(
+        flimflam::kEnabledTechnologiesProperty, 0);
   }
 
   virtual void DisableTechnology(
@@ -265,11 +273,8 @@ class ShillManagerClientStubImpl : public ShillManagerClient,
     MessageLoop::current()->PostTask(FROM_HERE, callback);
     base::StringValue type_value(type);
     enabled_list->Remove(type_value, NULL);
-    MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&ShillManagerClientStubImpl::NotifyObserversPropertyChanged,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   flimflam::kEnabledTechnologiesProperty));
+    CallNotifyObserversPropertyChanged(
+        flimflam::kEnabledTechnologiesProperty, 0);
   }
 
   virtual void ConfigureService(
@@ -294,49 +299,89 @@ class ShillManagerClientStubImpl : public ShillManagerClient,
   // ShillManagerClient::TestInterface overrides.
 
   virtual void AddDevice(const std::string& device_path) OVERRIDE {
-    GetListProperty(flimflam::kDevicesProperty)->Append(
-        base::Value::CreateStringValue(device_path));
+    if (GetListProperty(flimflam::kDevicesProperty)->AppendIfNotPresent(
+            base::Value::CreateStringValue(device_path))) {
+      CallNotifyObserversPropertyChanged(flimflam::kDevicesProperty, 0);
+    }
   }
 
   virtual void RemoveDevice(const std::string& device_path) OVERRIDE {
     base::StringValue device_path_value(device_path);
-    GetListProperty(flimflam::kDevicesProperty)->Remove(
-        device_path_value, NULL);
+    if (GetListProperty(flimflam::kDevicesProperty)->Remove(
+            device_path_value, NULL)) {
+      CallNotifyObserversPropertyChanged(flimflam::kDevicesProperty, 0);
+    }
   }
 
   virtual void AddService(const std::string& service_path,
                           bool add_to_watch_list) OVERRIDE {
-    GetListProperty(flimflam::kServicesProperty)->Append(
-        base::Value::CreateStringValue(service_path));
-    if (add_to_watch_list) {
-      GetListProperty(flimflam::kServiceWatchListProperty)->Append(
-          base::Value::CreateStringValue(service_path));
+    if (GetListProperty(flimflam::kServicesProperty)->AppendIfNotPresent(
+            base::Value::CreateStringValue(service_path))) {
+      CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
+    }
+    if (add_to_watch_list &&
+        GetListProperty(
+            flimflam::kServiceWatchListProperty)->AppendIfNotPresent(
+                base::Value::CreateStringValue(service_path))) {
+      CallNotifyObserversPropertyChanged(
+          flimflam::kServiceWatchListProperty, 0);
     }
   }
 
+  virtual void InsertService(const std::string& service_path,
+                             size_t index) OVERRIDE {
+    base::StringValue path_value(service_path);
+    base::ListValue* service_list =
+        GetListProperty(flimflam::kServicesProperty);
+    base::ListValue::iterator iter =
+        std::find_if(service_list->begin(), service_list->end(),
+                     ValueEquals(&path_value));
+        service_list->Find(path_value);
+    if (iter != service_list->end())
+      service_list->Erase(iter, NULL);
+    service_list->Insert(index, path_value.DeepCopy());
+    CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
+  }
+
   virtual void RemoveService(const std::string& service_path) OVERRIDE {
     base::StringValue service_path_value(service_path);
-    GetListProperty(flimflam::kServicesProperty)->Remove(
-        service_path_value, NULL);
-    GetListProperty(flimflam::kServiceWatchListProperty)->Remove(
-        service_path_value, NULL);
+    if (GetListProperty(flimflam::kServicesProperty)->Remove(
+            service_path_value, NULL)) {
+      CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
+    }
+    if (GetListProperty(flimflam::kServiceWatchListProperty)->Remove(
+            service_path_value, NULL)) {
+      CallNotifyObserversPropertyChanged(
+          flimflam::kServiceWatchListProperty, 0);
+    }
   }
 
   virtual void AddTechnology(const std::string& type, bool enabled) OVERRIDE {
-    GetListProperty(flimflam::kAvailableTechnologiesProperty)->Append(
-        base::Value::CreateStringValue(type));
-    if (enabled) {
-      GetListProperty(flimflam::kEnabledTechnologiesProperty)->Append(
-          base::Value::CreateStringValue(type));
+    if (GetListProperty(flimflam::kAvailableTechnologiesProperty)->
+        AppendIfNotPresent(base::Value::CreateStringValue(type))) {
+      CallNotifyObserversPropertyChanged(
+          flimflam::kAvailableTechnologiesProperty, 0);
+    }
+    if (enabled &&
+        GetListProperty(flimflam::kEnabledTechnologiesProperty)->
+        AppendIfNotPresent(base::Value::CreateStringValue(type))) {
+      CallNotifyObserversPropertyChanged(
+          flimflam::kEnabledTechnologiesProperty, 0);
     }
   }
 
   virtual void RemoveTechnology(const std::string& type) OVERRIDE {
     base::StringValue type_value(type);
-    GetListProperty(flimflam::kAvailableTechnologiesProperty)->Remove(
-        type_value, NULL);
-    GetListProperty(flimflam::kEnabledTechnologiesProperty)->Remove(
-        type_value, NULL);
+    if (GetListProperty(flimflam::kAvailableTechnologiesProperty)->Remove(
+            type_value, NULL)) {
+      CallNotifyObserversPropertyChanged(
+          flimflam::kAvailableTechnologiesProperty, 0);
+    }
+    if (GetListProperty(flimflam::kEnabledTechnologiesProperty)->Remove(
+            type_value, NULL)) {
+      CallNotifyObserversPropertyChanged(
+          flimflam::kEnabledTechnologiesProperty, 0);
+    }
   }
 
   virtual void ClearProperties() OVERRIDE {
@@ -365,6 +410,18 @@ class ShillManagerClientStubImpl : public ShillManagerClient,
     callback.Run(DBUS_METHOD_CALL_SUCCESS, stub_properties_);
   }
 
+  void CallNotifyObserversPropertyChanged(const std::string& property,
+                                          int delay_seconds) {

[Greg Spencer (Chromium), 2012/11/05 22:07:23]

Is seconds enough granularity?  Seems like milliseconds would be more flexible,
even if sub millisecond values are not used at the moment.

[stevenjb, 2012/11/05 22:42:35]

Done.

+    if (!initialized_)
+      return;
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&ShillManagerClientStubImpl::NotifyObserversPropertyChanged,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   property),
+        base::TimeDelta::FromSeconds(delay_seconds));
+  }
+
   void NotifyObserversPropertyChanged(const std::string& property) {
     base::Value* value = NULL;
     if (!stub_properties_.GetWithoutPathExpansion(property, &value)) {
@@ -386,6 +443,7 @@ class ShillManagerClientStubImpl : public ShillManagerClient,
     return list_property;
   }
 
+  bool initialized_;

[hashimoto, 2012/11/05 04:03:03]

Please add comments to describe that we need this variable even if no one can
add itself as an observer until the ShillManagerClient finishes its construction
because we notify observers asynchronously.

[stevenjb, 2012/11/05 22:42:35]

Good point. Removed this.

   base::DictionaryValue stub_properties_;
   ObserverList<ShillPropertyChangedObserver> observer_list_;