Add TestInterface methods for adding shill stub devices and services

This will help write functional tests for network handlers written on top of the Shill client code.

BUG=154072
TEST=NetworkSmsHandlerTest.SmsHandlerDbusStub passes


Review URL: https://chromiumcodereview.appspot.com/11365022

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

11 files changed, 443 insertions, 72 deletions

diff --git a/chromeos/dbus/mock_shill_device_client.h b/chromeos/dbus/mock_shill_device_client.h
index fa0372c..26b8032 100644
--- a/chromeos/dbus/mock_shill_device_client.h
+++ b/chromeos/dbus/mock_shill_device_client.h
@@ -71,6 +71,7 @@ class MockShillDeviceClient : public ShillDeviceClient {
                                 const std::string& carrier,
                                 const base::Closure& callback,
                                 const ErrorCallback& error_callback));
+  MOCK_METHOD0(GetTestInterface, TestInterface*());
 };
 
 }  // namespace chromeos
diff --git a/chromeos/dbus/mock_shill_manager_client.h b/chromeos/dbus/mock_shill_manager_client.h
index b022520..eb55a71 100644
--- a/chromeos/dbus/mock_shill_manager_client.h
+++ b/chromeos/dbus/mock_shill_manager_client.h
@@ -42,6 +42,7 @@ class MockShillManagerClient : public ShillManagerClient {
   MOCK_METHOD3(GetService, void(const base::DictionaryValue& properties,
                                 const ObjectPathCallback& callback,
                                 const ErrorCallback& error_callback));
+  MOCK_METHOD0(GetTestInterface, TestInterface*());
 };
 
 }  // namespace chromeos
diff --git a/chromeos/dbus/mock_shill_service_client.h b/chromeos/dbus/mock_shill_service_client.h
index 12d14bb..434d9ca 100644
--- a/chromeos/dbus/mock_shill_service_client.h
+++ b/chromeos/dbus/mock_shill_service_client.h
@@ -52,6 +52,7 @@ class MockShillServiceClient : public ShillServiceClient {
   MOCK_METHOD2(CallActivateCellularModemAndBlock,
                bool(const dbus::ObjectPath& service_path,
                     const std::string& carrier));
+  MOCK_METHOD0(GetTestInterface, TestInterface*());
 };
 
 }  // namespace chromeos
diff --git a/chromeos/dbus/shill_device_client.cc b/chromeos/dbus/shill_device_client.cc
index b2cc011..39ca916 100644
--- a/chromeos/dbus/shill_device_client.cc
+++ b/chromeos/dbus/shill_device_client.cc
@@ -188,6 +188,10 @@ class ShillDeviceClientImpl : public ShillDeviceClient {
         &method_call, callback, error_callback);
   }
 
+  virtual TestInterface* GetTestInterface() OVERRIDE {
+    return NULL;
+  }
+
  private:
   typedef std::map<std::string, ShillClientHelper*> HelperMap;
 
