Clean up wake on wifi handling

chrome/browser/chromeos/net/wake_on_wifi_manager.cc

Index: chrome/browser/chromeos/net/wake_on_wifi_manager.cc
diff --git a/chrome/browser/chromeos/net/wake_on_wifi_manager.cc b/chrome/browser/chromeos/net/wake_on_wifi_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a8cceefc5247ab52dbc8da8c50003f5d5eb14369
--- /dev/null
+++ b/chrome/browser/chromeos/net/wake_on_wifi_manager.cc
@@ -0,0 +1,211 @@
+// Copyright (c) 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 "chrome/browser/chromeos/net/wake_on_wifi_manager.h"
+
+#include <string>
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/sys_info.h"
+#include "chrome/browser/browser_process.h"

[Lei Zhang, 2014/11/18 01:57:54]

not used?

[Chirantan Ekbote, 2014/11/18 20:37:39]

Done.

+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/services/gcm/gcm_profile_service.h"
+#include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
+#include "chromeos/login/login_state.h"
+#include "chromeos/network/device_state.h"
+#include "chromeos/network/network_device_handler.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_state_handler.h"
+#include "chromeos/network/network_type_pattern.h"
+#include "components/gcm_driver/gcm_connection_observer.h"
+#include "components/gcm_driver/gcm_driver.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+#include "net/base/ip_endpoint.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace chromeos {
+
+namespace {
+
+const char kWakeOnNone[] = "none";
+const char kWakeOnPacket[] = "packet";
+const char kWakeOnSsid[] = "ssid";
+const char kWakeOnPacketAndSsid[] = "packet_and_ssid";
+
+std::string WakeOnWifiFeatureToString(
+    chromeos::WakeOnWifiManager::WakeOnWifiFeature feature) {

[Lei Zhang, 2014/11/18 01:57:54]

no need for chromeos:: inside namespace chromeos.

[Chirantan Ekbote, 2014/11/18 20:37:39]

Done.

+  switch (feature) {
+    case chromeos::WakeOnWifiManager::WAKE_ON_NONE:
+      return kWakeOnNone;
+    case chromeos::WakeOnWifiManager::WAKE_ON_PACKET:
+      return kWakeOnPacket;
+    case chromeos::WakeOnWifiManager::WAKE_ON_SSID:
+      return kWakeOnSsid;
+    case chromeos::WakeOnWifiManager::WAKE_ON_PACKET_AND_SSID:
+      return kWakeOnPacketAndSsid;
+  }
+
+  NOTREACHED() << "Unknown wake on wifi feature: " << feature;
+  return std::string();
+}
+
+// Weak pointer.  This class is owned by ChromeBrowserMainPartsChromeos.
+chromeos::WakeOnWifiManager* g_wake_on_wifi_manager = NULL;
+
+}  // namespace
+
+// Simple class that listens for a connection to the GCM server and passes the
+// connection information down to shill.  Each profile gets its own instance of
+// this class.
+class WakeOnWifiManager::WakeOnPacketConnectionObserver
+    : public gcm::GCMConnectionObserver {
+ public:
+  explicit WakeOnPacketConnectionObserver(Profile* profile)
+      : profile_(profile),
+        ip_endpoint_(net::IPEndPoint()) {
+    gcm::GCMProfileServiceFactory::GetForProfile(profile_)
+        ->driver()
+        ->AddConnectionObserver(this);
+  }
+
+  ~WakeOnPacketConnectionObserver() override {
+    if (!(ip_endpoint_ == net::IPEndPoint()))
+      OnDisconnected();
+
+    gcm::GCMProfileServiceFactory::GetForProfile(profile_)
+        ->driver()
+        ->RemoveConnectionObserver(this);
+  }
+
+  // gcm::GCMConnectionObserver overrides.
+
+  void OnConnected(const net::IPEndPoint& ip_endpoint) override {
+    ip_endpoint_ = ip_endpoint;
+
+    NetworkHandler::Get()
+        ->network_device_handler()
+        ->AddWifiWakeOnPacketConnection(
+            ip_endpoint_,
+            base::Bind(&base::DoNothing),
+            network_handler::ErrorCallback());
+  }
+
+  void OnDisconnected() override {
+    if (ip_endpoint_ == net::IPEndPoint()) {

[Lei Zhang, 2014/11/18 01:57:54]

I wonder if net::IPEndPoint should have a valid() method, but that's a
discussion for a different CL.

[Chirantan Ekbote, 2014/11/18 20:37:39]

I had a look at this to see if it would be easy to do. 
net::IPEndPoint::ToString claims that it will return an empty string if the
address is invalid so that was promising.  But then I tracked it to
net_util::IPAddressToString
(https://code.google.com/p/chromium/codesearch#chromium/src/net/base/net_util....),
which simply checks if the address length matches an ipv4 or ipv6 length and
then hits a CHECK(false) if it doesn't match either. 
net::IPEndPoint::GetSockAddrFamily does the same thing.

So the takeaway is (1) the docstring for net::IPEndPoint::ToString is a dirty
liar and (2) the only way to check if an endpoint is valid is to see if the
address length matches an ipv4 or ipv6 address length.

+      LOG(WARNING) << "Received GCMConnectionObserver::OnDisconnected without "
+                   << "a valid IPEndPoint.";
+      return;
+    }
+
+    NetworkHandler::Get()
+        ->network_device_handler()
+        ->RemoveWifiWakeOnPacketConnection(
+            ip_endpoint_,
+            base::Bind(&base::DoNothing),
+            network_handler::ErrorCallback());
+
+    ip_endpoint_ = net::IPEndPoint();
+  }
+
+ private:
+  Profile* profile_;
+  net::IPEndPoint ip_endpoint_;
+
+  DISALLOW_COPY_AND_ASSIGN(WakeOnPacketConnectionObserver);
+};
+
+// static
+WakeOnWifiManager* WakeOnWifiManager::Get() {
+  DCHECK(g_wake_on_wifi_manager);
+  return g_wake_on_wifi_manager;
+}
+
+WakeOnWifiManager::WakeOnWifiManager() {
+  // This class must be constructed before any users are logged in, i.e., before
+  // any profiles are created or added to the ProfileManager.  Additionally,
+  // IsUserLoggedIn always returns true when we are not running on a chromebook
+  // so this check should only run on actual chromebooks.

[stevenjb, 2014/11/14 18:57:57]

nit: Use "Chrome OS device" or just "device" (since device can include
"chromebox" and potentially other designators)

[Chirantan Ekbote, 2014/11/18 01:54:35]

Done.

+  CHECK(!base::SysInfo::IsRunningOnChromeOS() ||
+        !LoginState::Get()->IsUserLoggedIn());
+  DCHECK(!g_wake_on_wifi_manager);
+  g_wake_on_wifi_manager = this;
+
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_PROFILE_ADDED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_PROFILE_DESTROYED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+
+  NetworkHandler::Get()
+      ->network_device_handler()
+      ->RemoveAllWifiWakeOnPacketConnections(
+          base::Bind(&base::DoNothing),
+          network_handler::ErrorCallback());
+}
+
+WakeOnWifiManager::~WakeOnWifiManager() {
+  DCHECK(g_wake_on_wifi_manager);
+  g_wake_on_wifi_manager = NULL;
+}
+
+void WakeOnWifiManager::OnPreferenceChanged(
+    WakeOnWifiManager::WakeOnWifiFeature feature) {
+  const DeviceState* device =
+      NetworkHandler::Get()->network_state_handler()->GetDeviceStateByType(
+          NetworkTypePattern::WiFi());
+  if (!device)
+    return;
+
+  std::string feature_string(WakeOnWifiFeatureToString(feature));
+  DCHECK(!feature_string.empty());
+
+  NetworkHandler::Get()->network_device_handler()->SetDeviceProperty(
+      device->path(),
+      shill::kWakeOnWiFiFeaturesEnabledProperty,
+      base::StringValue(feature_string),
+      base::Bind(&base::DoNothing),
+      network_handler::ErrorCallback());
+}
+
+void WakeOnWifiManager::Observe(int type,
+                                const content::NotificationSource& source,
+                                const content::NotificationDetails& details) {
+  switch (type) {
+    case chrome::NOTIFICATION_PROFILE_ADDED: {
+      OnProfileAdded(content::Source<Profile>(source).ptr());
+      break;
+    }
+    case chrome::NOTIFICATION_PROFILE_DESTROYED: {
+      OnProfileDestroyed(content::Source<Profile>(source).ptr());
+      break;
+    }
+    default:
+      NOTREACHED();
+  }
+}
+
+void WakeOnWifiManager::OnProfileAdded(Profile* profile) {
+  if (connection_observers_.find(profile) != connection_observers_.end())
+    return;
+
+  connection_observers_[profile] =
+      linked_ptr<WakeOnWifiManager::WakeOnPacketConnectionObserver>(
+          new WakeOnWifiManager::WakeOnPacketConnectionObserver(profile));
+}
+
+void WakeOnWifiManager::OnProfileDestroyed(Profile* profile) {
+  const auto iter = connection_observers_.find(profile);
+  if (iter == connection_observers_.end())
+    return;
+
+  connection_observers_.erase(iter);

[Lei Zhang, 2014/11/18 01:57:54]

As is, this entire function can just be: connection_observers_.erase(profile);

[Chirantan Ekbote, 2014/11/18 20:37:39]

Done.

+}
+
+}  // namespace chromeos