summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorjamescook@chromium.org <jamescook@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-07-17 17:21:45 +0000
committerjamescook@chromium.org <jamescook@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-07-17 17:21:45 +0000
commit46a19f626f5794f298cd1e4d8bdcbfcb07fd0b4e (patch)
tree5a04abc6c4c823a4ddf1ca0d76fcc1ea47ff009a
parentdddea5bb5ab70ac91118754eb09ccb30dcb178a2 (diff)
downloadchromium_src-46a19f626f5794f298cd1e4d8bdcbfcb07fd0b4e.zip
chromium_src-46a19f626f5794f298cd1e4d8bdcbfcb07fd0b4e.tar.gz
chromium_src-46a19f626f5794f298cd1e4d8bdcbfcb07fd0b4e.tar.bz2
Revert 283678 "Refactor code that defers extension background pa..."
This broke Chrome OS valgrind bots, for example: http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28valgrind%29%284%29/builds/27033 > 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 TBR=jamescook@chromium.org Review URL: https://codereview.chromium.org/399153002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283801 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--apps/shell/browser/shell_extensions_browser_client.cc11
-rw-r--r--apps/shell/browser/shell_extensions_browser_client.h5
-rw-r--r--chrome/browser/extensions/chrome_extensions_browser_client.cc33
-rw-r--r--chrome/browser/extensions/chrome_extensions_browser_client.h9
-rw-r--r--chrome/browser/extensions/chrome_notification_observer.cc43
-rw-r--r--chrome/browser/extensions/chrome_notification_observer.h3
-rw-r--r--chrome/browser/extensions/chrome_process_manager_delegate.cc143
-rw-r--r--chrome/browser/extensions/chrome_process_manager_delegate.h50
-rw-r--r--chrome/chrome_browser_extensions.gypi2
-rw-r--r--extensions/DEPS3
-rw-r--r--extensions/browser/extensions_browser_client.h10
-rw-r--r--extensions/browser/process_manager.cc98
-rw-r--r--extensions/browser/process_manager.h21
-rw-r--r--extensions/browser/process_manager_delegate.h34
-rw-r--r--extensions/browser/process_manager_unittest.cc122
-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
18 files changed, 174 insertions, 443 deletions
diff --git a/apps/shell/browser/shell_extensions_browser_client.cc b/apps/shell/browser/shell_extensions_browser_client.cc
index 54b0df8..76f4a83 100644
--- a/apps/shell/browser/shell_extensions_browser_client.cc
+++ b/apps/shell/browser/shell_extensions_browser_client.cc
@@ -183,9 +183,14 @@ void ShellExtensionsBrowserClient::GetEarlyExtensionPrefsObservers(
content::BrowserContext* context,
std::vector<ExtensionPrefsObserver*>* observers) const {}
-ProcessManagerDelegate*
-ShellExtensionsBrowserClient::GetProcessManagerDelegate() const {
- return NULL;
+bool ShellExtensionsBrowserClient::DeferLoadingBackgroundHosts(
+ BrowserContext* context) const {
+ return false;
+}
+
+bool ShellExtensionsBrowserClient::IsBackgroundPageAllowed(
+ BrowserContext* context) const {
+ return true;
}
scoped_ptr<ExtensionHostDelegate>
diff --git a/apps/shell/browser/shell_extensions_browser_client.h b/apps/shell/browser/shell_extensions_browser_client.h
index b6a20f7..2013c6d 100644
--- a/apps/shell/browser/shell_extensions_browser_client.h
+++ b/apps/shell/browser/shell_extensions_browser_client.h
@@ -59,7 +59,10 @@ class ShellExtensionsBrowserClient : public ExtensionsBrowserClient {
virtual void GetEarlyExtensionPrefsObservers(
content::BrowserContext* context,
std::vector<ExtensionPrefsObserver*>* observers) const OVERRIDE;
- virtual ProcessManagerDelegate* GetProcessManagerDelegate() const OVERRIDE;
+ virtual bool DeferLoadingBackgroundHosts(content::BrowserContext* context)
+ const OVERRIDE;
+ virtual bool IsBackgroundPageAllowed(content::BrowserContext* context)
+ const OVERRIDE;
virtual scoped_ptr<ExtensionHostDelegate> CreateExtensionHostDelegate()
OVERRIDE;
virtual bool DidVersionUpdate(content::BrowserContext* context) OVERRIDE;
diff --git a/chrome/browser/extensions/chrome_extensions_browser_client.cc b/chrome/browser/extensions/chrome_extensions_browser_client.cc
index db2ae3b..c66d023 100644
--- a/chrome/browser/extensions/chrome_extensions_browser_client.cc
+++ b/chrome/browser/extensions/chrome_extensions_browser_client.cc
@@ -21,6 +21,7 @@
#include "chrome/browser/external_protocol/external_protocol_handler.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/browser_finder.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/chrome_version_info.h"
@@ -41,14 +42,12 @@
#include "chrome/browser/extensions/activity_log/activity_log.h"
#include "chrome/browser/extensions/api/chrome_extensions_api_client.h"
#include "chrome/browser/extensions/api/content_settings/content_settings_service.h"
-#include "chrome/browser/extensions/chrome_process_manager_delegate.h"
#endif
namespace extensions {
ChromeExtensionsBrowserClient::ChromeExtensionsBrowserClient() {
#if defined(ENABLE_EXTENSIONS)
- process_manager_delegate_.reset(new ChromeProcessManagerDelegate);
api_client_.reset(new ChromeExtensionsAPIClient);
#endif
// Only set if it hasn't already been set (e.g. by a test).
@@ -159,15 +158,35 @@ void ChromeExtensionsBrowserClient::GetEarlyExtensionPrefsObservers(
#endif
}
-ProcessManagerDelegate*
-ChromeExtensionsBrowserClient::GetProcessManagerDelegate() const {
-#if defined(ENABLE_EXTENSIONS)
- return process_manager_delegate_.get();
+bool ChromeExtensionsBrowserClient::DeferLoadingBackgroundHosts(
+ content::BrowserContext* context) const {
+ Profile* profile = static_cast<Profile*>(context);
+
+ // The profile may not be valid yet if it is still being initialized.
+ // In that case, defer loading, since it depends on an initialized profile.
+ // http://crbug.com/222473
+ if (!g_browser_process->profile_manager()->IsValidProfile(profile))
+ return true;
+
+#if defined(OS_ANDROID)
+ return false;
#else
- return NULL;
+ // There are no browser windows open and the browser process was
+ // started to show the app launcher.
+ return chrome::GetTotalBrowserCountForProfile(profile) == 0 &&
+ CommandLine::ForCurrentProcess()->HasSwitch(switches::kShowAppList);
#endif
}
+bool ChromeExtensionsBrowserClient::IsBackgroundPageAllowed(
+ content::BrowserContext* context) const {
+ // Returns true if current session is Guest mode session and current
+ // browser context is *not* off-the-record. Such context is artificial and
+ // background page shouldn't be created in it.
+ return !static_cast<Profile*>(context)->IsGuestSession() ||
+ context->IsOffTheRecord();
+}
+
scoped_ptr<ExtensionHostDelegate>
ChromeExtensionsBrowserClient::CreateExtensionHostDelegate() {
return scoped_ptr<ExtensionHostDelegate>(new ChromeExtensionHostDelegate);
diff --git a/chrome/browser/extensions/chrome_extensions_browser_client.h b/chrome/browser/extensions/chrome_extensions_browser_client.h
index 5957838..3c4a2c6 100644
--- a/chrome/browser/extensions/chrome_extensions_browser_client.h
+++ b/chrome/browser/extensions/chrome_extensions_browser_client.h
@@ -26,7 +26,6 @@ namespace extensions {
class ChromeComponentExtensionResourceManager;
class ChromeExtensionsAPIClient;
-class ChromeProcessManagerDelegate;
class ContentSettingsPrefsObserver;
// Implementation of extensions::BrowserClient for Chrome, which includes
@@ -77,7 +76,10 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {
virtual void GetEarlyExtensionPrefsObservers(
content::BrowserContext* context,
std::vector<ExtensionPrefsObserver*>* observers) const OVERRIDE;
- virtual ProcessManagerDelegate* GetProcessManagerDelegate() const OVERRIDE;
+ virtual bool DeferLoadingBackgroundHosts(
+ content::BrowserContext* context) const OVERRIDE;
+ virtual bool IsBackgroundPageAllowed(
+ content::BrowserContext* context) const OVERRIDE;
virtual scoped_ptr<ExtensionHostDelegate> CreateExtensionHostDelegate()
OVERRIDE;
virtual bool DidVersionUpdate(content::BrowserContext* context) OVERRIDE;
@@ -100,9 +102,6 @@ class ChromeExtensionsBrowserClient : public ExtensionsBrowserClient {
ChromeNotificationObserver notification_observer_;
#if defined(ENABLE_EXTENSIONS)
- // Support for ProcessManager.
- scoped_ptr<ChromeProcessManagerDelegate> process_manager_delegate_;
-
// Client for API implementations.
scoped_ptr<ChromeExtensionsAPIClient> api_client_;
#endif
diff --git a/chrome/browser/extensions/chrome_notification_observer.cc b/chrome/browser/extensions/chrome_notification_observer.cc
index c8e6016..5d966c8 100644
--- a/chrome/browser/extensions/chrome_notification_observer.cc
+++ b/chrome/browser/extensions/chrome_notification_observer.cc
@@ -4,22 +4,59 @@
#include "chrome/browser/extensions/chrome_notification_observer.h"
+#include "base/logging.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
#include "chrome/common/extensions/features/feature_channel.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
#include "content/public/browser/render_process_host.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/browser/process_manager.h"
#include "extensions/common/extension_messages.h"
namespace extensions {
ChromeNotificationObserver::ChromeNotificationObserver() {
registrar_.Add(this,
+ chrome::NOTIFICATION_BROWSER_WINDOW_READY,
+ content::NotificationService::AllSources());
+ registrar_.Add(this,
content::NOTIFICATION_RENDERER_PROCESS_CREATED,
content::NotificationService::AllBrowserContextsAndSources());
}
ChromeNotificationObserver::~ChromeNotificationObserver() {}
+void ChromeNotificationObserver::OnBrowserWindowReady(Browser* browser) {
+ Profile* profile = browser->profile();
+ DCHECK(profile);
+
+ // Inform the process manager for this profile that the window is ready.
+ // We continue to observe the notification in case browser windows open for
+ // a related incognito profile or other regular profiles.
+ extensions::ProcessManager* manager =
+ ExtensionSystem::Get(profile)->process_manager();
+ if (!manager) // Tests may not have a process manager.
+ return;
+ DCHECK_EQ(profile, manager->GetBrowserContext());
+ manager->OnBrowserWindowReady();
+
+ // For incognito profiles also inform the original profile's process manager
+ // that the window is ready. This will usually be a no-op because the
+ // original profile's process manager should have been informed when the
+ // non-incognito window opened.
+ if (profile->IsOffTheRecord()) {
+ Profile* original_profile = profile->GetOriginalProfile();
+ extensions::ProcessManager* original_manager =
+ ExtensionSystem::Get(original_profile)->process_manager();
+ DCHECK(original_manager);
+ DCHECK_EQ(original_profile, original_manager->GetBrowserContext());
+ original_manager->OnBrowserWindowReady();
+ }
+}
+
void ChromeNotificationObserver::OnRendererProcessCreated(
content::RenderProcessHost* process) {
// Extensions need to know the channel for API restrictions. Send the channel
@@ -31,6 +68,12 @@ void ChromeNotificationObserver::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
switch (type) {
+ case chrome::NOTIFICATION_BROWSER_WINDOW_READY: {
+ Browser* browser = content::Source<Browser>(source).ptr();
+ OnBrowserWindowReady(browser);
+ break;
+ }
+
case content::NOTIFICATION_RENDERER_PROCESS_CREATED: {
content::RenderProcessHost* process =
content::Source<content::RenderProcessHost>(source).ptr();
diff --git a/chrome/browser/extensions/chrome_notification_observer.h b/chrome/browser/extensions/chrome_notification_observer.h
index 8045b15..a31dc87 100644
--- a/chrome/browser/extensions/chrome_notification_observer.h
+++ b/chrome/browser/extensions/chrome_notification_observer.h
@@ -25,7 +25,8 @@ class ChromeNotificationObserver : public content::NotificationObserver {
ChromeNotificationObserver();
virtual ~ChromeNotificationObserver();
- // Notification handlers:
+ // IPC message handlers:
+ void OnBrowserWindowReady(Browser* browser);
void OnRendererProcessCreated(content::RenderProcessHost* process);
// content::NotificationObserver overrides:
diff --git a/chrome/browser/extensions/chrome_process_manager_delegate.cc b/chrome/browser/extensions/chrome_process_manager_delegate.cc
deleted file mode 100644
index 12e0c16..0000000
--- a/chrome/browser/extensions/chrome_process_manager_delegate.cc
+++ /dev/null
@@ -1,143 +0,0 @@
-// 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.
-
-#include "chrome/browser/extensions/chrome_process_manager_delegate.h"
-
-#include "base/command_line.h"
-#include "base/logging.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/ui/browser.h"
-#include "content/public/browser/notification_service.h"
-#include "extensions/browser/extension_system.h"
-#include "extensions/browser/process_manager.h"
-#include "extensions/common/one_shot_event.h"
-
-#if !defined(OS_ANDROID)
-#include "chrome/browser/ui/browser_finder.h"
-#include "chrome/common/chrome_switches.h"
-#endif
-
-namespace extensions {
-
-ChromeProcessManagerDelegate::ChromeProcessManagerDelegate() {
- registrar_.Add(this,
- chrome::NOTIFICATION_BROWSER_WINDOW_READY,
- content::NotificationService::AllSources());
- registrar_.Add(this,
- chrome::NOTIFICATION_PROFILE_CREATED,
- content::NotificationService::AllSources());
- // TODO(jamescook): Move observation of NOTIFICATION_PROFILE_DESTROYED here.
- // http://crbug.com/392658
-}
-
-ChromeProcessManagerDelegate::~ChromeProcessManagerDelegate() {
-}
-
-bool ChromeProcessManagerDelegate::IsBackgroundPageAllowed(
- content::BrowserContext* context) const {
- Profile* profile = static_cast<Profile*>(context);
-
- // Disallow if the current session is a Guest mode session but the current
- // browser context is *not* off-the-record. Such context is artificial and
- // background page shouldn't be created in it.
- // http://crbug.com/329498
- return !(profile->IsGuestSession() && !profile->IsOffTheRecord());
-}
-
-bool ChromeProcessManagerDelegate::DeferCreatingStartupBackgroundHosts(
- content::BrowserContext* context) const {
- Profile* profile = static_cast<Profile*>(context);
-
- // The profile may not be valid yet if it is still being initialized.
- // In that case, defer loading, since it depends on an initialized profile.
- // Background hosts will be loaded later via NOTIFICATION_PROFILE_CREATED.
- // http://crbug.com/222473
- if (!g_browser_process->profile_manager()->IsValidProfile(profile))
- return true;
-
-#if defined(OS_ANDROID)
- return false;
-#else
- // There are no browser windows open and the browser process was
- // started to show the app launcher. Background hosts will be loaded later
- // via NOTIFICATION_BROWSER_WINDOW_READY. http://crbug.com/178260
- return chrome::GetTotalBrowserCountForProfile(profile) == 0 &&
- CommandLine::ForCurrentProcess()->HasSwitch(switches::kShowAppList);
-#endif
-}
-
-void ChromeProcessManagerDelegate::Observe(
- int type,
- const content::NotificationSource& source,
- const content::NotificationDetails& details) {
- switch (type) {
- case chrome::NOTIFICATION_BROWSER_WINDOW_READY: {
- Browser* browser = content::Source<Browser>(source).ptr();
- OnBrowserWindowReady(browser);
- break;
- }
- case chrome::NOTIFICATION_PROFILE_CREATED: {
- Profile* profile = content::Source<Profile>(source).ptr();
- OnProfileCreated(profile);
- break;
- }
-
- default:
- NOTREACHED();
- }
-}
-
-void ChromeProcessManagerDelegate::OnBrowserWindowReady(Browser* browser) {
- Profile* profile = browser->profile();
- DCHECK(profile);
-
- // If the extension system isn't ready yet the background hosts will be
- // created automatically when it is.
- ExtensionSystem* system = ExtensionSystem::Get(profile);
- if (!system->ready().is_signaled())
- return;
-
- // Inform the process manager for this profile that the window is ready.
- // We continue to observe the notification in case browser windows open for
- // a related incognito profile or other regular profiles.
- ProcessManager* manager = system->process_manager();
- if (!manager) // Tests may not have a process manager.
- return;
- DCHECK_EQ(profile, manager->GetBrowserContext());
- manager->MaybeCreateStartupBackgroundHosts();
-
- // For incognito profiles also inform the original profile's process manager
- // that the window is ready. This will usually be a no-op because the
- // original profile's process manager should have been informed when the
- // non-incognito window opened.
- if (profile->IsOffTheRecord()) {
- Profile* original_profile = profile->GetOriginalProfile();
- ProcessManager* original_manager =
- ExtensionSystem::Get(original_profile)->process_manager();
- DCHECK(original_manager);
- DCHECK_EQ(original_profile, original_manager->GetBrowserContext());
- original_manager->MaybeCreateStartupBackgroundHosts();
- }
-}
-
-void ChromeProcessManagerDelegate::OnProfileCreated(Profile* profile) {
- // Incognito profiles are handled by their original profile.
- if (profile->IsOffTheRecord())
- return;
-
- // The profile can be created before the extension system is ready.
- ProcessManager* manager = ExtensionSystem::Get(profile)->process_manager();
- if (!manager)
- return;
-
- // The profile might have been initialized asynchronously (in parallel with
- // extension system startup). Now that initialization is complete the
- // ProcessManager can load deferred background pages.
- manager->MaybeCreateStartupBackgroundHosts();
-}
-
-} // namespace extensions
diff --git a/chrome/browser/extensions/chrome_process_manager_delegate.h b/chrome/browser/extensions/chrome_process_manager_delegate.h
deleted file mode 100644
index 9bcc58a..0000000
--- a/chrome/browser/extensions/chrome_process_manager_delegate.h
+++ /dev/null
@@ -1,50 +0,0 @@
-// 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 CHROME_BROWSER_EXTENSIONS_CHROME_PROCESS_MANAGER_DELEGATE_H_
-#define CHROME_BROWSER_EXTENSIONS_CHROME_PROCESS_MANAGER_DELEGATE_H_
-
-#include "base/compiler_specific.h"
-#include "base/macros.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-#include "extensions/browser/process_manager_delegate.h"
-
-class Browser;
-class Profile;
-
-namespace extensions {
-
-// Support for ProcessManager. Controls cases where Chrome wishes to disallow
-// extension background pages or defer their creation.
-class ChromeProcessManagerDelegate : public ProcessManagerDelegate,
- public content::NotificationObserver {
- public:
- ChromeProcessManagerDelegate();
- virtual ~ChromeProcessManagerDelegate();
-
- // ProcessManagerDelegate implementation:
- virtual bool IsBackgroundPageAllowed(
- content::BrowserContext* context) const OVERRIDE;
- virtual bool DeferCreatingStartupBackgroundHosts(
- content::BrowserContext* context) const OVERRIDE;
-
- // content::NotificationObserver implementation:
- virtual void Observe(int type,
- const content::NotificationSource& source,
- const content::NotificationDetails& details) OVERRIDE;
-
- private:
- // Notification handlers.
- void OnBrowserWindowReady(Browser* browser);
- void OnProfileCreated(Profile* profile);
-
- content::NotificationRegistrar registrar_;
-
- DISALLOW_COPY_AND_ASSIGN(ChromeProcessManagerDelegate);
-};
-
-} // namespace extensions
-
-#endif // CHROME_BROWSER_EXTENSIONS_CHROME_PROCESS_MANAGER_DELEGATE_H_
diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi
index 0e955f7..194dab0e 100644
--- a/chrome/chrome_browser_extensions.gypi
+++ b/chrome/chrome_browser_extensions.gypi
@@ -45,8 +45,6 @@
'browser/extensions/chrome_extensions_browser_client.h',
'browser/extensions/chrome_notification_observer.cc',
'browser/extensions/chrome_notification_observer.h',
- 'browser/extensions/chrome_process_manager_delegate.cc',
- 'browser/extensions/chrome_process_manager_delegate.h',
'browser/extensions/component_loader.cc',
'browser/extensions/component_loader.h',
'browser/extensions/context_menu_matcher.cc',
diff --git a/extensions/DEPS b/extensions/DEPS
index 4077a55..0505b02 100644
--- a/extensions/DEPS
+++ b/extensions/DEPS
@@ -1,5 +1,4 @@
include_rules = [
- # Do not add Chrome dependencies. Much work went into removing them.
"+components/url_matcher",
"+content/public/common",
"+content/public/test",
@@ -14,7 +13,7 @@ include_rules = [
# NOTE: Please do not add includes without talking to the app shell team;
# see OWNERS for this directory.
#
- # TODO(jamescook): Remove this. http://crbug.com/392622
+ # TODO(jamescook): Remove these. http://crbug.com/162530
"!chrome/browser/chrome_notification_types.h",
]
diff --git a/extensions/browser/extensions_browser_client.h b/extensions/browser/extensions_browser_client.h
index cb810b6..293b2af 100644
--- a/extensions/browser/extensions_browser_client.h
+++ b/extensions/browser/extensions_browser_client.h
@@ -41,7 +41,6 @@ class ExtensionPrefsObserver;
class ExtensionSystem;
class ExtensionSystemProvider;
class InfoMap;
-class ProcessManagerDelegate;
class RuntimeAPIDelegate;
// Interface to allow the extensions module to make browser-process-specific
@@ -134,9 +133,12 @@ class ExtensionsBrowserClient {
content::BrowserContext* context,
std::vector<ExtensionPrefsObserver*>* observers) const = 0;
- // Returns the ProcessManagerDelegate shared across all BrowserContexts. May
- // return NULL in tests or for simple embedders.
- virtual ProcessManagerDelegate* GetProcessManagerDelegate() 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;
// Creates a new ExtensionHostDelegate instance.
virtual scoped_ptr<ExtensionHostDelegate> CreateExtensionHostDelegate() = 0;
diff --git a/extensions/browser/process_manager.cc b/extensions/browser/process_manager.cc
index 12afd7c..e6beddb 100644
--- a/extensions/browser/process_manager.cc
+++ b/extensions/browser/process_manager.cc
@@ -33,7 +33,6 @@
#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"
@@ -246,6 +245,8 @@ 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()) {
@@ -306,14 +307,11 @@ 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())
- return false;
-
- // Don't create hosts if the embedder doesn't allow it.
- ProcessManagerDelegate* delegate =
- ExtensionsBrowserClient::Get()->GetProcessManagerDelegate();
- if (delegate && !delegate->IsBackgroundPageAllowed(GetBrowserContext()))
+ if (extension->is_hosted_app() ||
+ !ExtensionsBrowserClient::Get()->
+ IsBackgroundPageAllowed(GetBrowserContext())) {
return false;
+ }
// Don't create multiple background hosts for an extension.
if (GetBackgroundHostForExtension(extension->id()))
@@ -622,6 +620,16 @@ 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();
}
@@ -640,10 +648,15 @@ void ProcessManager::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
switch (type) {
- case chrome::NOTIFICATION_EXTENSIONS_READY: {
- // TODO(jamescook): Convert this to use ExtensionSystem::ready() instead
- // of a notification.
- MaybeCreateStartupBackgroundHosts();
+ 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();
break;
}
@@ -771,24 +784,24 @@ void ProcessManager::OnDevToolsStateChanged(
}
}
-void ProcessManager::MaybeCreateStartupBackgroundHosts() {
- if (startup_background_hosts_created_)
- return;
-
- // The embedder might disallow background pages entirely.
- ProcessManagerDelegate* delegate =
- ExtensionsBrowserClient::Get()->GetProcessManagerDelegate();
- if (delegate && !delegate->IsBackgroundPageAllowed(GetBrowserContext()))
+void ProcessManager::CreateBackgroundHostsForProfileStartup() {
+ if (startup_background_hosts_created_ ||
+ !ExtensionsBrowserClient::Get()->
+ IsBackgroundPageAllowed(GetBrowserContext())) {
return;
+ }
- // 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;
+ 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());
- CreateStartupBackgroundHosts();
+ FOR_EACH_OBSERVER(ProcessManagerObserver,
+ observer_list_,
+ OnBackgroundHostStartup(*extension));
+ }
startup_background_hosts_created_ = true;
// Background pages should only be loaded once. To prevent any further loads
@@ -797,6 +810,14 @@ void ProcessManager::MaybeCreateStartupBackgroundHosts() {
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,
@@ -805,21 +826,6 @@ void ProcessManager::MaybeCreateStartupBackgroundHosts() {
}
}
-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);
@@ -884,6 +890,12 @@ 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
//
@@ -902,6 +914,8 @@ 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,
diff --git a/extensions/browser/process_manager.h b/extensions/browser/process_manager.h
index 80736e4..e580bbd 100644
--- a/extensions/browser/process_manager.h
+++ b/extensions/browser/process_manager.h
@@ -33,7 +33,6 @@ namespace extensions {
class Extension;
class ExtensionHost;
-class ProcessManagerDelegate;
class ProcessManagerObserver;
// Manages dynamic state of running Chromium extensions. There is one instance
@@ -123,9 +122,8 @@ class ProcessManager : public content::NotificationObserver {
// onSuspendCanceled() event to it.
void CancelSuspend(const Extension* extension);
- // Creates background hosts if the embedder is ready and they are not already
- // loaded.
- void MaybeCreateStartupBackgroundHosts();
+ // Ensures background hosts are loaded for a new browser window.
+ void OnBrowserWindowReady();
// Gets the BrowserContext associated with site_instance_ and all other
// related SiteInstances.
@@ -146,10 +144,6 @@ class ProcessManager : public content::NotificationObserver {
content::BrowserContext* original_context,
ProcessManager* original_manager);
- 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.
@@ -164,6 +158,10 @@ 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.
@@ -184,10 +182,6 @@ 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);
@@ -222,6 +216,9 @@ 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
deleted file mode 100644
index 7cc48c0..0000000
--- a/extensions/browser/process_manager_delegate.h
+++ /dev/null
@@ -1,34 +0,0 @@
-// 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 0d1f86b..b72b112 100644
--- a/extensions/browser/process_manager_unittest.cc
+++ b/extensions/browser/process_manager_unittest.cc
@@ -5,12 +5,10 @@
#include "extensions/browser/process_manager.h"
#include "chrome/browser/chrome_notification_types.h"
-#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "content/public/browser/content_browser_client.h"
#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/process_manager_delegate.h"
#include "extensions/browser/test_extensions_browser_client.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -35,35 +33,12 @@ 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_) {
extensions_browser_client_.SetIncognitoContext(&incognito_context_);
- extensions_browser_client_.set_process_manager_delegate(
- &process_manager_delegate_);
ExtensionsBrowserClient::Set(&extensions_browser_client_);
}
@@ -74,23 +49,6 @@ class ProcessManagerTest : public testing::Test {
BrowserContext* original_context() { return &original_context_; }
BrowserContext* incognito_context() { return &incognito_context_; }
- // testing::Test implementation.
- virtual void SetUp() OVERRIDE {
- // Needed for ExtensionRegistry.
- BrowserContextDependencyManager::GetInstance()
- ->CreateBrowserContextServicesForTest(&original_context_);
- BrowserContextDependencyManager::GetInstance()
- ->CreateBrowserContextServicesForTest(&incognito_context_);
- }
-
- virtual void TearDown() OVERRIDE {
- // Needed to clean up ExtensionRegistry.
- BrowserContextDependencyManager::GetInstance()
- ->DestroyBrowserContextServices(&incognito_context_);
- BrowserContextDependencyManager::GetInstance()
- ->DestroyBrowserContextServices(&original_context_);
- }
-
// Returns true if the notification |type| is registered for |manager| with
// source |context|. Pass NULL for |context| for all sources.
static bool IsRegistered(ProcessManager* manager,
@@ -100,10 +58,9 @@ class ProcessManagerTest : public testing::Test {
manager, type, content::Source<BrowserContext>(context));
}
- protected:
+ private:
TestBrowserContext original_context_;
TestBrowserContextIncognito incognito_context_;
- TestProcessManagerDelegate process_manager_delegate_;
TestExtensionsBrowserClient extensions_browser_client_;
DISALLOW_COPY_AND_ASSIGN(ProcessManagerTest);
@@ -168,83 +125,6 @@ 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::Create(original_context()));
- 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::Create(original_context()));
- 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::Create(original_context()));
- 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::Create(original_context()));
- 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) {
diff --git a/extensions/browser/test_extensions_browser_client.cc b/extensions/browser/test_extensions_browser_client.cc
index a01b128..dcd467f 100644
--- a/extensions/browser/test_extensions_browser_client.cc
+++ b/extensions/browser/test_extensions_browser_client.cc
@@ -15,9 +15,7 @@ namespace extensions {
TestExtensionsBrowserClient::TestExtensionsBrowserClient(
BrowserContext* main_context)
- : main_context_(main_context),
- incognito_context_(NULL),
- process_manager_delegate_(NULL) {
+ : main_context_(main_context), incognito_context_(NULL) {
DCHECK(main_context_);
DCHECK(!main_context_->IsOffTheRecord());
}
@@ -118,9 +116,14 @@ void TestExtensionsBrowserClient::GetEarlyExtensionPrefsObservers(
content::BrowserContext* context,
std::vector<ExtensionPrefsObserver*>* observers) const {}
-ProcessManagerDelegate* TestExtensionsBrowserClient::GetProcessManagerDelegate()
- const {
- return process_manager_delegate_;
+bool TestExtensionsBrowserClient::DeferLoadingBackgroundHosts(
+ BrowserContext* context) const {
+ return false;
+}
+
+bool TestExtensionsBrowserClient::IsBackgroundPageAllowed(
+ BrowserContext* context) const {
+ return true;
}
scoped_ptr<ExtensionHostDelegate>
diff --git a/extensions/browser/test_extensions_browser_client.h b/extensions/browser/test_extensions_browser_client.h
index 1cfffee..b793259 100644
--- a/extensions/browser/test_extensions_browser_client.h
+++ b/extensions/browser/test_extensions_browser_client.h
@@ -15,14 +15,10 @@ namespace extensions {
// this class should call ExtensionsBrowserClient::Set() with its instance.
class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
public:
- // |main_context| is required and must not be an incognito context.
+ // |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);
@@ -63,7 +59,10 @@ class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
virtual void GetEarlyExtensionPrefsObservers(
content::BrowserContext* context,
std::vector<ExtensionPrefsObserver*>* observers) const OVERRIDE;
- virtual ProcessManagerDelegate* GetProcessManagerDelegate() const OVERRIDE;
+ virtual bool DeferLoadingBackgroundHosts(
+ content::BrowserContext* context) const OVERRIDE;
+ virtual bool IsBackgroundPageAllowed(content::BrowserContext* context) const
+ OVERRIDE;
virtual scoped_ptr<ExtensionHostDelegate> CreateExtensionHostDelegate()
OVERRIDE;
virtual bool DidVersionUpdate(content::BrowserContext* context) OVERRIDE;
@@ -83,9 +82,6 @@ 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 3501961..4349183 100644
--- a/extensions/extensions.gyp
+++ b/extensions/extensions.gyp
@@ -435,7 +435,6 @@
'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',