@@ -215,45 +219,17 @@ class ShillDeviceClientImpl : public ShillDeviceClient {
 
 // A stub implementation of ShillDeviceClient.
 // Implemented: Stub cellular device for SMS testing.
-class ShillDeviceClientStubImpl : public ShillDeviceClient {
+class ShillDeviceClientStubImpl : public ShillDeviceClient,
+                                  public ShillDeviceClient::TestInterface {
  public:
   ShillDeviceClientStubImpl() : weak_ptr_factory_(this) {
-    // Add a cellular device for SMS. Note: name matches Manager entry.
-    const char kStubCellular1[] = "stub_cellular1";
-    base::DictionaryValue* cellular_properties = new base::DictionaryValue;
-    cellular_properties->SetWithoutPathExpansion(
-        flimflam::kTypeProperty,
-        base::Value::CreateStringValue(flimflam::kTypeCellular));
-    cellular_properties->SetWithoutPathExpansion(
-        flimflam::kDBusConnectionProperty,
-        base::Value::CreateStringValue("/stub"));
-    cellular_properties->SetWithoutPathExpansion(
-        flimflam::kDBusObjectProperty,
-        base::Value::CreateStringValue("/device/cellular1"));
-    stub_devices_.Set(kStubCellular1, cellular_properties);
-
-    // Create a second device stubbing a modem managed by
-    // ModemManager1 interfaces.
-    // Note: name matches Manager entry.
-    const char kStubCellular2[] = "stub_cellular2";
-    cellular_properties = new base::DictionaryValue;
-    cellular_properties->SetWithoutPathExpansion(
-        flimflam::kTypeProperty,
-        base::Value::CreateStringValue(flimflam::kTypeCellular));
-    cellular_properties->SetWithoutPathExpansion(
-        flimflam::kDBusConnectionProperty,
-        base::Value::CreateStringValue(":stub.0"));
-    cellular_properties->SetWithoutPathExpansion(
-        flimflam::kDBusObjectProperty,
-        base::Value::CreateStringValue(
-                "/org/freedesktop/ModemManager1/stub/0"));
-    stub_devices_.Set(kStubCellular2, cellular_properties);
+    SetDefaultProperties();
   }
 
   virtual ~ShillDeviceClientStubImpl() {}
 
-  ///////////////////////////////////
   // ShillDeviceClient overrides.
+
   virtual void AddPropertyChangedObserver(
       const dbus::ObjectPath& device_path,
       ShillPropertyChangedObserver* observer) OVERRIDE {}
@@ -273,7 +249,10 @@ class ShillDeviceClientStubImpl : public ShillDeviceClient {
 
   virtual base::DictionaryValue* CallGetPropertiesAndBlock(
       const dbus::ObjectPath& device_path) OVERRIDE {
-    return new base::DictionaryValue;
+    base::DictionaryValue* device_properties = NULL;
+    stub_devices_.GetDictionaryWithoutPathExpansion(
+        device_path.value(), &device_properties);
+    return device_properties;
   }
 
   virtual void ProposeScan(const dbus::ObjectPath& device_path,
@@ -366,7 +345,6 @@ class ShillDeviceClientStubImpl : public ShillDeviceClient {
     MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
-  // ShillDeviceClient override.
   virtual void SetCarrier(const dbus::ObjectPath& device_path,
                           const std::string& carrier,
                           const base::Closure& callback,
@@ -374,11 +352,53 @@ class ShillDeviceClientStubImpl : public ShillDeviceClient {
     MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
+  virtual ShillDeviceClient::TestInterface* GetTestInterface() OVERRIDE {
+    return this;
+  }
+
+  // ShillDeviceClient::TestInterface overrides.
+
+  virtual void AddDevice(const std::string& device_path,
+                         const std::string& type,
+                         const std::string& object_path,
+                         const std::string& connection_path) OVERRIDE {
+    base::DictionaryValue* properties = GetDeviceProperties(device_path);
+    properties->SetWithoutPathExpansion(
+        flimflam::kTypeProperty,
+        base::Value::CreateStringValue(type));
+    properties->SetWithoutPathExpansion(
+        flimflam::kDBusObjectProperty,
+        base::Value::CreateStringValue(object_path));
+    properties->SetWithoutPathExpansion(
+        flimflam::kDBusConnectionProperty,
+        base::Value::CreateStringValue(connection_path));
+  }
+
+  virtual void RemoveDevice(const std::string& device_path) OVERRIDE {
+    stub_devices_.RemoveWithoutPathExpansion(device_path, NULL);
+  }
+
+  virtual void ClearDevices() OVERRIDE {
+    stub_devices_.Clear();
+  }
+
  private:
+  void SetDefaultProperties() {
+    // Add a wifi device. Note: path matches Manager entry.
+    AddDevice("stub_wifi_device1", flimflam::kTypeWifi,
+              "/device/wifi1", "/stub");
+
+    // Add a cellular device. Used in SMS stub. Note: path matches
+    // Manager entry.
+    AddDevice("stub_cellular_device1", flimflam::kTypeCellular,
+              "/device/cellular1", "/stub");
+  }
+
   void PassStubDeviceProperties(const dbus::ObjectPath& device_path,
-                               const DictionaryValueCallback& callback) const {
+                                const DictionaryValueCallback& callback) const {
     const base::DictionaryValue* device_properties = NULL;
-    if (!stub_devices_.GetDictionary(device_path.value(), &device_properties)) {
+    if (!stub_devices_.GetDictionaryWithoutPathExpansion(
+            device_path.value(), &device_properties)) {
       base::DictionaryValue empty_dictionary;
       callback.Run(DBUS_METHOD_CALL_FAILURE, empty_dictionary);
       return;
@@ -393,6 +413,16 @@ class ShillDeviceClientStubImpl : public ShillDeviceClient {
                                      base::Bind(callback, status));
   }
 
+  base::DictionaryValue* GetDeviceProperties(const std::string& device_path) {
+    base::DictionaryValue* properties = NULL;
+    if (!stub_devices_.GetDictionaryWithoutPathExpansion(
+            device_path, &properties)) {
+      properties = new base::DictionaryValue;
+      stub_devices_.Set(device_path, properties);
+    }
+    return properties;
+  }
+
   // Dictionary of <device_name, Dictionary>.
   base::DictionaryValue stub_devices_;
 
diff --git a/chromeos/dbus/shill_device_client.h b/chromeos/dbus/shill_device_client.h
index 7ac7e20..f016430 100644
--- a/chromeos/dbus/shill_device_client.h
+++ b/chromeos/dbus/shill_device_client.h
@@ -40,6 +40,21 @@ class CHROMEOS_EXPORT ShillDeviceClient {
   typedef ShillClientHelper::DictionaryValueCallback DictionaryValueCallback;
   typedef ShillClientHelper::ErrorCallback ErrorCallback;
 
+  // Interface for setting up devices for testing.
+  // Accessed through GetTestInterface(), only implemented in the Stub Impl.
+  class TestInterface {
+   public:
+    virtual void AddDevice(const std::string& device_path,
+                           const std::string& type,
+                           const std::string& object_path,
+                           const std::string& connection_path) = 0;
+    virtual void RemoveDevice(const std::string& device_path) = 0;
+    virtual void ClearDevices() = 0;
+
+   protected:
+    ~TestInterface() {}
+  };
+
   virtual ~ShillDeviceClient();
 
   // Factory function, creates a new instance which is owned by the caller.
@@ -151,6 +166,9 @@ class CHROMEOS_EXPORT ShillDeviceClient {
                           const base::Closure& callback,
                           const ErrorCallback& error_callback) = 0;
 
+  // Returns an interface for testing (stub only), or returns NULL.
+  virtual TestInterface* GetTestInterface() = 0;
+
  protected:
   // Create() should be used instead.
   ShillDeviceClient();
diff --git a/chromeos/dbus/shill_manager_client.cc b/chromeos/dbus/shill_manager_client.cc
index 08f163b..caffa30 100644
--- a/chromeos/dbus/shill_manager_client.cc
+++ b/chromeos/dbus/shill_manager_client.cc
@@ -161,6 +161,10 @@ class ShillManagerClientImpl : public ShillManagerClient {
                                                   error_callback);
   }
 
+  virtual TestInterface* GetTestInterface() OVERRIDE {
+    return NULL;
+  }
+
  private:
   dbus::ObjectProxy* proxy_;
   ShillClientHelper helper_;
@@ -169,30 +173,29 @@ class ShillManagerClientImpl : public ShillManagerClient {
 };
 
 // A stub implementation of ShillManagerClient.
-// Implemented: Stub cellular DeviceList entry for SMS testing.
-class ShillManagerClientStubImpl : public 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) {
-    base::ListValue* device_list = new base::ListValue;
-    // Note: names match Device stub map.
-    const char kStubCellular1[] = "stub_cellular1";
-    const char kStubCellular2[] = "stub_cellular2";
-    device_list->Append(base::Value::CreateStringValue(kStubCellular1));
-    device_list->Append(base::Value::CreateStringValue(kStubCellular2));
-    stub_properties_.Set(flimflam::kDevicesProperty, device_list);
+    SetDefaultProperties();
   }
 
   virtual ~ShillManagerClientStubImpl() {}
 
-  //////////////////////////////////
   // ShillManagerClient overrides.
+
   virtual void AddPropertyChangedObserver(
-      ShillPropertyChangedObserver* observer) OVERRIDE {}
+      ShillPropertyChangedObserver* observer) OVERRIDE {
+    observer_list_.AddObserver(observer);
+  }
 
   virtual void RemovePropertyChangedObserver(
-      ShillPropertyChangedObserver* observer) OVERRIDE {}
+      ShillPropertyChangedObserver* observer) OVERRIDE {
+    observer_list_.RemoveObserver(observer);
+  }
 
-  // ShillManagerClient override.
   virtual void GetProperties(const DictionaryValueCallback& callback) OVERRIDE {
     MessageLoop::current()->PostTask(
         FROM_HERE, base::Bind(
@@ -201,12 +204,10 @@ class ShillManagerClientStubImpl : public ShillManagerClient {
             callback));
   }
 
-  // ShillManagerClient override.
   virtual base::DictionaryValue* CallGetPropertiesAndBlock() OVERRIDE {
-    return new base::DictionaryValue;
+    return stub_properties_.DeepCopy();
   }
 
-  // ShillManagerClient override.
   virtual void SetProperty(const std::string& name,
                            const base::Value& value,
                            const base::Closure& callback,
@@ -215,30 +216,62 @@ class ShillManagerClientStubImpl : public ShillManagerClient {
     MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
-  // ShillManagerClient override.
   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));
   }
 
-  // ShillManagerClient override.
   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));
   }
 
-  // ShillManagerClient override.
   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));
   }
 
-  // ShillManagerClient override.
   virtual void ConfigureService(
       const base::DictionaryValue& properties,
       const base::Closure& callback,
@@ -246,22 +279,115 @@ class ShillManagerClientStubImpl : public ShillManagerClient {
     MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
-  // ShillManagerClient override.
   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()));
+    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));
+  }
+
+  virtual void RemoveDevice(const std::string& device_path) OVERRIDE {
+    base::StringValue device_path_value(device_path);
+    GetListProperty(flimflam::kDevicesProperty)->Remove(
+        device_path_value, NULL);
+  }
+
+  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));
+    }
+  }
+
+  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);
+  }
+
+  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));
+    }
+  }
+
+  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);
+  }
+
+  virtual void ClearProperties() OVERRIDE {
+    stub_properties_.Clear();
   }
 
  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 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;
