This adds a private extension API to use for simple networking

chrome/browser/chromeos/extensions/networking_private_api.cc

+// Copyright (c) 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 "chrome/browser/chromeos/extensions/networking_private_api.h"
+
+#include "base/bind_helpers.h"
+#include "chrome/browser/chromeos/extensions/networking_private_api_factory.h"
+#include "chrome/browser/extensions/extension_function_registry.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/shill_manager_client.h"
+#include "chromeos/dbus/shill_service_client.h"
+#include "dbus/object_path.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace chromeos {
+
+namespace {
+
+// These are keys used by the JavaScript code to access information in the
+// result.
+const char kBssid[] = "bssid";
+const char kName[] = "name";
+const char kNetworkId[] = "networkId";
+const char kNetworkTypeAll[] = "all";
+const char kStatus[] = "status";
+const char kType[] = "type";
+
+// This creates a new dictionary that only contains the information we're
+// interested in passing on to JavaScript.

[stevenjb, 2013/01/17 00:04:30]

I thought we were going to use ONC properties for all JS dictionaries, to avoid
lots of slightly different property names?

Also, it seems like if the only property we need here not already in
NetworkState is WifiBSsid, we could just use NetworkStateHandler?

Eventually we're going to need something just like this that translates every
Service property for JS. It would be nice to only fetch services from the
Manager in only two places instead of three.

[Greg Spencer (Chromium), 2013/01/17 00:10:23]

Good point on the ONC, I'll switch it to that format.

I forgot to write out in the description for this change that in order to
prevent the other project from being dependent upon CONFUSE in order to ship,
I'm avoiding using the stuff that we have behind a flag.  As soon as we move it
out from behind the flag I was going to make the internals of this class change
to use the "new stuff" but for now just use Shill directly.  Or perhaps just
make the things it executes conditional on the flag for now.

+base::DictionaryValue* CreateFilteredResult(
+    const base::DictionaryValue& properties) {
+  scoped_ptr<base::DictionaryValue> filtered_result(new base::DictionaryValue);
+
+  std::string bssid;
+  if (properties.GetString(flimflam::kWifiBSsid, &bssid))
+    filtered_result->SetString(kBssid, bssid);
+  std::string name;
+  if (properties.GetString(flimflam::kNameProperty, &name))
+    filtered_result->SetString(kName, name);
+  std::string type;
+  if (properties.GetString(flimflam::kTypeProperty, &type))
+    filtered_result->SetString(kType, type);
+  std::string state;
+  if (properties.GetString(flimflam::kStateProperty, &state)) {
+    if (state == flimflam::kStateReady || state == flimflam::kStateOnline) {

[stevenjb, 2013/01/17 00:04:30]

This should use NetworkState::StateIsConnected()

+      filtered_result->SetString(kStatus, "connected");
+    } else if (state == flimflam::kStateAssociation ||
+        state == flimflam::kStatePortal ||
+        state == flimflam::kStateConfiguration) {

[stevenjb, 2013/01/17 00:04:30]

This should use NetworkState::StateIsConnecting()

+      filtered_result->SetString(kStatus, "connecting");

[stevenjb, 2013/01/17 00:04:30]

This should also be a const defined above

+    } else {
+      filtered_result->SetString(kStatus, "notConnected");

[stevenjb, 2013/01/17 00:04:30]

ditto

+    }
+  }
+
+  return filtered_result.release();
+}
+
+}  // namespace
+
+
+////////////////////////////////////////////////////////////////////////////////
+// NetworkingGetPropertiesFunction
+
+NetworkingGetPropertiesFunction::~NetworkingGetPropertiesFunction() {}

[stevenjb, 2013/01/17 00:04:30]

nit: I generally see/prefer } on a second line outside of inlined declarations

[Greg Spencer (Chromium), 2013/01/18 01:20:30]

Done.

