Add NetworkDeviceHandler::RequestRefreshIPConfigs

This is needed for https://codereview.chromium.org/17995002/.

BUG=189080
R=armansito@chromium.org, pneubeck@chromium.org

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

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

6 files changed, 185 insertions, 59 deletions

diff --git a/chromeos/network/network_configuration_handler.cc b/chromeos/network/network_configuration_handler.cc
index fc6354b..9f05955 100644
--- a/chromeos/network/network_configuration_handler.cc
+++ b/chromeos/network/network_configuration_handler.cc
@@ -32,8 +32,6 @@ namespace {
 const char kErrorsListTag[] = "errors";
 const char kClearPropertiesFailedError[] = "Error.ClearPropertiesFailed";
 const char kClearPropertiesFailedErrorMessage[] = "Clear properties failed";
-const char kDBusFailedError[] = "Error.DBusFailed";
-const char kDBusFailedErrorMessage[] = "DBus call failed.";
 
 void ClearPropertiesCallback(
     const std::vector<std::string>& names,
@@ -73,30 +71,6 @@ void ClearPropertiesCallback(
   }
 }
 
-// Used to translate the dbus dictionary callback into one that calls
-// the error callback if we have a failure.
-void GetPropertiesCallback(
-    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(
-        network_handler::CreateErrorData(service_path,
-                                         kDBusFailedError,
-                                         kDBusFailedErrorMessage));
-    // Because services are added and removed frequently, we will see these
-    // failures regularly, so log as events not errors.
-    NET_LOG_EVENT(base::StringPrintf("GetProperties failed: %d", call_status),
-                  service_path);
-    if (!error_callback.is_null())
-      error_callback.Run(kDBusFailedError, error_data.Pass());
-  } else if (!callback.is_null()) {
-    callback.Run(service_path, value);
-  }
-}
-
 // Strip surrounding "" from keys (if present).
 std::string StripQuotations(const std::string& in_str) {
   size_t len = in_str.length();
@@ -229,7 +203,7 @@ void NetworkConfigurationHandler::GetProperties(
     const network_handler::ErrorCallback& error_callback) const {
   DBusThreadManager::Get()->GetShillServiceClient()->GetProperties(
       dbus::ObjectPath(service_path),
-      base::Bind(&GetPropertiesCallback,
+      base::Bind(&network_handler::GetPropertiesCallback,
                  callback, error_callback, service_path));
 }
 
diff --git a/chromeos/network/network_device_handler.cc b/chromeos/network/network_device_handler.cc
index f3e94ae..802acc9 100644
--- a/chromeos/network/network_device_handler.cc
+++ b/chromeos/network/network_device_handler.cc
@@ -8,6 +8,7 @@
 #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"
@@ -41,19 +42,63 @@ std::string GetErrorNameForShillError(const std::string& shill_error_name) {
   return NetworkDeviceHandler::kErrorUnknown;
 }
 
-void InvokeShillErrorCallback(
-    const std::string& path,
+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),
-      path,
+      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(
+          flimflam::kIPConfigsProperty, &ip_configs)) {
+    NET_LOG_ERROR("RequestRefreshIPConfigs Failed", device_path);
+    network_handler::ShillErrorCallbackFunction(
+        "RequestRefreshIPConfigs Failed",
+        device_path,
+        error_callback,
+        std::string("Missing ") + flimflam::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();
+}
+
 }  // namespace
 
 const char NetworkDeviceHandler::kErrorFailure[] = "failure";
@@ -69,6 +114,26 @@ NetworkDeviceHandler::NetworkDeviceHandler() {
 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::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::SetCarrier(
     const std::string& device_path,
     const std::string& carrier,
@@ -78,8 +143,7 @@ void NetworkDeviceHandler::SetCarrier(
       dbus::ObjectPath(device_path),
       carrier,
       callback,
-      base::Bind(&NetworkDeviceHandler::HandleShillCallFailure,
-                 AsWeakPtr(), device_path, error_callback));
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
 }
 
 void NetworkDeviceHandler::RequirePin(
@@ -93,8 +157,7 @@ void NetworkDeviceHandler::RequirePin(
       pin,
       require_pin,
       callback,
-      base::Bind(&NetworkDeviceHandler::HandleShillCallFailure,
-                 AsWeakPtr(), device_path, error_callback));
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
 }
 
 void NetworkDeviceHandler::EnterPin(
@@ -106,8 +169,7 @@ void NetworkDeviceHandler::EnterPin(
       dbus::ObjectPath(device_path),
       pin,
       callback,
-      base::Bind(&NetworkDeviceHandler::HandleShillCallFailure,
-                 AsWeakPtr(), device_path, error_callback));
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
 }
 
 void NetworkDeviceHandler::UnblockPin(
@@ -121,8 +183,7 @@ void NetworkDeviceHandler::UnblockPin(
       puk,
       new_pin,
       callback,
-      base::Bind(&NetworkDeviceHandler::HandleShillCallFailure,
-                 AsWeakPtr(), device_path, error_callback));
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
 }
 
 void NetworkDeviceHandler::ChangePin(
@@ -136,16 +197,15 @@ void NetworkDeviceHandler::ChangePin(
       old_pin,
       new_pin,
       callback,
-      base::Bind(&NetworkDeviceHandler::HandleShillCallFailure,
-                 AsWeakPtr(), device_path, error_callback));
+      base::Bind(&HandleShillCallFailure, device_path, error_callback));
 }
 
-void NetworkDeviceHandler::HandleShillCallFailure(
+void NetworkDeviceHandler::HandleShillCallFailureForTest(
     const std::string& device_path,
     const network_handler::ErrorCallback& error_callback,
     const std::string& shill_error_name,
     const std::string& shill_error_message) {
-  InvokeShillErrorCallback(
+  HandleShillCallFailure(
       device_path, error_callback, shill_error_name, shill_error_message);
 }
 
diff --git a/chromeos/network/network_device_handler.h b/chromeos/network/network_device_handler.h
index 7d0a5d3..851f1f9 100644
--- a/chromeos/network/network_device_handler.h
+++ b/chromeos/network/network_device_handler.h
@@ -46,6 +46,21 @@ class CHROMEOS_EXPORT 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(
+      const std::string& device_path,
+      const network_handler::DictionaryResultCallback& callback,
+      const network_handler::ErrorCallback& error_callback) const;
+
+  // 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(
+      const std::string& device_path,
+      const base::Closure& callback,
+      const network_handler::ErrorCallback& error_callback);
+
   // Tells the device to set the modem carrier firmware, as specified by
   // |carrier|.
   //
@@ -143,7 +158,7 @@ class CHROMEOS_EXPORT NetworkDeviceHandler
 
   NetworkDeviceHandler();
 
-  void HandleShillCallFailure(
+  void HandleShillCallFailureForTest(
       const std::string& device_path,
       const network_handler::ErrorCallback& error_callback,
       const std::string& error_name,
diff --git a/chromeos/network/network_device_handler_unittest.cc b/chromeos/network/network_device_handler_unittest.cc
index 749eaf2..9c4cbc5 100644
--- a/chromeos/network/network_device_handler_unittest.cc
+++ b/chromeos/network/network_device_handler_unittest.cc
@@ -17,6 +17,8 @@ namespace chromeos {
 
 namespace {
 
+const char kDefaultCellularDevicePath[] = "stub_cellular_device";
+const char kDefaultWifiDevicePath[] = "stub_wifi_device";
 const char kResultSuccess[] = "success";
 
 }  // namespace
@@ -31,9 +33,24 @@ class NetworkDeviceHandlerTest : public testing::Test {
     message_loop_.RunUntilIdle();
     success_callback_ = base::Bind(&NetworkDeviceHandlerTest::SuccessCallback,
                                    base::Unretained(this));
+    properties_success_callback_ =
+        base::Bind(&NetworkDeviceHandlerTest::PropertiesSuccessCallback,
+                   base::Unretained(this));
     error_callback_ = base::Bind(&NetworkDeviceHandlerTest::ErrorCallback,
                                  base::Unretained(this));
     network_device_handler_.reset(new NetworkDeviceHandler());
+
+    ShillDeviceClient::TestInterface* device_test =
+        DBusThreadManager::Get()->GetShillDeviceClient()->GetTestInterface();
+    device_test->ClearDevices();
+    device_test->AddDevice(
+        kDefaultCellularDevicePath, flimflam::kTypeCellular, "cellular1");
+    device_test->AddDevice(
+        kDefaultWifiDevicePath, flimflam::kTypeWifi, "wifi1");
+    base::ListValue test_ip_configs;
+    test_ip_configs.AppendString("ip_config1");
+    device_test->SetDeviceProperty(
+        kDefaultWifiDevicePath, flimflam::kIPConfigsProperty, test_ip_configs);
   }
 
   virtual void TearDown() OVERRIDE {
@@ -61,77 +78,76 @@ class NetworkDeviceHandlerTest : public testing::Test {
     result_ = kResultSuccess;
   }
 
+  void PropertiesSuccessCallback(const std::string& device_path,
+                                 const base::DictionaryValue& properties) {
+    result_ = kResultSuccess;
+    properties_.reset(properties.DeepCopy());
+  }
+
   void InvokeDBusErrorCallback(
       const std::string& device_path,
       const network_handler::ErrorCallback& callback,
       const std::string& error_name) {
-    network_device_handler_->HandleShillCallFailure(
+    network_device_handler_->HandleShillCallFailureForTest(
         device_path, callback, error_name, "Error message.");
   }
 
  protected:
-
-  static const char kDefaultCellularDevicePath[];
-  static const char kDefaultWifiDevicePath[];
-
   std::string result_;
 
   scoped_ptr<NetworkDeviceHandler> network_device_handler_;
   base::MessageLoopForUI message_loop_;
   base::Closure success_callback_;
+  network_handler::DictionaryResultCallback properties_success_callback_;
   network_handler::ErrorCallback error_callback_;
+  scoped_ptr<base::DictionaryValue> properties_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NetworkDeviceHandlerTest);
 };
 
-const char NetworkDeviceHandlerTest::kDefaultCellularDevicePath[] =
-    "/device/stub_cellular_device";
-const char NetworkDeviceHandlerTest::kDefaultWifiDevicePath[] =
-    "/device/stub_wifi_device";
-
 TEST_F(NetworkDeviceHandlerTest, ErrorTranslation) {
   EXPECT_TRUE(result_.empty());
   network_handler::ErrorCallback callback =
       base::Bind(&NetworkDeviceHandlerTest::ErrorCallback,
                  base::Unretained(this));
 
-  network_device_handler_->HandleShillCallFailure(
+  network_device_handler_->HandleShillCallFailureForTest(
       kDefaultCellularDevicePath,
       callback,
       "org.chromium.flimflam.Error.Failure",
       "Error happened.");
   EXPECT_EQ(NetworkDeviceHandler::kErrorFailure, result_);
 
-  network_device_handler_->HandleShillCallFailure(
+  network_device_handler_->HandleShillCallFailureForTest(
       kDefaultCellularDevicePath,
       callback,
       "org.chromium.flimflam.Error.IncorrectPin",
       "Incorrect pin.");
   EXPECT_EQ(NetworkDeviceHandler::kErrorIncorrectPin, result_);
 
-  network_device_handler_->HandleShillCallFailure(
+  network_device_handler_->HandleShillCallFailureForTest(
       kDefaultCellularDevicePath,
       callback,
       "org.chromium.flimflam.Error.NotSupported",
       "Operation not supported.");
   EXPECT_EQ(NetworkDeviceHandler::kErrorNotSupported, result_);
 
-  network_device_handler_->HandleShillCallFailure(
+  network_device_handler_->HandleShillCallFailureForTest(
       kDefaultCellularDevicePath,
       callback,
       "org.chromium.flimflam.Error.PinBlocked",
       "PIN is blocked.");
   EXPECT_EQ(NetworkDeviceHandler::kErrorPinBlocked, result_);
 
-  network_device_handler_->HandleShillCallFailure(
+  network_device_handler_->HandleShillCallFailureForTest(
       kDefaultCellularDevicePath,
       callback,
       "org.chromium.flimflam.Error.PinRequired",
       "A PIN error has occurred.");
   EXPECT_EQ(NetworkDeviceHandler::kErrorPinRequired, result_);
 
-  network_device_handler_->HandleShillCallFailure(
+  network_device_handler_->HandleShillCallFailureForTest(
       kDefaultCellularDevicePath,
       callback,
       "org.chromium.flimflam.Error.WorldExploded",
@@ -139,6 +155,29 @@ TEST_F(NetworkDeviceHandlerTest, ErrorTranslation) {
   EXPECT_EQ(NetworkDeviceHandler::kErrorUnknown, result_);
 }
 
+TEST_F(NetworkDeviceHandlerTest, GetDeviceProperties) {
+  network_device_handler_->GetDeviceProperties(
+      kDefaultWifiDevicePath,
+      properties_success_callback_,
+      error_callback_);
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(kResultSuccess, result_);
+  std::string type;
+  properties_->GetString(flimflam::kTypeProperty, &type);
+  EXPECT_EQ(flimflam::kTypeWifi, type);
+}
+
+TEST_F(NetworkDeviceHandlerTest, RequestRefreshIPConfigs) {
+  network_device_handler_->RequestRefreshIPConfigs(
+      kDefaultWifiDevicePath,
+      success_callback_,
+      error_callback_);
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(kResultSuccess, result_);
+  // TODO(stevenjb): Add test interface to ShillIPConfigClient and test
+  // refresh calls.
+}
+
 TEST_F(NetworkDeviceHandlerTest, SetCarrier) {
   const char kCarrier[] = "carrier";
 
diff --git a/chromeos/network/network_handler_callbacks.cc b/chromeos/network/network_handler_callbacks.cc
index bbd4bcc..0dcc55f 100644
--- a/chromeos/network/network_handler_callbacks.cc
+++ b/chromeos/network/network_handler_callbacks.cc
@@ -11,6 +11,10 @@
 namespace chromeos {
 namespace network_handler {
 
+// None of these messages are user-facing, they should only appear in logs.
+const char kDBusFailedError[] = "Error.DBusFailed";
+const char kDBusFailedErrorMessage[] = "DBus call failed.";
+
 // These are names of fields in the error data dictionary for ErrorCallback.
 const char kErrorName[] = "errorName";
 const char kErrorDetail[] = "errorDetail";
@@ -61,5 +65,27 @@ void ShillErrorCallbackFunction(const std::string& error_name,
   error_callback.Run(error_name, error_data.Pass());
 }
 
+void GetPropertiesCallback(
+    const network_handler::DictionaryResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback,
+    const std::string& path,
+    DBusMethodCallStatus call_status,
+    const base::DictionaryValue& value) {
+  if (call_status != DBUS_METHOD_CALL_SUCCESS) {
+    scoped_ptr<base::DictionaryValue> error_data(
+        network_handler::CreateErrorData(path,
+                                         kDBusFailedError,
+                                         kDBusFailedErrorMessage));
+    // Because network services are added and removed frequently, we will see
+    // these failures regularly, so log as events not errors.
+    NET_LOG_EVENT(base::StringPrintf("GetProperties failed: %d", call_status),
+                  path);
+    if (!error_callback.is_null())
+      error_callback.Run(kDBusFailedError, error_data.Pass());
+  } else if (!callback.is_null()) {
+    callback.Run(path, value);
+  }
+}
+
 }  // namespace network_handler
 }  // namespace chromeos
diff --git a/chromeos/network/network_handler_callbacks.h b/chromeos/network/network_handler_callbacks.h
index 9134951..8ba2f7d 100644
--- a/chromeos/network/network_handler_callbacks.h
+++ b/chromeos/network/network_handler_callbacks.h
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "chromeos/chromeos_export.h"
+#include "chromeos/dbus/dbus_method_call_status.h"
 
 namespace base {
 class DictionaryValue;
@@ -56,6 +57,17 @@ CHROMEOS_EXPORT void ShillErrorCallbackFunction(
     const std::string& dbus_error_name,
     const std::string& dbus_error_message);
 
+// Callback for property getters used by NetworkConfigurationHandler
+// (for Network Services) and by NetworkDeviceHandler. Used to translate
+// the DBus Dictionary callback into one that calls the error callback
+// if |call_status| != DBUS_METHOD_CALL_SUCCESS.
+CHROMEOS_EXPORT void GetPropertiesCallback(
+    const network_handler::DictionaryResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback,
+    const std::string& path,
+    DBusMethodCallStatus call_status,
+    const base::DictionaryValue& value);
+
 }  // namespace network_handler
 }  // namespace chromeos