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..091e501cb1cda0707f26ba9b4e43fe26daffe54c 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_[pid] = child_process_id;
 }
 
 mach_port_t MachBroker::TaskForPid(base::ProcessHandle pid) const {
@@ -222,23 +224,17 @@ void MachBroker::BrowserChildProcessCrashed(const ChildProcessData& data) {
 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) {
@@ -272,8 +268,23 @@ void MachBroker::InvalidatePid(base::ProcessHandle pid) {
                                           it->second);
   MACH_LOG_IF(WARNING, kr != KERN_SUCCESS, kr) << "mach_port_deallocate";
   mach_map_.erase(it);
+  child_process_id_map_.erase(pid);
+}
+
+void MachBroker::InvalidateChildProcessId(int child_process_id) {
+  base::ProcessHandle pid = 0;
+  for (ChildProcessIdMap::iterator it = child_process_id_map_.begin();

[Charlie Reis, 2014/11/10 19:57:25]

Why not have the map go the other way?  Then you can look up the handle from
child_process_id (without iterating), and erase it from the map by
child_process_id after calling InvalidatePid.

[sramajay, 2014/11/11 14:16:05]

InvalidatePid is called from other BrowserChildProcessHostDisconnected() and
BrowserChildProcessCrashed(), in which ProcessHandle is passed on. If i change
as per above comment, then iteration has to be done inside InvalidatePid().
Iteration cannot be avoided. 

+      it != child_process_id_map_.end(); ++it) {
+    if (it->second == child_process_id) {
+      pid = it->first;
+      break;
+    }
+  }
+  if (pid)
+    InvalidatePid(pid);
 }
 
+
 // static
 std::string MachBroker::GetMachPortName() {
   const base::CommandLine* command_line =