Implement networkingPrivate.setWifiTDLSEnabledState

chromeos/network/network_device_handler_impl.cc

Index: chromeos/network/network_device_handler_impl.cc
diff --git a/chromeos/network/network_device_handler_impl.cc b/chromeos/network/network_device_handler_impl.cc
index 6728546c99004c502fbe21773461959ca279d610..f883700b686c3bfaf7034fce9a2f69ea62098024 100644
--- a/chromeos/network/network_device_handler_impl.cc
+++ b/chromeos/network/network_device_handler_impl.cc
@@ -6,6 +6,8 @@
 
 #include "base/bind.h"
 #include "base/location.h"
+#include "base/message_loop/message_loop_proxy.h"
+#include "base/time/time.h"
 #include "base/values.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_device_client.h"
@@ -134,6 +136,70 @@ void SetDevicePropertyInternal(
       base::Bind(&HandleShillCallFailure, device_path, error_callback));
 }
 
+// Forward declare for PostDelayedTask.
+void CallPerformTDLSOperation(
+    const std::string& device_path,
+    const std::string& operation,
+    const std::string& ip_or_mac_address,
+    const network_handler::StringResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback);
+
+void TDLSOperationCallback(
+    const std::string& device_path,
+    const std::string& operation,
+    const std::string& ip_or_mac_address,
+    const network_handler::StringResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback,
+    const std::string& result) {
+  std::string event_desc = "TDLSOperationCallback: " + operation;
+  if (!result.empty())
+    event_desc += ": " + result;
+  NET_LOG_EVENT(event_desc, device_path);
+  if (operation != shill::kTDLSSetupOperation) {
+    if (!callback.is_null())
+      callback.Run(result);
+    return;
+  }
+  // Setup operation will retry if there was an error (result is not empty),
+  // or request the status.
+  std::string next_operation;
+  base::TimeDelta request_delay;
+  if (!result.empty()) {
+    const int64 kReRequestDelayMs = 1000;
+    next_operation = shill::kTDLSSetupOperation;
+    request_delay = base::TimeDelta::FromMilliseconds(kReRequestDelayMs);

[Paul Stewart, 2014/02/06 22:58:40]

I'm not sure I understand, but:

 - Does this just retry ad-infinitum?  Two issues if that's true:
   - If the error is other than kErrorResultInProgress, you stand to just sit
here forever.
   - If the error is kErrorResultInProgress you still stand to sit here forever
if the IP address never resolves.

[stevenjb, 2014/02/12 23:22:52]

You're right, we definitely need a timeout here. I'm going to start with 5s. If
it takes longer than that, the Extension can decide to retry or fail.

+  } else {
+    next_operation =  shill::kTDLSStatusOperation;

[Paul Stewart, 2014/02/06 22:58:40]

What is request_delay set to at this point?  Does the compiler warn against
uninitialized variables below?

[Paul Stewart, 2014/02/06 23:01:43]

Just spotted this was an object, which probably is set to some zero value.  You
may not want this value to be zero since the TDLS setup in the driver / firmware
won't necessarily be instant.  Since you return a result to your callback on the
first return of operation != shill::kTDLSSetupOperation, I'd suggest a small (5
second?) delay here.

[stevenjb, 2014/02/12 23:22:52]

I will set the delay to 500ms so that there is an opportunity for early feedback
to the user. I added a getWifiTDLSStatus method so that if the status is still
other than 'Connected' or 'Nonexistent' the extension can decide to poll the
status or not.

+  }
+
+  base::MessageLoopProxy::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&CallPerformTDLSOperation,
+                 device_path, next_operation, ip_or_mac_address,
+                 callback, error_callback),
+      request_delay);
+}
+
+void CallPerformTDLSOperation(
+    const std::string& device_path,
+    const std::string& operation,
+    const std::string& ip_or_mac_address,
+    const network_handler::StringResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  NET_LOG_EVENT("CallPerformTDLSOperation: " + operation, device_path);
+  DBusThreadManager::Get()->GetShillDeviceClient()->PerformTDLSOperation(
+      dbus::ObjectPath(device_path),
+      operation,
+      ip_or_mac_address,
+      base::Bind(&TDLSOperationCallback,
+                 device_path, operation, ip_or_mac_address,
+                 callback, error_callback),
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 "Device.PerformTDLSOperation Failed: " + operation,
+                 device_path,
+                 error_callback));
+}
+
 }  // namespace
 
 NetworkDeviceHandlerImpl::~NetworkDeviceHandlerImpl() {
@@ -278,6 +344,28 @@ void NetworkDeviceHandlerImpl::SetCellularAllowRoaming(
   ApplyCellularAllowRoamingToShill();
 }
 
+void NetworkDeviceHandlerImpl::SetWifiTDLSEnabled(
+    const std::string& ip_or_mac_address,
+    bool enabled,
+    const network_handler::StringResultCallback& callback,
+    const network_handler::ErrorCallback& error_callback) {
+  const DeviceState* device_state =
+      network_state_handler_->GetDeviceStateByType(NetworkTypePattern::WiFi());
+  if (!device_state) {
+    if (error_callback.is_null())
+      return;
+    scoped_ptr<base::DictionaryValue> error_data(new base::DictionaryValue);
+    error_data->SetString(network_handler::kErrorName, kErrorDeviceMissing);
+    error_callback.Run(kErrorDeviceMissing, error_data.Pass());
+    return;
+  }
+  std::string operation =
+      enabled ? shill::kTDLSSetupOperation : shill::kTDLSTeardownOperation;
+  CallPerformTDLSOperation(device_state->path(),
+                           operation, ip_or_mac_address,
+                           callback, error_callback);
+}
+
 void NetworkDeviceHandlerImpl::DeviceListChanged() {
   ApplyCellularAllowRoamingToShill();
 }