Add network_handler_callbacks.cc

This does some cleanup to allow NetworkStateHandler to use the same error callback infrastructure as NetworkConfigurationHandler.
It also adds event logging for errors.

BUG=none

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

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

9 files changed, 193 insertions, 121 deletions

diff --git a/chromeos/network/network_configuration_handler.cc b/chromeos/network/network_configuration_handler.cc
index 8fb6832..56ee934 100644
--- a/chromeos/network/network_configuration_handler.cc
+++ b/chromeos/network/network_configuration_handler.cc
@@ -22,6 +22,8 @@ namespace chromeos {
 
 namespace {
 
+const char kLogModule[] = "NetworkConfigurationHandler";
+
 // None of these error messages are user-facing: they should only appear in
 // logs.
 const char kErrorsListTag[] = "errors";
@@ -30,31 +32,11 @@ const char kClearPropertiesFailedErrorMessage[] = "Clear properties failed";
 const char kDBusFailedError[] = "Error.DBusFailed";
 const char kDBusFailedErrorMessage[] = "DBus call failed.";
 
-// These are names of fields in the error data returned through the error
-// callbacks.
-const char kErrorName[] = "errorName";
-const char kErrorMessage[] = "errorMessage";
-const char kServicePath[] = "servicePath";
-
-base::DictionaryValue* CreateErrorData(const std::string& service_path,
-                                       const std::string& error_name,
-                                       const std::string& error_message) {
-  scoped_ptr<base::DictionaryValue> error_data(new base::DictionaryValue);
-  error_data->SetString(kErrorName, error_name);
-  error_data->SetString(kErrorMessage, error_message);
-  if (!service_path.empty())
-    error_data->SetString(kServicePath, service_path);
-  LOG(ERROR) << "NetworkConfigurationHandler Received an error("
-             << error_name << ") for service path '" << service_path << "':"
-             << error_message;
-  return error_data.release();
-}
-
 void ClearPropertiesCallback(
     const std::vector<std::string>& names,
     const std::string& service_path,
     const base::Closure& callback,
-    const NetworkHandlerErrorCallback& error_callback,
+    const network_handler::ErrorCallback& error_callback,
     const base::ListValue& result) {
   bool some_failed = false;
   for (size_t i = 0; i < result.GetSize(); ++i) {
@@ -73,9 +55,11 @@ void ClearPropertiesCallback(
     DCHECK(names.size() == result.GetSize())
         << "Result wrong size from ClearProperties.";
     scoped_ptr<base::DictionaryValue> error_data(
-        CreateErrorData(service_path,
-                        kClearPropertiesFailedError,
-                        kClearPropertiesFailedErrorMessage));
+        network_handler::CreateErrorData(service_path,
+                                         kClearPropertiesFailedError,
+                                         kClearPropertiesFailedErrorMessage));
+    LOG(ERROR) << "ClearPropertiesCallback Failed for service path: "
+               << service_path;
     error_data->Set("errors", result.DeepCopy());
     scoped_ptr<base::ListValue> name_list(new base::ListValue);
     name_list->AppendStrings(names);
@@ -86,47 +70,29 @@ void ClearPropertiesCallback(
   }
 }
 
-void ClearPropertiesErrorCallback(
-    const std::string& service_path,
-    const NetworkHandlerErrorCallback& error_callback,
-    const std::string& error_name,
-    const std::string& error_message) {
-  // Add a new error.
-  scoped_ptr<base::DictionaryValue> error_data(
-      CreateErrorData(service_path, error_name, error_message));
-  error_callback.Run(kClearPropertiesFailedError, error_data.Pass());
-}
-
 // Used to translate the dbus dictionary callback into one that calls
 // the error callback if we have a failure.
 void RunCallbackWithDictionaryValue(
-    const NetworkHandlerDictionaryResultCallback& callback,
-    const NetworkHandlerErrorCallback& error_callback,
+    const network_handler::DictionaryResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback,
     const std::string& service_path,
     DBusMethodCallStatus call_status,
     const base::DictionaryValue& value) {
   if (call_status != DBUS_METHOD_CALL_SUCCESS) {
     scoped_ptr<base::DictionaryValue> error_data(
-        CreateErrorData(service_path,
-                        kDBusFailedError,
-                        kDBusFailedErrorMessage));
+        network_handler::CreateErrorData(service_path,
+                                         kDBusFailedError,
+                                         kDBusFailedErrorMessage));
+    LOG(ERROR) << "CallbackWithDictionaryValue Failed for service path: "
+               << service_path;
     error_callback.Run(kDBusFailedError, error_data.Pass());
   } else {
     callback.Run(service_path, value);
   }
 }
 
-void RunErrorCallback(const std::string& service_path,
-                      const NetworkHandlerErrorCallback& error_callback,
-                      const std::string& error_name,
-                      const std::string& error_message) {
-  scoped_ptr<base::DictionaryValue> error_dict(
-      CreateErrorData(service_path, error_name, error_message));
-  error_callback.Run(error_name, error_dict.Pass());
-}
-
 void RunCreateNetworkCallback(
-    const NetworkHandlerStringResultCallback& callback,
+    const network_handler::StringResultCallback& callback,
     const dbus::ObjectPath& service_path) {
   callback.Run(service_path.value());
 }
@@ -141,8 +107,8 @@ NetworkConfigurationHandler::~NetworkConfigurationHandler() {
 
 void NetworkConfigurationHandler::GetProperties(
     const std::string& service_path,
-    const NetworkHandlerDictionaryResultCallback& callback,
-    const NetworkHandlerErrorCallback& error_callback) const {
+    const network_handler::DictionaryResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback) const {
   DBusThreadManager::Get()->GetShillServiceClient()->GetProperties(
       dbus::ObjectPath(service_path),
       base::Bind(&RunCallbackWithDictionaryValue,
@@ -155,18 +121,19 @@ void NetworkConfigurationHandler::SetProperties(
     const std::string& service_path,
     const base::DictionaryValue& properties,
     const base::Closure& callback,
-    const NetworkHandlerErrorCallback& error_callback) const {
+    const network_handler::ErrorCallback& error_callback) const {
   DBusThreadManager::Get()->GetShillManagerClient()->ConfigureService(
       properties,
       callback,
-      base::Bind(&RunErrorCallback, service_path, error_callback));
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 kLogModule, service_path, error_callback));
 }
 
 void NetworkConfigurationHandler::ClearProperties(
     const std::string& service_path,
     const std::vector<std::string>& names,
     const base::Closure& callback,
-    const NetworkHandlerErrorCallback& error_callback) {
+    const network_handler::ErrorCallback& error_callback) {
   DBusThreadManager::Get()->GetShillServiceClient()->ClearProperties(
       dbus::ObjectPath(service_path),
       names,
@@ -175,47 +142,52 @@ void NetworkConfigurationHandler::ClearProperties(
                  service_path,
                  callback,
                  error_callback),
-      base::Bind(&ClearPropertiesErrorCallback, service_path, error_callback));
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 kLogModule, service_path, error_callback));
 }
 
 void NetworkConfigurationHandler::Connect(
     const std::string& service_path,
     const base::Closure& callback,
-    const NetworkHandlerErrorCallback& error_callback) const {
+    const network_handler::ErrorCallback& error_callback) const {
   DBusThreadManager::Get()->GetShillServiceClient()->Connect(
       dbus::ObjectPath(service_path),
       callback,
-      base::Bind(&RunErrorCallback, service_path, error_callback));
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 kLogModule, service_path, error_callback));
 }
 
 void NetworkConfigurationHandler::Disconnect(
     const std::string& service_path,
     const base::Closure& callback,
-    const NetworkHandlerErrorCallback& error_callback) const {
+    const network_handler::ErrorCallback& error_callback) const {
   DBusThreadManager::Get()->GetShillServiceClient()->Disconnect(
       dbus::ObjectPath(service_path),
       callback,
-      base::Bind(&RunErrorCallback, service_path, error_callback));
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 kLogModule, service_path, error_callback));
 }
 
 void NetworkConfigurationHandler::CreateConfiguration(
     const base::DictionaryValue& properties,
-    const NetworkHandlerStringResultCallback& callback,
-    const NetworkHandlerErrorCallback& error_callback) const {
+    const network_handler::StringResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback) const {
   DBusThreadManager::Get()->GetShillManagerClient()->GetService(
       properties,
       base::Bind(&RunCreateNetworkCallback, callback),
-      base::Bind(&RunErrorCallback, "", error_callback));
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 kLogModule, "", error_callback));
 }
 
 void NetworkConfigurationHandler::RemoveConfiguration(
     const std::string& service_path,
     const base::Closure& callback,
-    const NetworkHandlerErrorCallback& error_callback) const {
+    const network_handler::ErrorCallback& error_callback) const {
   DBusThreadManager::Get()->GetShillServiceClient()->Remove(
       dbus::ObjectPath(service_path),
       callback,
-      base::Bind(&RunErrorCallback, service_path, error_callback));
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 kLogModule, service_path, error_callback));
 }
 
 }  // namespace chromeos
