Base infrastructure for Networking Private API on Windows and Mac.

chrome/browser/extensions/api/networking_private/networking_private_api_nonchromeos.cc

Index: chrome/browser/extensions/api/networking_private/networking_private_api_nonchromeos.cc
diff --git a/chrome/browser/extensions/api/networking_private/networking_private_api_nonchromeos.cc b/chrome/browser/extensions/api/networking_private/networking_private_api_nonchromeos.cc
index 79cb5bdc0edfd87cd5cc7b83223d424362bb8bf0..2a62402f67f9ef8b5186bda6cd414df3ca87ff27 100644
--- a/chrome/browser/extensions/api/networking_private/networking_private_api_nonchromeos.cc
+++ b/chrome/browser/extensions/api/networking_private/networking_private_api_nonchromeos.cc
@@ -10,6 +10,7 @@
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/extensions/api/networking_private/networking_private_process_client.h"
 #include "chrome/browser/extensions/event_names.h"
 #include "chrome/browser/extensions/event_router.h"
 #include "chrome/browser/extensions/extension_function_registry.h"
@@ -24,17 +25,25 @@ using extensions::EventRouter;
 using extensions::ExtensionSystem;
 namespace api = extensions::api::networking_private;
 
-////////////////////////////////////////////////////////////////////////////////
-// NetworkingPrivateGetPropertiesFunction
+namespace {

[stevenjb, 2013/08/13 23:39:04]

nit: WS

[mef, 2013/09/24 21:14:37]

Done.

+const char kNetworkingPrivateProcessClient[] = "NetworkingPrivateProcessClient";
 
+NetworkingPrivateProcessClient* GetProcessClientForProfile(Profile* profile) {
+  if (!profile->GetUserData(kNetworkingPrivateProcessClient)) {
+    NetworkingPrivateProcessClient* client =
+        new NetworkingPrivateProcessClient(profile);
+    profile->SetUserData(
+        kNetworkingPrivateProcessClient,

[cbentzel, 2013/08/19 19:46:34]

I'm a little confused about how the scope of the NetworkingPrivateProcessClient
is tied to that of the Profile.

In particular, it's a refcounted object due to UtilityProcessClient, but if I
understand this code correctly the profile holds a raw pointer to it through
SetUserData. It should probably use a refcounted pointer, or at least do manual
calls to AddRef here and Release when the Profile tears down.

[mef, 2013/08/21 17:25:34]

Good point. I have not think this through yet. 

I need NetworkingPrivateProcessClient to survive between function calls (to
preserve state and broadcast events), but I haven't found a good way to
determine when extension comes and goes to control its lifespan. Ben suggested
to base it on registration of event subscribers, and I'm planning to do that
unless there is some better way.

I will make sure to provide proper refcounting at that point.

[mef, 2013/09/24 21:14:37]

Done.

+        new base::UserDataAdapter<NetworkingPrivateProcessClient>(client));
+  }
 
-const char kNetworkingPrivateProperties[] = "NetworkingPrivateProperties";
+  return base::UserDataAdapter<NetworkingPrivateProcessClient>::Get(
+      profile, kNetworkingPrivateProcessClient);
+}

[stevenjb, 2013/08/13 23:39:04]

nit: WS

[mef, 2013/09/24 21:14:37]

Done.

+}  // namespace
 
