Remove NotificationService usage from WakeOnWifiManager.

WakeOnWifiConnectObserver seems like a good candidate to be converted
to a KeyedService, but its lifetime doesn't match up with Profile
lifetime very well (particularly due to being destroyed early in some
cases).

It's not necessary to listen for profile destruction because normal
profiles aren't destroyed until browser shutdown, by which time
WakeOnWifiManager is gone. OTR profiles aren't observed since they
are never added to the ProfileManager.

Bug: 268984
Change-Id: Ie669931e04661363b4080c329e708beeb00ba53c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1867124
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707528}

chrome/browser/chromeos/net/wake_on_wifi_connection_observer.h

@@ -9,7 +9,6 @@
 #include "base/macros.h"
 #include "chrome/browser/chromeos/net/wake_on_wifi_manager.h"
 #include "components/gcm_driver/gcm_connection_observer.h"
-#include "content/public/browser/notification_observer.h"
 #include "net/base/ip_endpoint.h"
 
 class Profile;
@@ -23,6 +22,10 @@ class NetworkDeviceHandler;
 // associated with a profile.
 class WakeOnWifiConnectionObserver : public gcm::GCMConnectionObserver {
  public:
+  WakeOnWifiConnectionObserver(Profile* profile,
+                               bool wifi_properties_received,
+                               WakeOnWifiManager::WakeOnWifiFeature feature,
+                               NetworkDeviceHandler* network_device_handler);
   ~WakeOnWifiConnectionObserver() override;
 
   // Handles the case when the wifi properties have been received along with
@@ -45,12 +48,6 @@ class WakeOnWifiConnectionObserver : public gcm::GCMConnectionObserver {
                            TestWakeOnWifiPacketRemove);
   FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiNoneAdd);
   FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiNoneRemove);
-  friend class WakeOnWifiManager;
-
-  WakeOnWifiConnectionObserver(Profile* profile,
-                               bool wifi_properties_received,
-                               WakeOnWifiManager::WakeOnWifiFeature feature,
-                               NetworkDeviceHandler* network_device_handler);
 
   void AddWakeOnPacketConnection();
   void RemoveWakeOnPacketConnection();

chrome/browser/chromeos/net/wake_on_wifi_manager.cc

@@ -13,10 +13,11 @@
 #include "base/memory/ptr_util.h"
 #include "base/system/sys_info.h"
 #include "base/values.h"
-#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/net/wake_on_wifi_connection_observer.h"
 #include "chrome/browser/gcm/gcm_profile_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "chromeos/login/login_state/login_state.h"
 #include "chromeos/network/device_state.h"
@@ -27,8 +28,6 @@
 #include "components/gcm_driver/gcm_driver.h"
 #include "components/gcm_driver/gcm_profile_service.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_source.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
@@ -94,12 +93,7 @@ WakeOnWifiManager::WakeOnWifiManager()
 
   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());
+  g_browser_process->profile_manager()->AddObserver(this);
 
   NetworkHandler::Get()->network_state_handler()->AddObserver(this, FROM_HERE);
 
@@ -107,13 +101,13 @@ WakeOnWifiManager::WakeOnWifiManager()
 }
 
 WakeOnWifiManager::~WakeOnWifiManager() {
-  DCHECK(g_wake_on_wifi_manager);
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  g_browser_process->profile_manager()->RemoveObserver(this);
   if (current_feature_ != NOT_SUPPORTED) {
     NetworkHandler::Get()->network_state_handler()->RemoveObserver(this,
                                                                    FROM_HERE);
   }
-  g_wake_on_wifi_manager = NULL;
+  DCHECK_EQ(this, g_wake_on_wifi_manager);
+  g_wake_on_wifi_manager = nullptr;
 }
 
 void WakeOnWifiManager::OnPreferenceChanged(
@@ -141,21 +135,21 @@ bool WakeOnWifiManager::WakeOnWifiSupported() {
   return current_feature_ != NOT_SUPPORTED && current_feature_ != INVALID;
 }
 
-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) {
+  auto result = connection_observers_.find(profile);
+
+  // Only add the profile if it is not already present.
+  if (result != connection_observers_.end())
+    return;
+
+  connection_observers_[profile] =
+      std::make_unique<WakeOnWifiConnectionObserver>(
+          profile, wifi_properties_received_, current_feature_,
+          NetworkHandler::Get()->network_device_handler());
+
+  gcm::GCMProfileServiceFactory::GetForProfile(profile)
+      ->driver()
+      ->WakeFromSuspendForHeartbeat(IsWakeOnPacketEnabled(current_feature_));
 }
 
 void WakeOnWifiManager::DeviceListChanged() {
@@ -222,8 +216,8 @@ void WakeOnWifiManager::GetDevicePropertiesCallback(
     connection_observers_.clear();
     NetworkHandler::Get()->network_state_handler()->RemoveObserver(this,
                                                                    FROM_HERE);
-    registrar_.RemoveAll();
     extension_event_observer_.reset();
+    g_browser_process->profile_manager()->RemoveObserver(this);
 
     return;
   }
@@ -249,26 +243,4 @@ void WakeOnWifiManager::GetDevicePropertiesCallback(
   }
 }
 
