Refactor code that defers extension background page loading

src/extensions depends on chrome::NOTIFICATION_PROFILE_CREATED to support deferred loading of extension background pages when the profile isn't ready yet. This is a layering violation.

* Remove Chrome concepts like "browser window ready" and "profile created" from ProcessManager. Introduce ProcessManagerDelegate with a general concept of deferring background page loading.
* Consolidate all the tricky Chrome-specific background page loading rules into ChromeProcessManagerDelegate. This keeps all the rules in one place. Annotate each block of special case code with the bug that inspired it.
* Extend unit test coverage for ProcessManager.

This will make it easier to eliminate chrome::NOTIFICATION_PROFILE_DESTROYED in ProcessManager in a later CL.

BUG=392658
TEST=unit_tests ProcessManagerTest, browser_tests ProcessManagerBrowserTest, manual

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

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

1 files changed, 42 insertions, 56 deletions

diff --git a/extensions/browser/process_manager.cc b/extensions/browser/process_manager.cc
index e6beddb..12afd7c 100644
--- a/extensions/browser/process_manager.cc
+++ b/extensions/browser/process_manager.cc
@@ -33,6 +33,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_delegate.h"
 #include "extensions/browser/process_manager_observer.h"
 #include "extensions/browser/view_type_utils.h"
 #include "extensions/common/constants.h"
@@ -245,8 +246,6 @@ ProcessManager::ProcessManager(BrowserContext* context,
                  content::NotificationService::AllSources());
   registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_CONNECTED,
                  content::NotificationService::AllSources());
