diff options
18 files changed, 443 insertions, 174 deletions
diff --git a/apps/shell/browser/shell_extensions_browser_client.cc b/apps/shell/browser/shell_extensions_browser_client.cc index 76f4a83..54b0df8 100644 --- a/apps/shell/browser/shell_extensions_browser_client.cc +++ b/apps/shell/browser/shell_extensions_browser_client.cc @@ -183,14 +183,9 @@ void ShellExtensionsBrowserClient::GetEarlyExtensionPrefsObservers( content::BrowserContext* context, std::vector<ExtensionPrefsObserver*>* observers) const {} -bool ShellExtensionsBrowserClient::DeferLoadingBackgroundHosts( - BrowserContext* context) const { - return false; -} - -bool ShellExtensionsBrowserClient::IsBackgroundPageAllowed( - BrowserContext* context) const { - return true; +ProcessManagerDelegate* +ShellExtensionsBrowserClient::GetProcessManagerDelegate() const { + return NULL; } scoped_ptr<ExtensionHostDelegate> diff --git a/apps/shell/browser/shell_extensions_browser_client.h b/apps/shell/browser/shell_extensions_browser_client.h index 2013c6d..b6a20f7 100644 --- a/apps/shell/browser/shell_extensions_browser_client.h +++ b/apps/shell/browser/shell_extensions_browser_client.h @@ -59,10 +59,7 @@ class ShellExtensionsBrowserClient : 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; diff --git a/chrome/browser/extensions/chrome_extensions_browser_client.cc b/chrome/browser/extensions/chrome_extensions_browser_client.cc index c66d023..db2ae3b 100644 --- a/chrome/browser/extensions/chrome_extensions_browser_client.cc +++ b/chrome/browser/extensions/chrome_extensions_browser_client.cc @@ -21,7 +21,6 @@ #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" @@ -42,12 +41,14 @@ #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). @@ -158,35 +159,15 @@ void ChromeExtensionsBrowserClient::GetEarlyExtensionPrefsObservers( #endif } -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; +ProcessManagerDelegate* +ChromeExtensionsBrowserClient::GetProcessManagerDelegate() const { +#if defined(ENABLE_EXTENSIONS) + return process_manager_delegate_.get(); #else - // 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); + return NULL; #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 3c4a2c6..5957838 100644 --- a/chrome/browser/extensions/chrome_extensions_browser_client.h +++ b/chrome/browser/extensions/chrome_extensions_browser_client.h @@ -26,6 +26,7 @@ namespace extensions { class ChromeComponentExtensionResourceManager; class ChromeExtensionsAPIClient; +class ChromeProcessManagerDelegate; class ContentSettingsPrefsObserver; // Implementation of extensions::BrowserClient for Chrome, which includes @@ -76,10 +77,7 @@ class ChromeExtensionsBrowserClient : 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; @@ -102,6 +100,9 @@ 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 5d966c8..c8e6016 100644 --- a/chrome/browser/extensions/chrome_notification_observer.cc +++ b/chrome/browser/extensions/chrome_notification_observer.cc @@ -4,59 +4,22 @@ #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 @@ -68,12 +31,6 @@ 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 a31dc87..8045b15 100644 --- a/chrome/browser/extensions/chrome_notification_observer.h +++ b/chrome/browser/extensions/chrome_notification_observer.h @@ -25,8 +25,7 @@ class ChromeNotificationObserver : public content::NotificationObserver { ChromeNotificationObserver(); virtual ~ChromeNotificationObserver(); - // IPC message handlers: - void OnBrowserWindowReady(Browser* browser); + // Notification handlers: 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 new file mode 100644 index 0000000..12e0c16 --- /dev/null +++ b/chrome/browser/extensions/chrome_process_manager_delegate.cc @@ -0,0 +1,143 @@ +// 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 new file mode 100644 index 0000000..9bcc58a --- /dev/null +++ b/chrome/browser/extensions/chrome_process_manager_delegate.h @@ -0,0 +1,50 @@ +// 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 194dab0e..0e955f7 100644 --- a/chrome/chrome_browser_extensions.gypi +++ b/chrome/chrome_browser_extensions.gypi @@ -45,6 +45,8 @@ '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 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/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/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, diff --git a/extensions/browser/process_manager.h b/extensions/browser/process_manager.h index e580bbd..80736e4 100644 --- a/extensions/browser/process_manager.h +++ b/extensions/browser/process_manager.h @@ -33,6 +33,7 @@ namespace extensions { class Extension; class ExtensionHost; +class ProcessManagerDelegate; class ProcessManagerObserver; // Manages dynamic state of running Chromium extensions. There is one instance @@ -122,8 +123,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. @@ -144,6 +146,10 @@ 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. @@ -158,10 +164,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. @@ -182,6 +184,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 +222,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..0d1f86b 100644 --- a/extensions/browser/process_manager_unittest.cc +++ b/extensions/browser/process_manager_unittest.cc @@ -5,10 +5,12 @@ #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" @@ -33,12 +35,35 @@ 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_); } @@ -49,6 +74,23 @@ 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, @@ -58,9 +100,10 @@ class ProcessManagerTest : public testing::Test { manager, type, content::Source<BrowserContext>(context)); } - private: + protected: TestBrowserContext original_context_; TestBrowserContextIncognito incognito_context_; + TestProcessManagerDelegate process_manager_delegate_; TestExtensionsBrowserClient extensions_browser_client_; DISALLOW_COPY_AND_ASSIGN(ProcessManagerTest); @@ -125,6 +168,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::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 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 4349183..3501961 100644 --- a/extensions/extensions.gyp +++ b/extensions/extensions.gyp @@ -435,6 +435,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', |