-void WakeOnWifiManager::OnProfileAdded(Profile* profile) {
-  auto result = connection_observers_.find(profile);
-
-  // Only add the profile if it is not already present.
-  if (result != connection_observers_.end())
-    return;
-
-  connection_observers_[profile] =
-      base::WrapUnique(new WakeOnWifiConnectionObserver(
-          profile, wifi_properties_received_, current_feature_,
-          NetworkHandler::Get()->network_device_handler()));
-
-  gcm::GCMProfileServiceFactory::GetForProfile(profile)
-      ->driver()
-      ->WakeFromSuspendForHeartbeat(
-          IsWakeOnPacketEnabled(current_feature_));
-}
-
-void WakeOnWifiManager::OnProfileDestroyed(Profile* profile) {
-  connection_observers_.erase(profile);
-}
-
 }  // namespace chromeos

chrome/browser/chromeos/net/wake_on_wifi_manager.h

@@ -7,13 +7,11 @@
 
 #include <map>
 
-#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/chromeos/power/extension_event_observer.h"
+#include "chrome/browser/profiles/profile_manager_observer.h"
 #include "chromeos/network/network_state_handler_observer.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 
 class Profile;
 
@@ -31,7 +29,7 @@ class WakeOnWifiConnectionObserver;
 // servers and sending that connection information down to shill.  This class is
 // owned by ChromeBrowserMainPartsChromeos.  This class is also NOT thread-safe
 // and should only be called on the UI thread.
-class WakeOnWifiManager : public content::NotificationObserver,
+class WakeOnWifiManager : public ProfileManagerObserver,
                           public NetworkStateHandlerObserver {
  public:
   enum WakeOnWifiFeature {
@@ -57,21 +55,14 @@ class WakeOnWifiManager : public content::NotificationObserver,
   // have not yet determined whether wake-on-wifi features are supported.
   bool WakeOnWifiSupported();
 
-  // content::NotificationObserver override.
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
+  // ProfileManagerObserver:
+  void OnProfileAdded(Profile* profile) override;
 
-  // NetworkStateHandlerObserver overrides.
+  // NetworkStateHandlerObserver:
   void DeviceListChanged() override;
   void DevicePropertiesUpdated(const DeviceState* device) override;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiPacketAdd);
-  FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiPacketRemove);
-  FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiNoneAdd);
-  FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiNoneRemove);
-
   // Sends the user's preference to shill, updates the timer used by the GCM
   // client to send heartbeats, and tells |extension_event_observer_| to block
   // (or not block) suspends based on the value of |current_feature_|.
@@ -84,10 +75,6 @@ class WakeOnWifiManager : public content::NotificationObserver,
   void GetDevicePropertiesCallback(const std::string& device_path,
                                    const base::DictionaryValue& properties);
 
-  // Called when a Profile is added or destroyed.
-  void OnProfileAdded(Profile* profile);
-  void OnProfileDestroyed(Profile* profile);
-
   WakeOnWifiFeature current_feature_;
 
   // Set to true once we have received the properties for the wifi device from
@@ -99,8 +86,6 @@ class WakeOnWifiManager : public content::NotificationObserver,
 
   std::unique_ptr<ExtensionEventObserver> extension_event_observer_;
 
-  content::NotificationRegistrar registrar_;
-
   base::WeakPtrFactory<WakeOnWifiManager> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(WakeOnWifiManager);