Remove NotificationService usage from ExtensionEventObserver.

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

chrome/browser/chromeos/power/extension_event_observer.cc

@@ -11,10 +11,10 @@
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/extensions/api/gcm.h"
-#include "content/public/browser/notification_service.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/common/extension.h"
@@ -61,26 +61,19 @@ struct ExtensionEventObserver::KeepaliveSources {
   std::set<uint64_t> pending_network_requests;
 };
 
-ExtensionEventObserver::ExtensionEventObserver()
-    : should_delay_suspend_(true), suspend_keepalive_count_(0) {
-  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
-                 content::NotificationService::AllBrowserContextsAndSources());
-  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
-                 content::NotificationService::AllBrowserContextsAndSources());
-
+ExtensionEventObserver::ExtensionEventObserver() {
   PowerManagerClient::Get()->AddObserver(this);
+  g_browser_process->profile_manager()->AddObserver(this);
 }
 
 ExtensionEventObserver::~ExtensionEventObserver() {
-  for (Profile* profile : active_profiles_)
-    extensions::ProcessManager::Get(profile)->RemoveObserver(this);
-
   for (const auto& pair : keepalive_sources_) {
     extensions::ExtensionHost* host =
         const_cast<extensions::ExtensionHost*>(pair.first);
     host->RemoveObserver(this);
   }
 
+  g_browser_process->profile_manager()->RemoveObserver(this);
   PowerManagerClient::Get()->RemoveObserver(this);
 }
 
@@ -101,22 +94,11 @@ void ExtensionEventObserver::SetShouldDelaySuspend(bool should_delay) {
   }
 }
 