diff --git a/chromeos/network/network_configuration_handler.h b/chromeos/network/network_configuration_handler.h
index bac54c5..a92756c 100644
--- a/chromeos/network/network_configuration_handler.h
+++ b/chromeos/network/network_configuration_handler.h
@@ -12,6 +12,7 @@
 #include "base/callback.h"
 #include "base/gtest_prod_util.h"
 #include "chromeos/chromeos_export.h"
+#include "chromeos/network/network_handler_callbacks.h"
 
 namespace base {
 class DictionaryValue;
@@ -19,22 +20,6 @@ class DictionaryValue;
 
 namespace chromeos {
 
-// An error callback used by both the configuration handler and the state
-// handler to receive error results from the API.
-// TODO(gspencer): move to a common header.
-typedef base::Callback<
-    void(const std::string& error_name,
-         const scoped_ptr<base::DictionaryValue> error_data)>
-    NetworkHandlerErrorCallback;
-
-typedef base::Callback<
-    void(const std::string& service_path,
-         const base::DictionaryValue& dictionary)>
-    NetworkHandlerDictionaryResultCallback;
-
-typedef base::Callback<void(const std::string& service_path)>
-    NetworkHandlerStringResultCallback;
-
 // The NetworkConfigurationHandler class is used to create and configure
 // networks in ChromeOS. It mostly calls through to the Shill service API, and
 // most calls are asynchronous for that reason. No calls will block on DBus
@@ -60,18 +45,20 @@ class CHROMEOS_EXPORT NetworkConfigurationHandler {
 
   // Gets the properties of the network with id |service_path|. See note on
   // |callback| and |error_callback|, in class description above.
-  void GetProperties(const std::string& service_path,
-                     const NetworkHandlerDictionaryResultCallback& callback,
-                     const NetworkHandlerErrorCallback& error_callback) const;
+  void GetProperties(
+      const std::string& service_path,
+      const network_handler::DictionaryResultCallback& callback,
+      const network_handler::ErrorCallback& error_callback) const;
 
   // Sets the properties of the network with id |service_path|. This means the
   // given properties will be merged with the existing settings, and it won't
   // clear any existing properties. See note on |callback| and |error_callback|,
   // in class description above.
-  void SetProperties(const std::string& service_path,
-                     const base::DictionaryValue& properties,
-                     const base::Closure& callback,
-                     const NetworkHandlerErrorCallback& error_callback) const;
+  void SetProperties(
+      const std::string& service_path,
+      const base::DictionaryValue& properties,
+      const base::Closure& callback,
+      const network_handler::ErrorCallback& error_callback) const;
 
   // Removes the properties with the given property paths. If any of them are
   // unable to be cleared, the |error_callback| will only be run once with
@@ -82,19 +69,19 @@ class CHROMEOS_EXPORT NetworkConfigurationHandler {
   void ClearProperties(const std::string& service_path,
                        const std::vector<std::string>& property_paths,
                        const base::Closure& callback,
-                       const NetworkHandlerErrorCallback& error_callback);
+                       const network_handler::ErrorCallback& error_callback);
 
   // Initiates a connection with network that has id |service_path|. See note on
   // |callback| and |error_callback|, in class description above.
   void Connect(const std::string& service_path,
                const base::Closure& callback,
-               const NetworkHandlerErrorCallback& error_callback) const;
+               const network_handler::ErrorCallback& error_callback) const;
 
   // Initiates a disconnect with the network at |service_path|. See note on
   // |callback| and |error_callback|, in class description above.
   void Disconnect(const std::string& service_path,
                   const base::Closure& callback,
-                  const NetworkHandlerErrorCallback& error_callback) const;
+                  const network_handler::ErrorCallback& error_callback) const;
 
 
   // Creates a network with the given properties in the active Shill profile,
@@ -103,8 +90,8 @@ class CHROMEOS_EXPORT NetworkConfigurationHandler {
   // description above.
   void CreateConfiguration(
       const base::DictionaryValue& properties,
-      const NetworkHandlerStringResultCallback& callback,
-      const NetworkHandlerErrorCallback& error_callback) const;
+      const network_handler::StringResultCallback& callback,
+      const network_handler::ErrorCallback& error_callback) const;
 
   // Removes the network |service_path| from the remembered network list in the
   // active Shill profile. The network may still show up in the visible networks
@@ -113,7 +100,7 @@ class CHROMEOS_EXPORT NetworkConfigurationHandler {
   void RemoveConfiguration(
       const std::string& service_path,
       const base::Closure& callback,
-      const NetworkHandlerErrorCallback& error_callback) const;
+      const network_handler::ErrorCallback& error_callback) const;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NetworkConfigurationHandler);
diff --git a/chromeos/network/network_handler_callbacks.cc b/chromeos/network/network_handler_callbacks.cc
new file mode 100644
index 0000000..f10a12f
--- /dev/null
+++ b/chromeos/network/network_handler_callbacks.cc
@@ -0,0 +1,51 @@
+// 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/network/network_handler_callbacks.h"
+
+#include "base/logging.h"
+#include "base/values.h"
+#include "chromeos/network/network_event_log.h"
+
+namespace chromeos {
+namespace network_handler {
+
+const char kLogModule[] = "ShillError";
+
+// These are names of fields in the error data dictionary for ErrorCallback.
+const char kErrorName[] = "errorName";
+const char kErrorMessage[] = "errorMessage";
+const char kServicePath[] = "servicePath";
+
+base::DictionaryValue* CreateErrorData(const std::string& service_path,
+                                       const std::string& error_name,
+                                       const std::string& error_message) {
+  base::DictionaryValue* error_data(new base::DictionaryValue);
+  error_data->SetString(kErrorName, error_name);
+  error_data->SetString(kErrorMessage, error_message);
+  if (!service_path.empty())
+    error_data->SetString(kServicePath, service_path);
+  return error_data;
+}
+
+void ShillErrorCallbackFunction(const std::string& module,
+                                const std::string& path,
+                                const ErrorCallback& error_callback,
+                                const std::string& error_name,
+                                const std::string& error_message) {
+  std::string error = "Shill Error in " + module;
+  if (!path.empty())
+    error += " For " + path;
+  error += ": " + error_name + " : " + error_message;
+  LOG(ERROR) << error;
+  network_event_log::AddEntry(kLogModule, module, error);
+  if (error_callback.is_null())
+    return;
+  scoped_ptr<base::DictionaryValue> error_data(
+      CreateErrorData(path, error_name, error_message));
+  error_callback.Run(error_name, error_data.Pass());
+}
+
+} // namespace network_handler
+}  // namespace chromeos
diff --git a/chromeos/network/network_handler_callbacks.h b/chromeos/network/network_handler_callbacks.h
new file mode 100644
index 0000000..6e586b9
--- /dev/null
+++ b/chromeos/network/network_handler_callbacks.h
@@ -0,0 +1,49 @@
+// 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.
+
+#ifndef CHROMEOS_NETWORK_NETWORK_HANDLER_CALLBACKS_H_
+#define CHROMEOS_NETWORK_NETWORK_HANDLER_CALLBACKS_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+
+namespace base {
+class DictionaryValue;
+}
+
+namespace chromeos {
+namespace network_handler {
+
+// An error callback used by both the configuration handler and the state
+// handler to receive error results from the API.
+typedef base::Callback<
+  void(const std::string& error_name,
+       const scoped_ptr<base::DictionaryValue> error_data)> ErrorCallback;
+
+typedef base::Callback<
+  void(const std::string& service_path,
+       const base::DictionaryValue& dictionary)> DictionaryResultCallback;
+
+typedef base::Callback<
+  void(const std::string& service_path)> StringResultCallback;
+
+// Create a DictionaryValue for passing to ErrorCallback
+base::DictionaryValue* CreateErrorData(const std::string& service_path,
+                                       const std::string& error_name,
+                                       const std::string& error_message);
+
+// Callback for Shill errors. |path| may be blank if not relevant.
+// Logs an error and calls |error_callback| if not null.
+void ShillErrorCallbackFunction(const std::string& module,
+                                const std::string& path,
+                                const ErrorCallback& error_callback,
+                                const std::string& error_name,
+                                const std::string& error_message);
+
+}  // namespace network_handler
+}  // namespace chromeos
+
+#endif  // CHROMEOS_NETWORK_NETWORK_HANDLER_CALLBACKS_H_
diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc
index f8d2ef8..8b4ff3c 100644
--- a/chromeos/network/network_state_handler.cc
+++ b/chromeos/network/network_state_handler.cc
@@ -79,9 +79,12 @@ bool NetworkStateHandler::TechnologyEnabled(
   return enabled_technologies_.find(technology) != enabled_technologies_.end();
 }
 
