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
parent a83a71ca
...@@ -30,8 +30,9 @@ ChromeProcessManagerDelegate::ChromeProcessManagerDelegate() { ...@@ -30,8 +30,9 @@ ChromeProcessManagerDelegate::ChromeProcessManagerDelegate() {
registrar_.Add(this, registrar_.Add(this,
chrome::NOTIFICATION_PROFILE_CREATED, chrome::NOTIFICATION_PROFILE_CREATED,
content::NotificationService::AllSources()); content::NotificationService::AllSources());
// TODO(jamescook): Move observation of NOTIFICATION_PROFILE_DESTROYED here. registrar_.Add(this,
// http://crbug.com/392658 chrome::NOTIFICATION_PROFILE_DESTROYED,
content::NotificationService::AllSources());
} }
ChromeProcessManagerDelegate::~ChromeProcessManagerDelegate() { ChromeProcessManagerDelegate::~ChromeProcessManagerDelegate() {
...@@ -85,7 +86,11 @@ void ChromeProcessManagerDelegate::Observe( ...@@ -85,7 +86,11 @@ void ChromeProcessManagerDelegate::Observe(
OnProfileCreated(profile); OnProfileCreated(profile);
break; break;
} }
case chrome::NOTIFICATION_PROFILE_DESTROYED: {
Profile* profile = content::Source<Profile>(source).ptr();
OnProfileDestroyed(profile);
break;
}
default: default:
NOTREACHED(); NOTREACHED();
} }
...@@ -140,4 +145,25 @@ void ChromeProcessManagerDelegate::OnProfileCreated(Profile* profile) { ...@@ -140,4 +145,25 @@ void ChromeProcessManagerDelegate::OnProfileCreated(Profile* profile) {
manager->MaybeCreateStartupBackgroundHosts(); 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 } // namespace extensions
...@@ -39,6 +39,7 @@ class ChromeProcessManagerDelegate : public ProcessManagerDelegate, ...@@ -39,6 +39,7 @@ class ChromeProcessManagerDelegate : public ProcessManagerDelegate,
// Notification handlers. // Notification handlers.
void OnBrowserWindowReady(Browser* browser); void OnBrowserWindowReady(Browser* browser);
void OnProfileCreated(Profile* profile); void OnProfileCreated(Profile* profile);
void OnProfileDestroyed(Profile* profile);
content::NotificationRegistrar registrar_; content::NotificationRegistrar registrar_;
......
...@@ -263,12 +263,6 @@ ProcessManager::ProcessManager(BrowserContext* context, ...@@ -263,12 +263,6 @@ ProcessManager::ProcessManager(BrowserContext* context,
content::NotificationService::AllSources()); content::NotificationService::AllSources());
registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_CONNECTED, registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_CONNECTED,
content::NotificationService::AllSources()); 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 // Note: event_page_idle_time_ must be sufficiently larger (e.g. 2x) than
// kKeepaliveThrottleIntervalInSeconds in ppapi/proxy/plugin_globals. // kKeepaliveThrottleIntervalInSeconds in ppapi/proxy/plugin_globals.
...@@ -633,6 +627,14 @@ void ProcessManager::CancelSuspend(const Extension* extension) { ...@@ -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 { content::BrowserContext* ProcessManager::GetBrowserContext() const {
return site_instance_->GetBrowserContext(); return site_instance_->GetBrowserContext();
} }
...@@ -745,14 +747,6 @@ void ProcessManager::Observe(int type, ...@@ -745,14 +747,6 @@ void ProcessManager::Observe(int type,
break; 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: default:
NOTREACHED(); NOTREACHED();
} }
...@@ -854,14 +848,6 @@ void ProcessManager::CloseBackgroundHost(ExtensionHost* host) { ...@@ -854,14 +848,6 @@ void ProcessManager::CloseBackgroundHost(ExtensionHost* host) {
CHECK(background_hosts_.find(host) == background_hosts_.end()); 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) { void ProcessManager::UnregisterExtension(const std::string& extension_id) {
// The lazy_keepalive_count may be greater than zero at this point because // The lazy_keepalive_count may be greater than zero at this point because
// RenderViewHosts are still alive. During extension reloading, they will // RenderViewHosts are still alive. During extension reloading, they will
......
...@@ -128,6 +128,9 @@ class ProcessManager : public content::NotificationObserver { ...@@ -128,6 +128,9 @@ class ProcessManager : public content::NotificationObserver {
// loaded. // loaded.
void MaybeCreateStartupBackgroundHosts(); void MaybeCreateStartupBackgroundHosts();
// Called on shutdown to close our extension hosts.
void CloseBackgroundHosts();
// Gets the BrowserContext associated with site_instance_ and all other // Gets the BrowserContext associated with site_instance_ and all other
// related SiteInstances. // related SiteInstances.
content::BrowserContext* GetBrowserContext() const; content::BrowserContext* GetBrowserContext() const;
...@@ -165,9 +168,6 @@ class ProcessManager : public content::NotificationObserver { ...@@ -165,9 +168,6 @@ class ProcessManager : public content::NotificationObserver {
content::BrowserContext* original_context, content::BrowserContext* original_context,
ExtensionRegistry* registry); ExtensionRegistry* registry);
// Called on browser shutdown to close our extension hosts.
void CloseBackgroundHosts();
// content::NotificationObserver: // content::NotificationObserver:
virtual void Observe(int type, virtual void Observe(int type,
const content::NotificationSource& source, const content::NotificationSource& source,
......
...@@ -169,23 +169,10 @@ TEST_F(ProcessManagerTest, ExtensionNotificationRegistration) { ...@@ -169,23 +169,10 @@ TEST_F(ProcessManagerTest, ExtensionNotificationRegistration) {
chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED, chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
incognito_context())); 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. // Some are not observed at all.
EXPECT_FALSE(IsRegistered(manager2.get(), EXPECT_FALSE(IsRegistered(manager2.get(),
chrome::NOTIFICATION_EXTENSIONS_READY, chrome::NOTIFICATION_EXTENSIONS_READY,
original_context())); 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 // Test that startup background hosts are created when the extension system
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment