summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorscheib@chromium.org <scheib@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-21 18:25:34 +0000
committerscheib@chromium.org <scheib@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-21 18:25:34 +0000
commitbc118e52c97b739a72a0987a368075429e923ba6 (patch)
tree8a4643f292fd61d2d7342562a88e9df51f06522a
parent3b09d5e1d53f61291e20e2701c4ada568a2ae765 (diff)
downloadchromium_src-bc118e52c97b739a72a0987a368075429e923ba6.zip
chromium_src-bc118e52c97b739a72a0987a368075429e923ba6.tar.gz
chromium_src-bc118e52c97b739a72a0987a368075429e923ba6.tar.bz2
Revert of Unload all apps / extensions when deleting a profile. (https://codereview.chromium.org/266343002/)
Reason for revert: Caused crash in official builds when deleting profiles while the chrome:chrome-signin tab is open BUG=374683 TBR=kalman, sky Original issue's description: > 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 > TEST=Manual testing as described on http://crbug.com/368684#c1 > > 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/289283013 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271926 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/background/background_contents_service.cc3
-rw-r--r--chrome/browser/chrome_notification_types.h6
-rw-r--r--chrome/browser/extensions/extension_service.cc16
-rw-r--r--chrome/browser/extensions/extension_service.h5
-rw-r--r--chrome/browser/extensions/extension_service_unittest.cc40
-rw-r--r--chrome/browser/profiles/profile_manager.cc7
-rw-r--r--chrome/browser/ui/browser.cc5
-rw-r--r--extensions/browser/process_manager.cc5
-rw-r--r--extensions/common/extension.h12
9 files changed, 12 insertions, 87 deletions
diff --git a/chrome/browser/background/background_contents_service.cc b/chrome/browser/background/background_contents_service.cc
index b83decd..0d21871 100644
--- a/chrome/browser/background/background_contents_service.cc
+++ b/chrome/browser/background/background_contents_service.cc
@@ -468,8 +468,7 @@ void BackgroundContentsService::Observe(
case UnloadedExtensionInfo::REASON_DISABLE: // Fall through.
case UnloadedExtensionInfo::REASON_TERMINATE: // Fall through.
case UnloadedExtensionInfo::REASON_UNINSTALL: // Fall through.
- case UnloadedExtensionInfo::REASON_BLACKLIST: // Fall through.
- case UnloadedExtensionInfo::REASON_PROFILE_SHUTDOWN:
+ case UnloadedExtensionInfo::REASON_BLACKLIST:
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 9303cac..6e1adf4 100644
--- a/chrome/browser/chrome_notification_types.h
+++ b/chrome/browser/chrome_notification_types.h
@@ -287,12 +287,6 @@ enum NotificationType {
// 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 04dac42..7a357fd 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -301,9 +301,6 @@ ExtensionService::ExtensionService(Profile* profile,
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,
@@ -2168,10 +2165,6 @@ void ExtensionService::Observe(int type,
OnChromeUpdateAvailable());
break;
}
- case chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED: {
- OnProfileDestructionStarted();
- break;
- }
default:
NOTREACHED() << "Unexpected notification type.";
@@ -2435,12 +2428,3 @@ void ExtensionService::UnloadAllExtensionsInternal() {
// 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 1e5f5b5..adafa07 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -549,11 +549,6 @@ class ExtensionService
// 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 0fef7ad..a5ed2ba 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -165,7 +165,6 @@ using extensions::FeatureSwitch;
using extensions::Manifest;
using extensions::PermissionSet;
using extensions::TestExtensionSystem;
-using extensions::UnloadedExtensionInfo;
using extensions::URLPatternSet;
namespace keys = extensions::manifest_keys;
@@ -667,12 +666,10 @@ class ExtensionServiceTest
: public ExtensionServiceTestBase, public content::NotificationObserver {
public:
ExtensionServiceTest()
- : unloaded_reason_(UnloadedExtensionInfo::REASON_UNDEFINED),
- installed_(NULL),
+ : 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());
@@ -697,11 +694,10 @@ class ExtensionServiceTest
}
case chrome::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED: {
- UnloadedExtensionInfo* unloaded_info =
- content::Details<UnloadedExtensionInfo>(details).ptr();
- const Extension* e = unloaded_info->extension;
+ const Extension* e =
+ content::Details<extensions::UnloadedExtensionInfo>(
+ details)->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
@@ -1254,7 +1250,6 @@ class ExtensionServiceTest
protected:
extensions::ExtensionList loaded_;
std::string unloaded_id_;
- UnloadedExtensionInfo::Reason unloaded_reason_;
const Extension* installed_;
bool was_update_;
std::string old_name_;
@@ -4202,7 +4197,6 @@ TEST_F(ExtensionServiceTest, UninstallExtension) {
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) {
@@ -4210,7 +4204,6 @@ TEST_F(ExtensionServiceTest, UninstallTerminatedExtension) {
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.
@@ -4218,7 +4211,6 @@ TEST_F(ExtensionServiceTest, UninstallExtensionHelper) {
InitializeEmptyExtensionService();
InstallCRX(data_dir_.AppendASCII("good.crx"), INSTALL_NEW);
UninstallExtension(good_crx, true);
- EXPECT_EQ(UnloadedExtensionInfo::REASON_UNINSTALL, unloaded_reason_);
}
TEST_F(ExtensionServiceTest, UninstallExtensionHelperTerminated) {
@@ -4226,7 +4218,6 @@ TEST_F(ExtensionServiceTest, UninstallExtensionHelperTerminated) {
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
@@ -6962,24 +6953,3 @@ TEST_F(ExtensionServiceTest, ReconcileKnownDisabledWithSideEnable) {
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 1746f90..5d67669 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -1073,13 +1073,6 @@ void ProfileManager::FinishDeletingProfile(const base::FilePath& profile_dir) {
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 f3e71c1..0643a18 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -445,11 +445,6 @@ Browser::Browser(const CreateParams& params)
}
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);
diff --git a/extensions/browser/process_manager.cc b/extensions/browser/process_manager.cc
index b110aa3..9e5f0c6 100644
--- a/extensions/browser/process_manager.cc
+++ b/extensions/browser/process_manager.cc
@@ -436,10 +436,7 @@ void ProcessManager::DecrementLazyKeepaliveCount(const Extension* extension) {
void ProcessManager::DecrementLazyKeepaliveCount(
const std::string& extension_id) {
int& count = background_page_data_[extension_id].lazy_keepalive_count;
- DCHECK(count > 0 ||
- !ExtensionRegistry::Get(GetBrowserContext())
- ->enabled_extensions()
- .Contains(extension_id));
+ DCHECK_GT(count, 0);
// If we reach a zero keepalive count when the lazy background page is about
// to be closed, incrementing close_sequence_id will cancel the close
diff --git a/extensions/common/extension.h b/extensions/common/extension.h
index 6679252..4000910 100644
--- a/extensions/common/extension.h
+++ b/extensions/common/extension.h
@@ -522,13 +522,11 @@ struct InstalledExtensionInfo {
struct UnloadedExtensionInfo {
// TODO(DHNishi): Move this enum to ExtensionRegistryObserver.
enum Reason {
- REASON_UNDEFINED, // Undefined state used to initialize variables.
- REASON_DISABLE, // Extension is being disabled.
- REASON_UPDATE, // Extension is being updated to a newer version.
- REASON_UNINSTALL, // Extension is being uninstalled.
- REASON_TERMINATE, // Extension has terminated.
- REASON_BLACKLIST, // Extension has been blacklisted.
- REASON_PROFILE_SHUTDOWN, // Profile is being shut down.
+ REASON_DISABLE, // Extension is being disabled.
+ REASON_UPDATE, // Extension is being updated to a newer version.
+ REASON_UNINSTALL, // Extension is being uninstalled.
+ REASON_TERMINATE, // Extension has terminated.
+ REASON_BLACKLIST, // Extension has been blacklisted.
};
Reason reason;