-void NetworkStateHandler::SetTechnologyEnabled(const std::string& technology,
-                                               bool enabled) {
-  shill_property_handler_->SetTechnologyEnabled(technology, enabled);
+void NetworkStateHandler::SetTechnologyEnabled(
+    const std::string& technology,
+    bool enabled,
+    const network_handler::ErrorCallback& error_callback) {
+  shill_property_handler_->SetTechnologyEnabled(
+      technology, enabled, error_callback);
 }
 
 const DeviceState* NetworkStateHandler::GetDeviceState(
diff --git a/chromeos/network/network_state_handler.h b/chromeos/network/network_state_handler.h
index d8c2d3d..d8614d9 100644
--- a/chromeos/network/network_state_handler.h
+++ b/chromeos/network/network_state_handler.h
@@ -15,6 +15,7 @@
 #include "base/observer_list.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/network/managed_state.h"
+#include "chromeos/network/network_handler_callbacks.h"
 #include "chromeos/network/shill_property_handler.h"
 
 namespace base {
@@ -66,8 +67,11 @@ class CHROMEOS_EXPORT NetworkStateHandler
   bool TechnologyEnabled(const std::string& technology) const;
 
   // Asynchronously sets the enabled state for |technology|.
-  // Note: Modifes Manager state. TODO(stevenjb): Add a completion callback.
-  void SetTechnologyEnabled(const std::string& technology, bool enabled);
+  // Note: Modifes Manager state. Calls |error_callback| on failure.
+  void SetTechnologyEnabled(
+      const std::string& technology,
+      bool enabled,
+      const network_handler::ErrorCallback& error_callback);
 
   // Finds and returns a device state by |device_path| or NULL if not found.
   const DeviceState* GetDeviceState(const std::string& device_path) const;
@@ -107,6 +111,9 @@ class CHROMEOS_EXPORT NetworkStateHandler
   // observer calls).
   void GetNetworkList(NetworkStateList* list) const;
 
+ protected:
+  NetworkStateHandler();
+
   // ShillPropertyHandler::Listener overrides.
 
   // This adds new entries to the managed list specified by |type| and deletes
@@ -151,9 +158,6 @@ class CHROMEOS_EXPORT NetworkStateHandler
   virtual void ManagedStateListChanged(
       ManagedState::ManagedType type) OVERRIDE;
 
- protected:
-  NetworkStateHandler();
-
   // Called in Initialize(). Called explicitly by tests after adding
   // test observers.
   void InitShillPropertyHandler();
diff --git a/chromeos/network/network_state_handler_unittest.cc b/chromeos/network/network_state_handler_unittest.cc
index 915a187..448bf4c 100644
--- a/chromeos/network/network_state_handler_unittest.cc
+++ b/chromeos/network/network_state_handler_unittest.cc
@@ -155,7 +155,8 @@ TEST_F(NetworkStateHandlerTest, NetworkStateHandlerTechnologyChanged) {
   EXPECT_EQ(1u, test_observer_->manager_changed_count());
   // Enable a technology.
   EXPECT_FALSE(network_state_handler_->TechnologyEnabled(flimflam::kTypeWimax));
-  network_state_handler_->SetTechnologyEnabled(flimflam::kTypeWimax, true);
+  network_state_handler_->SetTechnologyEnabled(
+      flimflam::kTypeWimax, true, network_handler::ErrorCallback());
   message_loop_.RunUntilIdle();
   // Ensure we get a manager changed callback when we change a property.
   EXPECT_EQ(2u, test_observer_->manager_changed_count());
diff --git a/chromeos/network/shill_property_handler.cc b/chromeos/network/shill_property_handler.cc
index 2fe6f99..ec9331b 100644
--- a/chromeos/network/shill_property_handler.cc
+++ b/chromeos/network/shill_property_handler.cc
@@ -28,12 +28,6 @@ const char kLogModule[] = "ShillPropertyHandler";
 // order, it should be reasonable to ignore services past this.
 const size_t kMaxObservedServices = 100;
 
-void ErrorCallbackFunction(const std::string& error_name,
-                           const std::string& error_message) {
-  // TODO(stevenjb): Add error logging.
-  LOG(ERROR) << "Shill Error: " << error_name << " : " << error_message;
-}
-
 const base::ListValue* GetListValue(const std::string& key,
                                     const base::Value& value) {
   const base::ListValue* vlist = NULL;
@@ -72,22 +66,29 @@ void ShillPropertyHandler::Init() {
 
 void ShillPropertyHandler::SetTechnologyEnabled(
     const std::string& technology,
-    bool enabled) {
+    bool enabled,
+    const network_handler::ErrorCallback& error_callback) {
   if (enabled) {
-    shill_manager_->EnableTechnology(technology,
-                                     base::Bind(&base::DoNothing),
-                                     base::Bind(&ErrorCallbackFunction));
+    shill_manager_->EnableTechnology(
+        technology,
+        base::Bind(&base::DoNothing),
+        base::Bind(&network_handler::ShillErrorCallbackFunction,
+                   kLogModule, technology, error_callback));
   } else {
-    shill_manager_->DisableTechnology(technology,
-                                      base::Bind(&base::DoNothing),
-                                      base::Bind(&ErrorCallbackFunction));
+    shill_manager_->DisableTechnology(
+        technology,
+        base::Bind(&base::DoNothing),
+        base::Bind(&network_handler::ShillErrorCallbackFunction,
+                   kLogModule, technology, error_callback));
   }
 }
 
 void ShillPropertyHandler::RequestScan() const {
-  shill_manager_->RequestScan("",
-                              base::Bind(&base::DoNothing),
-                              base::Bind(&ErrorCallbackFunction));
+  shill_manager_->RequestScan(
+      "",
+      base::Bind(&base::DoNothing),
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 kLogModule, "", network_handler::ErrorCallback()));
 }
 
 void ShillPropertyHandler::RequestProperties(ManagedState::ManagedType type,
diff --git a/chromeos/network/shill_property_handler.h b/chromeos/network/shill_property_handler.h
index 3ab6bb8..0bb6e6b 100644
--- a/chromeos/network/shill_property_handler.h
+++ b/chromeos/network/shill_property_handler.h
@@ -14,6 +14,7 @@
 #include "chromeos/dbus/dbus_method_call_status.h"
 #include "chromeos/dbus/shill_property_changed_observer.h"
 #include "chromeos/network/managed_state.h"
+#include "chromeos/network/network_handler_callbacks.h"
 
 namespace base {
 class DictionaryValue;
@@ -93,8 +94,11 @@ class CHROMEOS_EXPORT ShillPropertyHandler
   void Init();
 
   // Asynchronously sets the enabled state for |technology|.
-  // Note: Modifes Manager state. TODO(stevenjb): Add a completion callback.
-  void SetTechnologyEnabled(const std::string& technology, bool enabled);
+  // Note: Modifes Manager state. Calls |error_callback| on failure.
+  void SetTechnologyEnabled(
+      const std::string& technology,
+      bool enabled,
+      const network_handler::ErrorCallback& error_callback);
 
   // Requests an immediate network scan.
   void RequestScan() const;