summaryrefslogtreecommitdiffstats
path: root/extensions
diff options
context:
space:
mode:
authorjamescook@chromium.org <jamescook@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-07-22 02:13:47 +0000
committerjamescook@chromium.org <jamescook@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-07-22 02:13:47 +0000
commit6b54fda654446046577e4b9c10c48eb61b022ebb (patch)
tree4707735b1e04dc6669d4baebc6e28dfa31f19c55 /extensions
parente25bbc6831ccc2d0a924c3b0d8ce79469beff32c (diff)
downloadchromium_src-6b54fda654446046577e4b9c10c48eb61b022ebb.zip
chromium_src-6b54fda654446046577e4b9c10c48eb61b022ebb.tar.gz
chromium_src-6b54fda654446046577e4b9c10c48eb61b022ebb.tar.bz2
Reland: 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. (Original CL https://codereview.chromium.org/381283002 broke valgrind bots because it was initializing left-over BrowserContextKeyedServices from tests running earlier in the same process.) BUG=392658 TEST=unit_tests ProcessManagerTest, browser_tests ProcessManagerBrowserTest, manual Review URL: https://codereview.chromium.org/408523005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284593 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'extensions')
-rw-r--r--extensions/DEPS3
-rw-r--r--extensions/browser/extension_registry.h2
-rw-r--r--extensions/browser/extensions_browser_client.h10
-rw-r--r--extensions/browser/lazy_background_task_queue_unittest.cc5
-rw-r--r--extensions/browser/process_manager.cc159
-rw-r--r--extensions/browser/process_manager.h43
-rw-r--r--extensions/browser/process_manager_delegate.h34
-rw-r--r--extensions/browser/process_manager_unittest.cc131
-rw-r--r--extensions/browser/test_extensions_browser_client.cc15
-rw-r--r--extensions/browser/test_extensions_browser_client.h14
-rw-r--r--extensions/extensions.gyp1
11 files changed, 291 insertions, 126 deletions
diff --git a/extensions/DEPS b/extensions/DEPS
index 0505b02..4077a55 100644
--- a/extensions/DEPS
+++ b/extensions/DEPS
@@ -1,4 +1,5 @@
include_rules = [
+ # Do not add Chrome dependencies. Much work went into removing them.
"+components/url_matcher",
"+content/public/common",
"+content/public/test",
@@ -13,7 +14,7 @@ include_rules = [
# NOTE: Please do not add includes without talking to the app shell team;
# see OWNERS for this directory.
#
- # TODO(jamescook): Remove these. http://crbug.com/162530
+ # TODO(jamescook): Remove this. http://crbug.com/392622
"!chrome/browser/chrome_notification_types.h",
]
diff --git a/extensions/browser/extension_registry.h b/extensions/browser/extension_registry.h
index 75724cd..d11111c 100644
--- a/extensions/browser/extension_registry.h
+++ b/extensions/browser/extension_registry.h
@@ -43,6 +43,8 @@ class ExtensionRegistry : public KeyedService {
// Returns the instance for the given |browser_context|.
static ExtensionRegistry* Get(content::BrowserContext* browser_context);
+ content::BrowserContext* browser_context() const { return browser_context_; }
+
// NOTE: These sets are *eventually* mututally exclusive, but an extension can
// appear in two sets for short periods of time.
const ExtensionSet& enabled_extensions() const {
diff --git a/extensions/browser/extensions_browser_client.h b/extensions/browser/extensions_browser_client.h
index 293b2af..cb810b6 100644
--- a/extensions/browser/extensions_browser_client.h
+++ b/extensions/browser/extensions_browser_client.h
@@ -41,6 +41,7 @@ class ExtensionPrefsObserver;
class ExtensionSystem;
class ExtensionSystemProvider;
class InfoMap;
+class ProcessManagerDelegate;
class RuntimeAPIDelegate;
// Interface to allow the extensions module to make browser-process-specific
@@ -133,12 +134,9 @@ class ExtensionsBrowserClient {
content::BrowserContext* context,
std::vector<ExtensionPrefsObserver*>* observers) const = 0;
- // Returns true if loading background pages should be deferred.
- virtual bool DeferLoadingBackgroundHosts(
- content::BrowserContext* context) const = 0;
-
- virtual bool IsBackgroundPageAllowed(
- content::BrowserContext* context) const = 0;
+ // Returns the ProcessManagerDelegate shared across all BrowserContexts. May
+ // return NULL in tests or for simple embedders.
+ virtual ProcessManagerDelegate* GetProcessManagerDelegate() const = 0;
// Creates a new ExtensionHostDelegate instance.
virtual scoped_ptr<ExtensionHostDelegate> CreateExtensionHostDelegate() = 0;
diff --git a/extensions/browser/lazy_background_task_queue_unittest.cc b/extensions/browser/lazy_background_task_queue_unittest.cc
index 0a991b4..bda663c 100644
--- a/extensions/browser/lazy_background_task_queue_unittest.cc
+++ b/extensions/browser/lazy_background_task_queue_unittest.cc
@@ -11,6 +11,7 @@
#include "chrome/browser/extensions/test_extension_system.h"
#include "chrome/test/base/testing_profile.h"
#include "content/public/test/test_browser_thread_bundle.h"
+#include "extensions/browser/extension_registry.h"
#include "extensions/browser/process_manager.h"
#include "extensions/common/extension.h"
#include "extensions/common/extension_builder.h"
@@ -22,7 +23,9 @@ namespace extensions {
class TestProcessManager : public ProcessManager {
public:
explicit TestProcessManager(Profile* profile)
- : ProcessManager(profile, profile->GetOriginalProfile()),
+ : ProcessManager(profile,
+ profile->GetOriginalProfile(),
+ ExtensionRegistry::Get(profile)),
create_count_(0) {}
virtual ~TestProcessManager() {}
diff --git a/extensions/browser/process_manager.cc b/extensions/browser/process_manager.cc
index e6beddb..72fbd2c 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"
@@ -105,7 +106,8 @@ class IncognitoProcessManager : public ProcessManager {
public:
IncognitoProcessManager(BrowserContext* incognito_context,
BrowserContext* original_context,
- ProcessManager* original_manager);
+ ProcessManager* original_manager,
+ ExtensionRegistry* extension_registry);
virtual ~IncognitoProcessManager() {}
virtual bool CreateBackgroundHost(const Extension* extension,
const GURL& url) OVERRIDE;
@@ -190,6 +192,7 @@ struct ProcessManager::BackgroundPageData {
// static
ProcessManager* ProcessManager::Create(BrowserContext* context) {
+ ExtensionRegistry* extension_registry = ExtensionRegistry::Get(context);
ExtensionsBrowserClient* client = ExtensionsBrowserClient::Get();
if (client->IsGuestSession(context)) {
// In the guest session, there is a single off-the-record context. Unlike
@@ -197,7 +200,7 @@ ProcessManager* ProcessManager::Create(BrowserContext* context) {
// created regardless of whether extensions use "spanning" or "split"
// incognito behavior.
BrowserContext* original_context = client->GetOriginalContext(context);
- return new ProcessManager(context, original_context);
+ return new ProcessManager(context, original_context, extension_registry);
}
if (context->IsOffTheRecord()) {
@@ -205,31 +208,46 @@ ProcessManager* ProcessManager::Create(BrowserContext* context) {
ProcessManager* original_manager =
ExtensionSystem::Get(original_context)->process_manager();
return new IncognitoProcessManager(
- context, original_context, original_manager);
+ context, original_context, original_manager, extension_registry);
}
- return new ProcessManager(context, context);
+ return new ProcessManager(context, context, extension_registry);
+}
+
+// static
+ProcessManager* ProcessManager::CreateForTesting(
+ BrowserContext* context,
+ ExtensionRegistry* extension_registry) {
+ DCHECK(!context->IsOffTheRecord());
+ return new ProcessManager(context, context, extension_registry);
}
// static
ProcessManager* ProcessManager::CreateIncognitoForTesting(
BrowserContext* incognito_context,
BrowserContext* original_context,
- ProcessManager* original_manager) {
+ ProcessManager* original_manager,
+ ExtensionRegistry* extension_registry) {
DCHECK(incognito_context->IsOffTheRecord());
DCHECK(!original_context->IsOffTheRecord());
- return new IncognitoProcessManager(
- incognito_context, original_context, original_manager);
+ return new IncognitoProcessManager(incognito_context,
+ original_context,
+ original_manager,
+ extension_registry);
}
ProcessManager::ProcessManager(BrowserContext* context,
- BrowserContext* original_context)
+ BrowserContext* original_context,
+ ExtensionRegistry* extension_registry)
: site_instance_(SiteInstance::Create(context)),
+ extension_registry_(extension_registry),
startup_background_hosts_created_(false),
devtools_callback_(base::Bind(&ProcessManager::OnDevToolsStateChanged,
base::Unretained(this))),
last_background_close_sequence_id_(0),
weak_ptr_factory_(this) {
+ // ExtensionRegistry is shared between incognito and regular contexts.
+ DCHECK_EQ(original_context, extension_registry_->browser_context());
registrar_.Add(this, chrome::NOTIFICATION_EXTENSIONS_READY,
content::Source<BrowserContext>(original_context));
registrar_.Add(this,
@@ -245,8 +263,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 +323,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()))
@@ -360,11 +379,7 @@ const Extension* ProcessManager::GetExtensionForRenderViewHost(
if (!render_view_host->GetSiteInstance())
return NULL;
- ExtensionRegistry* registry = ExtensionRegistry::Get(GetBrowserContext());
- if (!registry)
- return NULL;
-
- return registry->enabled_extensions().GetByID(
+ return extension_registry_->enabled_extensions().GetByID(
GetExtensionID(render_view_host));
}
@@ -440,9 +455,7 @@ void ProcessManager::DecrementLazyKeepaliveCount(
const std::string& extension_id) {
int& count = background_page_data_[extension_id].lazy_keepalive_count;
DCHECK(count > 0 ||
- !ExtensionRegistry::Get(GetBrowserContext())
- ->enabled_extensions()
- .Contains(extension_id));
+ !extension_registry_->enabled_extensions().Contains(extension_id));
// If we reach a zero keepalive count when the lazy background page is about
// to be closed, incrementing close_sequence_id will cancel the close
@@ -620,16 +633,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 +651,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 +782,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 +808,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 +816,21 @@ void ProcessManager::CreateBackgroundHostsForProfileStartup() {
}
}
+void ProcessManager::CreateStartupBackgroundHosts() {
+ DCHECK(!startup_background_hosts_created_);
+ const ExtensionSet& enabled_extensions =
+ extension_registry_->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 +895,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
//
@@ -903,8 +902,9 @@ bool ProcessManager::DeferLoadingBackgroundHosts() const {
IncognitoProcessManager::IncognitoProcessManager(
BrowserContext* incognito_context,
BrowserContext* original_context,
- ProcessManager* original_manager)
- : ProcessManager(incognito_context, original_context),
+ ProcessManager* original_manager,
+ ExtensionRegistry* extension_registry)
+ : ProcessManager(incognito_context, original_context, extension_registry),
original_manager_(original_manager) {
DCHECK(incognito_context->IsOffTheRecord());
@@ -914,8 +914,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,
@@ -932,14 +930,11 @@ bool IncognitoProcessManager::CreateBackgroundHost(const Extension* extension,
}
SiteInstance* IncognitoProcessManager::GetSiteInstanceForURL(const GURL& url) {
- ExtensionRegistry* registry = ExtensionRegistry::Get(GetBrowserContext());
- if (registry) {
- const Extension* extension =
- registry->enabled_extensions().GetExtensionOrAppByURL(url);
- if (extension && !IncognitoInfo::IsSplitMode(extension)) {
- return original_manager_->GetSiteInstanceForURL(url);
- }
- }
+ const Extension* extension =
+ extension_registry_->enabled_extensions().GetExtensionOrAppByURL(url);
+ if (extension && !IncognitoInfo::IsSplitMode(extension))
+ return original_manager_->GetSiteInstanceForURL(url);
+
return ProcessManager::GetSiteInstanceForURL(url);
}
diff --git a/extensions/browser/process_manager.h b/extensions/browser/process_manager.h
index e580bbd..e3c6bcf 100644
--- a/extensions/browser/process_manager.h
+++ b/extensions/browser/process_manager.h
@@ -33,6 +33,8 @@ namespace extensions {
class Extension;
class ExtensionHost;
+class ExtensionRegistry;
+class ProcessManagerDelegate;
class ProcessManagerObserver;
// Manages dynamic state of running Chromium extensions. There is one instance
@@ -122,8 +124,9 @@ class ProcessManager : public content::NotificationObserver {
// onSuspendCanceled() event to it.
void CancelSuspend(const Extension* extension);
- // Ensures background hosts are loaded for a new browser window.
- void OnBrowserWindowReady();
+ // Creates background hosts if the embedder is ready and they are not already
+ // loaded.
+ void MaybeCreateStartupBackgroundHosts();
// Gets the BrowserContext associated with site_instance_ and all other
// related SiteInstances.
@@ -137,18 +140,30 @@ class ProcessManager : public content::NotificationObserver {
void SetKeepaliveImpulseDecrementCallbackForTesting(
const ImpulseCallbackForTesting& callback);
- // Creates an incognito-context instance for tests. Tests for non-incognito
- // contexts can just use Create() above.
+ // Creates a non-incognito instance for tests. |registry| allows unit tests
+ // to inject an ExtensionRegistry that is not managed by the usual
+ // BrowserContextKeyedServiceFactory system.
+ static ProcessManager* CreateForTesting(content::BrowserContext* context,
+ ExtensionRegistry* registry);
+
+ // Creates an incognito-context instance for tests.
static ProcessManager* CreateIncognitoForTesting(
content::BrowserContext* incognito_context,
content::BrowserContext* original_context,
- ProcessManager* original_manager);
+ ProcessManager* original_manager,
+ ExtensionRegistry* registry);
+
+ bool startup_background_hosts_created_for_test() const {
+ return startup_background_hosts_created_;
+ }
protected:
// If |context| is incognito pass the master context as |original_context|.
- // Otherwise pass the same context for both.
+ // Otherwise pass the same context for both. Pass the ExtensionRegistry for
+ // |context| as |registry|, or override it for testing.
ProcessManager(content::BrowserContext* context,
- content::BrowserContext* original_context);
+ content::BrowserContext* original_context,
+ ExtensionRegistry* registry);
// Called on browser shutdown to close our extension hosts.
void CloseBackgroundHosts();
@@ -158,10 +173,6 @@ class ProcessManager : public content::NotificationObserver {
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
- // Load all background pages once the profile data is ready and the pages
- // should be loaded.
- void CreateBackgroundHostsForProfileStartup();
-
content::NotificationRegistrar registrar_;
// The set of ExtensionHosts running viewless background extensions.
@@ -172,6 +183,9 @@ class ProcessManager : public content::NotificationObserver {
// browsing instance is created. This controls process grouping.
scoped_refptr<content::SiteInstance> site_instance_;
+ // Not owned. Also used by IncognitoProcessManager.
+ ExtensionRegistry* extension_registry_;
+
private:
friend class ProcessManagerTest;
@@ -182,6 +196,10 @@ class ProcessManager : public content::NotificationObserver {
typedef std::map<content::RenderViewHost*,
extensions::ViewType> ExtensionRenderViews;
+ // Load all background pages once the profile data is ready and the pages
+ // should be loaded.
+ void CreateStartupBackgroundHosts();
+
// Called just after |host| is created so it can be registered in our lists.
void OnBackgroundHostCreated(ExtensionHost* host);
@@ -216,9 +234,6 @@ class ProcessManager : public content::NotificationObserver {
// Clears background page data for this extension.
void ClearBackgroundPageData(const std::string& extension_id);
- // Returns true if loading background pages should be deferred.
- bool DeferLoadingBackgroundHosts() const;
-
void OnDevToolsStateChanged(content::DevToolsAgentHost*, bool attached);
// Contains all active extension-related RenderViewHost instances for all
diff --git a/extensions/browser/process_manager_delegate.h b/extensions/browser/process_manager_delegate.h
new file mode 100644
index 0000000..7cc48c0
--- /dev/null
+++ b/extensions/browser/process_manager_delegate.h
@@ -0,0 +1,34 @@
+// 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_DELEGATE_H_
+#define EXTENSIONS_BROWSER_PROCESS_MANAGER_DELEGATE_H_
+
+namespace content {
+class BrowserContext;
+};
+
+namespace extensions {
+
+// Customization of ProcessManager for the extension system embedder.
+class ProcessManagerDelegate {
+ public:
+ virtual ~ProcessManagerDelegate() {}
+
+ // Returns true if the embedder allows background pages for the given
+ // |context|.
+ virtual bool IsBackgroundPageAllowed(
+ content::BrowserContext* context) const = 0;
+
+ // Returns true if the embedder wishes to defer starting up the renderers for
+ // extension background pages. If the embedder returns true it must call
+ // ProcessManager::MaybeCreateStartupBackgroundHosts() when it is ready. See
+ // ChromeProcessManagerDelegate for examples of how this is useful.
+ virtual bool DeferCreatingStartupBackgroundHosts(
+ content::BrowserContext* context) const = 0;
+};
+
+} // namespace extensions
+
+#endif // EXTENSIONS_BROWSER_PROCESS_MANAGER_DELEGATE_H_
diff --git a/extensions/browser/process_manager_unittest.cc b/extensions/browser/process_manager_unittest.cc
index b72b112..e5fc807 100644
--- a/extensions/browser/process_manager_unittest.cc
+++ b/extensions/browser/process_manager_unittest.cc
@@ -9,6 +9,8 @@
#include "content/public/browser/notification_service.h"
#include "content/public/browser/site_instance.h"
#include "content/public/test/test_browser_context.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/process_manager_delegate.h"
#include "extensions/browser/test_extensions_browser_client.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -33,12 +35,37 @@ class TestBrowserContextIncognito : public TestBrowserContext {
DISALLOW_COPY_AND_ASSIGN(TestBrowserContextIncognito);
};
+// A trivial ProcessManagerDelegate.
+class TestProcessManagerDelegate : public ProcessManagerDelegate {
+ public:
+ TestProcessManagerDelegate()
+ : is_background_page_allowed_(true),
+ defer_creating_startup_background_hosts_(false) {}
+ virtual ~TestProcessManagerDelegate() {}
+
+ // ProcessManagerDelegate implementation.
+ virtual bool IsBackgroundPageAllowed(BrowserContext* context) const OVERRIDE {
+ return is_background_page_allowed_;
+ }
+ virtual bool DeferCreatingStartupBackgroundHosts(
+ BrowserContext* context) const OVERRIDE {
+ return defer_creating_startup_background_hosts_;
+ }
+
+ bool is_background_page_allowed_;
+ bool defer_creating_startup_background_hosts_;
+};
+
} // namespace
class ProcessManagerTest : public testing::Test {
public:
- ProcessManagerTest() : extensions_browser_client_(&original_context_) {
+ ProcessManagerTest()
+ : extension_registry_(&original_context_),
+ extensions_browser_client_(&original_context_) {
extensions_browser_client_.SetIncognitoContext(&incognito_context_);
+ extensions_browser_client_.set_process_manager_delegate(
+ &process_manager_delegate_);
ExtensionsBrowserClient::Set(&extensions_browser_client_);
}
@@ -48,6 +75,10 @@ class ProcessManagerTest : public testing::Test {
BrowserContext* original_context() { return &original_context_; }
BrowserContext* incognito_context() { return &incognito_context_; }
+ ExtensionRegistry* extension_registry() { return &extension_registry_; }
+ TestProcessManagerDelegate* process_manager_delegate() {
+ return &process_manager_delegate_;
+ }
// Returns true if the notification |type| is registered for |manager| with
// source |context|. Pass NULL for |context| for all sources.
@@ -61,6 +92,8 @@ class ProcessManagerTest : public testing::Test {
private:
TestBrowserContext original_context_;
TestBrowserContextIncognito incognito_context_;
+ ExtensionRegistry extension_registry_; // Shared between BrowserContexts.
+ TestProcessManagerDelegate process_manager_delegate_;
TestExtensionsBrowserClient extensions_browser_client_;
DISALLOW_COPY_AND_ASSIGN(ProcessManagerTest);
@@ -69,8 +102,8 @@ class ProcessManagerTest : public testing::Test {
// Test that notification registration works properly.
TEST_F(ProcessManagerTest, ExtensionNotificationRegistration) {
// Test for a normal context ProcessManager.
- scoped_ptr<ProcessManager> manager1(
- ProcessManager::Create(original_context()));
+ scoped_ptr<ProcessManager> manager1(ProcessManager::CreateForTesting(
+ original_context(), extension_registry()));
EXPECT_EQ(original_context(), manager1->GetBrowserContext());
EXPECT_EQ(0u, manager1->background_hosts().size());
@@ -90,8 +123,11 @@ TEST_F(ProcessManagerTest, ExtensionNotificationRegistration) {
original_context()));
// Test for an incognito context ProcessManager.
- scoped_ptr<ProcessManager> manager2(ProcessManager::CreateIncognitoForTesting(
- incognito_context(), original_context(), manager1.get()));
+ scoped_ptr<ProcessManager> manager2(
+ ProcessManager::CreateIncognitoForTesting(incognito_context(),
+ original_context(),
+ manager1.get(),
+ extension_registry()));
EXPECT_EQ(incognito_context(), manager2->GetBrowserContext());
EXPECT_EQ(0u, manager2->background_hosts().size());
@@ -125,6 +161,83 @@ TEST_F(ProcessManagerTest, ExtensionNotificationRegistration) {
incognito_context()));
}
+// Test that startup background hosts are created when the extension system
+// becomes ready.
+//
+// NOTE: This test and those that follow do not try to create ExtensionsHosts
+// because ExtensionHost is tightly coupled to WebContents and can't be
+// constructed in unit tests.
+TEST_F(ProcessManagerTest, CreateBackgroundHostsOnExtensionsReady) {
+ scoped_ptr<ProcessManager> manager(ProcessManager::CreateForTesting(
+ original_context(), extension_registry()));
+ ASSERT_FALSE(manager->startup_background_hosts_created_for_test());
+
+ // Simulate the extension system becoming ready.
+ content::NotificationService::current()->Notify(
+ chrome::NOTIFICATION_EXTENSIONS_READY,
+ content::Source<BrowserContext>(original_context()),
+ content::NotificationService::NoDetails());
+ EXPECT_TRUE(manager->startup_background_hosts_created_for_test());
+}
+
+// Test that startup background hosts can be created explicitly before the
+// extension system is ready (this is the normal pattern in Chrome).
+TEST_F(ProcessManagerTest, CreateBackgroundHostsExplicitly) {
+ scoped_ptr<ProcessManager> manager(ProcessManager::CreateForTesting(
+ original_context(), extension_registry()));
+ ASSERT_FALSE(manager->startup_background_hosts_created_for_test());
+
+ // Embedder explicitly asks for hosts to be created. Chrome does this on
+ // normal startup.
+ manager->MaybeCreateStartupBackgroundHosts();
+ EXPECT_TRUE(manager->startup_background_hosts_created_for_test());
+}
+
+// Test that the embedder can defer background host creation. Chrome does this
+// when the profile is created asynchronously, which may take a while.
+TEST_F(ProcessManagerTest, CreateBackgroundHostsDeferred) {
+ scoped_ptr<ProcessManager> manager(ProcessManager::CreateForTesting(
+ original_context(), extension_registry()));
+ ASSERT_FALSE(manager->startup_background_hosts_created_for_test());
+
+ // Don't create background hosts if the delegate says to defer them.
+ process_manager_delegate()->defer_creating_startup_background_hosts_ = true;
+ manager->MaybeCreateStartupBackgroundHosts();
+ EXPECT_FALSE(manager->startup_background_hosts_created_for_test());
+
+ // The extension system becoming ready still doesn't create the hosts.
+ content::NotificationService::current()->Notify(
+ chrome::NOTIFICATION_EXTENSIONS_READY,
+ content::Source<BrowserContext>(original_context()),
+ content::NotificationService::NoDetails());
+ EXPECT_FALSE(manager->startup_background_hosts_created_for_test());
+
+ // Once the embedder is ready the background hosts can be created.
+ process_manager_delegate()->defer_creating_startup_background_hosts_ = false;
+ manager->MaybeCreateStartupBackgroundHosts();
+ EXPECT_TRUE(manager->startup_background_hosts_created_for_test());
+}
+
+// Test that the embedder can disallow background host creation.
+// Chrome OS does this in guest mode.
+TEST_F(ProcessManagerTest, IsBackgroundHostAllowed) {
+ scoped_ptr<ProcessManager> manager(ProcessManager::CreateForTesting(
+ original_context(), extension_registry()));
+ ASSERT_FALSE(manager->startup_background_hosts_created_for_test());
+
+ // Don't create background hosts if the delegate disallows them.
+ process_manager_delegate()->is_background_page_allowed_ = false;
+ manager->MaybeCreateStartupBackgroundHosts();
+ EXPECT_FALSE(manager->startup_background_hosts_created_for_test());
+
+ // The extension system becoming ready still doesn't create the hosts.
+ content::NotificationService::current()->Notify(
+ chrome::NOTIFICATION_EXTENSIONS_READY,
+ content::Source<BrowserContext>(original_context()),
+ content::NotificationService::NoDetails());
+ EXPECT_FALSE(manager->startup_background_hosts_created_for_test());
+}
+
// Test that extensions get grouped in the right SiteInstance (and therefore
// process) based on their URLs.
TEST_F(ProcessManagerTest, ProcessGrouping) {
@@ -133,12 +246,14 @@ TEST_F(ProcessManagerTest, ProcessGrouping) {
// Extensions in different browser contexts should always be different
// SiteInstances.
- scoped_ptr<ProcessManager> manager1(
- ProcessManager::Create(original_context()));
+ scoped_ptr<ProcessManager> manager1(ProcessManager::CreateForTesting(
+ original_context(), extension_registry()));
// NOTE: This context is not associated with the TestExtensionsBrowserClient.
// That's OK because we're not testing regular vs. incognito behavior.
TestBrowserContext another_context;
- scoped_ptr<ProcessManager> manager2(ProcessManager::Create(&another_context));
+ ExtensionRegistry another_registry(&another_context);
+ scoped_ptr<ProcessManager> manager2(
+ ProcessManager::CreateForTesting(&another_context, &another_registry));
// Extensions with common origins ("scheme://id/") should be grouped in the
// same SiteInstance.
diff --git a/extensions/browser/test_extensions_browser_client.cc b/extensions/browser/test_extensions_browser_client.cc
index dcd467f..a01b128 100644
--- a/extensions/browser/test_extensions_browser_client.cc
+++ b/extensions/browser/test_extensions_browser_client.cc
@@ -15,7 +15,9 @@ namespace extensions {
TestExtensionsBrowserClient::TestExtensionsBrowserClient(
BrowserContext* main_context)
- : main_context_(main_context), incognito_context_(NULL) {
+ : main_context_(main_context),
+ incognito_context_(NULL),
+ process_manager_delegate_(NULL) {
DCHECK(main_context_);
DCHECK(!main_context_->IsOffTheRecord());
}
@@ -116,14 +118,9 @@ void TestExtensionsBrowserClient::GetEarlyExtensionPrefsObservers(
content::BrowserContext* context,
std::vector<ExtensionPrefsObserver*>* observers) const {}
-bool TestExtensionsBrowserClient::DeferLoadingBackgroundHosts(
- BrowserContext* context) const {
- return false;
-}
-
-bool TestExtensionsBrowserClient::IsBackgroundPageAllowed(
- BrowserContext* context) const {
- return true;
+ProcessManagerDelegate* TestExtensionsBrowserClient::GetProcessManagerDelegate()
+ const {
+ return process_manager_delegate_;
}
scoped_ptr<ExtensionHostDelegate>
diff --git a/extensions/browser/test_extensions_browser_client.h b/extensions/browser/test_extensions_browser_client.h
index b793259..1cfffee 100644
--- a/extensions/browser/test_extensions_browser_client.h
+++ b/extensions/browser/test_extensions_browser_client.h
@@ -15,10 +15,14 @@ namespace extensions {
// this class should call ExtensionsBrowserClient::Set() with its instance.
class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
public:
- // |context| is required and must not be an incognito context.
+ // |main_context| is required and must not be an incognito context.
explicit TestExtensionsBrowserClient(content::BrowserContext* main_context);
virtual ~TestExtensionsBrowserClient();
+ void set_process_manager_delegate(ProcessManagerDelegate* delegate) {
+ process_manager_delegate_ = delegate;
+ }
+
// Associates an incognito context with |main_context_|.
void SetIncognitoContext(content::BrowserContext* incognito_context);
@@ -59,10 +63,7 @@ class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
virtual void GetEarlyExtensionPrefsObservers(
content::BrowserContext* context,
std::vector<ExtensionPrefsObserver*>* observers) const OVERRIDE;
- virtual bool DeferLoadingBackgroundHosts(
- content::BrowserContext* context) const OVERRIDE;
- virtual bool IsBackgroundPageAllowed(content::BrowserContext* context) const
- OVERRIDE;
+ virtual ProcessManagerDelegate* GetProcessManagerDelegate() const OVERRIDE;
virtual scoped_ptr<ExtensionHostDelegate> CreateExtensionHostDelegate()
OVERRIDE;
virtual bool DidVersionUpdate(content::BrowserContext* context) OVERRIDE;
@@ -82,6 +83,9 @@ class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
content::BrowserContext* main_context_; // Not owned.
content::BrowserContext* incognito_context_; // Not owned, defaults to NULL.
+ // Not owned, defaults to NULL.
+ ProcessManagerDelegate* process_manager_delegate_;
+
DISALLOW_COPY_AND_ASSIGN(TestExtensionsBrowserClient);
};
diff --git a/extensions/extensions.gyp b/extensions/extensions.gyp
index 447dc32..35e8f38 100644
--- a/extensions/extensions.gyp
+++ b/extensions/extensions.gyp
@@ -433,6 +433,7 @@
'browser/pref_names.h',
'browser/process_manager.cc',
'browser/process_manager.h',
+ 'browser/process_manager_delegate.h',
'browser/process_manager_observer.h',
'browser/process_map.cc',
'browser/process_map.h',