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

9 files changed, 93 insertions, 16 deletions

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 @@ 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:
+        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 @@ 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 7a357fd..702b7fc 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -301,6 +301,9 @@ 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,
@@ -1398,10 +1401,12 @@ void ExtensionService::RemoveComponentExtension(
   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 @@ void ExtensionService::Observe(int type,
                         OnChromeUpdateAvailable());
       break;
     }
+    case chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED: {
+      OnProfileDestructionStarted();
+      break;
+    }
 
     default:
       NOTREACHED() << "Unexpected notification type.";
@@ -2428,3 +2437,12 @@ 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 adafa07..1e5f5b5 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -549,6 +549,11 @@ 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 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::FeatureSwitch;
 using extensions::Manifest;
 using extensions::PermissionSet;
 using extensions::TestExtensionSystem;
+using extensions::UnloadedExtensionInfo;
 using extensions::URLPatternSet;
 
 namespace keys = extensions::manifest_keys;
@@ -666,10 +667,12 @@ class ExtensionServiceTest
   : 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 @@ class ExtensionServiceTest
       }
 
       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 @@ class ExtensionServiceTest
  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 @@ 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) {
@@ -4204,6 +4210,7 @@ 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.
@@ -4211,6 +4218,7 @@ 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) {
@@ -4218,6 +4226,7 @@ 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
@@ -6953,3 +6962,24 @@ 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 5d67669..1746f90 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -1073,6 +1073,13 @@ 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 0643a18..f3e71c1 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -445,6 +445,11 @@ 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 9e5f0c6..b110aa3 100644
--- a/extensions/browser/process_manager.cc
+++ b/extensions/browser/process_manager.cc
@@ -436,7 +436,10 @@ 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_GT(count, 0);
+  DCHECK(count > 0 ||
+         !ExtensionRegistry::Get(GetBrowserContext())
+              ->enabled_extensions()
+              .Contains(extension_id));
 
   // 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 4000910..6679252 100644
--- a/extensions/common/extension.h
+++ b/extensions/common/extension.h
@@ -522,11 +522,13 @@ struct InstalledExtensionInfo {
 struct UnloadedExtensionInfo {
   // TODO(DHNishi): Move this enum to ExtensionRegistryObserver.
   enum Reason {
-    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_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 reason;