diff options
author | jamescook@chromium.org <jamescook@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-25 05:24:37 +0000 |
---|---|---|
committer | jamescook@chromium.org <jamescook@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-25 05:24:37 +0000 |
commit | 9602db41e8152f6dd3f2a27460b7db5a5783087c (patch) | |
tree | 2ca095245c96e6f3b189bc949edae167e30839d6 | |
parent | a83a71cab8c73e00a1e12ce23579620a966c107a (diff) | |
download | chromium_src-9602db41e8152f6dd3f2a27460b7db5a5783087c.zip chromium_src-9602db41e8152f6dd3f2a27460b7db5a5783087c.tar.gz chromium_src-9602db41e8152f6dd3f2a27460b7db5a5783087c.tar.bz2 |
Remove chrome::NOTIFICATION_PROFILE_DESTROYED usage from src/extensions
The extensions module should not be listening to Chrome notifications.
Move the observation to ChromeProcessManagerDelegate.
BUG=396083
TEST=browser_tests ProcessManager* and unit_tests ProcessManager*
Review URL: https://codereview.chromium.org/415933003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285482 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/extensions/chrome_process_manager_delegate.cc | 32 | ||||
-rw-r--r-- | chrome/browser/extensions/chrome_process_manager_delegate.h | 1 | ||||
-rw-r--r-- | extensions/browser/process_manager.cc | 30 | ||||
-rw-r--r-- | extensions/browser/process_manager.h | 6 | ||||
-rw-r--r-- | extensions/browser/process_manager_unittest.cc | 13 |
5 files changed, 41 insertions, 41 deletions
diff --git a/chrome/browser/extensions/chrome_process_manager_delegate.cc b/chrome/browser/extensions/chrome_process_manager_delegate.cc index 12e0c16..511e5cd 100644 --- a/chrome/browser/extensions/chrome_process_manager_delegate.cc +++ b/chrome/browser/extensions/chrome_process_manager_delegate.cc @@ -30,8 +30,9 @@ ChromeProcessManagerDelegate::ChromeProcessManagerDelegate() { registrar_.Add(this, chrome::NOTIFICATION_PROFILE_CREATED, content::NotificationService::AllSources()); - // TODO(jamescook): Move observation of NOTIFICATION_PROFILE_DESTROYED here. - // http://crbug.com/392658 + registrar_.Add(this, + chrome::NOTIFICATION_PROFILE_DESTROYED, + content::NotificationService::AllSources()); } ChromeProcessManagerDelegate::~ChromeProcessManagerDelegate() { @@ -85,7 +86,11 @@ void ChromeProcessManagerDelegate::Observe( OnProfileCreated(profile); break; } - + case chrome::NOTIFICATION_PROFILE_DESTROYED: { + Profile* profile = content::Source<Profile>(source).ptr(); + OnProfileDestroyed(profile); + break; + } default: NOTREACHED(); } @@ -140,4 +145,25 @@ void ChromeProcessManagerDelegate::OnProfileCreated(Profile* profile) { manager->MaybeCreateStartupBackgroundHosts(); } +void ChromeProcessManagerDelegate::OnProfileDestroyed(Profile* profile) { + // Close background hosts when the last profile is closed so that they + // have time to shutdown various objects on different threads. The + // ProfileManager destructor is called too late in the shutdown sequence. + // http://crbug.com/15708 + ProcessManager* manager = ExtensionSystem::Get(profile)->process_manager(); + if (manager) + manager->CloseBackgroundHosts(); + + // If this profile owns an incognito profile, but it is destroyed before the + // incognito profile is destroyed, then close the incognito background hosts + // as well. This happens in a few tests. http://crbug.com/138843 + if (!profile->IsOffTheRecord() && profile->HasOffTheRecordProfile()) { + ProcessManager* incognito_manager = + ExtensionSystem::Get(profile->GetOffTheRecordProfile()) + ->process_manager(); + if (incognito_manager) + incognito_manager->CloseBackgroundHosts(); + } +} + } // namespace extensions diff --git a/chrome/browser/extensions/chrome_process_manager_delegate.h b/chrome/browser/extensions/chrome_process_manager_delegate.h index 9bcc58a..e2f0fbe 100644 --- a/chrome/browser/extensions/chrome_process_manager_delegate.h +++ b/chrome/browser/extensions/chrome_process_manager_delegate.h @@ -39,6 +39,7 @@ class ChromeProcessManagerDelegate : public ProcessManagerDelegate, // Notification handlers. void OnBrowserWindowReady(Browser* browser); void OnProfileCreated(Profile* profile); + void OnProfileDestroyed(Profile* profile); content::NotificationRegistrar registrar_; diff --git a/extensions/browser/process_manager.cc b/extensions/browser/process_manager.cc index 72fbd2c..d111d62 100644 --- a/extensions/browser/process_manager.cc +++ b/extensions/browser/process_manager.cc @@ -263,12 +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_DESTROYED, - content::Source<BrowserContext>(context)); - if (context->IsOffTheRecord()) { - registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - content::Source<BrowserContext>(original_context)); - } // Note: event_page_idle_time_ must be sufficiently larger (e.g. 2x) than // kKeepaliveThrottleIntervalInSeconds in ppapi/proxy/plugin_globals. @@ -633,6 +627,14 @@ void ProcessManager::CancelSuspend(const Extension* extension) { } } +void ProcessManager::CloseBackgroundHosts() { + for (ExtensionHostSet::iterator iter = background_hosts_.begin(); + iter != background_hosts_.end();) { + ExtensionHostSet::iterator current = iter++; + delete *current; + } +} + content::BrowserContext* ProcessManager::GetBrowserContext() const { return site_instance_->GetBrowserContext(); } @@ -745,14 +747,6 @@ void ProcessManager::Observe(int type, break; } - case chrome::NOTIFICATION_PROFILE_DESTROYED: { - // Close background hosts when the last browser is closed so that they - // have time to shutdown various objects on different threads. Our - // destructor is called too late in the shutdown sequence. - CloseBackgroundHosts(); - break; - } - default: NOTREACHED(); } @@ -854,14 +848,6 @@ void ProcessManager::CloseBackgroundHost(ExtensionHost* host) { CHECK(background_hosts_.find(host) == background_hosts_.end()); } -void ProcessManager::CloseBackgroundHosts() { - for (ExtensionHostSet::iterator iter = background_hosts_.begin(); - iter != background_hosts_.end(); ) { - ExtensionHostSet::iterator current = iter++; - delete *current; - } -} - void ProcessManager::UnregisterExtension(const std::string& extension_id) { // The lazy_keepalive_count may be greater than zero at this point because // RenderViewHosts are still alive. During extension reloading, they will diff --git a/extensions/browser/process_manager.h b/extensions/browser/process_manager.h index e3c6bcf..a5422bc7 100644 --- a/extensions/browser/process_manager.h +++ b/extensions/browser/process_manager.h @@ -128,6 +128,9 @@ class ProcessManager : public content::NotificationObserver { // loaded. void MaybeCreateStartupBackgroundHosts(); + // Called on shutdown to close our extension hosts. + void CloseBackgroundHosts(); + // Gets the BrowserContext associated with site_instance_ and all other // related SiteInstances. content::BrowserContext* GetBrowserContext() const; @@ -165,9 +168,6 @@ class ProcessManager : public content::NotificationObserver { content::BrowserContext* original_context, ExtensionRegistry* registry); - // Called on browser shutdown to close our extension hosts. - void CloseBackgroundHosts(); - // content::NotificationObserver: virtual void Observe(int type, const content::NotificationSource& source, diff --git a/extensions/browser/process_manager_unittest.cc b/extensions/browser/process_manager_unittest.cc index f42e227..121f110 100644 --- a/extensions/browser/process_manager_unittest.cc +++ b/extensions/browser/process_manager_unittest.cc @@ -169,23 +169,10 @@ TEST_F(ProcessManagerTest, ExtensionNotificationRegistration) { chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED, incognito_context())); - // Some notifications are observed for both incognito and original. - EXPECT_TRUE(IsRegistered(manager2.get(), - chrome::NOTIFICATION_PROFILE_DESTROYED, - original_context())); - EXPECT_TRUE(IsRegistered(manager2.get(), - chrome::NOTIFICATION_PROFILE_DESTROYED, - incognito_context())); - // Some are not observed at all. EXPECT_FALSE(IsRegistered(manager2.get(), chrome::NOTIFICATION_EXTENSIONS_READY, original_context())); - - // This notification is observed for incognito contexts only. - EXPECT_TRUE(IsRegistered(manager2.get(), - chrome::NOTIFICATION_PROFILE_DESTROYED, - incognito_context())); } // Test that startup background hosts are created when the extension system |