-struct NetworkingPrivatePropertiesData : base::SupportsUserData::Data {
-  explicit NetworkingPrivatePropertiesData(const base::DictionaryValue* prop) :
-    properties_(prop->DeepCopy()) { }
-  scoped_ptr<base::DictionaryValue> properties_;
-};
+////////////////////////////////////////////////////////////////////////////////
+// NetworkingPrivateGetPropertiesFunction
 
 NetworkingPrivateGetPropertiesFunction::
   ~NetworkingPrivateGetPropertiesFunction() {
@@ -45,38 +54,33 @@ bool NetworkingPrivateGetPropertiesFunction::RunImpl() {
       api::GetProperties::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
 
-  // If there are properties set by SetProperties function, use those.
-  NetworkingPrivatePropertiesData* stored_properties =
-    static_cast<NetworkingPrivatePropertiesData*> (
-        profile()->GetUserData(kNetworkingPrivateProperties));
-  if (stored_properties != NULL) {
-    SetResult(stored_properties->properties_.release());
-    SendResponse(true);
-    return true;
-  }
+  NetworkingPrivateProcessClient* process_client =
+      GetProcessClientForProfile(profile_);
 
-  const std::string network_properties =
-    "{\"ConnectionState\":\"NotConnected\","
-     "\"GUID\":\"stub_wifi2\","
-     "\"Name\":\"wifi2_PSK\","
-     "\"Type\":\"WiFi\","
-     "\"WiFi\":{"
-       "\"Frequency\":5000,"
-       "\"FrequencyList\":[2400,5000],"
-       "\"SSID\":\"stub_wifi2\","
-       "\"Security\":\"WPA-PSK\","
-       "\"SignalStrength\":80}}";
-
-  if (params->network_guid == "nonexistent_path") {
-    error_ = "Error.DBusFailed";
-    SendResponse(false);
-  } else {
-    SetResult(base::JSONReader::Read(network_properties));
-    SendResponse(true);
-  }
+  process_client->GetProperties(
+      params->network_guid,  // service path
+      base::Bind(&NetworkingPrivateGetPropertiesFunction::GetPropertiesSuccess,
+                 this),
+      base::Bind(&NetworkingPrivateGetPropertiesFunction::GetPropertiesFailed,
+                 this));
   return true;
 }
 
+void NetworkingPrivateGetPropertiesFunction::GetPropertiesSuccess(
+    const std::string& service_path,
+    const base::DictionaryValue& dictionary) {
+  base::DictionaryValue* network_properties = dictionary.DeepCopy();

[stevenjb, 2013/08/13 23:39:04]

nit: no need for local variable (and makes ownership slightly less clear)

[mef, 2013/09/11 20:55:08]

Done.

+  SetResult(network_properties);
+  SendResponse(true);
+}
+
+void NetworkingPrivateGetPropertiesFunction::GetPropertiesFailed(
+    const std::string& error_name,
+    scoped_ptr<base::DictionaryValue> error_data) {
+  error_ = error_name;
+  SendResponse(false);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NetworkingPrivateGetManagedPropertiesFunction
 
@@ -188,13 +192,28 @@ bool NetworkingPrivateSetPropertiesFunction::RunImpl() {
   scoped_ptr<base::DictionaryValue> properties_dict(
       params->properties.ToValue());
 
-  // Store properties_dict in profile to return from GetProperties.
-  profile()->SetUserData(kNetworkingPrivateProperties,
-    new NetworkingPrivatePropertiesData(properties_dict.get()));
-  SendResponse(true);
+  NetworkingPrivateProcessClient* process_client =
+      GetProcessClientForProfile(profile_);
+
+  process_client->SetProperties(
+      params->network_guid,  // service path
+      *properties_dict,
+      base::Bind(&NetworkingPrivateSetPropertiesFunction::ResultCallback, this),
+      base::Bind(&NetworkingPrivateSetPropertiesFunction::ErrorCallback, this));
   return true;
 }
 
+void NetworkingPrivateSetPropertiesFunction::ErrorCallback(
+    const std::string& error_name,
+    const scoped_ptr<base::DictionaryValue> error_data) {
+  error_ = error_name;
+  SendResponse(false);
+}
+
+void NetworkingPrivateSetPropertiesFunction::ResultCallback() {
+  SendResponse(true);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NetworkingPrivateGetVisibleNetworksFunction
 
@@ -206,68 +225,42 @@ bool NetworkingPrivateGetVisibleNetworksFunction::RunImpl() {
   scoped_ptr<api::GetVisibleNetworks::Params> params =
       api::GetVisibleNetworks::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
-  const std::string networks_json =
-      "[{"
-      "    \"ConnectionState\": \"Connected\","
-      "    \"GUID\": \"stub_ethernet\","
-      "    \"Name\": \"eth0\","
-      "    \"Type\": \"Ethernet\""
-      "  },"
-      "  {"
-      "    \"ConnectionState\": \"Connected\","
-      "    \"GUID\": \"stub_wifi1\","
-      "    \"Name\": \"wifi1\","
-      "    \"Type\": \"WiFi\","
-      "    \"WiFi\": {"
-      "      \"AutoConnect\": false,"
-      "      \"Security\": \"WEP-PSK\","
-      "      \"SignalStrength\": 0"
-      "    }"
-      "  },"
-      "  {"
-      "    \"ConnectionState\": \"Connected\","
-      "    \"GUID\": \"stub_vpn1\","
-      "    \"Name\": \"vpn1\","
-      "    \"Type\": \"VPN\","
-      "    \"VPN\": {"
-      "      \"AutoConnect\": false"
-      "    }"
-      "  },"
-      "  {"
-      "    \"ConnectionState\": \"NotConnected\","
-      "    \"GUID\": \"stub_wifi2\","
-      "    \"Name\": \"wifi2_PSK\","
-      "    \"Type\": \"WiFi\","
-      "    \"WiFi\": {"
-      "      \"AutoConnect\": false,"
-      "      \"Security\": \"WPA-PSK\","
-      "      \"SignalStrength\": 80"
-      "    }"
-      "  },"
-      "  {"
-      "    \"Cellular\": {"
-      "      \"ActivateOverNonCellularNetwork\": false,"
-      "      \"ActivationState\": \"not-activated\","
-      "      \"NetworkTechnology\": \"GSM\","
-      "      \"RoamingState\": \"home\""
-      "    },"
-      "    \"ConnectionState\": \"NotConnected\","
-      "    \"GUID\": \"stub_cellular1\","
-      "    \"Name\": \"cellular1\","
-      "    \"Type\": \"Cellular\""
-      "  }]";
-  ListValue* visible_networks =
-      static_cast<ListValue*>(base::JSONReader::Read(networks_json));
-  // If caller only needs WiFi networks, then remove all other networks.
-  if (params->type == api::GetVisibleNetworks::Params::TYPE_WIFI) {
-    visible_networks->Remove(4, NULL);
-    visible_networks->Remove(2, NULL);
-    visible_networks->Remove(0, NULL);
+
+  NetworkingPrivateProcessClient* process_client =
+      GetProcessClientForProfile(profile_);
+
+  process_client->GetVisibleNetworks(base::Bind(
+      &NetworkingPrivateGetVisibleNetworksFunction::ResultCallback, this));
+
+  return true;
+}
+
+void NetworkingPrivateGetVisibleNetworksFunction::ResultCallback(
+    const base::ListValue& network_list) {
+  scoped_ptr<api::GetVisibleNetworks::Params> params =
+      api::GetVisibleNetworks::Params::Create(*args_);
+  ListValue* result = new ListValue();
+  std::string params_type =
+      api::GetVisibleNetworks::Params::ToString(params->type);
+  bool request_all =
+      (params->type == api::GetVisibleNetworks::Params::TYPE_ALL);
+
+  // Copy networks of requested type;
+  for (base::ListValue::const_iterator it = network_list.begin();
+       it != network_list.end();
+       ++it) {
+    const base::Value* v = *it;

[Greg Spencer (Chromium), 2013/08/13 23:59:30]

Please use a more descriptive variable name than 'v'.

[mef, 2013/09/24 21:14:37]

Done.

+    const DictionaryValue* network = NULL;
+    std::string network_type;
+    if (request_all || (v->GetAsDictionary(&network) &&
+                        network->GetString("Type", &network_type) &&

[stevenjb, 2013/08/13 23:39:04]

This should be defined as a constant somewhere, either using an ONC constant or
something defined in NetworkingPrivateProcessClient.

[mef, 2013/09/11 20:55:08]

Done.

+                        network_type == params_type)) {

[stevenjb, 2013/08/13 23:39:04]

Do we want to ensure that |v| is a dictionary? It might also make the code more
clear if we do:

if (!v->GetAsDictionary(&network)) {
  LOG(ERROR)...
  continue;
}
if (!request_all) {
  std::string network_type;
  network->GetString(kType, &network_type);
  if (network_type != params_type)
    continue;
}

[mef, 2013/09/11 20:55:08]

Done.

+      result->Append(v->DeepCopy());
+    }
   }
 
-  SetResult(visible_networks);
+  SetResult(result);
   SendResponse(true);
-  return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -278,20 +271,9 @@ NetworkingPrivateRequestNetworkScanFunction::
 }
 
 bool NetworkingPrivateRequestNetworkScanFunction::RunImpl() {
-  // Generate onNetworkListChanged event.
-  std::vector<std::string> changes;
-  changes.push_back("stub_ethernet");
-  changes.push_back("stub_wifi1");
-  changes.push_back("stub_vpn1");
-  changes.push_back("stub_wifi2");
-  changes.push_back("stub_cellular1");
-
-  EventRouter* event_router = ExtensionSystem::Get(profile_)->event_router();
-  scoped_ptr<base::ListValue> args(api::OnNetworkListChanged::Create(changes));
-  scoped_ptr<extensions::Event> extension_event(new extensions::Event(
-      kOnNetworkListChanged, args.Pass()));
-  event_router->BroadcastEvent(extension_event.Pass());
-
+  NetworkingPrivateProcessClient* process_client =
+      GetProcessClientForProfile(profile_);
+  process_client->RequestNetworkScan();
   return true;
 }
 
@@ -302,53 +284,31 @@ NetworkingPrivateStartConnectFunction::
   ~NetworkingPrivateStartConnectFunction() {
 }
 
+void NetworkingPrivateStartConnectFunction::ConnectionStartSuccess() {
+  SendResponse(true);
+}
+
+void NetworkingPrivateStartConnectFunction::ConnectionStartFailed(
+    const std::string& error_name,
+    const scoped_ptr<base::DictionaryValue> error_data) {
+  error_ = error_name;
+  SendResponse(false);
+}
+
 bool NetworkingPrivateStartConnectFunction::RunImpl() {
   scoped_ptr<api::StartConnect::Params> params =
       api::StartConnect::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
-  if (params->network_guid == "nonexistent_path") {
-    error_ = "not-found";
-    SendResponse(false);
-  } else {
-    SendResponse(true);
-    // Set Properties to reflect connected state
-    const std::string network_properties =
-      "{\"ConnectionState\":\"Connected\","
-       "\"GUID\":\"stub_wifi2\","
-       "\"Name\":\"wifi2_PSK\","
-       "\"Type\":\"WiFi\","
-       "\"WiFi\":{"
-         "\"SSID\":\"stub_wifi2\","
-         "\"Security\":\"WPA-PSK\","
-         "\"SignalStrength\":80}}";
-
-    // Store network_properties in profile to return from GetProperties.
-    profile()->SetUserData(kNetworkingPrivateProperties,
-      new NetworkingPrivatePropertiesData(
-        static_cast<DictionaryValue*>(
-          base::JSONReader::Read(network_properties))));
-
-    // Broadcast NetworksChanged Event that network is connected
-    EventRouter* event_router = ExtensionSystem::Get(profile_)->event_router();
-    scoped_ptr<base::ListValue> args(api::OnNetworksChanged::Create(
-        std::vector<std::string>(1, params->network_guid)));
-    scoped_ptr<extensions::Event> netchanged_event(
-        new extensions::Event(kOnNetworksChanged, args.Pass()));
-    event_router->BroadcastEvent(netchanged_event.Pass());
-
-    // Generate NetworkListChanged event.
-    std::vector<std::string> list;
-    list.push_back("stub_ethernet");
-    list.push_back("stub_wifi2");
-    list.push_back("stub_vpn1");
-    list.push_back("stub_wifi1");
-    list.push_back("stub_cellular1");
-
-    scoped_ptr<base::ListValue> arg2(api::OnNetworkListChanged::Create(list));
-    scoped_ptr<extensions::Event> netlist_event(new extensions::Event(
-        kOnNetworkListChanged, arg2.Pass()));
-    event_router->BroadcastEvent(netlist_event.Pass());
-  }
+
+  NetworkingPrivateProcessClient* process_client =
+      GetProcessClientForProfile(profile_);
+
+  process_client->StartConnect(
+      params->network_guid,  // service path
+      base::Bind(&NetworkingPrivateStartConnectFunction::ConnectionStartSuccess,
+                 this),
+      base::Bind(&NetworkingPrivateStartConnectFunction::ConnectionStartFailed,
+                 this));
   return true;
 }
 
@@ -359,24 +319,33 @@ NetworkingPrivateStartDisconnectFunction::
   ~NetworkingPrivateStartDisconnectFunction() {
 }
 
+void NetworkingPrivateStartDisconnectFunction::DisconnectionStartSuccess() {
+  SendResponse(true);
+}
+
+void NetworkingPrivateStartDisconnectFunction::DisconnectionStartFailed(
+    const std::string& error_name,
+    const scoped_ptr<base::DictionaryValue> error_data) {
+  error_ = error_name;
+  SendResponse(false);
+}
+
 bool NetworkingPrivateStartDisconnectFunction::RunImpl() {
   scoped_ptr<api::StartDisconnect::Params> params =
       api::StartDisconnect::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params);
-  if (params->network_guid == "nonexistent_path") {
-    error_ = "not-found";
-    SendResponse(false);
-  } else {
-    SendResponse(true);
-
-    // Send Event that network is disconnected. Listener will use GetProperties.
-    EventRouter* event_router = ExtensionSystem::Get(profile_)->event_router();
-    scoped_ptr<base::ListValue> args(api::OnNetworksChanged::Create(
-        std::vector<std::string>(1, params->network_guid)));
-    scoped_ptr<extensions::Event> extension_event(
-        new extensions::Event(kOnNetworksChanged, args.Pass()));
-    event_router->BroadcastEvent(extension_event.Pass());
-  }
+
+  NetworkingPrivateProcessClient* process_client =
+      GetProcessClientForProfile(profile_);
+
+  process_client->StartDisconnect(
+      params->network_guid,  // service path
+      base::Bind(
+          &NetworkingPrivateStartDisconnectFunction::DisconnectionStartSuccess,
+          this),
+      base::Bind(
+          &NetworkingPrivateStartDisconnectFunction::DisconnectionStartFailed,
+          this));
   return true;
 }