-  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_CREATED,
-                 content::Source<BrowserContext>(original_context));
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
                  content::Source<BrowserContext>(context));
   if (context->IsOffTheRecord()) {
@@ -307,11 +306,14 @@ bool ProcessManager::CreateBackgroundHost(const Extension* extension,
                                           const GURL& url) {
   // Hosted apps are taken care of from BackgroundContentsService. Ignore them
   // here.
-  if (extension->is_hosted_app() ||
-      !ExtensionsBrowserClient::Get()->
-          IsBackgroundPageAllowed(GetBrowserContext())) {
+  if (extension->is_hosted_app())
+    return false;
+
+  // Don't create hosts if the embedder doesn't allow it.
+  ProcessManagerDelegate* delegate =
+      ExtensionsBrowserClient::Get()->GetProcessManagerDelegate();
+  if (delegate && !delegate->IsBackgroundPageAllowed(GetBrowserContext()))
     return false;
-  }
 
   // Don't create multiple background hosts for an extension.
   if (GetBackgroundHostForExtension(extension->id()))
@@ -620,16 +622,6 @@ void ProcessManager::CancelSuspend(const Extension* extension) {
   }
 }
 
-void ProcessManager::OnBrowserWindowReady() {
-  // If the extension system isn't ready yet the background hosts will be
-  // created via NOTIFICATION_EXTENSIONS_READY below.
-  ExtensionSystem* system = ExtensionSystem::Get(GetBrowserContext());
-  if (!system->ready().is_signaled())
-    return;
-
-  CreateBackgroundHostsForProfileStartup();
-}
-
 content::BrowserContext* ProcessManager::GetBrowserContext() const {
   return site_instance_->GetBrowserContext();
 }
@@ -648,15 +640,10 @@ void ProcessManager::Observe(int type,
                              const content::NotificationSource& source,
                              const content::NotificationDetails& details) {
   switch (type) {
-    case chrome::NOTIFICATION_EXTENSIONS_READY:
-    case chrome::NOTIFICATION_PROFILE_CREATED: {
-       // Don't load background hosts now if the loading should be deferred.
-       // Instead they will be loaded when a browser window for this profile
-       // (or an incognito profile from this profile) is ready.
-       if (DeferLoadingBackgroundHosts())
-         break;
-
-      CreateBackgroundHostsForProfileStartup();
+    case chrome::NOTIFICATION_EXTENSIONS_READY: {
+      // TODO(jamescook): Convert this to use ExtensionSystem::ready() instead
+      // of a notification.
+      MaybeCreateStartupBackgroundHosts();
       break;
     }
 
@@ -784,24 +771,24 @@ void ProcessManager::OnDevToolsStateChanged(
   }
 }
 
-void ProcessManager::CreateBackgroundHostsForProfileStartup() {
-  if (startup_background_hosts_created_ ||
-      !ExtensionsBrowserClient::Get()->
-          IsBackgroundPageAllowed(GetBrowserContext())) {
+void ProcessManager::MaybeCreateStartupBackgroundHosts() {
+  if (startup_background_hosts_created_)
     return;
-  }
 
-  const ExtensionSet& enabled_extensions =
-      ExtensionRegistry::Get(GetBrowserContext())->enabled_extensions();
-  for (ExtensionSet::const_iterator extension = enabled_extensions.begin();
-       extension != enabled_extensions.end();
-       ++extension) {
-    CreateBackgroundHostForExtensionLoad(this, extension->get());
+  // The embedder might disallow background pages entirely.
+  ProcessManagerDelegate* delegate =
+      ExtensionsBrowserClient::Get()->GetProcessManagerDelegate();
+  if (delegate && !delegate->IsBackgroundPageAllowed(GetBrowserContext()))
+    return;
 
-    FOR_EACH_OBSERVER(ProcessManagerObserver,
-                      observer_list_,
-                      OnBackgroundHostStartup(*extension));
-  }
+  // The embedder might want to defer background page loading. For example,
+  // Chrome defers background page loading when it is launched to show the app
+  // list, then triggers a load later when a browser window opens.
+  if (delegate &&
+      delegate->DeferCreatingStartupBackgroundHosts(GetBrowserContext()))
+    return;
+
+  CreateStartupBackgroundHosts();
   startup_background_hosts_created_ = true;
 
   // Background pages should only be loaded once. To prevent any further loads
@@ -810,14 +797,6 @@ void ProcessManager::CreateBackgroundHostsForProfileStartup() {
       ExtensionsBrowserClient::Get()->GetOriginalContext(GetBrowserContext());
   if (registrar_.IsRegistered(
           this,
-          chrome::NOTIFICATION_PROFILE_CREATED,
-          content::Source<BrowserContext>(original_context))) {
-    registrar_.Remove(this,
-                      chrome::NOTIFICATION_PROFILE_CREATED,
-                      content::Source<BrowserContext>(original_context));
-  }
-  if (registrar_.IsRegistered(
-          this,
           chrome::NOTIFICATION_EXTENSIONS_READY,
           content::Source<BrowserContext>(original_context))) {
     registrar_.Remove(this,
@@ -826,6 +805,21 @@ void ProcessManager::CreateBackgroundHostsForProfileStartup() {
   }
 }
 
+void ProcessManager::CreateStartupBackgroundHosts() {
+  DCHECK(!startup_background_hosts_created_);
+  const ExtensionSet& enabled_extensions =
+      ExtensionRegistry::Get(GetBrowserContext())->enabled_extensions();
+  for (ExtensionSet::const_iterator extension = enabled_extensions.begin();
+       extension != enabled_extensions.end();
+       ++extension) {
+    CreateBackgroundHostForExtensionLoad(this, extension->get());
+
+    FOR_EACH_OBSERVER(ProcessManagerObserver,
+                      observer_list_,
+                      OnBackgroundHostStartup(*extension));
+  }
+}
+
 void ProcessManager::OnBackgroundHostCreated(ExtensionHost* host) {
   DCHECK_EQ(GetBrowserContext(), host->browser_context());
   background_hosts_.insert(host);
@@ -890,12 +884,6 @@ void ProcessManager::ClearBackgroundPageData(const std::string& extension_id) {
   }
 }
 
-bool ProcessManager::DeferLoadingBackgroundHosts() const {
-  // The extensions embedder may have special rules about background hosts.
-  return ExtensionsBrowserClient::Get()->DeferLoadingBackgroundHosts(
-      GetBrowserContext());
-}
-
 //
 // IncognitoProcessManager
 //
@@ -914,8 +902,6 @@ IncognitoProcessManager::IncognitoProcessManager(
   // in the NOTIFICATION_BROWSER_WINDOW_READY notification handler.
   registrar_.Remove(this, chrome::NOTIFICATION_EXTENSIONS_READY,
                     content::Source<BrowserContext>(original_context));
-  registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_CREATED,
-                    content::Source<BrowserContext>(original_context));
 }
 
 bool IncognitoProcessManager::CreateBackgroundHost(const Extension* extension,