Removed the ProcessHandle from the RendererClosedDetails payload

content/browser/mach_broker_mac.mm

Index: content/browser/mach_broker_mac.mm
diff --git a/content/browser/mach_broker_mac.mm b/content/browser/mach_broker_mac.mm
index f2e5f985c32a792b19455d96e9b762bc3a655eb7..273481de432dde2a98a4ac91cff2ab02adae040f 100644
--- a/content/browser/mach_broker_mac.mm
+++ b/content/browser/mach_broker_mac.mm
@@ -195,11 +195,13 @@ void MachBroker::EnsureRunning() {
   }
 }
 
-void MachBroker::AddPlaceholderForPid(base::ProcessHandle pid) {
+void MachBroker::AddPlaceholderForPid(base::ProcessHandle pid,
+                                      int child_process_id) {
   lock_.AssertAcquired();
 
   DCHECK_EQ(0u, mach_map_.count(pid));
   mach_map_[pid] = MACH_PORT_NULL;
+  child_process_id_map_[child_process_id] = pid;
 }
 
 mach_port_t MachBroker::TaskForPid(base::ProcessHandle pid) const {
@@ -212,33 +214,27 @@ mach_port_t MachBroker::TaskForPid(base::ProcessHandle pid) const {
 
 void MachBroker::BrowserChildProcessHostDisconnected(
     const ChildProcessData& data) {
-  InvalidatePid(data.handle);
+  InvalidateChildProcessId(data.id);
 }
 
 void MachBroker::BrowserChildProcessCrashed(const ChildProcessData& data) {
-  InvalidatePid(data.handle);
+  InvalidateChildProcessId(data.id);
 }
 
 void MachBroker::Observe(int type,
                          const NotificationSource& source,
                          const NotificationDetails& details) {
-  // TODO(rohitrao): These notifications do not always carry the proper PIDs,
-  // especially when the renderer is already gone or has crashed.  Find a better
-  // way to listen for child process deaths.  http://crbug.com/55734
-  base::ProcessHandle handle = 0;
   switch (type) {
-    case NOTIFICATION_RENDERER_PROCESS_CLOSED:
-      handle = Details<RenderProcessHost::RendererClosedDetails>(
-          details)->handle;
-      break;
     case NOTIFICATION_RENDERER_PROCESS_TERMINATED:
-      handle = Source<RenderProcessHost>(source)->GetHandle();
+    case NOTIFICATION_RENDERER_PROCESS_CLOSED: {
+      RenderProcessHost* host = Source<RenderProcessHost>(source).ptr();
+      InvalidateChildProcessId(host->GetID());
       break;
+    }
     default:
       NOTREACHED() << "Unexpected notification";
       break;
   }
-  InvalidatePid(handle);
 }
 
 MachBroker::MachBroker() : listener_thread_started_(false) {
@@ -262,16 +258,19 @@ void MachBroker::FinalizePid(base::ProcessHandle pid,
     it->second = task_port;
 }
 
-void MachBroker::InvalidatePid(base::ProcessHandle pid) {
+void MachBroker::InvalidateChildProcessId(int child_process_id) {
   base::AutoLock lock(lock_);
-  MachBroker::MachMap::iterator it = mach_map_.find(pid);
-  if (it == mach_map_.end())
+  MachBroker::ChildProcessIdMap::iterator it =
+    child_process_id_map_.find(child_process_id);

[Mark Mentovai, 2014/11/17 19:17:46]

Continuation lines get a 4-space indent, not a 2-space one.

[sramajay, 2014/11/18 14:15:19]

Done.

+  if (it == child_process_id_map_.end())
     return;
 
+  base::ProcessHandle pid = it->second;
   kern_return_t kr = mach_port_deallocate(mach_task_self(),

[Mark Mentovai, 2014/11/17 19:17:46]

You should use mach_map_.find() and handle the case where the PID isn’t in
mach_map_ by skipping the mach_port_deallocate() and mach_map_.erase(). Since
you’ll have an iterator, you can also use it with the erase() on line 272,
instead of having to do the same map lookup twice.

It will happen that you’ll get an entry for child_process_id in
child_process_id_map_ but no entry for the pid in mach_map_ if FinalizePid()
doesn’t get called before the child process dies.

[sramajay, 2014/11/18 14:15:19]

Done.

-                                          it->second);
+                                          mach_map_[pid]);
   MACH_LOG_IF(WARNING, kr != KERN_SUCCESS, kr) << "mach_port_deallocate";
-  mach_map_.erase(it);
+  mach_map_.erase(pid);
+  child_process_id_map_.erase(it);
 }
 
 // static