WiFi client for GCD bootstrapping

chrome/browser/local_discovery/wifi/wifi_manager_nonchromeos.cc

+// Copyright 2014 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 "base/cancelable_callback.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "base/threading/thread.h"
+#include "chrome/browser/local_discovery/wifi/wifi_manager_nonchromeos.h"
+#include "components/onc/onc_constants.h"
+#include "components/wifi/wifi_service.h"
+#include "content/public/browser/browser_thread.h"
+
+using ::wifi::WiFiService;
+
+namespace local_discovery {
+
+namespace wifi {
+
+namespace {
+
+const int kConnectionTimeoutSeconds = 10;
+
+scoped_ptr<base::DictionaryValue> MakeProperties(const std::string& ssid,
+                                                 const std::string& password) {
+  scoped_ptr<base::DictionaryValue> properties(new base::DictionaryValue);
+
+  properties->SetString(onc::network_config::kType, onc::network_type::kWiFi);
+  base::DictionaryValue* wifi = new base::DictionaryValue;
+  properties->Set(onc::network_config::kWiFi, wifi);
+
+  wifi->SetString(onc::wifi::kSSID, ssid);
+  if (!password.empty()) {
+    wifi->SetString(onc::wifi::kPassphrase, password);
+    // TODO(noamsml): Allow choosing security mechanism in a more fine-grained
+    // manner.
+    wifi->SetString(onc::wifi::kSecurity, onc::wifi::kWPA2_PSK);
+  } else {
+    wifi->SetString(onc::wifi::kSecurity, onc::wifi::kNone);
+  }
+
+  return properties.Pass();
+}
+
+}  // namespace
+
+class WifiManagerNonChromeos::WifiServiceWrapper {
+ public:
+  typedef base::Callback<void(const base::Closure&)> PostClosureCallback;
+  typedef base::Callback<void(scoped_ptr<std::vector<NetworkProperties> >
+                                  ssid_list)> NetworkListUpdateCallback;
+  typedef base::Callback<void(const WifiManager::SSIDListCallback& callback,
+                              scoped_ptr<std::vector<NetworkProperties> >
+                                  ssid_list)> NetworkListCallbackCallback;

[stevenjb, 2014/05/27 16:49:08]

nit: I think NetworkListPostCallback might be more clear.

[Noam Samuel, 2014/05/27 22:24:07]

Made obsolete by the next comment.

+
+  WifiServiceWrapper(
+      const PostClosureCallback& post_callback,
+      const NetworkListUpdateCallback& network_list_update,
+      const NetworkListCallbackCallback& network_list_post_callback);
+
+  ~WifiServiceWrapper();
+
+  void Start();
+
+  void GetSSIDList(const WifiManager::SSIDListCallback& callback);
+
+  void ConfigureAndConnectPskNetwork(
+      const std::string& ssid,
+      const std::string& password,
+      const WifiManager::SuccessCallback& callback);
+
+  base::WeakPtr<WifiManagerNonChromeos::WifiServiceWrapper> AsWeakPtr();
+
+  void RequestScan();
+
+  void ConnectToNetworkByID(const std::string& network_guid,
+                            const WifiManager::SuccessCallback& callback);
+
+  void RequestNetworkCredentials(
+      const std::string& network_guid,
+      const WifiManager::CredentialsCallback& callback);
+
+ private:
+  void GetSSIDListInternal(std::vector<NetworkProperties>* ssid_list);
+
+  void OnNetworkListChangedEvent(const std::vector<std::string>& network_guids);
+
+  void OnNetworksChangedEvent(const std::vector<std::string>& network_guids);
+
+  std::string GetConnectedGUID();
+
+  bool IsConnected(const std::string& network_guid);
+
+  void OnConnectToNetworkTimeout();
+
+  void PostClosure(const base::Closure& closure);
+
+  scoped_ptr<WiFiService> wifi_service_;
+
+  PostClosureCallback post_callback_;
+  NetworkListUpdateCallback network_list_update_;
+  NetworkListCallbackCallback network_list_post_callback_;
+
+  WifiManager::SuccessCallback connect_success_callback_;
+  base::CancelableClosure connect_failure_callback_;
+  std::string connected_network_guid_;  // SSID of previously connected network.
+  // SSID of network we are connecting to.

[stevenjb, 2014/05/27 16:49:08]

nit: WS above, maybe put the comment for connected_network_guid_ above it also
(with WS) for consistency. (but see next comment).

[Noam Samuel, 2014/05/27 22:24:07]

Looks like it fits next to the variable now.

+  std::string connecting_network_guid_;
+
+  scoped_refptr<base::MessageLoopProxy> callback_runner_;
+
+  base::WeakPtrFactory<WifiServiceWrapper> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(WifiServiceWrapper);
+};
+
+WifiManagerNonChromeos::WifiServiceWrapper::WifiServiceWrapper(
+    const PostClosureCallback& post_callback,
+    const NetworkListUpdateCallback& network_list_update,
+    const NetworkListCallbackCallback& network_list_post_callback)
+    : post_callback_(post_callback),
+      network_list_update_(network_list_update),
+      network_list_post_callback_(network_list_post_callback),

[stevenjb, 2014/05/27 16:49:08]

nit: This is only a suggestion, I haven't carefully mapped out any
thread-saftery issues but it seems like it would be fine: Rather than pass three
separate callbacks to the Wrapper, maybe just pass a WeakPtr to
WifiManagerNonChromeos to this class and call its methods directly. Since this
class is a member of WifiManagerNonChromeos, it can access private methods of
WifiManagerNonChromeos.

[Noam Samuel, 2014/05/27 22:24:07]

Done.

+      callback_runner_(base::MessageLoopProxy::current()),
+      weak_factory_(this) {
+}
+
+WifiManagerNonChromeos::WifiServiceWrapper::~WifiServiceWrapper() {
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::Start() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  wifi_service_.reset(WiFiService::Create());
+
+  wifi_service_->Initialize(base::MessageLoopProxy::current());
+
+  wifi_service_->SetEventObservers(
+      base::MessageLoopProxy::current(),
+      base::Bind(&WifiServiceWrapper::OnNetworksChangedEvent,
+                 base::Unretained(this)),
+      base::Bind(&WifiServiceWrapper::OnNetworkListChangedEvent,
+                 base::Unretained(this)));
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::GetSSIDList(
+    const WifiManager::SSIDListCallback& callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+
+  scoped_ptr<std::vector<NetworkProperties> > ssid_list(
+      new std::vector<NetworkProperties>);
+  GetSSIDListInternal(ssid_list.get());
+
+  callback_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(
+          network_list_post_callback_, callback, base::Passed(&ssid_list)));
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::ConfigureAndConnectPskNetwork(
+    const std::string& ssid,
+    const std::string& password,
+    const WifiManager::SuccessCallback& callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  scoped_ptr<base::DictionaryValue> properties = MakeProperties(ssid, password);
+
+  std::string network_guid;
+  std::string error_string;
+  // Will fail without changing system state if network already exists.
+  wifi_service_->CreateNetwork(
+      false, properties.Pass(), &network_guid, &error_string);
+

[stevenjb, 2014/05/27 16:49:08]

nit: if (error_string.empty) { ConnectToNetworkByID(network_guid, callback);
return; }

It adds a call to ConnectToNetworkByID, but makes the rest of the function
easier to read.

[Noam Samuel, 2014/05/27 22:24:07]

Done.

+  // If we cannot create the network, attempt to configure and connect to an
+  // existing network.
+  bool error = false;
+  if (!error_string.empty()) {
+    error_string.clear();
+    std::vector<NetworkProperties> network_property_list;
+    GetSSIDListInternal(&network_property_list);
+
+    network_guid.clear();
+    for (size_t i = 0; i < network_property_list.size(); i++) {
+      // TODO(noamsml): Handle case where there are multiple networks with the
+      // same SSID but different security.
+      if (network_property_list[i].ssid == ssid) {
+        network_guid = network_property_list[i].internal_id;
+        break;
+      }
+    }
+
+    if (network_guid.empty()) {
+      error = true;  // Could not create the network and it is not configured.

[stevenjb, 2014/05/27 16:49:08]

nit: If this whole if (!error_string.empty()) {} clause were put into a helper
method, we can early return 'false' here and below, making this a little bit
more readable, e.g.

if (GetExistingNetwork(&network_guid))
  ConnectToNetworkByID(network_guid, callback);
else
  PostClosure(base::Bind(callback, false /* success */));

[Noam Samuel, 2014/05/27 22:24:07]

Done.

+    } else {
+      properties = MakeProperties(ssid, password);
+      wifi_service_->SetProperties(
+          network_guid, properties.Pass(), &error_string);
+
+      if (!error_string.empty()) {
+        LOG(ERROR) << "Could not set properties on network: " << error_string;
+        error = true;
+      }
+    }
+  }
+
+  if (!error) {
+    ConnectToNetworkByID(network_guid, callback);
+  } else {
+    PostClosure(base::Bind(callback, false /* success */));
+  }
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::OnNetworkListChangedEvent(
+    const std::vector<std::string>& network_guids) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  scoped_ptr<std::vector<NetworkProperties> > ssid_list(
+      new std::vector<NetworkProperties>);
+  GetSSIDListInternal(ssid_list.get());
+  callback_runner_->PostTask(
+      FROM_HERE, base::Bind(network_list_update_, base::Passed(&ssid_list)));
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::OnNetworksChangedEvent(
+    const std::vector<std::string>& network_guids) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  if (connecting_network_guid_.empty() ||
+      !IsConnected(connecting_network_guid_)) {
+    return;
+  }
+
+  connecting_network_guid_.clear();
+  connect_failure_callback_.Cancel();
+
+  PostClosure(base::Bind(connect_success_callback_, true));
+
+  connect_success_callback_.Reset();
+}
+
+base::WeakPtr<WifiManagerNonChromeos::WifiServiceWrapper>
+WifiManagerNonChromeos::WifiServiceWrapper::AsWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::RequestScan() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  wifi_service_->RequestNetworkScan();
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::ConnectToNetworkByID(
+    const std::string& network_guid,
+    const WifiManager::SuccessCallback& callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+
+  std::string connected_network_id = GetConnectedGUID();
+  std::string error_string;
+  wifi_service_->StartConnect(network_guid, &error_string);
+
+  bool error = false;
+  if (!error_string.empty()) {
+    LOG(ERROR) << "Could not connect to network by ID: " << error_string;
+    error = true;
+    wifi_service_->StartConnect(connected_network_id, &error_string);

[stevenjb, 2014/05/27 16:49:08]

nit: PostClosure(base::Bind(callback, false /* success */)); return;

That way we can elim 'error' and the non-trivial IsConnected() call below.

[Noam Samuel, 2014/05/27 22:24:07]

Done.

+  }
+
+  bool connected = IsConnected(network_guid);

[stevenjb, 2014/05/27 16:49:08]

nit: if (IsConnected(network_guid)) { PostClosure(base::Bind(callback, true /*
success */)); return; }

[Noam Samuel, 2014/05/27 22:24:07]

Done.

+
+  if (error || connected) {
+    PostClosure(base::Bind(callback, !error));
+    return;
+  }
+
+  connect_success_callback_ = callback;
+  connecting_network_guid_ = network_guid;
+  connected_network_guid_ = connected_network_id;
+
+  connect_failure_callback_.Reset(base::Bind(
+      &WifiServiceWrapper::OnConnectToNetworkTimeout, base::Unretained(this)));
+
+  base::MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      connect_failure_callback_.callback(),
+      base::TimeDelta::FromSeconds(kConnectionTimeoutSeconds));
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::OnConnectToNetworkTimeout() {
+  bool connected = IsConnected(connecting_network_guid_);
+  std::string error_string;
+
+  if (!connected)
+    wifi_service_->StartConnect(connected_network_guid_, &error_string);

[stevenjb, 2014/05/27 16:49:08]

We should add a comment here. 'connected' vs. 'connecting' is subtle and I was
confused for a while (again I think) as to what this was doing.

[Noam Samuel, 2014/05/27 22:24:07]

Done.

+
+  connecting_network_guid_.clear();
+
+  PostClosure(base::Bind(connect_success_callback_, connected));

[stevenjb, 2014/05/27 16:49:08]

Add /* success */ comment, or maybe just rename 'connected' -> 'success'.

[Noam Samuel, 2014/05/27 22:24:07]

Done.

+
+  connect_success_callback_.Reset();
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::RequestNetworkCredentials(
+    const std::string& network_guid,
+    const WifiManager::CredentialsCallback& callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+
+  bool error = false;
+  std::string ssid;
+  std::string key;
+
+#if defined(OS_WIN)
+  NOTIMPLEMENTED();
+  error = true
+#else
+  base::DictionaryValue properties;
+  std::string error_string;
+  wifi_service_->GetProperties(network_guid, &properties, &error_string);
+
+  if (!error_string.empty()) {
+    LOG(ERROR) << "Could not get network properties: " << error_string;
+    error = true;
+  }
+
+  if (!properties.GetString(onc::network_config::kName, &ssid)) {
+    LOG(ERROR) << "Could not get network SSID";
+    error = true;
+  }
+
+  if (!error) {
+    wifi_service_->GetKeyFromSystem(network_guid, &key, &error_string);
+
+    if (!error_string.empty()) {
+      LOG(ERROR) << "Could not get key from system: " << error_string;
+      error = true;
+    }
+  }
+#endif  // OS_WIN
+      PostClosure(base::Bind(callback, !error, ssid, key));

[stevenjb, 2014/05/27 16:49:08]

Fix indent.
nit: 'error' -> 'success' (since that's what gets passed to 'callback')

[Noam Samuel, 2014/05/27 22:24:07]

Done.

+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::GetSSIDListInternal(
+    std::vector<NetworkProperties>* ssid_list) {
+  base::ListValue visible_networks;
+
+  wifi_service_->GetVisibleNetworks(onc::network_type::kWiFi,
+                                    &visible_networks);
+
+  for (size_t i = 0; i < visible_networks.GetSize(); i++) {
+    const base::DictionaryValue* network_value = NULL;
+    NetworkProperties network_properties;
+    std::string connection_status;
+
+    if (!visible_networks.GetDictionary(i, &network_value) ||
+        !network_value->GetString(onc::network_config::kName,
+                                  &network_properties.ssid) ||
+        !network_value->GetString(onc::network_config::kGUID,
+                                  &network_properties.internal_id) ||
+        !network_value->GetString(onc::network_config::kConnectionState,
+                                  &connection_status)) {
+      NOTREACHED();
+      continue;

[stevenjb, 2014/05/27 16:49:08]

nit: In general we shouldn't handle NOTREACHED code. If network_value == NULL we
will crash, which is fine. Otherwise we should either LOG(ERROR) and continue,
or leave the NOTREACHED and add the entry with bogus properties (i.e. remove the
continue).

[Noam Samuel, 2014/05/27 22:24:08]

Done.

+    }
+
+    network_properties.connected =
+        (connection_status == onc::connection_state::kConnected);
+
+    ssid_list->push_back(network_properties);
+  }
+}
+
+std::string WifiManagerNonChromeos::WifiServiceWrapper::GetConnectedGUID() {
+  std::vector<NetworkProperties> ssid_list;
+  GetSSIDListInternal(&ssid_list);
+
+  for (std::vector<NetworkProperties>::const_iterator it = ssid_list.begin();
+       it != ssid_list.end();
+       ++it) {
+    if (it->connected)
+      return it->internal_id;
+  }
+
+  return "";
+}
+
+bool WifiManagerNonChromeos::WifiServiceWrapper::IsConnected(
+    const std::string& network_guid) {
+  std::vector<NetworkProperties> ssid_list;
+  GetSSIDListInternal(&ssid_list);
+
+  for (std::vector<NetworkProperties>::const_iterator it = ssid_list.begin();
+       it != ssid_list.end();
+       ++it) {
+    if (it->connected && it->internal_id == network_guid)
+      return true;
+  }
+
+  return false;
+}
+
+void WifiManagerNonChromeos::WifiServiceWrapper::PostClosure(
+    const base::Closure& closure) {
+  callback_runner_->PostTask(FROM_HERE, base::Bind(post_callback_, closure));
+}
+
+scoped_ptr<WifiManager> WifiManager::Create() {
+  return scoped_ptr<WifiManager>(new WifiManagerNonChromeos());
+}
+
+WifiManagerNonChromeos::WifiManagerNonChromeos()
+    : wifi_wrapper_(NULL), weak_factory_(this) {
+}
+
+WifiManagerNonChromeos::~WifiManagerNonChromeos() {
+  if (wifi_wrapper_) {
+    content::BrowserThread::DeleteSoon(
+        content::BrowserThread::FILE, FROM_HERE, wifi_wrapper_);
+  }
+}
+
+void WifiManagerNonChromeos::Start() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  task_runner_ = content::BrowserThread::GetMessageLoopProxyForThread(
+      content::BrowserThread::FILE);
+
+  // Allocated on UI thread, but all initialization is done on file
+  // thread. Destroyed on file thread, which should be safe since all of the
+  // thread-unsafe members are created on the file thread.
+  wifi_wrapper_ = new WifiServiceWrapper(
+      base::Bind(&WifiManagerNonChromeos::PostClosure,
+                 weak_factory_.GetWeakPtr()),
+      base::Bind(&WifiManagerNonChromeos::OnNetworkListChanged,
+                 weak_factory_.GetWeakPtr()),
+      base::Bind(&WifiManagerNonChromeos::PostSSIDListCallback,
+                 weak_factory_.GetWeakPtr()));
+
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&WifiServiceWrapper::Start, wifi_wrapper_->AsWeakPtr()));
+}
+
+void WifiManagerNonChromeos::GetSSIDList(const SSIDListCallback& callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  task_runner_->PostTask(FROM_HERE,
+                         base::Bind(&WifiServiceWrapper::GetSSIDList,
+                                    wifi_wrapper_->AsWeakPtr(),
+                                    callback));
+}
+
+void WifiManagerNonChromeos::RequestScan() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&WifiServiceWrapper::RequestScan, wifi_wrapper_->AsWeakPtr()));
+}
+
+void WifiManagerNonChromeos::OnNetworkListChanged(
+    scoped_ptr<std::vector<NetworkProperties> > ssid_list) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  FOR_EACH_OBSERVER(NetworkListObserver,
+                    network_list_observers_,
+                    OnNetworkListChanged(*ssid_list));
+}
+
+void WifiManagerNonChromeos::PostClosure(const base::Closure& callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  callback.Run();
+}
+
+void WifiManagerNonChromeos::PostSSIDListCallback(
+    const SSIDListCallback& callback,
+    scoped_ptr<std::vector<NetworkProperties> > ssid_list) {
+  callback.Run(*ssid_list);
+}
+
+void WifiManagerNonChromeos::ConfigureAndConnectNetwork(
+    const std::string& ssid,
+    const WifiCredentials& credentials,
+    const SuccessCallback& callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&WifiServiceWrapper::ConfigureAndConnectPskNetwork,
+                 wifi_wrapper_->AsWeakPtr(),
+                 ssid,
+                 credentials.psk,
+                 callback));
+}
+
+void WifiManagerNonChromeos::ConnectToNetworkByID(
+    const std::string& internal_id,
+    const SuccessCallback& callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  task_runner_->PostTask(FROM_HERE,
+                         base::Bind(&WifiServiceWrapper::ConnectToNetworkByID,
+                                    wifi_wrapper_->AsWeakPtr(),
+                                    internal_id,
+                                    callback));
+}
+
+void WifiManagerNonChromeos::RequestNetworkCredentials(
+    const std::string& internal_id,
+    const CredentialsCallback& callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&WifiServiceWrapper::RequestNetworkCredentials,
+                 wifi_wrapper_->AsWeakPtr(),
+                 internal_id,
+                 callback));
+}
+
+void WifiManagerNonChromeos::AddNetworkListObserver(
+    NetworkListObserver* observer) {
+  network_list_observers_.AddObserver(observer);
+}
+
+void WifiManagerNonChromeos::RemoveNetworkListObserver(
+    NetworkListObserver* observer) {
+  network_list_observers_.RemoveObserver(observer);
+}
+
+}  // namespace wifi
+
+}  // namespace local_discovery