+  }
+
   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.
diff --git a/chromeos/dbus/shill_manager_client.h b/chromeos/dbus/shill_manager_client.h
index 8034830..01dd63a 100644
--- a/chromeos/dbus/shill_manager_client.h
+++ b/chromeos/dbus/shill_manager_client.h
@@ -32,6 +32,23 @@ class CHROMEOS_EXPORT ShillManagerClient {
   typedef ShillClientHelper::DictionaryValueCallback DictionaryValueCallback;
   typedef ShillClientHelper::ErrorCallback ErrorCallback;
 
+  // Interface for setting up devices, services, and technologies for testing.
+  // Accessed through GetTestInterface(), only implemented in the Stub Impl.
+  class TestInterface {
+   public:
+    virtual void AddDevice(const std::string& device_path) = 0;
+    virtual void RemoveDevice(const std::string& device_path) = 0;
+    virtual void AddService(const std::string& service_path,
+                            bool add_to_watch_list) = 0;
+    virtual void RemoveService(const std::string& service_path) = 0;
+    virtual void AddTechnology(const std::string& type, bool enabled) = 0;
+    virtual void RemoveTechnology(const std::string& type) = 0;
+    virtual void ClearProperties() = 0;
+
+   protected:
+    ~TestInterface() {}
+  };
+
   virtual ~ShillManagerClient();
 
   // Factory function, creates a new instance which is owned by the caller.
@@ -96,6 +113,9 @@ class CHROMEOS_EXPORT ShillManagerClient {
                           const ObjectPathCallback& callback,
                           const ErrorCallback& error_callback) = 0;
 
+  // Returns an interface for testing (stub only), or returns NULL.
+  virtual TestInterface* GetTestInterface() = 0;
+
  protected:
   // Create() should be used instead.
   ShillManagerClient();
diff --git a/chromeos/dbus/shill_service_client.cc b/chromeos/dbus/shill_service_client.cc
index 46fe0f4..f208966 100644
--- a/chromeos/dbus/shill_service_client.cc
+++ b/chromeos/dbus/shill_service_client.cc
@@ -153,6 +153,10 @@ class ShillServiceClientImpl : public ShillServiceClient {
     return GetHelper(service_path)->CallVoidMethodAndBlock(&method_call);
   }
 
+  virtual ShillServiceClient::TestInterface* GetTestInterface() OVERRIDE {
+    return NULL;
+  }
+
  private:
   typedef std::map<std::string, ShillClientHelper*> HelperMap;
 
@@ -179,28 +183,37 @@ class ShillServiceClientImpl : public ShillServiceClient {
 };
 
 // A stub implementation of ShillServiceClient.
-class ShillServiceClientStubImpl : public ShillServiceClient {
+class ShillServiceClientStubImpl : public ShillServiceClient,
+                                   public ShillServiceClient::TestInterface {
  public:
-  ShillServiceClientStubImpl() : weak_ptr_factory_(this) {}
+  ShillServiceClientStubImpl() : weak_ptr_factory_(this) {
+    SetDefaultProperties();
+  }
 
-  virtual ~ShillServiceClientStubImpl() {}
+  virtual ~ShillServiceClientStubImpl() {
+  }
 
-  ///////////////////////////////////
   // ShillServiceClient overrides.
+
   virtual void AddPropertyChangedObserver(
       const dbus::ObjectPath& service_path,
-      ShillPropertyChangedObserver* observer) OVERRIDE {}
+      ShillPropertyChangedObserver* observer) OVERRIDE {
+    observer_list_.AddObserver(observer);
+  }
 
   virtual void RemovePropertyChangedObserver(
       const dbus::ObjectPath& service_path,
-      ShillPropertyChangedObserver* observer) OVERRIDE {}
+      ShillPropertyChangedObserver* observer) OVERRIDE {
+    observer_list_.RemoveObserver(observer);
+  }
 
   virtual void GetProperties(const dbus::ObjectPath& service_path,
                              const DictionaryValueCallback& callback) OVERRIDE {
     MessageLoop::current()->PostTask(
         FROM_HERE,
-        base::Bind(&ShillServiceClientStubImpl::PassEmptyDictionaryValue,
+        base::Bind(&ShillServiceClientStubImpl::PassStubDictionaryValue,
                    weak_ptr_factory_.GetWeakPtr(),
+                   service_path,
                    callback));
   }
 
@@ -209,13 +222,31 @@ class ShillServiceClientStubImpl : public ShillServiceClient {
                            const base::Value& value,
                            const base::Closure& callback,
                            const ErrorCallback& error_callback) OVERRIDE {
+    base::DictionaryValue* dict = NULL;
+    if (!stub_services_.GetDictionaryWithoutPathExpansion(
+            service_path.value(), &dict)) {
+      error_callback.Run("StubError", "Service not found");
+      return;
+    }
+    dict->SetWithoutPathExpansion(name, value.DeepCopy());
     MessageLoop::current()->PostTask(FROM_HERE, callback);
+    MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&ShillServiceClientStubImpl::NotifyObserversPropertyChanged,
+                   weak_ptr_factory_.GetWeakPtr(), service_path, name));
   }
 
   virtual void ClearProperty(const dbus::ObjectPath& service_path,
                              const std::string& name,
                              const base::Closure& callback,
                              const ErrorCallback& error_callback) OVERRIDE {
+    base::DictionaryValue* dict = NULL;
+    if (!stub_services_.GetDictionaryWithoutPathExpansion(
+            service_path.value(), &dict)) {
+      error_callback.Run("StubError", "Service not found");
+      return;
+    }
+    dict->Remove(name, NULL);
     MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
@@ -251,12 +282,120 @@ class ShillServiceClientStubImpl : public ShillServiceClient {
     return true;
   }
 
+  virtual ShillServiceClient::TestInterface* GetTestInterface() OVERRIDE {
+    return this;
+  }
+
+  // ShillServiceClient::TestInterface overrides.
+
+  virtual void AddService(const std::string& service_path,
+                          const std::string& name,
+                          const std::string& type,
+                          const std::string& state,
+                          const std::string& security) OVERRIDE {
+    base::DictionaryValue* properties = GetServiceProperties(service_path);
+    properties->SetWithoutPathExpansion(
+        flimflam::kSSIDProperty,
+        base::Value::CreateStringValue(service_path));
+    properties->SetWithoutPathExpansion(
+        flimflam::kNameProperty,
+        base::Value::CreateStringValue(name));
+    properties->SetWithoutPathExpansion(
+        flimflam::kTypeProperty,
+        base::Value::CreateStringValue(type));
+    properties->SetWithoutPathExpansion(
+        flimflam::kStateProperty,
+        base::Value::CreateStringValue(state));
+    properties->SetWithoutPathExpansion(
+        flimflam::kSecurityProperty,
+        base::Value::CreateStringValue(security));
+  }
+
+  virtual void RemoveService(const std::string& service_path) {
+    stub_services_.RemoveWithoutPathExpansion(service_path, NULL);
+  }
+
+  virtual void SetServiceProperty(const std::string& service_path,
+                                  const std::string& property,
+                                  const base::Value& value) OVERRIDE {
+    SetProperty(dbus::ObjectPath(service_path), property, value,
+                base::Bind(&base::DoNothing), ErrorCallback());
+  }
+
+  virtual void ClearServices() OVERRIDE {
+    stub_services_.Clear();
+  }
+
  private:
-  void PassEmptyDictionaryValue(const DictionaryValueCallback& callback) const {
-    base::DictionaryValue dictionary;
-    callback.Run(DBUS_METHOD_CALL_SUCCESS, dictionary);
+  void SetDefaultProperties() {
+    // Add stub services. Note: names match Manager stub impl.
+    AddService("stub_ethernet", "eth0",
+               flimflam::kTypeEthernet,
+               flimflam::kStateOnline,
+               flimflam::kSecurityNone);
+    AddService("stub_wifi1", "wifi1",
+               flimflam::kTypeWifi,
+               flimflam::kStateOnline,
+               flimflam::kSecurityNone);
+    AddService("stub_wifi2", "wifi2_PSK",
+               flimflam::kTypeWifi,
+               flimflam::kStateIdle,
+               flimflam::kSecurityPsk);
+    AddService("stub_cellular1", "cellular1",
+               flimflam::kTypeCellular,
+               flimflam::kStateIdle,
+               flimflam::kSecurityNone);
+    base::StringValue technology_value(flimflam::kNetworkTechnologyGsm);
+    SetServiceProperty("stub_cellular1",
+                       flimflam::kNetworkTechnologyProperty,
+                       technology_value);
+  }
+
+  void PassStubDictionaryValue(const dbus::ObjectPath& service_path,
+                               const DictionaryValueCallback& callback) {
+    base::DictionaryValue* dict = NULL;
+    if (!stub_services_.GetDictionaryWithoutPathExpansion(
+            service_path.value(), &dict)) {
+      base::DictionaryValue empty_dictionary;
+      callback.Run(DBUS_METHOD_CALL_FAILURE, empty_dictionary);
+      return;
+    }
+    callback.Run(DBUS_METHOD_CALL_SUCCESS, *dict);
+  }
+
+  void NotifyObserversPropertyChanged(const dbus::ObjectPath& service_path,
+                                      const std::string& property) {
+    base::DictionaryValue* dict = NULL;
+    std::string path = service_path.value();
+    if (!stub_services_.GetDictionaryWithoutPathExpansion(path, &dict)) {
+      LOG(ERROR) << "Notify for unknown service: " << path;
+      return;
+    }
+    base::Value* value = NULL;
+    if (!dict->GetWithoutPathExpansion(property, &value)) {
+      LOG(ERROR) << "Notify for unknown property: "
+                 << path << " : " << property;
+      return;
+    }
+    FOR_EACH_OBSERVER(ShillPropertyChangedObserver,
+                      observer_list_,
+                      OnPropertyChanged(property, *value));
   }
 
+  base::DictionaryValue* GetServiceProperties(const std::string& service_path) {
+    base::DictionaryValue* properties = NULL;
+    if (!stub_services_.GetDictionaryWithoutPathExpansion(
+            service_path, &properties)) {
+      properties = new base::DictionaryValue;
+      stub_services_.Set(service_path, properties);
+    }
+    return properties;
+  }
+
+
+  base::DictionaryValue stub_services_;
+  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<ShillServiceClientStubImpl> weak_ptr_factory_;
diff --git a/chromeos/dbus/shill_service_client.h b/chromeos/dbus/shill_service_client.h
index 0e30243..0563aa4 100644
--- a/chromeos/dbus/shill_service_client.h
+++ b/chromeos/dbus/shill_service_client.h
@@ -39,12 +39,30 @@ class CHROMEOS_EXPORT ShillServiceClient {
   typedef ShillClientHelper::DictionaryValueCallback DictionaryValueCallback;
   typedef ShillClientHelper::ErrorCallback ErrorCallback;
 
+  // Interface for setting up services for testing.
+  // Accessed through GetTestInterface(), only implemented in the Stub Impl.
+  class TestInterface {
+   public:
+    virtual void AddService(const std::string& service_path,
+                            const std::string& name,
+                            const std::string& type,
+                            const std::string& state,
+                            const std::string& security) = 0;
+    virtual void RemoveService(const std::string& service_path) = 0;
+    virtual void SetServiceProperty(const std::string& service_path,
+                                    const std::string& property,
+                                    const base::Value& value) = 0;
+    virtual void ClearServices() = 0;
+
+   protected:
+    ~TestInterface() {}
+  };
   virtual ~ShillServiceClient();
 
   // Factory function, creates a new instance which is owned by the caller.
   // For normal usage, access the singleton via DBusThreadManager::Get().
   static ShillServiceClient* Create(DBusClientImplementationType type,
-                                       dbus::Bus* bus);
+                                    dbus::Bus* bus);
 
   // Adds a property changed |observer| to the service at |service_path|.
   virtual void AddPropertyChangedObserver(
@@ -111,6 +129,9 @@ class CHROMEOS_EXPORT ShillServiceClient {
       const dbus::ObjectPath& service_path,
       const std::string& carrier) = 0;
 
+  // Returns an interface for testing (stub only), or returns NULL.
+  virtual TestInterface* GetTestInterface() = 0;
+
  protected:
   // Create() should be used instead.
   ShillServiceClient();
diff --git a/chromeos/network/network_sms_handler.cc b/chromeos/network/network_sms_handler.cc
index 7734d7e..f70314b0 100644
--- a/chromeos/network/network_sms_handler.cc
+++ b/chromeos/network/network_sms_handler.cc
@@ -400,7 +400,8 @@ void NetworkSmsHandler::DevicePropertiesCallback(
     DBusMethodCallStatus call_status,
     const base::DictionaryValue& properties) {
   if (call_status != DBUS_METHOD_CALL_SUCCESS) {
-    LOG(ERROR) << "NetworkSmsHandler: ERROR: " << call_status;
+    LOG(ERROR) << "NetworkSmsHandler: ERROR: " << call_status
+               << " For: " << device_path;
     return;
   }
 
diff --git a/chromeos/network/network_sms_handler_unittest.cc b/chromeos/network/network_sms_handler_unittest.cc
index a201864..a821ce8 100644
--- a/chromeos/network/network_sms_handler_unittest.cc
+++ b/chromeos/network/network_sms_handler_unittest.cc
@@ -12,7 +12,10 @@
 #include "base/message_loop.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/shill_device_client.h"
+#include "chromeos/dbus/shill_manager_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
@@ -54,6 +57,15 @@ class NetworkSmsHandlerTest : public testing::Test {
   virtual void SetUp() OVERRIDE {
     // Initialize DBusThreadManager with a stub implementation.
     DBusThreadManager::InitializeWithStub();
+    ShillManagerClient::TestInterface* manager_test =
+        DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface();
+    ASSERT_TRUE(manager_test);
+    manager_test->AddDevice("stub_cellular_device2");
+    ShillDeviceClient::TestInterface* device_test =
+        DBusThreadManager::Get()->GetShillDeviceClient()->GetTestInterface();
+    ASSERT_TRUE(device_test);
+    device_test->AddDevice("stub_cellular_device2", flimflam::kTypeCellular,
+                           "/org/freedesktop/ModemManager1/stub/0", ":stub.0");
   }
 
   virtual void TearDown() OVERRIDE {
@@ -85,6 +97,7 @@ TEST_F(NetworkSmsHandlerTest, SmsHandlerDbusStub) {
   const std::set<std::string>& messages(test_observer->messages());
   // Note: The following string corresponds to values in
   // ModemMessagingClientStubImpl and SmsClientStubImpl.
+  // TODO(stevenjb): Use a TestInterface to set this up to remove dependency.
   const char kMessage1[] = "SMSClientStubImpl: Test Message: /SMS/0";
   EXPECT_EQ(messages.find(kMessage1), messages.end());