Remove RenderProcessHost notifications from the signin manager.

Remove RenderProcessHost notifications from the signin manager.

BUG=170921
TEST=everything still works

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244300

[Andrew T Wilson (Slow), 2014-01-10 10:02:44]

lgtm

[commit-bot: I haz the power, 2014-01-10 15:37:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/126613002/1

[commit-bot: I haz the power, 2014-01-10 15:37:12]

Failed to apply patch for chrome/browser/signin/signin_manager.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/signin/signin_manager.cc
  Hunk #2 succeeded at 98 with fuzz 1.
  Hunk #3 succeeded at 129 (offset 12 lines).
  Hunk #4 FAILED at 605.
  1 out of 4 hunks FAILED -- saving rejects to file
chrome/browser/signin/signin_manager.cc.rej

Patch:       chrome/browser/signin/signin_manager.cc
Index: chrome/browser/signin/signin_manager.cc
diff --git a/chrome/browser/signin/signin_manager.cc
b/chrome/browser/signin/signin_manager.cc
index
ad725364b65bbbb0c6c7743fe324ee203f829eb2..b1a0ab1b7b651b66a93c163d57ecc8dd17d658ad
100644
--- a/chrome/browser/signin/signin_manager.cc
+++ b/chrome/browser/signin/signin_manager.cc
@@ -46,6 +46,7 @@ using namespace signin_internals_util;
 
 using content::BrowserThread;
 using content::ChildProcessHost;
+using content::RenderProcessHost;
 
 namespace {
 
@@ -97,12 +98,10 @@ void SigninManager::SetSigninProcess(int process_id) {
           signin_host_id_ != ChildProcessHost::kInvalidChildProcessUniqueId)
       << "Replacing in-use signin process.";
   signin_host_id_ = process_id;
-  const content::RenderProcessHost* process =
-      content::RenderProcessHost::FromID(process_id);
-  DCHECK(process);
-  registrar_.Add(this,
-                 content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-                 content::Source<content::RenderProcessHost>(process));
+  RenderProcessHost* host = RenderProcessHost::FromID(process_id);
+  DCHECK(host);
+  host->AddObserver(this);
+  signin_hosts_observed_.insert(host);
 }
 
 void SigninManager::ClearSigninProcess() {
@@ -118,6 +117,12 @@ bool SigninManager::HasSigninProcess() const {
 }
 
 SigninManager::~SigninManager() {
+  std::set<RenderProcessHost*>::iterator i;
+  for (i = signin_hosts_observed_.begin();
+       i != signin_hosts_observed_.end();
+       ++i) {
+    (*i)->RemoveObserver(this);
+  }
 }
 
 void SigninManager::InitTokenService() {
@@ -600,22 +605,14 @@ void SigninManager::OnGetUserInfoFailure(const
GoogleServiceAuthError& error) {
   OnClientLoginFailure(error);
 }
 
-void SigninManager::Observe(int type,
-                            const content::NotificationSource& source,
-                            const content::NotificationDetails& details) {
-  DCHECK_EQ(content::NOTIFICATION_RENDERER_PROCESS_TERMINATED, type);
-
-  // It's possible we're listening to a "stale" renderer because it was
-  // replaced with a new process by process-per-site. In either case,
-  // stop listening to it, but only reset signin_host_id_ tracking
-  // if this was from the current signin process.
-  registrar_.Remove(this,
-                    content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-                    source);
-  if (signin_host_id_ ==
-      content::Source<content::RenderProcessHost>(source)->GetID()) {
+void SigninManager::RenderProcessHostDestroyed(RenderProcessHost* host) {
+  // It's possible we're listening to a "stale" renderer because it was
replaced
+  // with a new process by process-per-site. In either case, stop observing it,
+  // but only reset signin_host_id_ tracking if this was from the current
signin
+  // process.
+  signin_hosts_observed_.erase(host);
+  if (signin_host_id_ == host->GetID())
     signin_host_id_ = ChildProcessHost::kInvalidChildProcessUniqueId;
-  }
 }
 
 void SigninManager::ProhibitSignout(bool prohibit_signout) {

[commit-bot: I haz the power, 2014-01-10 16:09:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/126613002/270001

[commit-bot: I haz the power, 2014-01-10 23:17:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/126613002/270001

[commit-bot: I haz the power, 2014-01-11 01:58:17]

Change committed as 244300