Unload all apps / extensions immediately when deleting a profile.
Previously apps could remain running with references to profiles that had been deleted by users, but before the browser shut down and profiles were fully removed. Problems included E.g. opening a link in an app would open a tab in the deleted profile.
Relanding patch: ShutdownStartupCycle failed in build [49353], this patch was speculatively reverted in r269383 [49355], but ShutdownStartupCycle failed again in [49362] after the revert had landed.
[49353] http://build.chromium.org/p/chromium.mac/builders/Mac%2010.6%20Tests%20%28dbg%29%281%29/builds/49353
[49355] http://build.chromium.org/p/chromium.mac/builders/Mac%2010.6%20Tests%20%28dbg%29%281%29/builds/49355
[49362] http://build.chromium.org/p/chromium.mac/builders/Mac%2010.6%20Tests%20%28dbg%29%281%29/builds/49362
BUG=368684, 374683
TEST=Manual testing as described on http://crbug.com/368684#c1 and http://crbug.com/374683#c7
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269343
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270890
Review URL: https://codereview.chromium.org/266343002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272090 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/background/background_contents_service.cc b/chrome/browser/background/background_contents_service.cc
index 45e84c1..5be9d4d 100644
--- a/chrome/browser/background/background_contents_service.cc
+++ b/chrome/browser/background/background_contents_service.cc
@@ -468,7 +468,8 @@
case UnloadedExtensionInfo::REASON_DISABLE: // Fall through.
case UnloadedExtensionInfo::REASON_TERMINATE: // Fall through.
case UnloadedExtensionInfo::REASON_UNINSTALL: // Fall through.
- case UnloadedExtensionInfo::REASON_BLACKLIST:
+ case UnloadedExtensionInfo::REASON_BLACKLIST: // Fall through.
+ case UnloadedExtensionInfo::REASON_PROFILE_SHUTDOWN:
ShutdownAssociatedBackgroundContents(base::ASCIIToUTF16(
content::Details<UnloadedExtensionInfo>(details)->
extension->id()));
diff --git a/chrome/browser/chrome_notification_types.h b/chrome/browser/chrome_notification_types.h
index 6e1adf4..9303cac 100644
--- a/chrome/browser/chrome_notification_types.h
+++ b/chrome/browser/chrome_notification_types.h
@@ -287,6 +287,12 @@
// The details are none and the source is the new profile.
NOTIFICATION_PROFILE_ADDED,
+ // Sent early in the process of destroying a Profile, at the time a user
+ // initiates the deletion of a profile versus the much later time when the
+ // profile object is actually destroyed (use NOTIFICATION_PROFILE_DESTROYED).
+ // The details are none and the source is a Profile*.
+ NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
+
// Sent before a Profile is destroyed. This notification is sent both for
// normal and OTR profiles.
// The details are none and the source is a Profile*.
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 7a357fd..702b7fc4 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -301,6 +301,9 @@
content::NotificationService::AllBrowserContextsAndSources());
registrar_.Add(this, chrome::NOTIFICATION_UPGRADE_RECOMMENDED,
content::NotificationService::AllBrowserContextsAndSources());
+ registrar_.Add(this,
+ chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
+ content::Source<Profile>(profile_));
pref_change_registrar_.Init(profile->GetPrefs());
base::Closure callback =
base::Bind(&ExtensionService::OnExtensionInstallPrefChanged,
@@ -1398,10 +1401,12 @@
scoped_refptr<const Extension> extension(
GetExtensionById(extension_id, false));
UnloadExtension(extension_id, UnloadedExtensionInfo::REASON_UNINSTALL);
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
- content::Source<Profile>(profile_),
- content::Details<const Extension>(extension.get()));
+ if (extension.get()) {
+ content::NotificationService::current()->Notify(
+ chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
+ content::Source<Profile>(profile_),
+ content::Details<const Extension>(extension.get()));
+ }
}
void ExtensionService::UnloadAllExtensionsForTest() {
@@ -2165,6 +2170,10 @@
OnChromeUpdateAvailable());
break;
}
+ case chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED: {
+ OnProfileDestructionStarted();
+ break;
+ }
default:
NOTREACHED() << "Unexpected notification type.";
@@ -2428,3 +2437,12 @@
// EXTENSION_UNLOADED since that implies that the extension has been disabled
// or uninstalled.
}
+
+void ExtensionService::OnProfileDestructionStarted() {
+ ExtensionIdSet ids_to_unload = registry_->enabled_extensions().GetIDs();
+ for (ExtensionIdSet::iterator it = ids_to_unload.begin();
+ it != ids_to_unload.end();
+ ++it) {
+ UnloadExtension(*it, UnloadedExtensionInfo::REASON_PROFILE_SHUTDOWN);
+ }
+}
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index adafa07..1e5f5b50 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -549,6 +549,11 @@
// Used only by test code.
void UnloadAllExtensionsInternal();
+ // Disable apps & extensions now to stop them from running after a profile
+ // has been conceptually deleted. Don't wait for full browser shutdown and
+ // the actual profile objects to be destroyed.
+ void OnProfileDestructionStarted();
+
// The normal profile associated with this ExtensionService.
Profile* profile_;
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index a5ed2ba..0fef7ad 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -165,6 +165,7 @@
using extensions::Manifest;
using extensions::PermissionSet;
using extensions::TestExtensionSystem;
+using extensions::UnloadedExtensionInfo;
using extensions::URLPatternSet;
namespace keys = extensions::manifest_keys;
@@ -666,10 +667,12 @@
: public ExtensionServiceTestBase, public content::NotificationObserver {
public:
ExtensionServiceTest()
- : installed_(NULL),
+ : unloaded_reason_(UnloadedExtensionInfo::REASON_UNDEFINED),
+ installed_(NULL),
was_update_(false),
override_external_install_prompt_(
- FeatureSwitch::prompt_for_external_extensions(), false) {
+ FeatureSwitch::prompt_for_external_extensions(),
+ false) {
registrar_.Add(this,
chrome::NOTIFICATION_EXTENSION_LOADED_DEPRECATED,
content::NotificationService::AllSources());
@@ -694,10 +697,11 @@
}
case chrome::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED: {
- const Extension* e =
- content::Details<extensions::UnloadedExtensionInfo>(
- details)->extension;
+ UnloadedExtensionInfo* unloaded_info =
+ content::Details<UnloadedExtensionInfo>(details).ptr();
+ const Extension* e = unloaded_info->extension;
unloaded_id_ = e->id();
+ unloaded_reason_ = unloaded_info->reason;
extensions::ExtensionList::iterator i =
std::find(loaded_.begin(), loaded_.end(), e);
// TODO(erikkay) fix so this can be an assert. Right now the tests
@@ -1250,6 +1254,7 @@
protected:
extensions::ExtensionList loaded_;
std::string unloaded_id_;
+ UnloadedExtensionInfo::Reason unloaded_reason_;
const Extension* installed_;
bool was_update_;
std::string old_name_;
@@ -4197,6 +4202,7 @@
EXPECT_EQ(1u, registry_->enabled_extensions().size());
UninstallExtension(good_crx, false);
EXPECT_EQ(0u, registry_->enabled_extensions().size());
+ EXPECT_EQ(UnloadedExtensionInfo::REASON_UNINSTALL, unloaded_reason_);
}
TEST_F(ExtensionServiceTest, UninstallTerminatedExtension) {
@@ -4204,6 +4210,7 @@
InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW);
TerminateExtension(good_crx);
UninstallExtension(good_crx, false);
+ EXPECT_EQ(UnloadedExtensionInfo::REASON_TERMINATE, unloaded_reason_);
}
// Tests the uninstaller helper.
@@ -4211,6 +4218,7 @@
InitializeEmptyExtensionService();
InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW);
UninstallExtension(good_crx, true);
+ EXPECT_EQ(UnloadedExtensionInfo::REASON_UNINSTALL, unloaded_reason_);
}
TEST_F(ExtensionServiceTest, UninstallExtensionHelperTerminated) {
@@ -4218,6 +4226,7 @@
InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW);
TerminateExtension(good_crx);
UninstallExtension(good_crx, true);
+ EXPECT_EQ(UnloadedExtensionInfo::REASON_TERMINATE, unloaded_reason_);
}
// An extension disabled because of unsupported requirements should re-enabled
@@ -6953,3 +6962,24 @@
EXPECT_EQ(expected_disabled_extensions,
registry_->disabled_extensions().GetIDs());
}
+
+// Tests a profile being destroyed correctly disables extensions.
+TEST_F(ExtensionServiceTest, DestroyingProfileClearsExtensions) {
+ InitializeEmptyExtensionService();
+
+ InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW);
+ EXPECT_NE(UnloadedExtensionInfo::REASON_PROFILE_SHUTDOWN, unloaded_reason_);
+ EXPECT_EQ(1u, registry_->enabled_extensions().size());
+ EXPECT_EQ(0u, registry_->disabled_extensions().size());
+ EXPECT_EQ(0u, registry_->terminated_extensions().size());
+ EXPECT_EQ(0u, registry_->blacklisted_extensions().size());
+
+ service_->Observe(chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
+ content::Source<Profile>(profile_.get()),
+ content::NotificationService::NoDetails());
+ EXPECT_EQ(UnloadedExtensionInfo::REASON_PROFILE_SHUTDOWN, unloaded_reason_);
+ EXPECT_EQ(0u, registry_->enabled_extensions().size());
+ EXPECT_EQ(0u, registry_->disabled_extensions().size());
+ EXPECT_EQ(0u, registry_->terminated_extensions().size());
+ EXPECT_EQ(0u, registry_->blacklisted_extensions().size());
+}
diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc
index 5d67669..1746f905 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -1073,6 +1073,13 @@
Profile* profile = GetProfileByPath(profile_dir);
if (profile) {
+ // TODO: Migrate additional code in this block to observe this notification
+ // instead of being implemented here.
+ content::NotificationService::current()->Notify(
+ chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
+ content::Source<Profile>(profile),
+ content::NotificationService::NoDetails());
+
// By this point, all in-progress downloads for the profile being deleted
// must have been canceled (crbug.com/336725).
DCHECK(DownloadServiceFactory::GetForBrowserContext(profile)->
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 0643a18..f3e71c1 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -445,6 +445,11 @@
}
Browser::~Browser() {
+ // Stop observing notifications before continuing with destruction. Profile
+ // destruction will unload extensions and reentrant calls to Browser:: should
+ // be avoided while it is being torn down.
+ registrar_.RemoveAll();
+
// The tab strip should not have any tabs at this point.
DCHECK(tab_strip_model_->empty());
tab_strip_model_->RemoveObserver(this);
