Break extensions ProcessManager dependency on Runtime API

* Introduce a ProcessManagerObserver
* Make the Chrome extension RuntimeAPI implementation use it to send onStartup
  events.

BUG=354552
TEST=browser_tests *Extension*

Review URL: https://codereview.chromium.org/212603020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260478 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/runtime/runtime_api.cc

@@ -211,8 +211,16 @@ void RuntimeAPI::OnExtensionsReady() {
 
   registered_for_updates_ = true;
 
-  ExtensionSystem::Get(browser_context_)->extension_service()->
-      AddUpdateObserver(this);
+  ExtensionSystem* extension_system = ExtensionSystem::Get(browser_context_);
+  extension_system->extension_service()->AddUpdateObserver(this);
+
+  // RuntimeAPI is redirected in incognito, so |browser_context_| is never
+  // incognito. We don't observe incognito ProcessManagers but that is OK
+  // because we don't send onStartup events to incognito browser contexts.
+  DCHECK(!browser_context_->IsOffTheRecord());
+  // Some tests use partially constructed Profiles without a process manager.
+  if (extension_system->process_manager())
+    extension_system->process_manager()->AddObserver(this);
 }
 
 void RuntimeAPI::OnExtensionLoaded(const Extension* extension) {
@@ -264,6 +272,16 @@ void RuntimeAPI::OnExtensionUninstalled(const Extension* extension) {
   RuntimeEventRouter::OnExtensionUninstalled(profile, extension->id());
 }
 
+void RuntimeAPI::Shutdown() {
+  // ExtensionSystem deletes its ProcessManager during the Shutdown() phase, so
+  // the observer must be removed here and not in the RuntimeAPI destructor.
+  ProcessManager* process_manager =
+      ExtensionSystem::Get(browser_context_)->process_manager();
+  // Some tests use partially constructed Profiles without a process manager.
+  if (process_manager)
+    process_manager->RemoveObserver(this);
+}
+
 void RuntimeAPI::OnAppUpdateAvailable(const Extension* extension) {
   Profile* profile = Profile::FromBrowserContext(browser_context_);
   RuntimeEventRouter::DispatchOnUpdateAvailableEvent(
@@ -275,6 +293,10 @@ void RuntimeAPI::OnChromeUpdateAvailable() {
   RuntimeEventRouter::DispatchOnBrowserUpdateAvailableEvent(profile);
 }
 
+void RuntimeAPI::OnBackgroundHostStartup(const Extension* extension) {
+  RuntimeEventRouter::DispatchOnStartupEvent(browser_context_, extension->id());
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 // static

chrome/browser/extensions/api/runtime/runtime_api.h

@@ -12,6 +12,7 @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "extensions/browser/browser_context_keyed_api_factory.h"
+#include "extensions/browser/process_manager_observer.h"
 #include "extensions/browser/update_observer.h"
 
 class Profile;
@@ -33,7 +34,8 @@ class ExtensionHost;
 // its related incognito instance.
 class RuntimeAPI : public BrowserContextKeyedAPI,
                    public content::NotificationObserver,
-                   public extensions::UpdateObserver {
+                   public UpdateObserver,
+                   public ProcessManagerObserver {
  public:
   static BrowserContextKeyedAPIFactory<RuntimeAPI>* GetFactoryInstance();
 
@@ -57,11 +59,15 @@ class RuntimeAPI : public BrowserContextKeyedAPI,
   static const char* service_name() { return "RuntimeAPI"; }
   static const bool kServiceRedirectedInIncognito = true;
   static const bool kServiceIsNULLWhileTesting = true;
+  virtual void Shutdown() OVERRIDE;
 
   // extensions::UpdateObserver overrides:
   virtual void OnAppUpdateAvailable(const Extension* extension) OVERRIDE;
   virtual void OnChromeUpdateAvailable() OVERRIDE;
 
+  // ProcessManagerObserver implementation:
+  virtual void OnBackgroundHostStartup(const Extension* extension) OVERRIDE;
+
   content::BrowserContext* browser_context_;
 
   // True if we should dispatch the chrome.runtime.onInstalled event with

extensions/browser/process_manager.cc

@@ -14,7 +14,6 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/extensions/api/runtime/runtime_api.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/devtools_agent_host.h"
@@ -33,6 +32,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extensions_browser_client.h"
+#include "extensions/browser/process_manager_observer.h"
 #include "extensions/browser/view_type_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_messages.h"
@@ -282,6 +282,14 @@ const ProcessManager::ViewSet ProcessManager::GetAllViews() const {
   return result;
 }
 
+void ProcessManager::AddObserver(ProcessManagerObserver* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void ProcessManager::RemoveObserver(ProcessManagerObserver* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
 bool ProcessManager::CreateBackgroundHost(const Extension* extension,
                                           const GURL& url) {
   // Hosted apps are taken care of from BackgroundContentsService. Ignore them
@@ -771,8 +779,9 @@ void ProcessManager::CreateBackgroundHostsForProfileStartup() {
        ++extension) {
     CreateBackgroundHostForExtensionLoad(this, extension->get());
 
-    RuntimeEventRouter::DispatchOnStartupEvent(GetBrowserContext(),
-                                               (*extension)->id());
+    FOR_EACH_OBSERVER(ProcessManagerObserver,
+                      observer_list_,
+                      OnBackgroundHostStartup(*extension));
   }
   startup_background_hosts_created_ = true;
 

extensions/browser/process_manager.h

@@ -13,6 +13,7 @@
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "base/time/time.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -32,6 +33,7 @@ namespace extensions {
 
 class Extension;
 class ExtensionHost;
+class ProcessManagerObserver;
 
 // Manages dynamic state of running Chromium extensions. There is one instance
 // of this class per Profile. OTR Profiles have a separate instance that keeps
@@ -51,6 +53,10 @@ class ProcessManager : public content::NotificationObserver {
   typedef std::set<content::RenderViewHost*> ViewSet;
   const ViewSet GetAllViews() const;
 
+  // The typical observer interface.
+  void AddObserver(ProcessManagerObserver* observer);
+  void RemoveObserver(ProcessManagerObserver* observer);
+
   // Creates a new UI-less extension instance.  Like CreateViewHost, but not
   // displayed anywhere.  Returns false if no background host can be created,
   // for example for hosted apps and extensions that aren't enabled in
@@ -238,6 +244,8 @@ class ProcessManager : public content::NotificationObserver {
   ImpulseCallbackForTesting keepalive_impulse_callback_for_testing_;
   ImpulseCallbackForTesting keepalive_impulse_decrement_callback_for_testing_;
 
+  ObserverList<ProcessManagerObserver> observer_list_;
+
   base::WeakPtrFactory<ProcessManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ProcessManager);

extensions/browser/process_manager_observer.h

+// 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.
+
+#ifndef EXTENSIONS_BROWSER_PROCESS_MANAGER_OBSERVER_H_
+#define EXTENSIONS_BROWSER_PROCESS_MANAGER_OBSERVER_H_
+
+namespace extensions {
+class Extension;
+
+class ProcessManagerObserver {
+ public:
+  virtual ~ProcessManagerObserver() {}
+
+  // Called immediately after an extension background host is started.
+  virtual void OnBackgroundHostStartup(const Extension* extension) = 0;
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_BROWSER_PROCESS_MANAGER_OBSERVER_H_

extensions/extensions.gyp

@@ -317,6 +317,7 @@
         'browser/pref_names.h',
         'browser/process_manager.cc',
         'browser/process_manager.h',
+        'browser/process_manager_observer.h',
         'browser/process_map.cc',
         'browser/process_map.h',
         'browser/process_map_factory.cc',