Windows-specific implementation of Networking Private API.

chrome/utility/wifi/wifi_service_win.cc

Index: chrome/utility/wifi/wifi_service_win.cc
diff --git a/chrome/utility/wifi/wifi_service_win.cc b/chrome/utility/wifi/wifi_service_win.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b9c70d7782c61a1b34d6d6443a282b58a3d79ce3
--- /dev/null
+++ b/chrome/utility/wifi/wifi_service_win.cc
@@ -0,0 +1,802 @@
+// Copyright 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/utility/wifi/wifi_service.h"
+
+#include <iphlpapi.h>
+#include <objbase.h>
+#include <wlanapi.h>
+
+#include "base/bind.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop.h"
+#include "base/strings/string16.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "components/onc/onc_constants.h"
+
+namespace wifi {

[Jói, 2013/10/19 21:14:45]

I believe in the src/chrome folder we're mostly getting rid of namespaces. That
and the class names are already prefixed WiFi.

OTOH, I wonder if this code (I mean all of the chrome/utility/wifi code) should
live in src/chrome or whether it is separate enough from src/chrome (and
potentially reusable by other top-level targets such as Android WebView) to live
in src/components. See the following:

http://www.chromium.org/developers/design-documents/browser-components
http://www.chromium.org/developers/design-documents/browser-components/cookbook

[mef, 2013/10/21 15:37:16]

Interesting. I'll check the component idea. Per discussion with cbentzel@ there
is no immediate plans to make this code reusable anywhere but in
networkingPrivate API. I've modeled utility/wifi/ after
utility/media_galleries/, which use namespaces.

[Jói, 2013/10/21 16:44:03]

If there are no plans to use it other than for extension APIs, then there is no
need to move it to src/components. I would however recommend adding a
restrictive DEPS file to help ensure that your code doesn't start depending on
"everything" in src/chrome (i.e. it looks like it can depend only on its own
directlry under src/chrome and nothing else except lower layers).

I think you probably don't want a namespace if it's staying under src/chrome.

[mef, 2013/10/21 19:42:00]

Done. If you don't have strong objections I'd keep the namespace - it was
suggested by stevenjb@ and seems consistent with utility/local_discovery,
utility/media_galleries, etc. 
OTO the chrome/networking_private_handler.* is using namespace chrome.

Please let me know.

+
+// Implementation of WiFiService for Windows.
+class WiFiServiceImpl : public WiFiService, base::NonThreadSafe {
+ public:
+  WiFiServiceImpl();
+  virtual ~WiFiServiceImpl();
+
+  // WiFiService interface implementation.
+
+  virtual void GetProperties(const std::string& network_guid,
+                             const NetworkPropertiesCallback& callback,
+                             const ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void GetState(const std::string& network_guid,
+                        const NetworkPropertiesCallback& callback,
+                        const ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void GetManagedProperties(
+      const std::string& network_guid,
+      const DictionaryResultCallback& callback,
+      const ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void SetProperties(const std::string& network_guid,
+                             const base::DictionaryValue& properties,
+                             const StringResultCallback& callback,
+                             const ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void GetVisibleNetworks(const NetworkListCallback& callback,
+                                  const ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void RequestNetworkScan() OVERRIDE;
+
+  virtual void StartConnect(const std::string& network_guid,
+                            const StringResultCallback& callback,
+                            const ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void StartDisconnect(const std::string& network_guid,
+                               const StringResultCallback& callback,
+                               const ErrorCallback& error_callback) OVERRIDE;
+
+  virtual void SetNetworksChangedObserver(
+      const NetworkGuidListCallback& observer) OVERRIDE;
+
+  virtual void SetNetworkListChangedObserver(
+      const NetworkGuidListCallback& observer) OVERRIDE;
+
+ private:
+  // Static callback for Windows WLAN_NOTIFICATION. Calls OnWlanNotification
+  // on WiFiServiceImpl passed back as |context|.
+  static void __stdcall OnWlanNotificationCallback(
+      PWLAN_NOTIFICATION_DATA wlan_notification_data,
+      PVOID context);
+
+  // Callback for Windows WLAN_NOTIFICATION. Called on random thread from
+  // OnWlanNotificationCallback. Handles network connectivity and scan complete
+  // notification and posts tasks to main thread.
+  void OnWlanNotification(PWLAN_NOTIFICATION_DATA wlan_notification_data);
+
+  // Handles NetworkScanComplete notification on main thread. Sends
+  // |NetworkListChanged| event with new list of visible networks.
+  void OnNetworkScanCompleteOnMainThread();
+
+  // Wait up to |kMaxAttempts| with |kAttemptDelayMs| delay for connection
+  // to network with |network_guid|. Reset DHCP and Notify that |NetworkChanged|
+  // upon success.
+  void WaitForNetworkConnect(const std::string& network_guid, int attempt);
+
+  // Check |error_code| and if is not |ERROR_SUCCESS|, then run |error_callback|
+  // with |error_name|.
+  bool CheckError(const ErrorCallback& error_callback,
+                  const std::string& error_name,
+                  DWORD error_code) const;
+
+  // Return |iterator| to network identified by |network_guid| in |networks|
+  // list.
+  NetworkList::const_iterator FindNetwork(
+      const NetworkList& networks,
+      const std::string& network_guid) const;
+
+  // Save currently connected network profile and return its
+  // |connected_network_guid|, so it can be re-connected later.
+  DWORD SaveCurrentConnectedNetwork(std::string* connected_network_guid);
+
+  // Sort networks, so connected/connecting is up front, then by type:
+  // Ethernet, WiFi, Cellular, VPN
+  static void SortNetworks(NetworkList* networks);
+
+  // Open a WLAN client handle, register for WLAN notifications.
+  DWORD OpenClientHandle();
+
+  // Reset DHCP on wireless adapter to speed up reconnect after chromecast

[Jói, 2013/10/19 21:14:45]

Is this code specific to Chromecast? If not, suggest explaining in more general
terms or using Chromecast only as an example (adding an e.g.)

[mef, 2013/10/21 15:37:16]

Done.

+  // device reset
+  DWORD ResetDHCP();

[Jói, 2013/10/19 21:14:45]

Here you upper-case DHCP; in OnWlanNotification and FindAdapterIndexMapByGuid
you do not upper-case WLAN or GUID (there are other examples, search for Ssid,
Guid, Wlan, DHCP). Suggest trying to be consistent within your own file. 
Whether to upper-case or not is up to you, the Chrome codebase isn't consistent
on this.

[mef, 2013/10/21 15:37:16]

Done. I've chosen upper-case.

+
+  // Find |adapter_index_map| by |interface_guid| for DHCP reset.
+  DWORD FindAdapterIndexMapByGuid(const GUID& interface_guid,
+                                  IP_ADAPTER_INDEX_MAP* adapter_index_map);
+
+  // Ensure that |client_| handle is initialized.
+  DWORD EnsureInitialized();
+
+  // Close |client_| handle if it is open.
+  DWORD CloseClientHandle();
+
+  // Get |profile_name| from unique |network_guid|.
+  base::string16 ProfileNameFromGuid(const std::string& network_guid) const {
+    return base::UTF8ToUTF16(network_guid);
+  }
+
+  // Get |dot11Ssid| from unique |network_guid|.
+  DOT11_SSID SsidFromGuid(const std::string& network_guid) const;
+
+  // Get unique |network_guid| string based on |dot11Ssid|.
+  std::string GuidFromSsid(const DOT11_SSID& dot11Ssid) const {

[afontan, 2013/10/21 16:55:23]

dot11Ssid is not a GUID and it is not unique. You can have multiple profiles for
the same SSID (a typical scenario is for example 2.4 and 5 GHz).

[mef, 2013/10/21 19:42:00]

I could make it an interface-ssid-bssid combination. How unique do we want them
to be? Right now this api is able to connect to arbitrary unsecured network
(e.g. Chromekey1501, GoogleGuest) or previously connected secured network if
there is a saved profile. What would be a scenario to connect to particular BSS?

[afontan, 2013/10/21 20:51:02]

Correct. The 5GHz/2.4GHz with same SSID is the scenario that comes to mind.

[mef, 2013/10/24 23:44:47]

Done. Per discussion with tbarzic@ we'll keep the same guid for both networks,
but will allow explicit parameter to connect on specific frequency.

+    return std::string(reinterpret_cast<const char*>(dot11Ssid.ucSSID),
+                       dot11Ssid.uSSIDLength);
+  }
+
+  // Get network |ssid| string based on |wlan|.
+  std::string SsidFromWlan(const WLAN_AVAILABLE_NETWORK& wlan) const {
+    return GuidFromSsid(wlan.dot11Ssid);
+  }
+
+  // Get unique |network_guid| string based on |wlan|.
+  std::string GuidFromWlan(const WLAN_AVAILABLE_NETWORK& wlan) const {

[afontan, 2013/10/21 16:55:23]

ditto.

[mef, 2013/10/24 23:44:47]

Done. Same as GUIDFromSSID().

+    return SsidFromWlan(wlan);
+  }
+
+  // Deduce |onc::wifi| security from |alg|.
+  std::string SecurityFromDot11AuthAlg(DOT11_AUTH_ALGORITHM alg) const;
+
+  // Populate |properties| based on |wlan| and its corresponding bss info from
+  // |wlan_bss_list|.
+  void NetworkPropertiesFromAvailableNetwork(const WLAN_AVAILABLE_NETWORK& wlan,
+                                             const WLAN_BSS_LIST& wlan_bss_list,
+                                             NetworkProperties* properties);
+  // Get the list of visible wireless networks.
+  DWORD GetVisibleNetworkList(NetworkList* network_list);
+
+  // Find currently connected network if any. Populate |connected_network_guid|
+  // on success.
+  DWORD FindConnectedNetwork(std::string* connected_network_guid);
+
+  // Connect to network |network_guid| using previosly stored profile if exists,
+  // or just network sid.
+  DWORD Connect(const std::string& network_guid);
+
+  // Disconnect from currently connected network if any.
+  DWORD Disconnect();
+
+  // Save temporary wireless profile for |network_guid|.
+  DWORD SaveTempProfile(const std::string& network_guid);
+
+  // Get previously stored |profile_xml| for |network_guid|.
+  DWORD GetProfile(const std::string& network_guid, std::string* profile_xml);
+
+  // Return true if there is previously stored profile xml for |network_guid|.
+  bool HaveProfile(const std::string& network_guid);
+
+  // Notify |network_list_changed_observer_| that list of visible networks has
+  // changed to |networks|.
+  void NotifyNetworkListChanged(const NetworkList& networks);
+
+  // Notify |networks_changed_observer_| that network |network_guid| status has
+  // changed.
+  void NotifyNetworkChanged(const std::string& network_guid);
+
+  // Wlan Service Handle.

[Jói, 2013/10/19 21:14:45]

Title case seems unnecessary, here and next couple of comments.

[mef, 2013/10/21 15:37:16]

Done.

+  HANDLE client_;
+  // Wlan Interface Guid.

[Jói, 2013/10/19 21:14:45]

A better comment might be "GUID of the currently connected interface, if any,
otherwise the GUID of one of the WLAN interfaces."

[mef, 2013/10/21 15:37:16]

Done.

+  GUID interface_guid_;
+  // Preserved Wlan Profile Xml.
+  std::map<std::string, std::string> saved_profiles_xml_;
+  // Observer to get notified when network(s) have changed (e.g. connect).
+  NetworkGuidListCallback networks_changed_observer_;
+  // Observer to get notified when network list has changed (scan complete).
+  NetworkGuidListCallback network_list_changed_observer_;
+  // Task runner to post tasks on main thread.
+  scoped_refptr<base::TaskRunner> task_runner_;
+  // If |false|, then |networks_changed_observer_| is not notified.
+  bool enable_notify_network_changed_;
+  // Number of attempts to check that network has connected successfully.
+  static const int kMaxAttempts = 100;
+  // Delay between attempts to check that network has connected successfully.
+  static const int kAttemptDelayMs = 100;
+  // Delay after DHCP Renew to allow IP address to be acquired.
+  static const int kDhcpRenewDelayMs = 5000;
+
+  DISALLOW_COPY_AND_ASSIGN(WiFiServiceImpl);
+};
+
+WiFiServiceImpl::WiFiServiceImpl()
+    : client_(NULL), enable_notify_network_changed_(true) {}
+
+WiFiServiceImpl::~WiFiServiceImpl() { CloseClientHandle(); }
+
+void WiFiServiceImpl::GetProperties(const std::string& network_guid,
+                                    const NetworkPropertiesCallback& callback,

[Jói, 2013/10/19 21:14:45]

I'm assuming the interface here and other places is designed using callbacks
instead of just returning values because on some other platforms you can't
return synchronously? If this is not the case we should discuss.

[mef, 2013/10/21 15:37:16]

Yes, AFAIR Mac OS X WiFiFoundation uses callbacks interface, but I need to
double check.

[Jói, 2013/10/21 16:44:03]

OK, fine by me, just checking.

+                                    const ErrorCallback& error_callback) {
+  DWORD error = EnsureInitialized();
+  if (error == ERROR_SUCCESS) {
+    NetworkList network_list;
+    error = GetVisibleNetworkList(&network_list);
+    if (error == ERROR_SUCCESS && !network_list.empty()) {
+      NetworkList::const_iterator it = FindNetwork(network_list, network_guid);
+      if (it != network_list.end()) {
+        DLOG(INFO) << "Get Properties: " << network_guid << ":"
+                   << it->connection_state;
+        callback.Run(network_guid, *it);

[Jói, 2013/10/19 21:14:45]

Could return right after this, no need to CheckError.

[mef, 2013/10/21 15:37:16]

Done.

+      } else {
+        error = ERROR_NOT_FOUND;
+      }
+    }
+  }
+
+  CheckError(error_callback, "Error.DBusFailed", error);
+}
+
+void WiFiServiceImpl::GetState(const std::string& network_guid,
+                               const NetworkPropertiesCallback& callback,
+                               const ErrorCallback& error_callback) {}

[Jói, 2013/10/19 21:14:45]

Might want to comment here and GetManagedProperties why the implementation does
nothing (not even invoke the |callback|).

[mef, 2013/10/21 15:37:16]

Done.

+
+void WiFiServiceImpl::GetManagedProperties(
+    const std::string& network_guid,
+    const DictionaryResultCallback& callback,
+    const ErrorCallback& error_callback) {}
+
+void WiFiServiceImpl::SetProperties(const std::string& network_guid,
+                                    const base::DictionaryValue& properties,
+                                    const StringResultCallback& callback,
+                                    const ErrorCallback& error_callback) {}
+
+void WiFiServiceImpl::GetVisibleNetworks(const NetworkListCallback& callback,
+                                         const ErrorCallback& error_callback) {
+  DWORD error = EnsureInitialized();
+
+  if (error == ERROR_SUCCESS) {
+    NetworkList network_list;
+    error = GetVisibleNetworkList(&network_list);
+    if (error == ERROR_SUCCESS && !network_list.empty()) {
+      SortNetworks(&network_list);

[afontan, 2013/10/21 16:55:23]

I would have expected the API to retrieve the networks asynchronously and then
invoke the callback.

[mef, 2013/10/21 19:42:00]

Is it something that could block for considerable time?

+      callback.Run(network_list);

[Jói, 2013/10/19 21:14:45]

Could return here, no need to CheckError.

This probably goes for every place you CheckError at the end of the function; if
you ran the non-error |callback| you aren't going to also run the
|error_callback|.

[mef, 2013/10/21 15:37:16]

Done.

+    }
+  }
+
+  CheckError(error_callback, "Error.DBusFailed", error);
+}
+
+void WiFiServiceImpl::RequestNetworkScan() {
+  DWORD error = EnsureInitialized();
+  if (error == ERROR_SUCCESS) {
+    WlanScan(client_, &interface_guid_, NULL, NULL, NULL);
+  }
+}
+
+void WiFiServiceImpl::StartConnect(const std::string& network_guid,
+                                   const StringResultCallback& callback,
+                                   const ErrorCallback& error_callback) {
+  DLOG(INFO) << "Start Connect: " << network_guid;
+  DWORD error = EnsureInitialized();
+  if (error == ERROR_SUCCESS) {
+    std::string connected_network_guid;
+    error = SaveCurrentConnectedNetwork(&connected_network_guid);
+    if (error == ERROR_SUCCESS) {
+      // Connect only if network |network_guid| is not connected already.
+      if (network_guid != connected_network_guid)
+        error = Connect(network_guid);

[afontan, 2013/10/21 16:55:23]

One thing we were asked is to avoid the network location wizard to pop up when
we change networks. There is a registry key that controls that
(Software\Microsoft\Windows NT\CurrentVersion\Network\NwCategoryWizard -> Show).
This is the same as if the user had clicked do not remind me.

Ideally you would disable at the beginning and restore it when you are done. In
this way the pop up does not show while you are automatically changing networks.
Unfortunately I do not know of an API that can disable the pop up temporarily.

[mef, 2013/10/21 19:42:00]

Cool, Will do.

[mef, 2013/10/22 20:06:01]

Done.

+      if (error == ERROR_SUCCESS) {
+        callback.Run(network_guid);

[Jói, 2013/10/19 21:14:45]

Just checking: Are you sure you want to run this before NotifyNetworkChanged and
before you WaitForNetworkConnect?

[mef, 2013/10/21 15:37:16]

I think so. Logically caller is start the Connect, and callback tells that that
start is successful, and afterwards all network notifications are fired.

[Jói, 2013/10/21 16:44:03]

OK.

+        // Notify that previously connected network has changed.
+        NotifyNetworkChanged(connected_network_guid);
+        // Start waiting for network connection state change.
+        if (!networks_changed_observer_.is_null()) {
+          // Disable automatic network change notifications.
+          enable_notify_network_changed_ = false;
+          WaitForNetworkConnect(network_guid, 0);
+        }
+      }
+    }
+  }
+  CheckError(error_callback, "Error.DBusFailed", error);
+}
+
+void WiFiServiceImpl::StartDisconnect(const std::string& network_guid,
+                                      const StringResultCallback& callback,
+                                      const ErrorCallback& error_callback) {
+  DLOG(INFO) << "Start Disconnect: " << network_guid;
+  DWORD error = EnsureInitialized();
+  if (error == ERROR_SUCCESS) {
+    std::string connected_network_guid;
+    error = SaveCurrentConnectedNetwork(&connected_network_guid);
+    if (error == ERROR_SUCCESS && network_guid == connected_network_guid) {
+      error = Disconnect();
+      if (error == ERROR_SUCCESS) {
+        NotifyNetworkChanged(network_guid);
+        callback.Run(network_guid);
+      }
+    }
+  }
+  CheckError(error_callback, "Error.DBusFailed", error);

[Jói, 2013/10/19 21:14:45]

Error.DBusFailed is a bit of a weird error for Windows... is this ChromeOS
legacy or some such?

[mef, 2013/10/21 15:37:16]

Done. Yes, it was a ChromeOS legacy, but it doesn't have to be as extension
doesn't check error type, so I've made up a new error constant.

+}
+
+void WiFiServiceImpl::SetNetworksChangedObserver(
+    const NetworkGuidListCallback& observer) {
+  networks_changed_observer_ = observer;
+}
+
+void WiFiServiceImpl::SetNetworkListChangedObserver(
+    const NetworkGuidListCallback& observer) {
+  network_list_changed_observer_ = observer;
+}
+
+void WiFiServiceImpl::OnWlanNotificationCallback(
+    PWLAN_NOTIFICATION_DATA wlan_notification_data,
+    PVOID context) {
+  WiFiServiceImpl* service = reinterpret_cast<WiFiServiceImpl*>(context);
+  service->OnWlanNotification(wlan_notification_data);
+}
+
+void WiFiServiceImpl::OnWlanNotification(
+    PWLAN_NOTIFICATION_DATA wlan_notification_data) {
+  PWLAN_CONNECTION_NOTIFICATION_DATA wlan_connection_data =
+      reinterpret_cast<PWLAN_CONNECTION_NOTIFICATION_DATA>(
+          wlan_notification_data->pData);
+
+  switch (wlan_notification_data->NotificationCode) {
+    case wlan_notification_acm_disconnected:
+    case wlan_notification_acm_connection_complete:
+    case wlan_notification_acm_connection_attempt_fail:
+      task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&WiFiServiceImpl::NotifyNetworkChanged,
+                     base::Unretained(this),
+                     GuidFromSsid(wlan_connection_data->dot11Ssid)));
+      break;
+    case wlan_notification_acm_scan_complete:
+      task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&WiFiServiceImpl::OnNetworkScanCompleteOnMainThread,
+                     base::Unretained(this)));
+      break;
+  }

[Jói, 2013/10/19 21:14:45]

I would add a default: case that DCHECKs.

[mef, 2013/10/21 15:37:16]

I can't, there are more notification types that we don't handle.

[Jói, 2013/10/21 16:44:03]

If you get one of the other notifications, then your cast to
PWLAN_CONNECTION_NOTIFICATION_DATA is invalid. I guess it's "safe" in that you
won't access it if it isn't one of these types, but I think it'd be better to
return early (before casting) if it isn't one of the types you handle. That
would imply another switch statement before the cast, I guess.

[mef, 2013/10/21 19:42:00]

Done.

+}
+
+void WiFiServiceImpl::OnNetworkScanCompleteOnMainThread() {
+  NetworkList networks;
+  DWORD error = GetVisibleNetworkList(&networks);

[Jói, 2013/10/19 21:14:45]

Why do you need to do this before notifying that the list has changed, and why
do you notify only on success? A comment might be helpful.

[mef, 2013/10/21 15:37:16]

Done. Added a comment. Not sure what would a good action be if
GetVisibleNetworkList returns an error. DLOG? DCHECK?

[Jói, 2013/10/21 16:44:03]

DCHECK seems right if you expect this to always succeed.

[mef, 2013/10/21 19:42:00]

Done.

+  if (error == ERROR_SUCCESS)
+    NotifyNetworkListChanged(networks);
+}
+
+void WiFiServiceImpl::WaitForNetworkConnect(const std::string& network_guid,
+                                            int attempt) {
+  if (attempt > kMaxAttempts) {
+    enable_notify_network_changed_ = true;

[Jói, 2013/10/19 21:14:45]

Why is this is set to true when max attempts have been reached?

[mef, 2013/10/21 15:37:16]

At this point we don't expect this network to get connected, so we need to
re-enable automatic notifications.

+    return;
+  }
+  std::string connected_network_guid;
+  DWORD error = FindConnectedNetwork(&connected_network_guid);
+  if (network_guid == connected_network_guid) {
+    DLOG(INFO) << "WiFi Connected, Reset DHCP: " << network_guid;
+    enable_notify_network_changed_ = true;
+    // Reset DHCP to speed up the connection after Chromekey factory reset.

[Jói, 2013/10/19 21:14:45]

Same question on whether code is specific to Chromecast etc.

[mef, 2013/10/21 15:37:16]

Done. Added better comment.

+    error = ResetDHCP();
+    if (error == ERROR_SUCCESS) {
+      base::MessageLoop::current()->PostDelayedTask(

[Jói, 2013/10/19 21:14:45]

Why is this delayed if resetting DHCP was successful?

[mef, 2013/10/21 15:37:16]

I've added comment, but would love to change it to something more deterministic
if possible (is there some notification that I can subscribe to?).

[Jói, 2013/10/21 16:44:03]

I'm not aware of a notification you could subscribe to. I guess you could check
the adapter in the delayed task to see if it has an IP address, if not, delay
more, up to some maximum.

[mef, 2013/10/21 19:42:00]

Sounds good, I'll try to find appropriate API.

[mef, 2013/10/22 20:06:01]

Done. Upon further testing it turned that IP Address is renewed upon return from
IpRenewAddress, and previously observed flakiness which I've attributed to
address renewal delay was actually caused by out-of-band network change
notifications. 

+          FROM_HERE,
+          base::Bind(&WiFiServiceImpl::NotifyNetworkChanged,
+                     base::Unretained(this),
+                     network_guid),
+          base::TimeDelta::FromMilliseconds(kDhcpRenewDelayMs));
+    } else {
+      NotifyNetworkChanged(network_guid);
+    }
+  } else {
+    // Continue waiting for network connection state change.
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&WiFiServiceImpl::WaitForNetworkConnect,
+                   base::Unretained(this),
+                   network_guid,
+                   ++attempt),
+        base::TimeDelta::FromMilliseconds(kAttemptDelayMs));
+  }
+}
+
+bool WiFiServiceImpl::CheckError(const ErrorCallback& error_callback,
+                                 const std::string& error_name,
+                                 DWORD error_code) const {
+  if (error_code != ERROR_SUCCESS) {
+    DLOG(ERROR) << "WiFiService Error " << error_code << ": " << error_name;
+    scoped_ptr<base::DictionaryValue> error_data(new base::DictionaryValue);
+    error_data->SetInteger("Win32ErrorCode", error_code);
+    error_callback.Run(error_name, error_data.Pass());
+    return true;
+  }
+  return false;
+}
+
+WiFiService::NetworkList::const_iterator WiFiServiceImpl::FindNetwork(
+    const WiFiService::NetworkList& networks,
+    const std::string& network_guid) const {
+  for (NetworkList::const_iterator it = networks.begin(); it != networks.end();
+       ++it) {
+    if (it->guid == network_guid)
+      return it;
+  }
+  return networks.end();
+}
+
+DWORD WiFiServiceImpl::SaveCurrentConnectedNetwork(
+    std::string* connected_network_guid) {
+  // Find currently connected network.
+  DWORD error = FindConnectedNetwork(connected_network_guid);
+  if (error == ERROR_SUCCESS && !connected_network_guid->empty()) {
+    if (error == ERROR_SUCCESS) {
+      SaveTempProfile(*connected_network_guid);
+      std::string profile_xml;
+      error = GetProfile(*connected_network_guid, &profile_xml);
+      if (error == ERROR_SUCCESS) {
+        saved_profiles_xml_[*connected_network_guid] = profile_xml;
+      }
+    }
+  }
+  return error;
+}
+
+void WiFiServiceImpl::SortNetworks(NetworkList* networks) {
+  networks->sort(WiFiService::NetworkProperties::OrderByType);
+}
+
+DWORD WiFiServiceImpl::OpenClientHandle() {
+  CloseClientHandle();
+
+  DWORD error = ERROR_SUCCESS;
+  DWORD service_version = 0;
+
+  // Open a handle to the service.
+  error = WlanOpenHandle(WLAN_API_VERSION, NULL, &service_version, &client_);
+

[afontan, 2013/10/21 16:55:23]

I assume using WLAN_API_VERSION is fine (given that we only use 1.0 methods) but
in the Chromecast setup we use the same version 1.0 for all windows OS (although
we only support win 7 and 8). This code uses 2.0 for win 7/8 but you then you do
not use the negotiated version to know what API is being used.

[mef, 2013/10/21 19:42:00]

Could you elaborate on suggested change? Also, I've tried Chromecast APP on
Windows XP and it didn't seem to work (at least on my VM). Is this expected?

[afontan, 2013/10/21 20:51:02]

Correct. Today we only support win 7/8. For vista or XP today we send the users
to manual web setup from the install site (where they manually connect to
Chromecast). It would be great if you could do better as Chrome supports
XP/Vista so it would be more consistent but the main issue with supporting XP is
that the wireless drivers out there often do things very inconsistently.

What I meant is that I believe that the WLAN_API_VERSION macro will
automatically expand to 2 in > Vista. The service_version will tell you exactly
the API version that was negotiated. You are not using the API negotiated value
to take decisions, as implicitly you are only using APIs supported in both 1 and
2.

In that case why not just ask for API 1? otherwise if you ever use an API that
does not exist in version 1, you will break XP (unless you consider the value
returned in service_version). So the question is, if you only use the version 1
APIs anyway, should you still request version 2 for win7/8? In the chromecast
setup we ask for API 1 but not sure that is the best option as I am not sure if
there are some behavior changes between 1/2 even for the same set of APIs, if
there is no behavior change it would make sense to request version 1.

http://msdn.microsoft.com/en-us/library/windows/desktop/ms706759(v=vs.85).aspx

Note, I just wanted to point it out so you take the decision with all the
information or can dig deeper to understand what is best in your scenario, it is
a difference with our current implementation and makes sense to understand the
trade-offs.

[mef, 2013/10/22 20:06:01]

Done.

+  PWLAN_INTERFACE_INFO_LIST pIntfList = NULL;

[Jói, 2013/10/19 21:14:45]

pIntfList is non-compliant naming; interface_list, perhaps?

[mef, 2013/10/21 15:37:16]

Done.

+  if (error == ERROR_SUCCESS) {
+    // Enumerate wireless interfaces.
+    error = WlanEnumInterfaces(client_, NULL, &pIntfList);
+    if (error == ERROR_SUCCESS) {
+      if (pIntfList != NULL && pIntfList->dwNumberOfItems != 0) {
+        // Remember first interface.

[Jói, 2013/10/19 21:14:45]

What is special about the first interface, is this just so that we have some
GUID if none of the interfaces turn out to be connected?

[mef, 2013/10/21 15:37:16]

Yes.

+        interface_guid_ = pIntfList->InterfaceInfo[0].InterfaceGuid;
+        // Try to find connected interface.

[Jói, 2013/10/19 21:14:45]

nit: find connected -> find a connected

[mef, 2013/10/21 15:37:16]

Done.

+        for (DWORD itf = 0; itf < pIntfList->dwNumberOfItems; ++itf) {
+          if (pIntfList->InterfaceInfo[itf].isState ==
+              wlan_interface_state_connected) {
+            // Found connected interface, remember it!
+            interface_guid_ = pIntfList->InterfaceInfo[itf].InterfaceGuid;

[afontan, 2013/10/21 16:55:23]

What if we want to setup with other wifi interfaces because they already have
the profile for the network requested (or if the first failed and you want to
try to setup with the second)? It is not common having more than one wifi but
could happen. Using only the first interface or the one connected looks more
like a decision that could be taken at application level. In the current
chromecast setup I think we try to find the interface that has the specific
profile we look for.

[mef, 2013/10/21 19:42:00]

Interesting. I suppose this loop could potentially select 5GHz-only adapter and
will be unable to connect to 2.4GHz-only ChromeCast device. Should I use
WlanQueryInterface with 
wlan_intf_opcode_channel_number opcode to select only 2.4 GHz interface
regardless of its connection state?

[afontan, 2013/10/21 20:51:02]

The issue is that we decided to try to setup first from the 5GHz network as if
it is successful we know the household is good to go. If it fails, then we
connect to the 2.4GHz. If that fails then it is a total failure but if it
succeeds it is just a partial success as the user may experience issues with
devices that connect to 5GHz (macbooks prefer it, Android prefers it too) so we
notify them and send them to a page that explains the isolation configuration in
detail.

We did this because we found that some home routers had this issue where they
named the 5GHz and 2.4GHz with the same SSID but the networks were isolated.
Note that this type of setup procedure is only possible in Windows, MAC and
Android can't control the band they want to connect to.

+            break;
+          }
+        }
+        WlanRegisterNotification(client_,
+                                 WLAN_NOTIFICATION_SOURCE_ALL,
+                                 FALSE,
+                                 OnWlanNotificationCallback,
+                                 this,
+                                 NULL,
+                                 NULL);
+      } else {
+        error = ERROR_NOINTERFACE;
+      }
+    }
+    // Clean up.
+    if (pIntfList != NULL)
+      WlanFreeMemory(pIntfList);
+  }
+  return error;
+}
+
+DWORD WiFiServiceImpl::ResetDHCP() {
+  IP_ADAPTER_INDEX_MAP adapter_index_map = {0};
+  DWORD error = FindAdapterIndexMapByGuid(interface_guid_, &adapter_index_map);
+  if (error == ERROR_SUCCESS) {
+    error = IpReleaseAddress(&adapter_index_map);
+    if (error == ERROR_SUCCESS) {
+      error = IpRenewAddress(&adapter_index_map);
+    }
+  }
+  return error;
+}
+
+DWORD WiFiServiceImpl::FindAdapterIndexMapByGuid(
+    const GUID& interface_guid,
+    IP_ADAPTER_INDEX_MAP* adapter_index_map) {
+  string16 guid_string;
+  const int kGUIDSize = 39;

[Jói, 2013/10/19 21:14:45]

GUID vs. Guid -> be consistent through your code.

[mef, 2013/10/21 15:37:16]

Done.

+  ::StringFromGUID2(
+      interface_guid, WriteInto(&guid_string, kGUIDSize), kGUIDSize);
+
+  ULONG buffer_length = 0;
+  DWORD error = GetInterfaceInfo(NULL, &buffer_length);
+  if (error == ERROR_INSUFFICIENT_BUFFER) {
+    scoped_ptr<unsigned char[]> buffer(new unsigned char[buffer_length]);
+    IP_INTERFACE_INFO* interface_info =
+        reinterpret_cast<IP_INTERFACE_INFO*>(buffer.get());
+    error = GetInterfaceInfo(interface_info, &buffer_length);
+    if (error == ERROR_SUCCESS) {
+      for (int adapter = 0; adapter < interface_info->NumAdapters; ++adapter) {
+        if (EndsWith(
+                interface_info->Adapter[adapter].Name, guid_string, false)) {
+          *adapter_index_map = interface_info->Adapter[adapter];
+          break;
+        }
+      }
+    }
+  }
+  return error;
+}
+
+DWORD WiFiServiceImpl::EnsureInitialized() {
+  DCHECK(CalledOnValidThread());
+  if (task_runner_.get() == NULL)
+    task_runner_ = base::MessageLoopProxy::current();
+
+  if (client_ != NULL)

[Jói, 2013/10/19 21:14:45]

Suggest leading with this (right after the DCHECK) since it's the
already-initialized case.

[mef, 2013/10/21 15:37:16]

Done.

+    return ERROR_SUCCESS;
+  return OpenClientHandle();
+}
+
+DWORD WiFiServiceImpl::CloseClientHandle() {
+  DWORD error = ERROR_SUCCESS;
+  if (client_ != NULL) {
+    WlanCloseHandle(client_, NULL);

[Jói, 2013/10/19 21:14:45]

should be:  error = WlanCloseHandle(client_, NULL);

[mef, 2013/10/21 15:37:16]

Done.

+    client_ = NULL;
+  }
+  return error;
+}
+
+DOT11_SSID WiFiServiceImpl::SsidFromGuid(
+    const std::string& network_guid) const {
+  DOT11_SSID ssid = {0};
+  if (network_guid.length() <= DOT11_SSID_MAX_LENGTH) {

[Jói, 2013/10/19 21:14:45]

Should there be e.g. a DCHECK in case the length is more than that?

[mef, 2013/10/21 15:37:16]

Done.

+    ssid.uSSIDLength = network_guid.length();
+    strncpy(reinterpret_cast<char*>(ssid.ucSSID),
+            network_guid.c_str(),
+            ssid.uSSIDLength);
+  }
+  return ssid;
+}
+
+std::string WiFiServiceImpl::SecurityFromDot11AuthAlg(
+    DOT11_AUTH_ALGORITHM alg) const {
+  // TODO(mef): Figure out correct mapping.
+  switch (alg) {
+    case DOT11_AUTH_ALGO_RSNA:
+      return onc::wifi::kWPA_EAP;
+    case DOT11_AUTH_ALGO_RSNA_PSK:
+      return onc::wifi::kWPA_PSK;
+    case DOT11_AUTH_ALGO_80211_SHARED_KEY:
+      return onc::wifi::kWEP_PSK;
+    case DOT11_AUTH_ALGO_80211_OPEN:
+      return onc::wifi::kNone;
+    default:
+      return onc::wifi::kWPA_EAP;
+  }
+  return onc::wifi::kWPA_EAP;

[Jói, 2013/10/19 21:14:45]

This line is not needed, since you already have a return from the default case.

[mef, 2013/10/21 15:37:16]

Done.

+}
+
+void WiFiServiceImpl::NetworkPropertiesFromAvailableNetwork(
+    const WLAN_AVAILABLE_NETWORK& wlan,
+    const WLAN_BSS_LIST& wlan_bss_list,
+    NetworkProperties* properties) {
+  if (wlan.dwFlags & WLAN_AVAILABLE_NETWORK_CONNECTED) {
+    properties->connection_state = onc::connection_state::kConnected;
+  } else {
+    properties->connection_state = onc::connection_state::kNotConnected;
+  }
+
+  properties->ssid = SsidFromWlan(wlan);
+  properties->name = properties->ssid;
+  properties->guid = GuidFromWlan(wlan);
+  properties->type = onc::network_type::kWiFi;
+
+  for (size_t bss = 0; bss < wlan_bss_list.dwNumberOfItems; ++bss) {
+    const WLAN_BSS_ENTRY& bss_entry(wlan_bss_list.wlanBssEntries[bss]);
+    if (bss_entry.dot11Ssid.uSSIDLength == wlan.dot11Ssid.uSSIDLength &&
+        0 == memcmp(bss_entry.dot11Ssid.ucSSID,
+                    wlan.dot11Ssid.ucSSID,
+                    bss_entry.dot11Ssid.uSSIDLength)) {
+      if (bss_entry.ulChCenterFrequency < 3000000)
+        properties->frequency = kFrequency2400;
+      else
+        properties->frequency = kFrequency5000;
+      properties->frequency_list.push_back(properties->frequency);
+      properties->bssid = WiFiService::NetworkProperties::MacAddressAsString(

[Jói, 2013/10/19 21:14:45]

Is the WiFiService:: prefix needed here?

[mef, 2013/10/21 15:37:16]

Done.

+          bss_entry.dot11Bssid);
+    }
+  }
+  properties->frequency_list.sort();
+  properties->frequency_list.unique();
+  properties->security =
+      SecurityFromDot11AuthAlg(wlan.dot11DefaultAuthAlgorithm);
+  properties->signal_strength = wlan.wlanSignalQuality;
+}
+
+// Get the list of visible wireless networks
+DWORD WiFiServiceImpl::GetVisibleNetworkList(NetworkList* network_list) {
+  DWORD error = ERROR_SUCCESS;
+
+  if (client_ == NULL) {

[Jói, 2013/10/19 21:14:45]

Should there be a NOTREACHED() here to help you catch incorrect usage in debug
builds?

[mef, 2013/10/21 15:37:16]

Done.

[Jói, 2013/10/21 16:44:03]

You still need a NOTREACHED() in EnsureInitialized when client_ is NULL.

[mef, 2013/10/21 19:42:00]

Should I add explicit 'Initialize()' method as currently there isn't one?

+    return ERROR_NOINTERFACE;
+  }
+
+  PWLAN_AVAILABLE_NETWORK_LIST pVList = NULL;

[Jói, 2013/10/19 21:14:45]

pVList -> network_list (for example)

[mef, 2013/10/21 15:37:16]

Done.

+  PWLAN_BSS_LIST pWlanBssList = NULL;

[Jói, 2013/10/19 21:14:45]

pWlanBssList -> bss_list (for example)

[mef, 2013/10/21 15:37:16]

Done.

+
+  error =

[Jói, 2013/10/19 21:14:45]

nit: I think it would be more common to break after the ( in this situation.

[mef, 2013/10/21 15:37:16]

Done.

+      WlanGetAvailableNetworkList(client_, &interface_guid_, 0, NULL, &pVList);
+
+  if (error == ERROR_SUCCESS && NULL != pVList) {
+    error = WlanGetNetworkBssList(client_,
+                                  &interface_guid_,
+                                  NULL,
+                                  dot11_BSS_type_any,
+                                  FALSE,
+                                  NULL,
+                                  &pWlanBssList);
+    if (error == ERROR_SUCCESS && NULL != pWlanBssList) {
+      for (DWORD i = 0; i < pVList->dwNumberOfItems; ++i) {
+        network_list->push_back(NetworkProperties());
+        NetworkPropertiesFromAvailableNetwork(
+            pVList->Network[i], *pWlanBssList, &network_list->back());
+      }
+    }
+  }
+
+  // clean up
+  if (pVList != NULL) {
+    WlanFreeMemory(pVList);
+  }
+  if (pWlanBssList != NULL) {
+    WlanFreeMemory(pWlanBssList);
+  }
+  return error;
+}
+
+// Find currently connected network.
+DWORD WiFiServiceImpl::FindConnectedNetwork(
+    std::string* connected_network_guid) {
+  DWORD error = ERROR_SUCCESS;
+
+  if (client_ == NULL) {
+    return ERROR_NOINTERFACE;

[Jói, 2013/10/19 21:14:45]

NOTREACHED() ?

[mef, 2013/10/21 15:37:16]

Done.

+  }
+
+  PWLAN_AVAILABLE_NETWORK_LIST pVList = NULL;

[Jói, 2013/10/19 21:14:45]

pVList -> network_list (for example)

[mef, 2013/10/21 15:37:16]

Done.

+
+  error =
+      WlanGetAvailableNetworkList(client_, &interface_guid_, 0, NULL, &pVList);

[Jói, 2013/10/19 21:14:45]

same nit re indenting; more common to indent right after (

[mef, 2013/10/21 15:37:16]

Done.

[afontan, 2013/10/21 16:55:23]

Flags is currently 0 but you should include hidden profiles.

[mef, 2013/10/21 19:42:00]

Done. But why?

[afontan, 2013/10/21 20:51:02]

Because the user may have configured a hidden nerwork that has no public SSID,
we do support this scenario where they need to type the SSID manually.

+
+  if (error == ERROR_SUCCESS && NULL != pVList) {
+    for (DWORD i = 0; i < pVList->dwNumberOfItems; ++i) {
+      const WLAN_AVAILABLE_NETWORK& wlan = pVList->Network[i];
+      if (wlan.dwFlags & WLAN_AVAILABLE_NETWORK_CONNECTED) {
+        *connected_network_guid = GuidFromWlan(wlan);
+        break;
+      }
+    }
+  }
+
+  // clean up
+  if (pVList != NULL) {
+    WlanFreeMemory(pVList);
+  }
+
+  return error;
+}
+
+DWORD WiFiServiceImpl::Connect(const std::string& network_guid) {

[afontan, 2013/10/21 16:55:23]

One scenario we had to cover based on dogfooding was to connect to a network
with a SSID using the 5GHz profile (assuming the same SSID supports both 2.4 and
5 GHz), then try to setup Chromecast.

If we are unable, it means that the user has isolation between 2.4 and 5GHz. At
that point we need to connect to 2.4GHz and if we are successful, still provide
a warning to the user that he may not be able to use Chromecast properly (for
example if the laptop decides to use 5GHz).

For this scenario we need to be able to specify the specific WLAN_BSS_ENTRY. 

[mef, 2013/10/21 19:42:00]

Interesting. Could you elaborate on this? Currently networkingPrivate API
doesn't 
provide parameters to specify BSSID or encryption key/password. 
Adding tbarzic@ for extension knowledge.

I could make network_guid to correspond to every BSSID, but that would mean that
in the office I get 12 GoogleGuests to choose from.

[tbarzic, 2013/10/21 20:13:29]

On ChromeOs we can't connect to a specific profile. The best we can do is to get
the list of frequencies for a network and the one that is used. The extension
has different behaviour when setup is done based on this information.

[afontan, 2013/10/21 20:51:02]

We get a list of band filtered Bss entries that contains the 5GHz entries that
have the same SSID or are hidden networks and we use that list of Bss entries to
connect. In this way we will not connect to the 2.4GHz network even if the SSID
matches.

WlanConnect takes a PWLAN_CONNECTION_PARAMETERS that takes a desired bssid list:

http://msdn.microsoft.com/en-us/library/windows/desktop/ms706613(v=vs.85).aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/ms706851(v=vs.85).aspx

I assume you have access to the setup code as I see you have the DHCP
workaround, look for ConnectWithXML in wlanManager.

[mef, 2013/10/24 23:44:47]

Done. On separate CL.

+  DWORD error = ERROR_SUCCESS;
+
+  if (client_ == NULL) {
+    return ERROR_NOINTERFACE;

[Jói, 2013/10/19 21:14:45]

NOTREACHED() ?

Here and all remaining "return ERROR_NOINTERFACE" locations.

[mef, 2013/10/21 15:37:16]

Done.

+  }
+
+  base::string16 profile_name = ProfileNameFromGuid(network_guid);
+
+  if (HaveProfile(network_guid)) {
+    WLAN_CONNECTION_PARAMETERS wlan_params = {
+        wlan_connection_mode_profile, profile_name.c_str(), NULL,

[Jói, 2013/10/19 21:14:45]

Suggest one per line.

[mef, 2013/10/21 15:37:16]

Done.

+        NULL,                         dot11_BSS_type_any,   0};
+    error = ::WlanConnect(client_, &interface_guid_, &wlan_params, NULL);
+  } else {
+    DOT11_SSID ssid = SsidFromGuid(network_guid);
+    WLAN_CONNECTION_PARAMETERS wlan_params = {
+        wlan_connection_mode_discovery_unsecure, NULL, &ssid, NULL,

[Jói, 2013/10/19 21:14:45]

Suggest one per line.

[mef, 2013/10/21 15:37:16]

Done.

+        dot11_BSS_type_infrastructure,           0};
+    error = ::WlanConnect(client_, &interface_guid_, &wlan_params, NULL);

[Jói, 2013/10/19 21:14:45]

Note that per
http://msdn.microsoft.com/en-us/library/windows/desktop/ms706844(v=vs.85).aspx,
only the wlan_connection_mode_profile is supported on XP SP3 and older. Not 100%
clear whether we already dropped support for XP so thought I'd mention it.

[mef, 2013/10/21 15:37:16]

Yes. This and unsupported WlanGetNetworkBssList function break XP at this point.

[Jói, 2013/10/21 16:44:03]

This needs to be fixed before check-in. According to
https://support.google.com/chrome/answer/95411?hl=en we support XP SP2 and
newer.

For the dot11_BSS_type_infrastructure constant, it is probably just a matter of
not calling the function with that constant.

For the WlanGetNetworkBssList function though, since you are delay-loading a DLL
with the assumption that the function is there, this will break on old enough
versions of XP. The way to fix it is to use explicit LoadLibrary and do your own
conditional function binding. There are existing examples of this in the code I
think; search for GetProcAddress and you should find them.

[mef, 2013/10/21 19:42:00]

Sounds good to be on the safe side, although from
http://msdn.microsoft.com/en-us/library/windows/desktop/ms706735(v=vs.85).aspx
and my experiment on XP SP3 the function is there, it just returns
'ERROR_NOT_SUPPORTED'.

+  }
+
+  return error;
+}
+
+DWORD WiFiServiceImpl::Disconnect() {
+  DWORD error = ERROR_SUCCESS;
+
+  if (client_ == NULL) {
+    return ERROR_NOINTERFACE;
+  }
+
+  error = ::WlanDisconnect(client_, &interface_guid_, NULL);
+  return error;
+}
+
+DWORD WiFiServiceImpl::SaveTempProfile(const std::string& network_guid) {
+  DWORD error = ERROR_SUCCESS;
+
+  if (client_ == NULL) {
+    return ERROR_NOINTERFACE;
+  }
+
+  base::string16 profile_name = ProfileNameFromGuid(network_guid);
+
+  error = ::WlanSaveTemporaryProfile(
+      client_, &interface_guid_, profile_name.c_str(), NULL, 0, true, NULL);

[Jói, 2013/10/19 21:14:45]

Should the profile perhaps be saved per-user? See WLAN_PROFILE_USER for the
dwFlags parameter in
http://msdn.microsoft.com/en-us/library/windows/desktop/ms706778(v=vs.85).aspx

[mef, 2013/10/21 15:37:16]

Done.

[afontan, 2013/10/21 16:55:23]

In the current Chromecast implementation we save it for all users but I agree
that seems more reasonable to only save it for the current user.

[mef, 2013/10/21 19:42:00]

Done.

+  return error;
+}
+
+DWORD WiFiServiceImpl::GetProfile(const std::string& network_guid,
+                                  std::string* profile_xml) {
+  DWORD error = ERROR_SUCCESS;
+
+  if (client_ == NULL) {
+    return ERROR_NOINTERFACE;
+  }
+
+  base::string16 profile_name = ProfileNameFromGuid(network_guid);
+  LPWSTR str_profile_xml = NULL;
+  error = ::WlanGetProfile(client_,

[Jói, 2013/10/19 21:14:45]

Suggest using :: in front of all the Win32 APIs you're calling. Makes things
clearer.

[mef, 2013/10/21 15:37:16]

Done.

+                           &interface_guid_,
+                           profile_name.c_str(),
+                           NULL,
+                           &str_profile_xml,
+                           NULL,
+                           NULL);
+
+  if (error == ERROR_SUCCESS && str_profile_xml != NULL) {
+    *profile_xml = base::UTF16ToUTF8(str_profile_xml);
+  }
+  // clean up
+  if (str_profile_xml != NULL) {
+    WlanFreeMemory(str_profile_xml);
+  }
+
+  return error;
+}
+
+bool WiFiServiceImpl::HaveProfile(const std::string& network_guid) {
+  DWORD error = ERROR_SUCCESS;
+  std::string profile_xml;
+  return GetProfile(network_guid, &profile_xml) == ERROR_SUCCESS;
+}
+
+void WiFiServiceImpl::NotifyNetworkListChanged(const NetworkList& networks) {
+  if (network_list_changed_observer_.is_null())
+    return;
+
+  WiFiService::NetworkGuidList current_networks;
+  for (WiFiService::NetworkList::const_iterator it = networks.begin();
+       it != networks.end();
+       ++it) {
+    current_networks.push_back(it->guid);
+  }
+  network_list_changed_observer_.Run(current_networks);
+}
+
+void WiFiServiceImpl::NotifyNetworkChanged(const std::string& network_guid) {
+  if (enable_notify_network_changed_ && !networks_changed_observer_.is_null()) {
+    DLOG(INFO) << "NotifyNetworkChanged: " << network_guid;
+    WiFiService::NetworkGuidList changed_networks(1, network_guid);

[Jói, 2013/10/19 21:14:45]

Is the WiFiService:: prefix needed here?

[mef, 2013/10/21 15:37:16]

Done.

+    networks_changed_observer_.Run(changed_networks);
+  }
+}
+
+WiFiService* WiFiService::CreateService() { return new WiFiServiceImpl(); }
+
+}  // namespace wifi