+
+bool NetworkingGetPropertiesFunction::RunImpl() {
+  std::string service_path;
+  if (!args_->GetString(0, &service_path))
+    return false;
+
+  DBusThreadManager::Get()->GetShillServiceClient()->GetProperties(
+      dbus::ObjectPath(service_path), base::Bind(
+          &NetworkingGetPropertiesFunction::ResultCallback, this));
+  return true;
+}
+
+void NetworkingGetPropertiesFunction::ResultCallback(
+    DBusMethodCallStatus call_status,
+    const base::DictionaryValue& result) {
+  scoped_ptr<base::DictionaryValue> filtered_result(
+      CreateFilteredResult(result));
+
+  SetResult(filtered_result.release());
+  SendResponse(true);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// NetworkingGetVisibleNetworksFunction::ScopedDecrementer
+
+NetworkingGetVisibleNetworksFunction::ScopedDecrementer::ScopedDecrementer(
+    NetworkingGetVisibleNetworksFunction* parent,
+    scoped_refptr<NetworkingGetVisibleNetworksFunction::ResultList> result_list)
+: parent_(parent), result_list_(result_list)  {

[stevenjb, 2013/01/17 00:04:30]

align, separate line per member

[Greg Spencer (Chromium), 2013/01/18 01:20:30]

Done.

+}
+
+NetworkingGetVisibleNetworksFunction::ScopedDecrementer::~ScopedDecrementer() {
+  result_list_->count--;

[stevenjb, 2013/01/17 00:04:30]

pre-decrement

[Greg Spencer (Chromium), 2013/01/18 01:20:30]

Done.

+  if (result_list_->count == 0) {
+    parent_->SetResult(result_list_->list.release());
+    parent_->SendResponse(true);
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// NetworkingGetVisibleNetworksFunction
+
+NetworkingGetVisibleNetworksFunction::~NetworkingGetVisibleNetworksFunction() {}
+
+bool NetworkingGetVisibleNetworksFunction::RunImpl() {
+  std::string network_type;
+  if (!args_->GetString(0, &network_type))
+    return false;
+
+  DBusThreadManager::Get()->GetShillManagerClient()->GetProperties(
+      base::Bind(
+          &NetworkingGetVisibleNetworksFunction::ManagerPropertiesCallback,
+          this,
+          network_type));
+  return true;
+}
+
+// For each of the available services, fire off a request for its properties.
+void NetworkingGetVisibleNetworksFunction::ManagerPropertiesCallback(
+    const std::string& network_type,
+    DBusMethodCallStatus call_status,
+    const base::DictionaryValue& result) {
+  const base::ListValue* available_services;
+  if (!result.GetList(flimflam::kServicesProperty, &available_services)) {
+    error_ = "Error.noServices";
+    SendResponse(false);
+    return;
+  }
+
+  scoped_refptr<ResultList> results(new ResultList);
+  results->count = available_services->GetSize();
+  for (size_t i = 0; i < available_services->GetSize(); ++i) {
+    std::string service_path;
+    available_services->GetString(i, &service_path);
+    DBusThreadManager::Get()->GetShillServiceClient()->GetProperties(
+        dbus::ObjectPath(service_path),
+        base::Bind(
+            &NetworkingGetVisibleNetworksFunction::ServicePropertiesCallback,
+            this,
+            service_path,
+            network_type,
+            results));
+  }
+}
+
+// If this network is of the appropriate type, add it to the results and
+// decrement the count.  If the count hits zero, then send the result.
+void NetworkingGetVisibleNetworksFunction::ServicePropertiesCallback(
+    const std::string& service_path,
+    const std::string& network_type,
+    scoped_refptr<ResultList> result_list,
+    DBusMethodCallStatus call_status,
+    const base::DictionaryValue& result) {
+  // We must decrement the count each time we get a callback, regardless of the
+  // outcome.
+  ScopedDecrementer decrementer(this, result_list);
+
+  if (call_status != DBUS_METHOD_CALL_SUCCESS)
+    return;
+
+  std::string type;
+  if (!result.GetString(flimflam::kTypeProperty, &type))
+    return;
+
+  if (type == network_type || network_type == kNetworkTypeAll) {
+    scoped_ptr<base::DictionaryValue> network_properties(
+        CreateFilteredResult(result));
+    network_properties->SetString(kNetworkId, service_path);
+    result_list->list->Append(network_properties.release());
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// NetworkingRequestConnectFunction
+
+NetworkingRequestConnectFunction::~NetworkingRequestConnectFunction() {}
+
+void  NetworkingRequestConnectFunction::ConnectSuccess() {
+  SendResponse(true);
+}
+
+void NetworkingRequestConnectFunction::ConnectFailed(
+    const std::string& errorName,
+    const std::string& errorMessage) {
+  error_ = errorName;
+  SendResponse(false);
+}
+
+bool NetworkingRequestConnectFunction::RunImpl() {
+  std::string service_path;
+  if (!args_->GetString(0, &service_path))
+    return false;
+
+  DBusThreadManager::Get()->GetShillServiceClient()->Connect(
+      dbus::ObjectPath(service_path),
+      base::Bind(&NetworkingRequestConnectFunction::ConnectSuccess, this),
+      base::Bind(&NetworkingRequestConnectFunction::ConnectFailed, this));
+  return true;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// NetworkingRequestDisconnectFunction
+
+NetworkingRequestDisconnectFunction::~NetworkingRequestDisconnectFunction() {}
+
+void  NetworkingRequestDisconnectFunction::ConnectSuccess() {
+  SendResponse(true);

[stevenjb, 2013/01/17 00:04:30]

Note: this does not mean that we connected, just that the DBus message
succeeded. We should either document that, or wait for the State change before
sending a response.

[Greg Spencer (Chromium), 2013/01/18 01:20:30]

I documented it.  The Javascript can wait for a property change event if it
wants to.

+}
+
+void NetworkingRequestDisconnectFunction::ConnectFailed(
+    const std::string& errorName,
+    const std::string& errorMessage) {
+  error_ = errorName;
+  SendResponse(false);
+}
+
+bool NetworkingRequestDisconnectFunction::RunImpl() {
+  std::string service_path;
+  if (!args_->GetString(0, &service_path))
+    return false;
+
+  DBusThreadManager::Get()->GetShillServiceClient()->Connect(
+      dbus::ObjectPath(service_path),
+      base::Bind(&NetworkingRequestDisconnectFunction::ConnectSuccess, this),
+      base::Bind(&NetworkingRequestDisconnectFunction::ConnectFailed, this));
+  return true;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// NetworkingPrivateAPI
+
+NetworkingPrivateAPI::NetworkingPrivateAPI(Profile* profile) {
+  ExtensionFunctionRegistry* registry =
+      ExtensionFunctionRegistry::GetInstance();
+  registry->RegisterFunction<NetworkingGetPropertiesFunction>();
+  registry->RegisterFunction<NetworkingGetVisibleNetworksFunction>();
+  registry->RegisterFunction<NetworkingRequestConnectFunction>();
+  registry->RegisterFunction<NetworkingRequestDisconnectFunction>();
+}
+
+NetworkingPrivateAPI::~NetworkingPrivateAPI() {
+}
+
+void NetworkingPrivateAPI::Shutdown() {
+}
+
+// static
+NetworkingPrivateAPI* NetworkingPrivateAPI::Get(Profile* profile) {
+  return NetworkingPrivateAPIFactory::GetForProfile(profile);
+}
+
+}  // namespace chromeos