Separate NetworkDeviceHandler interface from its implementation.

This is required for unit testing classes using NetworkDeviceHandler.

While there, adapts the NetworkDeviceHandler unit test to set up its own Shill DBusClients (i.e. no other clients are created and the default Shill stub environment isn't used).

BUG=323537

Review URL: https://codereview.chromium.org/113873002

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

10 files changed, 439 insertions, 312 deletions

diff --git a/chromeos/chromeos.gyp b/chromeos/chromeos.gyp
index 3d74363..2279bb4 100644
--- a/chromeos/chromeos.gyp
+++ b/chromeos/chromeos.gyp
@@ -265,6 +265,8 @@
         'network/network_connection_handler.h',
         'network/network_device_handler.cc',
         'network/network_device_handler.h',
+        'network/network_device_handler_impl.cc',
+        'network/network_device_handler_impl.h',
         'network/network_event_log.cc',
         'network/network_event_log.h',
         'network/network_handler.cc',
diff --git a/chromeos/dbus/fake_dbus_thread_manager.cc b/chromeos/dbus/fake_dbus_thread_manager.cc
index 3dfffef..a57a7c2 100644
--- a/chromeos/dbus/fake_dbus_thread_manager.cc
+++ b/chromeos/dbus/fake_dbus_thread_manager.cc
@@ -69,16 +69,8 @@ void FakeDBusThreadManager::SetFakeClients() {
   SetCryptohomeClient(scoped_ptr<CryptohomeClient>(new FakeCryptohomeClient));
   SetDebugDaemonClient(
       scoped_ptr<DebugDaemonClient>(new FakeDebugDaemonClient));
-  SetShillManagerClient(
-      scoped_ptr<ShillManagerClient>(new FakeShillManagerClient));
-  SetShillDeviceClient(
-      scoped_ptr<ShillDeviceClient>(new FakeShillDeviceClient));
-  SetShillIPConfigClient(
-      scoped_ptr<ShillIPConfigClient>(new FakeShillIPConfigClient));
-  SetShillServiceClient(
-      scoped_ptr<ShillServiceClient>(new FakeShillServiceClient));
-  SetShillProfileClient(
-      scoped_ptr<ShillProfileClient>(new FakeShillProfileClient));
+
+  SetFakeShillClients();
 
   FakeGsmSMSClient* gsm_sms_client = new FakeGsmSMSClient();
   gsm_sms_client->set_sms_test_message_switch_present(
@@ -112,6 +104,19 @@ void FakeDBusThreadManager::SetFakeClients() {
   SetPowerPolicyController(make_scoped_ptr(new PowerPolicyController));
 }
 
+void FakeDBusThreadManager::SetFakeShillClients() {
+  SetShillManagerClient(
+      scoped_ptr<ShillManagerClient>(new FakeShillManagerClient));
+  SetShillDeviceClient(
+      scoped_ptr<ShillDeviceClient>(new FakeShillDeviceClient));
+  SetShillIPConfigClient(
+      scoped_ptr<ShillIPConfigClient>(new FakeShillIPConfigClient));
+  SetShillServiceClient(
+      scoped_ptr<ShillServiceClient>(new FakeShillServiceClient));
+  SetShillProfileClient(
+      scoped_ptr<ShillProfileClient>(new FakeShillProfileClient));
+}
+
 void FakeDBusThreadManager::SetBluetoothAdapterClient(
     scoped_ptr<BluetoothAdapterClient> client) {
   bluetooth_adapter_client_ = client.Pass();
diff --git a/chromeos/dbus/fake_dbus_thread_manager.h b/chromeos/dbus/fake_dbus_thread_manager.h
index ace5690a..b541ffb 100644
--- a/chromeos/dbus/fake_dbus_thread_manager.h
+++ b/chromeos/dbus/fake_dbus_thread_manager.h
@@ -31,6 +31,9 @@ class CHROMEOS_EXPORT FakeDBusThreadManager : public DBusThreadManager {
   // Creates and sets all fake DBusClients and the PowerPolicyController.
   void SetFakeClients();
 
+  // Creates and sets all fake Shill DBusClients.
+  void SetFakeShillClients();
+
   void SetBluetoothAdapterClient(scoped_ptr<BluetoothAdapterClient> client);
   void SetBluetoothAgentManagerClient(
       scoped_ptr<BluetoothAgentManagerClient> client);
diff --git a/chromeos/network/network_device_handler.cc b/chromeos/network/network_device_handler.cc
index 4814d85..813cf661 100644
--- a/chromeos/network/network_device_handler.cc
+++ b/chromeos/network/network_device_handler.cc
@@ -4,122 +4,8 @@
 
 #include "chromeos/network/network_device_handler.h"
 
-#include "base/bind.h"
-#include "base/values.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/shill_device_client.h"
-#include "chromeos/dbus/shill_ipconfig_client.h"
-#include "chromeos/network/device_state.h"
-#include "chromeos/network/network_event_log.h"
-#include "dbus/object_path.h"
-#include "third_party/cros_system_api/dbus/service_constants.h"
-
 namespace chromeos {
 
-namespace {
-
-// TODO(armansito): Add bindings for these to service_constants.h
-// (crbug.com/256889)
-const char kShillErrorFailure[] = "org.chromium.flimflam.Error.Failure";
-const char kShillErrorNotSupported[] =
-    "org.chromium.flimflam.Error.NotSupported";
-
-std::string GetErrorNameForShillError(const std::string& shill_error_name) {
-  // TODO(armansito): Use the new SIM error names once the ones below get
-  // deprecated (crbug.com/256855)
-  if (shill_error_name == kShillErrorFailure)
-    return NetworkDeviceHandler::kErrorFailure;
-  if (shill_error_name == kShillErrorNotSupported)
-    return NetworkDeviceHandler::kErrorNotSupported;
-  if (shill_error_name == shill::kErrorIncorrectPinMsg)
-    return NetworkDeviceHandler::kErrorIncorrectPin;
-  if (shill_error_name == shill::kErrorPinBlockedMsg)
-    return NetworkDeviceHandler::kErrorPinBlocked;
-  if (shill_error_name == shill::kErrorPinRequiredMsg)
-    return NetworkDeviceHandler::kErrorPinRequired;
-  return NetworkDeviceHandler::kErrorUnknown;
-}
-
-void HandleShillCallFailure(
-    const std::string& device_path,
-    const network_handler::ErrorCallback& error_callback,
-    const std::string& shill_error_name,
-    const std::string& shill_error_message) {
-  network_handler::ShillErrorCallbackFunction(
-      GetErrorNameForShillError(shill_error_name),
-      device_path,
-      error_callback,
-      shill_error_name,
-      shill_error_message);
-}
-
-void IPConfigRefreshCallback(const std::string& ipconfig_path,
-                             DBusMethodCallStatus call_status) {
-  if (call_status != DBUS_METHOD_CALL_SUCCESS) {
-    NET_LOG_ERROR(
-        base::StringPrintf("IPConfigs.Refresh Failed: %d", call_status),
-        ipconfig_path);
-  } else {
-    NET_LOG_EVENT("IPConfigs.Refresh Succeeded", ipconfig_path);
-  }
-}
-
-void RefreshIPConfigsCallback(
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback,
-    const std::string& device_path,
-    const base::DictionaryValue& properties) {
-  const ListValue* ip_configs;
-  if (!properties.GetListWithoutPathExpansion(
-          shill::kIPConfigsProperty, &ip_configs)) {
-    NET_LOG_ERROR("RequestRefreshIPConfigs Failed", device_path);
-    network_handler::ShillErrorCallbackFunction(
-        "RequestRefreshIPConfigs Failed",
-        device_path,
-        error_callback,
-        std::string("Missing ") + shill::kIPConfigsProperty, "");
-    return;
-  }
-
-  for (size_t i = 0; i < ip_configs->GetSize(); i++) {
-    std::string ipconfig_path;
-    if (!ip_configs->GetString(i, &ipconfig_path))
-      continue;
-    DBusThreadManager::Get()->GetShillIPConfigClient()->Refresh(
-        dbus::ObjectPath(ipconfig_path),
-        base::Bind(&IPConfigRefreshCallback, ipconfig_path));
-  }
-  // It is safe to invoke |callback| here instead of waiting for the
-  // IPConfig.Refresh callbacks to complete because the Refresh DBus calls will
-  // be executed in order and thus before any further DBus requests that
-  // |callback| may issue.
-  if (!callback.is_null())
-    callback.Run();
-}
-
-void ProposeScanCallback(
-    const std::string& device_path,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback,
-    DBusMethodCallStatus call_status) {
-  if (call_status != DBUS_METHOD_CALL_SUCCESS) {
-    NET_LOG_ERROR(
-        base::StringPrintf("Device.ProposeScan failed: %d", call_status),
-        device_path);
-    network_handler::ShillErrorCallbackFunction(
-        "Device.ProposeScan Failed",
-        device_path,
-        error_callback,
-        base::StringPrintf("DBus call failed: %d", call_status), "");
-    return;
-  }
-  NET_LOG_EVENT("Device.ProposeScan succeeded.", device_path);
-  if (!callback.is_null())
-    callback.Run();
-}
-
-}  // namespace
-
 const char NetworkDeviceHandler::kErrorFailure[] = "failure";
 const char NetworkDeviceHandler::kErrorIncorrectPin[] = "incorrect-pin";
 const char NetworkDeviceHandler::kErrorNotSupported[] = "not-supported";
@@ -127,131 +13,10 @@ const char NetworkDeviceHandler::kErrorPinBlocked[] = "pin-blocked";
 const char NetworkDeviceHandler::kErrorPinRequired[] = "pin-required";
 const char NetworkDeviceHandler::kErrorUnknown[] = "unknown";
 
-NetworkDeviceHandler::~NetworkDeviceHandler() {
-}
-
-void NetworkDeviceHandler::GetDeviceProperties(
-    const std::string& device_path,
-    const network_handler::DictionaryResultCallback& callback,
-    const network_handler::ErrorCallback& error_callback) const {
-  DBusThreadManager::Get()->GetShillDeviceClient()->GetProperties(
-      dbus::ObjectPath(device_path),
-      base::Bind(&network_handler::GetPropertiesCallback,
-                 callback, error_callback, device_path));
-}
-
-void NetworkDeviceHandler::SetDeviceProperty(
-    const std::string& device_path,
-    const std::string& name,
-    const base::Value& value,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) {
-  DBusThreadManager::Get()->GetShillDeviceClient()->SetProperty(
-      dbus::ObjectPath(device_path),
-      name,
-      value,
-      callback,
-      base::Bind(&HandleShillCallFailure, device_path, error_callback));
-}
-
-void NetworkDeviceHandler::RequestRefreshIPConfigs(
-    const std::string& device_path,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) {
-  GetDeviceProperties(device_path,
-                      base::Bind(&RefreshIPConfigsCallback,
-                                 callback, error_callback),
-                      error_callback);
-}
-
-void NetworkDeviceHandler::ProposeScan(
-    const std::string& device_path,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) {
-  DBusThreadManager::Get()->GetShillDeviceClient()->ProposeScan(
-      dbus::ObjectPath(device_path),
-      base::Bind(&ProposeScanCallback, device_path, callback, error_callback));
-}
-
-void NetworkDeviceHandler::RegisterCellularNetwork(
-    const std::string& device_path,
-    const std::string& network_id,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) {
-  DBusThreadManager::Get()->GetShillDeviceClient()->Register(
-      dbus::ObjectPath(device_path),
-      network_id,
-      callback,
-      base::Bind(&HandleShillCallFailure, device_path, error_callback));
-}
-
-void NetworkDeviceHandler::SetCarrier(
-    const std::string& device_path,
-    const std::string& carrier,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) {
-  DBusThreadManager::Get()->GetShillDeviceClient()->SetCarrier(
-      dbus::ObjectPath(device_path),
-      carrier,
-      callback,
-      base::Bind(&HandleShillCallFailure, device_path, error_callback));
-}
-
-void NetworkDeviceHandler::RequirePin(
-    const std::string& device_path,
-    bool require_pin,
-    const std::string& pin,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) {
-  DBusThreadManager::Get()->GetShillDeviceClient()->RequirePin(
-      dbus::ObjectPath(device_path),
-      pin,
-      require_pin,
-      callback,
-      base::Bind(&HandleShillCallFailure, device_path, error_callback));
-}
-
-void NetworkDeviceHandler::EnterPin(
-    const std::string& device_path,
-    const std::string& pin,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) {
-  DBusThreadManager::Get()->GetShillDeviceClient()->EnterPin(
-      dbus::ObjectPath(device_path),
-      pin,
-      callback,
-      base::Bind(&HandleShillCallFailure, device_path, error_callback));
-}
-
-void NetworkDeviceHandler::UnblockPin(
-    const std::string& device_path,
-    const std::string& puk,
-    const std::string& new_pin,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) {
-  DBusThreadManager::Get()->GetShillDeviceClient()->UnblockPin(
-      dbus::ObjectPath(device_path),
-      puk,
-      new_pin,
-      callback,
-      base::Bind(&HandleShillCallFailure, device_path, error_callback));
-}
-
-void NetworkDeviceHandler::ChangePin(
-    const std::string& device_path,
-    const std::string& old_pin,
-    const std::string& new_pin,
-    const base::Closure& callback,
-    const network_handler::ErrorCallback& error_callback) {
-  DBusThreadManager::Get()->GetShillDeviceClient()->ChangePin(
-      dbus::ObjectPath(device_path),
-      old_pin,
-      new_pin,
-      callback,
-      base::Bind(&HandleShillCallFailure, device_path, error_callback));
+NetworkDeviceHandler::NetworkDeviceHandler() {
 }
 
-NetworkDeviceHandler::NetworkDeviceHandler() {
+NetworkDeviceHandler::~NetworkDeviceHandler() {
 }
 
 }  // namespace chromeos
diff --git a/chromeos/network/network_device_handler.h b/chromeos/network/network_device_handler.h
index d675cfa..fe454f4 100644
--- a/chromeos/network/network_device_handler.h
+++ b/chromeos/network/network_device_handler.h
@@ -8,16 +8,12 @@
 #include <string>
 
 #include "base/callback.h"
-#include "base/memory/weak_ptr.h"
 #include "chromeos/chromeos_export.h"
-#include "chromeos/network/network_handler.h"
 #include "chromeos/network/network_handler_callbacks.h"
 
 namespace base {
-
 class Value;
-
-}  // namespace base
+}
 
 namespace chromeos {
 
@@ -29,16 +25,13 @@ namespace chromeos {
 // basically a singleton, but with explicit lifetime management.
 //
 // Note on callbacks: Because all the functions here are meant to be
-// asynchronous, they all take a |callback| of some type, and an
-// |error_callback|. When the operation succeeds, |callback| will be called, and
-// when it doesn't, |error_callback| will be called with information about the
-// error, including a symbolic name for the error and often some error message
-// that is suitable for logging. None of the error message text is meant for
-// user consumption.
-
+// asynchronous, they take a |callback| of some type, and an |error_callback|.
+// When the operation succeeds, |callback| will be called, and when it doesn't,
+// |error_callback| will be called with information about the error, including a
+// symbolic name for the error and often some error message that is suitable for
+// logging. None of the error message text is meant for user consumption.
 class CHROMEOS_EXPORT NetworkDeviceHandler {
  public:
-
   // Constants for |error_name| from |error_callback|.
   static const char kErrorFailure[];
   static const char kErrorIncorrectPin[];
@@ -48,31 +41,32 @@ class CHROMEOS_EXPORT NetworkDeviceHandler {
   static const char kErrorPinRequired[];
   static const char kErrorUnknown[];
 
+  NetworkDeviceHandler();
   virtual ~NetworkDeviceHandler();
 
   // Gets the properties of the device with id |device_path|. See note on
   // |callback| and |error_callback|, in class description above.
-  void GetDeviceProperties(
+  virtual void GetDeviceProperties(
       const std::string& device_path,
       const network_handler::DictionaryResultCallback& callback,
-      const network_handler::ErrorCallback& error_callback) const;
+      const network_handler::ErrorCallback& error_callback) const = 0;
 
   // Sets the value of property |name| on device with id |device_path| to
   // |value|.
-  void SetDeviceProperty(
+  virtual void SetDeviceProperty(
       const std::string& device_path,
       const std::string& name,
       const base::Value& value,
       const base::Closure& callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
   // Requests a refresh of the IP configuration for the device specified by
   // |device_path| if it exists. This will apply any newly configured
   // properties and renew the DHCP lease.
-  void RequestRefreshIPConfigs(
+  virtual void RequestRefreshIPConfigs(
       const std::string& device_path,
       const base::Closure& callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
   // Requests a network scan on the device specified by |device_path|.
   // For cellular networks, the result of this call gets asynchronously stored
@@ -84,10 +78,10 @@ class CHROMEOS_EXPORT NetworkDeviceHandler {
   // of requesting a network scan is Manager.RequestScan, however shill
   // currently doesn't support cellular network scans via Manager.RequestScan.
   // Remove this method once shill supports it (crbug.com/262356).
-  void ProposeScan(
+  virtual void ProposeScan(
       const std::string& device_path,
       const base::Closure& callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
   // Tells the device specified by |device_path| to register to the cellular
   // network with id |network_id|. If |network_id| is empty then registration
@@ -95,11 +89,11 @@ class CHROMEOS_EXPORT NetworkDeviceHandler {
   // with the home network.
   // This call is only available on cellular devices and will fail with
   // Error.NotSupported on all other technologies.
-  void RegisterCellularNetwork(
+  virtual void RegisterCellularNetwork(
       const std::string& device_path,
       const std::string& network_id,
       const base::Closure& callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
   // Tells the device to set the modem carrier firmware, as specified by
   // |carrier|.
@@ -110,11 +104,11 @@ class CHROMEOS_EXPORT NetworkDeviceHandler {
   //    - |carrier| doesn't match one of the supported carriers, as reported by
   //    - Shill.
   //    - Operation is not supported by the device.
-  void SetCarrier(
+  virtual void SetCarrier(
       const std::string& device_path,
       const std::string& carrier,
       const base::Closure& callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
   // SIM PIN/PUK methods
 
@@ -133,12 +127,12 @@ class CHROMEOS_EXPORT NetworkDeviceHandler {
   //
   // This method applies to Cellular devices only. The call will fail with a
   // "not-supported" error if called on a non-cellular device.
-  void RequirePin(
+  virtual void RequirePin(
       const std::string& device_path,
       bool require_pin,
       const std::string& pin,
       const base::Closure& callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
   // Sends the PIN code |pin| to the device |device_path|.
   //
@@ -150,11 +144,11 @@ class CHROMEOS_EXPORT NetworkDeviceHandler {
   //
   // This method applies to Cellular devices only. The call will fail with a
   // "not-supported" error if called on a non-cellular device.
-  void EnterPin(
+  virtual void EnterPin(
       const std::string& device_path,
       const std::string& pin,
       const base::Closure& callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
   // Sends the PUK code |puk| to the SIM to unblock a blocked SIM. On success,
   // the SIM will be unblocked and its PIN code will be set to |pin|.
@@ -166,12 +160,12 @@ class CHROMEOS_EXPORT NetworkDeviceHandler {
   //
   // This method applies to Cellular devices only. The call will fail with a
   // "not-supported" error if called on a non-cellular device.
-  void UnblockPin(
+  virtual void UnblockPin(
       const std::string& device_path,
       const std::string& puk,
       const std::string& new_pin,
       const base::Closure& callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
   // Tells the device to change the PIN code used to unlock a locked SIM card.
   //
@@ -184,19 +178,15 @@ class CHROMEOS_EXPORT NetworkDeviceHandler {
   //
   // This method applies to Cellular devices only. The call will fail with a
   // "not-supported" error if called on a non-cellular device.
-  void ChangePin(
+  virtual void ChangePin(
       const std::string& device_path,
       const std::string& old_pin,
       const std::string& new_pin,
       const base::Closure& callback,
-      const network_handler::ErrorCallback& error_callback);
+      const network_handler::ErrorCallback& error_callback) = 0;
 
- private:
-  friend class NetworkHandler;
-  friend class NetworkDeviceHandlerTest;
-
-  NetworkDeviceHandler();
 
+ private:
   DISALLOW_COPY_AND_ASSIGN(NetworkDeviceHandler);
 };
 
diff --git a/chromeos/network/network_device_handler_impl.cc b/chromeos/network/network_device_handler_impl.cc
new file mode 100644
index 0000000..206fddf
--- /dev/null
+++ b/chromeos/network/network_device_handler_impl.cc
@@ -0,0 +1,248 @@
+// Copyright 2013 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/network/network_device_handler_impl.h"
+
+#include "base/bind.h"
+#include "base/values.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/shill_device_client.h"
+#include "chromeos/dbus/shill_ipconfig_client.h"
+#include "chromeos/network/device_state.h"
+#include "chromeos/network/network_event_log.h"
+#include "dbus/object_path.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace chromeos {
+
+namespace {
+
+// TODO(armansito): Add bindings for these to service_constants.h
+// (crbug.com/256889)
+const char kShillErrorFailure[] = "org.chromium.flimflam.Error.Failure";
+const char kShillErrorNotSupported[] =
+    "org.chromium.flimflam.Error.NotSupported";
+
+std::string GetErrorNameForShillError(const std::string& shill_error_name) {
+  // TODO(armansito): Use the new SIM error names once the ones below get
+  // deprecated (crbug.com/256855)
+  if (shill_error_name == kShillErrorFailure)
+    return NetworkDeviceHandler::kErrorFailure;
+  if (shill_error_name == kShillErrorNotSupported)
+    return NetworkDeviceHandler::kErrorNotSupported;
+  if (shill_error_name == shill::kErrorIncorrectPinMsg)
+    return NetworkDeviceHandler::kErrorIncorrectPin;
+  if (shill_error_name == shill::kErrorPinBlockedMsg)
+    return NetworkDeviceHandler::kErrorPinBlocked;
+  if (shill_error_name == shill::kErrorPinRequiredMsg)
+    return NetworkDeviceHandler::kErrorPinRequired;
+  return NetworkDeviceHandler::kErrorUnknown;
+}
+
+void HandleShillCallFailure(
+    const std::string& device_path,
+    const network_handler::ErrorCallback& error_callback,
+    const std::string& shill_error_name,
+    const std::string& shill_error_message) {
+  network_handler::ShillErrorCallbackFunction(
+      GetErrorNameForShillError(shill_error_name),
+      device_path,
+      error_callback,
+      shill_error_name,
+      shill_error_message);
+}
+
+void IPConfigRefreshCallback(const std::string& ipconfig_path,
+                             DBusMethodCallStatus call_status) {
+  if (call_status != DBUS_METHOD_CALL_SUCCESS) {
+    NET_LOG_ERROR(
+        base::StringPrintf("IPConfigs.Refresh Failed: %d", call_status),
+        ipconfig_path);
+  } else {
+    NET_LOG_EVENT("IPConfigs.Refresh Succeeded", ipconfig_path);
+  }
+}
+
+void RefreshIPConfigsCallback(
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback,
+    const std::string& device_path,
+    const base::DictionaryValue& properties) {
+  const ListValue* ip_configs;
+  if (!properties.GetListWithoutPathExpansion(
+          shill::kIPConfigsProperty, &ip_configs)) {
+    NET_LOG_ERROR("RequestRefreshIPConfigs Failed", device_path);
+    network_handler::ShillErrorCallbackFunction(
+        "RequestRefreshIPConfigs Failed",
+        device_path,
+        error_callback,
+        std::string("Missing ") + shill::kIPConfigsProperty, "");
+    return;
+  }
+
+  for (size_t i = 0; i < ip_configs->GetSize(); i++) {
+    std::string ipconfig_path;
+    if (!ip_configs->GetString(i, &ipconfig_path))
+      continue;
+    DBusThreadManager::Get()->GetShillIPConfigClient()->Refresh(
+        dbus::ObjectPath(ipconfig_path),
+        base::Bind(&IPConfigRefreshCallback, ipconfig_path));
+  }
+  // It is safe to invoke |callback| here instead of waiting for the
+  // IPConfig.Refresh callbacks to complete because the Refresh DBus calls will
+  // be executed in order and thus before any further DBus requests that
+  // |callback| may issue.
+  if (!callback.is_null())
+    callback.Run();
+}
+
+void ProposeScanCallback(
+    const std::string& device_path,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback,
+    DBusMethodCallStatus call_status) {
+  if (call_status != DBUS_METHOD_CALL_SUCCESS) {
+    NET_LOG_ERROR(
+        base::StringPrintf("Device.ProposeScan failed: %d", call_status),
+        device_path);
+    network_handler::ShillErrorCallbackFunction(
+        "Device.ProposeScan Failed",
+        device_path,
+        error_callback,
+        base::StringPrintf("DBus call failed: %d", call_status), "");
+    return;
+  }
+  NET_LOG_EVENT("Device.ProposeScan succeeded.", device_path);
+  if (!callback.is_null())
+    callback.Run();
+}
+
+}  // namespace
+
+NetworkDeviceHandlerImpl::~NetworkDeviceHandlerImpl() {}
+
+void NetworkDeviceHandlerImpl::GetDeviceProperties(
+    const std::string& device_path,
+    const network_handler::DictionaryResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback) const {
+  DBusThreadManager::Get()->GetShillDeviceClient()->GetProperties(
+      dbus::ObjectPath(device_path),
+      base::Bind(&network_handler::GetPropertiesCallback,
+                 callback, error_callback, device_path));
+}
+
+void NetworkDeviceHandlerImpl::SetDeviceProperty(
+    const std::string& device_path,
+    const std::string& name,
+    const base::Value& value,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  DBusThreadManager::Get()->GetShillDeviceClient()->SetProperty(
+      dbus::ObjectPath(device_path),
+      name,
+      value,
+      callback,
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
+}
+
+void NetworkDeviceHandlerImpl::RequestRefreshIPConfigs(
+    const std::string& device_path,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  GetDeviceProperties(device_path,
+                      base::Bind(&RefreshIPConfigsCallback,
+                                 callback, error_callback),
+                      error_callback);
+}
+
+void NetworkDeviceHandlerImpl::ProposeScan(
+    const std::string& device_path,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  DBusThreadManager::Get()->GetShillDeviceClient()->ProposeScan(
+      dbus::ObjectPath(device_path),
+      base::Bind(&ProposeScanCallback, device_path, callback, error_callback));
+}
+
+void NetworkDeviceHandlerImpl::RegisterCellularNetwork(
+    const std::string& device_path,
+    const std::string& network_id,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  DBusThreadManager::Get()->GetShillDeviceClient()->Register(
+      dbus::ObjectPath(device_path),
+      network_id,
+      callback,
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
+}
+
+void NetworkDeviceHandlerImpl::SetCarrier(
+    const std::string& device_path,
+    const std::string& carrier,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  DBusThreadManager::Get()->GetShillDeviceClient()->SetCarrier(
+      dbus::ObjectPath(device_path),
+      carrier,
+      callback,
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
+}
+
+void NetworkDeviceHandlerImpl::RequirePin(
+    const std::string& device_path,
+    bool require_pin,
+    const std::string& pin,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  DBusThreadManager::Get()->GetShillDeviceClient()->RequirePin(
+      dbus::ObjectPath(device_path),
+      pin,
+      require_pin,
+      callback,
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
+}
+
+void NetworkDeviceHandlerImpl::EnterPin(
+    const std::string& device_path,
+    const std::string& pin,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  DBusThreadManager::Get()->GetShillDeviceClient()->EnterPin(
+      dbus::ObjectPath(device_path),
+      pin,
+      callback,
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
+}
+
+void NetworkDeviceHandlerImpl::UnblockPin(
+    const std::string& device_path,
+    const std::string& puk,
+    const std::string& new_pin,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  DBusThreadManager::Get()->GetShillDeviceClient()->UnblockPin(
+      dbus::ObjectPath(device_path),
+      puk,
+      new_pin,
+      callback,
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
+}
+
+void NetworkDeviceHandlerImpl::ChangePin(
+    const std::string& device_path,
+    const std::string& old_pin,
+    const std::string& new_pin,
+    const base::Closure& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  DBusThreadManager::Get()->GetShillDeviceClient()->ChangePin(
+      dbus::ObjectPath(device_path),
+      old_pin,
+      new_pin,
+      callback,
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
+}
+
+NetworkDeviceHandlerImpl::NetworkDeviceHandlerImpl() {}
+
+}  // namespace chromeos
diff --git a/chromeos/network/network_device_handler_impl.h b/chromeos/network/network_device_handler_impl.h
new file mode 100644
index 0000000..358b64b
--- /dev/null
+++ b/chromeos/network/network_device_handler_impl.h
@@ -0,0 +1,102 @@
+// Copyright 2013 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.
+
+#ifndef CHROMEOS_NETWORK_NETWORK_DEVICE_HANDLER_IMPL_H_
+#define CHROMEOS_NETWORK_NETWORK_DEVICE_HANDLER_IMPL_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/compiler_specific.h"
+#include "chromeos/chromeos_export.h"
+#include "chromeos/network/network_device_handler.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_handler_callbacks.h"
+#include "chromeos/network/network_state_handler_observer.h"
+
+namespace chromeos {
+
+class NetworkStateHandler;
+
+class CHROMEOS_EXPORT NetworkDeviceHandlerImpl
+    : public NetworkDeviceHandler,
+      public NetworkStateHandlerObserver {
+ public:
+  virtual ~NetworkDeviceHandlerImpl();
+
+  // NetworkDeviceHandler overrides
+  virtual void GetDeviceProperties(
+      const std::string& device_path,
+      const network_handler::DictionaryResultCallback& callback,
+      const network_handler::ErrorCallback& error_callback) const OVERRIDE;
+
+  virtual void SetDeviceProperty(
+      const std::string& device_path,
+      const std::string& name,
+      const base::Value& value,
+      const base::Closure& callback,
+      const network_handler::ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void RequestRefreshIPConfigs(
+      const std::string& device_path,
+      const base::Closure& callback,
+      const network_handler::ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void ProposeScan(const std::string& device_path,
+                           const base::Closure& callback,
+                           const network_handler::ErrorCallback& error_callback)
+      OVERRIDE;
+
+  virtual void RegisterCellularNetwork(
+      const std::string& device_path,
+      const std::string& network_id,
+      const base::Closure& callback,
+      const network_handler::ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void SetCarrier(const std::string& device_path,
+                          const std::string& carrier,
+                          const base::Closure& callback,
+                          const network_handler::ErrorCallback& error_callback)
+      OVERRIDE;
+
+  virtual void RequirePin(const std::string& device_path,
+                          bool require_pin,
+                          const std::string& pin,
+                          const base::Closure& callback,
+                          const network_handler::ErrorCallback& error_callback)
+      OVERRIDE;
+
+  virtual void EnterPin(const std::string& device_path,
+                        const std::string& pin,
+                        const base::Closure& callback,
+                        const network_handler::ErrorCallback& error_callback)
+      OVERRIDE;
+
+  virtual void UnblockPin(const std::string& device_path,
+                          const std::string& puk,
+                          const std::string& new_pin,
+                          const base::Closure& callback,
+                          const network_handler::ErrorCallback& error_callback)
+      OVERRIDE;
+
+  virtual void ChangePin(const std::string& device_path,
+                         const std::string& old_pin,
+                         const std::string& new_pin,
+                         const base::Closure& callback,
+                         const network_handler::ErrorCallback& error_callback)
+      OVERRIDE;
+
+ private:
+  friend class NetworkHandler;
+  friend class NetworkDeviceHandlerTest;
+
+  NetworkDeviceHandlerImpl();
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkDeviceHandlerImpl);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_NETWORK_NETWORK_DEVICE_HANDLER_IMPL_H_
diff --git a/chromeos/network/network_device_handler_unittest.cc b/chromeos/network/network_device_handler_unittest.cc
index 372b50c..4185b75 100644
--- a/chromeos/network/network_device_handler_unittest.cc
+++ b/chromeos/network/network_device_handler_unittest.cc
@@ -2,14 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chromeos/network/network_device_handler.h"
-
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/values.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/shill_device_client.h"
+#include "chromeos/dbus/fake_dbus_thread_manager.h"
+#include "chromeos/dbus/fake_shill_device_client.h"
+#include "chromeos/dbus/fake_shill_manager_client.h"
+#include "chromeos/network/network_device_handler_impl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
@@ -26,24 +26,24 @@ const char kResultSuccess[] = "success";
 
 class NetworkDeviceHandlerTest : public testing::Test {
  public:
-  NetworkDeviceHandlerTest() {}
+  NetworkDeviceHandlerTest() : fake_device_client_(NULL) {}
   virtual ~NetworkDeviceHandlerTest() {}
 
   virtual void SetUp() OVERRIDE {
-    DBusThreadManager::InitializeWithStub();
+    FakeDBusThreadManager* dbus_manager = new FakeDBusThreadManager;
+    dbus_manager->SetFakeShillClients();
+
+    fake_device_client_ = new FakeShillDeviceClient;
+    dbus_manager->SetShillDeviceClient(
+        scoped_ptr<ShillDeviceClient>(fake_device_client_));
+    DBusThreadManager::InitializeForTesting(dbus_manager);
 
     ShillDeviceClient::TestInterface* device_test =
-        DBusThreadManager::Get()->GetShillDeviceClient()->GetTestInterface();
-    device_test->ClearDevices();
+        fake_device_client_->GetTestInterface();
     device_test->AddDevice(
         kDefaultCellularDevicePath, shill::kTypeCellular, "cellular1");
     device_test->AddDevice(kDefaultWifiDevicePath, shill::kTypeWifi, "wifi1");
 
-    base::FundamentalValue allow_roaming(false);
-    device_test->SetDeviceProperty(kDefaultCellularDevicePath,
-                                   shill::kCellularAllowRoamingProperty,
-                                   allow_roaming);
-
     base::ListValue test_ip_configs;
     test_ip_configs.AppendString("ip_config1");
     device_test->SetDeviceProperty(
@@ -56,7 +56,8 @@ class NetworkDeviceHandlerTest : public testing::Test {
                    base::Unretained(this));
     error_callback_ = base::Bind(&NetworkDeviceHandlerTest::ErrorCallback,
                                  base::Unretained(this));
-    network_device_handler_.reset(new NetworkDeviceHandler);
+
+    network_device_handler_.reset(new NetworkDeviceHandlerImpl);
   }
 
   virtual void TearDown() OVERRIDE {
@@ -82,6 +83,7 @@ class NetworkDeviceHandlerTest : public testing::Test {
  protected:
   std::string result_;
 
+  FakeShillDeviceClient* fake_device_client_;
   scoped_ptr<NetworkDeviceHandler> network_device_handler_;
   base::MessageLoopForUI message_loop_;
   base::Closure success_callback_;
@@ -104,30 +106,39 @@ TEST_F(NetworkDeviceHandlerTest, GetDeviceProperties) {
 }
 
 TEST_F(NetworkDeviceHandlerTest, SetDeviceProperty) {
-  // Check that GetDeviceProperties returns the expected initial values.
+
+  // Set the shill::kCellularAllowRoamingProperty to true. The call
+  // should succeed and the value should be set.
+  network_device_handler_->SetDeviceProperty(
+      kDefaultCellularDevicePath,
+      shill::kCellularAllowRoamingProperty,
+      base::FundamentalValue(true),
+      success_callback_,
+      error_callback_);
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(kResultSuccess, result_);
+
+  // GetDeviceProperties should return the value set by SetDeviceProperty.
   network_device_handler_->GetDeviceProperties(kDefaultCellularDevicePath,
                                                properties_success_callback_,
                                                error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
-  bool allow_roaming;
+  bool allow_roaming = false;
   EXPECT_TRUE(properties_->GetBooleanWithoutPathExpansion(
       shill::kCellularAllowRoamingProperty, &allow_roaming));
-  EXPECT_FALSE(allow_roaming);
+  EXPECT_TRUE(allow_roaming);
 
-  // Set the shill::kCellularAllowRoamingProperty to true. The call
-  // should succeed and the value should be set.
-  base::FundamentalValue allow_roaming_value(true);
+  // Repeat the same with value false.
   network_device_handler_->SetDeviceProperty(
       kDefaultCellularDevicePath,
       shill::kCellularAllowRoamingProperty,
-      allow_roaming_value,
+      base::FundamentalValue(false),
       success_callback_,
       error_callback_);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kResultSuccess, result_);
 
-  // GetDeviceProperties should return the value set by SetDeviceProperty.
   network_device_handler_->GetDeviceProperties(kDefaultCellularDevicePath,
                                                properties_success_callback_,
                                                error_callback_);
@@ -135,13 +146,13 @@ TEST_F(NetworkDeviceHandlerTest, SetDeviceProperty) {
   EXPECT_EQ(kResultSuccess, result_);
   EXPECT_TRUE(properties_->GetBooleanWithoutPathExpansion(
       shill::kCellularAllowRoamingProperty, &allow_roaming));
-  EXPECT_TRUE(allow_roaming);
+  EXPECT_FALSE(allow_roaming);
 
   // Set property on an invalid path.
   network_device_handler_->SetDeviceProperty(
       kUnknownCellularDevicePath,
       shill::kCellularAllowRoamingProperty,
-      allow_roaming_value,
+      base::FundamentalValue(true),
       success_callback_,
       error_callback_);
   message_loop_.RunUntilIdle();
diff --git a/chromeos/network/network_handler.cc b/chromeos/network/network_handler.cc
index 15ede66..2875862 100644
--- a/chromeos/network/network_handler.cc
+++ b/chromeos/network/network_handler.cc
@@ -13,7 +13,7 @@
 #include "chromeos/network/network_cert_migrator.h"
 #include "chromeos/network/network_configuration_handler.h"
 #include "chromeos/network/network_connection_handler.h"
-#include "chromeos/network/network_device_handler.h"
+#include "chromeos/network/network_device_handler_impl.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_profile_handler.h"
 #include "chromeos/network/network_profile_observer.h"
@@ -32,7 +32,7 @@ NetworkHandler::NetworkHandler()
   network_event_log::Initialize();
 
   network_state_handler_.reset(new NetworkStateHandler());
-  network_device_handler_.reset(new NetworkDeviceHandler());
+  network_device_handler_.reset(new NetworkDeviceHandlerImpl());
   network_profile_handler_.reset(new NetworkProfileHandler());
   network_configuration_handler_.reset(new NetworkConfigurationHandler());
   managed_network_configuration_handler_.reset(
diff --git a/chromeos/network/network_handler.h b/chromeos/network/network_handler.h
index bbb9e9c..6949d70 100644
--- a/chromeos/network/network_handler.h
+++ b/chromeos/network/network_handler.h
@@ -22,6 +22,7 @@ class NetworkCertMigrator;
 class NetworkConfigurationHandler;
 class NetworkConnectionHandler;
 class NetworkDeviceHandler;
+class NetworkDeviceHandlerImpl;
 class NetworkProfileHandler;
 class NetworkStateHandler;
 class NetworkSmsHandler;
@@ -69,7 +70,7 @@ class CHROMEOS_EXPORT NetworkHandler {
   // The order of these determines the (inverse) destruction order.
   scoped_refptr<base::MessageLoopProxy> message_loop_;
   scoped_ptr<NetworkStateHandler> network_state_handler_;
-  scoped_ptr<NetworkDeviceHandler> network_device_handler_;
+  scoped_ptr<NetworkDeviceHandlerImpl> network_device_handler_;
   scoped_ptr<NetworkProfileHandler> network_profile_handler_;
   scoped_ptr<NetworkConfigurationHandler> network_configuration_handler_;
   scoped_ptr<ManagedNetworkConfigurationHandlerImpl>