Move PROFILE_DESTROYED notification to ProfileDestroyer and observe it in ExtensionProcessManager.

1. EPM was not observing the correct notification to shut down its ExtensionHosts.
2. PROFILE_DESTROYED has to be sent early enough that the EPM can shut down ExtensionHosts before the ProfileDestroyer checks that all renderers are gone.

BUG=138843
TEST=In Debug build, have a split mode incognito extension installed. Open and close an incognito window; no crash. Close the regular profile window; no crash.
TBR=sky@chromium.org

Review URL: https://chromiumcodereview.appspot.com/10824020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148674 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/extension_process_manager.cc

@@ -151,8 +151,12 @@ ExtensionProcessManager::ExtensionProcessManager(Profile* profile)
                  content::Source<Profile>(profile));
   registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_CONNECTED,
                  content::NotificationService::AllSources());
-  registrar_.Add(this, content::NOTIFICATION_APP_TERMINATING,
-                 content::NotificationService::AllSources());
+  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
+                 content::Source<Profile>(profile));
+  if (profile->IsOffTheRecord()) {
+    registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
+                   content::Source<Profile>(original_profile));
+  }
   registrar_.Add(this, content::NOTIFICATION_DEVTOOLS_WINDOW_OPENING,
                  content::Source<content::BrowserContext>(profile));
   registrar_.Add(this, content::NOTIFICATION_DEVTOOLS_WINDOW_CLOSING,
@@ -610,7 +614,7 @@ void ExtensionProcessManager::Observe(
       break;
     }
 
-    case content::NOTIFICATION_APP_TERMINATING: {
+    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.

chrome/browser/profiles/off_the_record_profile_impl.cc

@@ -37,7 +37,6 @@
 #include "chrome/browser/ui/webui/chrome_url_data_manager_factory.h"
 #include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
 #include "chrome/common/chrome_constants.h"
-#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
@@ -47,6 +46,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/host_zoom_map.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/transport_security_state.h"
@@ -114,9 +114,7 @@ void OffTheRecordProfileImpl::Init() {
 }
 
 OffTheRecordProfileImpl::~OffTheRecordProfileImpl() {
-  content::NotificationService::current()->Notify(
-    chrome::NOTIFICATION_PROFILE_DESTROYED, content::Source<Profile>(this),
-    content::NotificationService::NoDetails());
+  MaybeSendDestroyedNotification();
 
   ChromePluginServiceFilter::GetInstance()->UnregisterResourceContext(
     io_data_.GetResourceContextNoInit());

chrome/browser/profiles/profile.cc

@@ -10,7 +10,10 @@
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/sync_prefs.h"
+#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
 
@@ -21,6 +24,7 @@
 
 Profile::Profile()
     : restored_last_session_(false),
+      sent_destroyed_notification_(false),
       accessibility_pause_level_(0) {
 }
 
@@ -117,3 +121,13 @@ bool Profile::IsSyncAccessible() {
   browser_sync::SyncPrefs prefs(GetPrefs());
   return ProfileSyncService::IsSyncEnabled() && !prefs.IsManaged();
 }
+
+void Profile::MaybeSendDestroyedNotification() {
+  if (!sent_destroyed_notification_) {
+    sent_destroyed_notification_ = true;
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_PROFILE_DESTROYED,
+        content::Source<Profile>(this),
+        content::NotificationService::NoDetails());
+  }
+}

chrome/browser/profiles/profile.h

@@ -390,6 +390,11 @@ class Profile : public content::BrowserContext {
   // disabled or controlled by configuration management.
   bool IsSyncAccessible();
 
+  // Send NOTIFICATION_PROFILE_DESTROYED for this Profile, if it has not
+  // already been sent. It is necessary because most Profiles are destroyed by
+  // ProfileDestroyer, but in tests, some are not.
+  void MaybeSendDestroyedNotification();
+
   // Creates an OffTheRecordProfile which points to this Profile.
   Profile* CreateOffTheRecordProfile();
 
@@ -409,6 +414,10 @@ class Profile : public content::BrowserContext {
  private:
   bool restored_last_session_;
 
+  // Used to prevent the notification that this Profile is destroyed from
+  // being sent twice.
+  bool sent_destroyed_notification_;
+
   // Accessibility events will only be propagated when the pause
   // level is zero.  PauseAccessibilityEvents and ResumeAccessibilityEvents
   // increment and decrement the level, respectively, rather than set it to

chrome/browser/profiles/profile_destroyer.cc

@@ -24,6 +24,8 @@ std::vector<ProfileDestroyer*>* ProfileDestroyer::pending_destroyers_ = NULL;
 // static
 void ProfileDestroyer::DestroyProfileWhenAppropriate(Profile* const profile) {
   DCHECK(profile);
+  profile->MaybeSendDestroyedNotification();
+
   std::vector<content::RenderProcessHost*> hosts;
   // Testing profiles can simply be deleted directly. Some tests don't setup
   // RenderProcessHost correctly and don't necessary run on the UI thread
@@ -85,7 +87,7 @@ ProfileDestroyer::ProfileDestroyer(
   pending_destroyers_->push_back(this);
   for (size_t i = 0; i < hosts.size(); ++i) {
     registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-        content::Source<content::RenderProcessHost>(hosts[i]));
+                   content::Source<content::RenderProcessHost>(hosts[i]));
     // For each of the notifications, we bump up our reference count.
     // It will go back to 0 and free us when all hosts are terminated.
     ++num_hosts_;
@@ -126,6 +128,8 @@ void ProfileDestroyer::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
   DCHECK(type == content::NOTIFICATION_RENDERER_PROCESS_TERMINATED);
+  registrar_.Remove(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
+                    source);
   DCHECK(num_hosts_ > 0);
   --num_hosts_;
   if (num_hosts_ == 0) {

chrome/browser/profiles/profile_impl.cc

@@ -473,10 +473,8 @@ void ProfileImpl::set_last_selected_directory(const FilePath& path) {
 }
 
 ProfileImpl::~ProfileImpl() {
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_PROFILE_DESTROYED,
-      content::Source<Profile>(this),
-      content::NotificationService::NoDetails());
+  MaybeSendDestroyedNotification();
+
   bool prefs_loaded = prefs_->GetInitializationStatus() !=
       PrefService::INITIALIZATION_STATUS_WAITING;
 

chrome/test/base/testing_profile.cc

@@ -248,11 +248,7 @@ void TestingProfile::FinishInit() {
 }
 
 TestingProfile::~TestingProfile() {
-  DCHECK(content::NotificationService::current());
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_PROFILE_DESTROYED,
-      content::Source<Profile>(static_cast<Profile*>(this)),
-      content::NotificationService::NoDetails());
+  MaybeSendDestroyedNotification();
 
   profile_dependency_manager_->DestroyProfileServices(this);