-void ExtensionEventObserver::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 ExtensionEventObserver::OnProfileAdded(Profile* profile) {
+  // Add the observer when |profile| is added and ProcessManager is available as
+  // a keyed service. It will be removed when the ProcessManager instance is
+  // shut down (OnProcessManagerShutdown).
+  process_manager_observers_.Add(extensions::ProcessManager::Get(profile));
 }
 
 void ExtensionEventObserver::OnBackgroundHostCreated(
@@ -135,6 +117,11 @@ void ExtensionEventObserver::OnBackgroundHostCreated(
     host->AddObserver(this);
 }
 
+void ExtensionEventObserver::OnProcessManagerShutdown(
+    extensions::ProcessManager* manager) {
+  process_manager_observers_.Remove(manager);
+}
+
 void ExtensionEventObserver::OnExtensionHostDestroyed(
     const extensions::ExtensionHost* host) {
   auto it = keepalive_sources_.find(host);
@@ -217,20 +204,6 @@ void ExtensionEventObserver::SuspendDone(const base::TimeDelta& duration) {
   suspend_readiness_callback_.Cancel();
 }
 
-void ExtensionEventObserver::OnProfileAdded(Profile* profile) {
-  auto result = active_profiles_.insert(profile);
-
-  if (result.second)
-    extensions::ProcessManager::Get(profile)->AddObserver(this);
-}
-
-void ExtensionEventObserver::OnProfileDestroyed(Profile* profile) {
-  if (active_profiles_.erase(profile) == 0)
-    return;
-
-  extensions::ProcessManager::Get(profile)->RemoveObserver(this);
-}
-
 void ExtensionEventObserver::OnSuspendImminent(bool dark_suspend) {
   DCHECK(block_suspend_token_.is_empty())
       << "OnSuspendImminent called while previous suspend attempt "

chrome/browser/chromeos/power/extension_event_observer.h

@@ -16,14 +16,13 @@
 #include "base/cancelable_callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
 #include "base/time/time.h"
 #include "base/unguessable_token.h"
+#include "chrome/browser/profiles/profile_manager_observer.h"
 #include "chromeos/dbus/power/power_manager_client.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-#include "content/public/browser/notification_source.h"
 #include "extensions/browser/extension_host_observer.h"
+#include "extensions/browser/process_manager.h"
 #include "extensions/browser/process_manager_observer.h"
 
 class Profile;
@@ -39,7 +38,7 @@ namespace chromeos {
 // that arrive from Google's GCM servers and network requests initiated by
 // extensions while processing the push messages.  This class is owned by
 // WakeOnWifiManager.
-class ExtensionEventObserver : public content::NotificationObserver,
+class ExtensionEventObserver : public ProfileManagerObserver,
                                public extensions::ProcessManagerObserver,
                                public extensions::ExtensionHostObserver,
                                public PowerManagerClient::Observer {
@@ -75,15 +74,14 @@ class ExtensionEventObserver : public content::NotificationObserver,
   // ExtensionEventObserver should or should not delay the system suspend.
   void SetShouldDelaySuspend(bool should_delay);
 
-  // content::NotificationObserver override.
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
+  // ProfileManagerObserver:
+  void OnProfileAdded(Profile* profile) override;
 
-  // extensions::ProcessManagerObserver overrides.
+  // extensions::ProcessManagerObserver:
   void OnBackgroundHostCreated(extensions::ExtensionHost* host) override;
+  void OnProcessManagerShutdown(extensions::ProcessManager* manager) override;
 
-  // extensions::ExtensionHostObserver overrides.
+  // extensions::ExtensionHostObserver:
   void OnExtensionHostDestroyed(const extensions::ExtensionHost* host) override;
   void OnBackgroundEventDispatched(const extensions::ExtensionHost* host,
                                    const std::string& event_name,
@@ -95,7 +93,7 @@ class ExtensionEventObserver : public content::NotificationObserver,
   void OnNetworkRequestDone(const extensions::ExtensionHost* host,
                             uint64_t request_id) override;
 
-  // PowerManagerClient::Observer overrides.
+  // PowerManagerClient::Observer:
   void SuspendImminent(power_manager::SuspendImminent::Reason reason) override;
   void DarkSuspendImminent() override;
   void SuspendDone(const base::TimeDelta& duration) override;
@@ -103,10 +101,6 @@ class ExtensionEventObserver : public content::NotificationObserver,
  private:
   friend class TestApi;
 
-  // Called when a new profile is created or destroyed.
-  void OnProfileAdded(Profile* profile);
-  void OnProfileDestroyed(Profile* profile);
-
   // Called when the system is about to perform a regular suspend or a dark
   // suspend.
   void OnSuspendImminent(bool dark_suspend);
@@ -120,10 +114,11 @@ class ExtensionEventObserver : public content::NotificationObserver,
                      std::unique_ptr<KeepaliveSources>>
       keepalive_sources_;
 
-  std::set<Profile*> active_profiles_;
+  ScopedObserver<extensions::ProcessManager, extensions::ProcessManagerObserver>
+      process_manager_observers_{this};
 
-  bool should_delay_suspend_;
-  int suspend_keepalive_count_;
+  bool should_delay_suspend_ = true;
+  int suspend_keepalive_count_ = 0;
 
   // |this| blocks Power Manager suspend with this token. When the token is
   // empty, |this| isn't blocking suspend.
@@ -131,8 +126,6 @@ class ExtensionEventObserver : public content::NotificationObserver,
 
   base::CancelableClosure suspend_readiness_callback_;
 
-  content::NotificationRegistrar registrar_;
-
   base::WeakPtrFactory<ExtensionEventObserver> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionEventObserver);

chrome/browser/chromeos/power/extension_event_observer_unittest.cc

@@ -50,12 +50,13 @@ class ExtensionEventObserverTest : public ChromeRenderViewHostTestHarness {
     PowerManagerClient::InitializeFake();
     profile_manager_ = std::make_unique<TestingProfileManager>(
         TestingBrowserProcess::GetGlobal());
-    extension_event_observer_ = std::make_unique<ExtensionEventObserver>();
-    test_api_ = extension_event_observer_->CreateTestApi();
 
     // Must be called from ::testing::Test::SetUp.
     ASSERT_TRUE(profile_manager_->SetUp());
 
+    extension_event_observer_ = std::make_unique<ExtensionEventObserver>();
+    test_api_ = extension_event_observer_->CreateTestApi();
+
     const char kUserProfile[] = "profile1@example.com";
     const AccountId account_id(AccountId::FromUserEmail(kUserProfile));
     fake_user_manager_->AddUser(account_id);

extensions/browser/process_manager.cc

@@ -289,6 +289,9 @@ void ProcessManager::Shutdown() {
   DCHECK(background_hosts_.empty());
   content::DevToolsAgentHost::RemoveObserver(this);
   site_instance_ = nullptr;
+
+  for (auto& observer : observer_list_)
+    observer.OnProcessManagerShutdown(this);
 }
 
 void ProcessManager::RegisterRenderFrameHost(

extensions/browser/process_manager_observer.h

@@ -16,6 +16,7 @@ class RenderFrameHost;
 namespace extensions {
 class Extension;
 class ExtensionHost;
+class ProcessManager;
 
 class ProcessManagerObserver : public base::CheckedObserver {
  public:
@@ -41,6 +42,9 @@ class ProcessManagerObserver : public base::CheckedObserver {
   virtual void OnExtensionFrameUnregistered(
       const std::string& extension_id,
       content::RenderFrameHost* render_frame_host) {}
+
+  // Called when the observed ProcessManager is shutting down.
+  virtual void OnProcessManagerShutdown(ProcessManager* manager) {}
 };
 
 }  // namespace extensions