Add chromeos::NetworkStateManager

chromeos/dbus/shill_manager_client.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/dbus/shill_manager_client.h"
 
 #include "base/bind.h"
 #include "base/chromeos/chromeos_version.h"
 #include "base/message_loop.h"
 #include "base/values.h"
@@ skipping 154 matching lines @@
     return NULL;
   }
 
  private:
   dbus::ObjectProxy* proxy_;
   ShillClientHelper helper_;
 
   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() {}
 
   // ShillManagerClient overrides.
 
   virtual void AddPropertyChangedObserver(
       ShillPropertyChangedObserver* observer) OVERRIDE {
     observer_list_.AddObserver(observer);
   }
@@ skipping 21 matching lines @@
                            const ErrorCallback& error_callback) OVERRIDE {
     stub_properties_.Set(name, value.DeepCopy());
     MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
   virtual void RequestScan(const std::string& type,
                            const base::Closure& callback,
                            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(
       const std::string& type,
       const base::Closure& callback,
       const ErrorCallback& error_callback) OVERRIDE {
     base::ListValue* enabled_list = NULL;
     if (!stub_properties_.GetListWithoutPathExpansion(
             flimflam::kEnabledTechnologiesProperty, &enabled_list)) {
       MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(error_callback, "StubError", "Property not found"));
       return;
     }
     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(
       const std::string& type,
       const base::Closure& callback,
       const ErrorCallback& error_callback) OVERRIDE {
     base::ListValue* enabled_list = NULL;
     if (!stub_properties_.GetListWithoutPathExpansion(
             flimflam::kEnabledTechnologiesProperty, &enabled_list)) {
       MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(error_callback, "StubError", "Property not found"));
       return;
     }
     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(
       const base::DictionaryValue& properties,
       const base::Closure& callback,
       const ErrorCallback& error_callback) OVERRIDE {
     MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
   virtual void GetService(
       const base::DictionaryValue& properties,
       const ObjectPathCallback& callback,
       const ErrorCallback& error_callback) OVERRIDE {
     MessageLoop::current()->PostTask(
         FROM_HERE, base::Bind(callback, dbus::ObjectPath()));
   }
 
   virtual ShillManagerClient::TestInterface* GetTestInterface() OVERRIDE {
     return this;
   }
 
   // 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 {
     stub_properties_.Clear();

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

No need to notify observers here? If so, please add comments to describe why.

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

Comment added to header.

   }
 
  private:
   void SetDefaultProperties() {
     // Stub Devices, Note: names match Device stub map.
     AddDevice("stub_wifi_device1");
     AddDevice("stub_cellular_device1");
 
     // Stub Services, Note: names match Service stub map.
     AddService("stub_ethernet", true);
     AddService("stub_wifi1", true);
     AddService("stub_wifi2", true);
     AddService("stub_cellular1", true);
 
     // Stub Technologies
     AddTechnology(flimflam::kTypeEthernet, true);
     AddTechnology(flimflam::kTypeWifi, true);
     AddTechnology(flimflam::kTypeCellular, true);
   }
 
   void PassStubProperties(const DictionaryValueCallback& callback) const {
     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)) {
       LOG(ERROR) << "Notify for unknown property: " << property;
       return;
     }
     FOR_EACH_OBSERVER(ShillPropertyChangedObserver,
                       observer_list_,
                       OnPropertyChanged(property, *value));
   }
 
   base::ListValue* GetListProperty(const std::string& property) {
     base::ListValue* list_property = NULL;
     if (!stub_properties_.GetListWithoutPathExpansion(
             property, &list_property)) {
       list_property = new base::ListValue;
       stub_properties_.Set(property, list_property);
     }
     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_;
 
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<ShillManagerClientStubImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ShillManagerClientStubImpl);
 };
 
 }  // namespace
 
 ShillManagerClient::ShillManagerClient() {}
 
 ShillManagerClient::~ShillManagerClient() {}
 
 // static
 ShillManagerClient* ShillManagerClient::Create(
     DBusClientImplementationType type,
     dbus::Bus* bus) {
   if (type == REAL_DBUS_CLIENT_IMPLEMENTATION)
     return new ShillManagerClientImpl(bus);
   DCHECK_EQ(STUB_DBUS_CLIENT_IMPLEMENTATION, type);
   return new ShillManagerClientStubImpl();
 }
 
 }  // namespace chromeos