Remove the ProcessHandle from the RendererClosedDetails payload

NOTIFICATION_RENDERER_PROCESS_CLOSED is sent out after the closing the render
process. ProcessHandle passed via this notification is invalid and useless. This
cl is to remove it from the notification payload.

MachBroker is relying on this notification to do clean-up of its map between
ProcessHandle and mach_port_t. Since ProcessHandle is invalid, there is a TODO
to handle it in proper way, which is also fixed as it is dependent.

Each Child Process (Renderer, Plugin) has its own id generated out of
ChildProcessHostImpl[1], which is a common unique id generator. Refer [2] for
documentation of this ID related to Render Process.

[1] https://code.google.com/p/chromium/codesearch#chromium/src/content/common/child_process_host_impl.cc&l=219&rcl=1415723309
[2] https://code.google.com/p/chromium/codesearch#chromium/src/content/public/browser/render_process_host.h&sq=package:chromium&rcl=1415723309&l=141

Related CLs - https://codereview.chromium.org/644473004/ and https://codereview.chromium.org/662103002/

BUG=432486

Review URL: https://codereview.chromium.org/681733003

Cr-Commit-Position: refs/heads/master@{#306407}

10 files changed, 164 insertions, 108 deletions

diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.cc b/chrome/browser/chromeos/memory/oom_priority_manager.cc
index 823207f..d192680 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager.cc
+++ b/chrome/browser/chromeos/memory/oom_priority_manager.cc
@@ -171,7 +171,7 @@ OomPriorityManager::TabStats::~TabStats() {
 }
 
 OomPriorityManager::OomPriorityManager()
-    : focused_tab_pid_(0),
+    : focused_tab_process_info_(std::make_pair(0, 0)),
       low_memory_observer_(new LowMemoryObserver),
       discard_count_(0),
       recent_tab_discard_(false) {
@@ -218,14 +218,14 @@ void OomPriorityManager::Stop() {
 
 std::vector<base::string16> OomPriorityManager::GetTabTitles() {
   TabStatsList stats = GetTabStatsOnUIThread();
-  base::AutoLock pid_to_oom_score_autolock(pid_to_oom_score_lock_);
+  base::AutoLock oom_score_autolock(oom_score_lock_);
   std::vector<base::string16> titles;
   titles.reserve(stats.size());
   TabStatsList::iterator it = stats.begin();
   for ( ; it != stats.end(); ++it) {
     base::string16 str;
     str.reserve(4096);
-    int score = pid_to_oom_score_[it->renderer_handle];
+    int score = oom_score_map_[it->child_process_host_id];
     str += base::IntToString16(score);
     str += base::ASCIIToUTF16(" - ");
     str += it->title;
@@ -456,10 +456,12 @@ bool OomPriorityManager::CompareTabStats(TabStats first,
 
 void OomPriorityManager::AdjustFocusedTabScoreOnFileThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  base::AutoLock pid_to_oom_score_autolock(pid_to_oom_score_lock_);
+  base::AutoLock oom_score_autolock(oom_score_lock_);
+  base::ProcessHandle pid = focused_tab_process_info_.second;
   content::ZygoteHost::GetInstance()->AdjustRendererOOMScore(
-      focused_tab_pid_, chrome::kLowestRendererOomScore);
-  pid_to_oom_score_[focused_tab_pid_] = chrome::kLowestRendererOomScore;
+      pid, chrome::kLowestRendererOomScore);
+  oom_score_map_[focused_tab_process_info_.first] =
+      chrome::kLowestRendererOomScore;
 }
 
 void OomPriorityManager::OnFocusTabScoreAdjustmentTimeout() {
@@ -472,35 +474,30 @@ void OomPriorityManager::OnFocusTabScoreAdjustmentTimeout() {
 void OomPriorityManager::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
-  base::ProcessHandle handle = 0;
-  base::AutoLock pid_to_oom_score_autolock(pid_to_oom_score_lock_);
+  base::AutoLock oom_score_autolock(oom_score_lock_);
   switch (type) {
-    case content::NOTIFICATION_RENDERER_PROCESS_CLOSED: {
-      handle =
-          content::Details<content::RenderProcessHost::RendererClosedDetails>(
-              details)->handle;
-      pid_to_oom_score_.erase(handle);
-      break;
-    }
+    case content::NOTIFICATION_RENDERER_PROCESS_CLOSED:
     case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED: {
-      handle = content::Source<content::RenderProcessHost>(source)->
-          GetHandle();
-      pid_to_oom_score_.erase(handle);
+      content::RenderProcessHost* host =
+          content::Source<content::RenderProcessHost>(source).ptr();
+      oom_score_map_.erase(host->GetID());
       break;
     }
     case content::NOTIFICATION_RENDER_WIDGET_VISIBILITY_CHANGED: {
       bool visible = *content::Details<bool>(details).ptr();
       if (visible) {
-        focused_tab_pid_ =
+        content::RenderProcessHost* render_host =
             content::Source<content::RenderWidgetHost>(source).ptr()->
-            GetProcess()->GetHandle();
+            GetProcess();
+        focused_tab_process_info_ = std::make_pair(render_host->GetID(),
+                                                   render_host->GetHandle());
 
         // If the currently focused tab already has a lower score, do not
         // set it. This can happen in case the newly focused tab is script
         // connected to the previous tab.
         ProcessScoreMap::iterator it;
-        it = pid_to_oom_score_.find(focused_tab_pid_);
-        if (it == pid_to_oom_score_.end()
+        it = oom_score_map_.find(focused_tab_process_info_.first);
+        if (it == oom_score_map_.end()
             || it->second != chrome::kLowestRendererOomScore) {
           // By starting a timer we guarantee that the tab is focused for
           // certain amount of time. Secondly, it also does not add overhead
@@ -581,6 +578,7 @@ OomPriorityManager::TabStatsList OomPriorityManager::GetTabStatsOnUIThread() {
         stats.is_discarded = model->IsTabDiscarded(i);
         stats.last_active = contents->GetLastActiveTime();
         stats.renderer_handle = contents->GetRenderProcessHost()->GetHandle();
+        stats.child_process_host_id = contents->GetRenderProcessHost()->GetID();
         stats.title = contents->GetTitle();
         stats.tab_contents_id = IdFromWebContents(contents);
         stats_list.push_back(stats);
@@ -596,9 +594,10 @@ OomPriorityManager::TabStatsList OomPriorityManager::GetTabStatsOnUIThread() {
 }
 
 // static
-std::vector<base::ProcessHandle> OomPriorityManager::GetProcessHandles(
+std::vector<OomPriorityManager::ProcessInfo>
+    OomPriorityManager::GetChildProcessInfos(
     const TabStatsList& stats_list) {
-  std::vector<base::ProcessHandle> process_handles;
+  std::vector<ProcessInfo> process_infos;
   std::set<base::ProcessHandle> already_seen;
   for (TabStatsList::const_iterator iterator = stats_list.begin();
        iterator != stats_list.end(); ++iterator) {
@@ -613,21 +612,21 @@ std::vector<base::ProcessHandle> OomPriorityManager::GetProcessHandles(
       continue;
     }
 
-    process_handles.push_back(iterator->renderer_handle);
+    process_infos.push_back(std::make_pair(
+        iterator->child_process_host_id, iterator->renderer_handle));
   }
-  return process_handles;
+  return process_infos;
 }
 
 void OomPriorityManager::AdjustOomPrioritiesOnFileThread(
     TabStatsList stats_list) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  base::AutoLock pid_to_oom_score_autolock(pid_to_oom_score_lock_);
+  base::AutoLock oom_score_autolock(oom_score_lock_);
 
   // Remove any duplicate PIDs. Order of the list is maintained, so each
   // renderer process will take on the oom_score_adj of the most important
   // (least likely to be killed) tab.
-  std::vector<base::ProcessHandle> process_handles =
-      GetProcessHandles(stats_list);
+  std::vector<ProcessInfo> process_infos = GetChildProcessInfos(stats_list);
 
   // Now we assign priorities based on the sorted list.  We're
   // assigning priorities in the range of kLowestRendererOomScore to
@@ -643,18 +642,17 @@ void OomPriorityManager::AdjustOomPrioritiesOnFileThread(
   const int kPriorityRange = chrome::kHighestRendererOomScore -
                              chrome::kLowestRendererOomScore;
   float priority_increment =
-      static_cast<float>(kPriorityRange) / process_handles.size();
-  for (std::vector<base::ProcessHandle>::iterator iterator =
-           process_handles.begin();
-       iterator != process_handles.end(); ++iterator) {
+      static_cast<float>(kPriorityRange) / process_infos.size();
+  for (const auto& process_info : process_infos) {
     int score = static_cast<int>(priority + 0.5f);
-    ProcessScoreMap::iterator it = pid_to_oom_score_.find(*iterator);
+    ProcessScoreMap::iterator it =
+        oom_score_map_.find(process_info.first);
     // If a process has the same score as the newly calculated value,
     // do not set it.
-    if (it == pid_to_oom_score_.end() || it->second != score) {
-      content::ZygoteHost::GetInstance()->AdjustRendererOOMScore(*iterator,
-                                                                 score);
-      pid_to_oom_score_[*iterator] = score;
+    if (it == oom_score_map_.end() || it->second != score) {
+      content::ZygoteHost::GetInstance()->AdjustRendererOOMScore(
+          process_info.second, score);
+      oom_score_map_[process_info.first] = score;
     }
     priority += priority_increment;
   }
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.h b/chrome/browser/chromeos/memory/oom_priority_manager.h
index fcf4d92..92ef696 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager.h
+++ b/chrome/browser/chromeos/memory/oom_priority_manager.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_CHROMEOS_MEMORY_OOM_PRIORITY_MANAGER_H_
 #define CHROME_BROWSER_CHROMEOS_MEMORY_OOM_PRIORITY_MANAGER_H_
 
+#include <utility>
 #include <vector>
 
 #include "base/compiler_specific.h"
@@ -79,6 +80,7 @@ class OomPriorityManager : public content::NotificationObserver {
     bool is_discarded;
     base::TimeTicks last_active;
     base::ProcessHandle renderer_handle;
+    int child_process_host_id;
     base::string16 title;
     int64 tab_contents_id;  // unique ID per WebContents
   };
@@ -111,10 +113,14 @@ class OomPriorityManager : public content::NotificationObserver {
   // Called when the timer fires, sets oom_adjust_score for all renderers.
   void AdjustOomPriorities();
 
-  // Returns a list of unique process handles from |stats_list|. If multiple
-  // tabs use the same process, returns the first process handle. This implies
+  // Pair to hold child process host id and ProcessHandle
+  typedef std::pair<int, base::ProcessHandle> ProcessInfo;
+
+  // Returns a list of child process host ids and ProcessHandles from
+  // |stats_list| with unique pids. If multiple tabs use the same process,
+  // returns the first child process host id and corresponding pid. This implies
   // that the processes are selected based on their "most important" tab.
-  static std::vector<base::ProcessHandle> GetProcessHandles(
+  static std::vector<ProcessInfo> GetChildProcessInfos(
       const TabStatsList& stats_list);
 
   // Called by AdjustOomPriorities.
@@ -137,12 +143,13 @@ class OomPriorityManager : public content::NotificationObserver {
   base::RepeatingTimer<OomPriorityManager> recent_tab_discard_timer_;
   content::NotificationRegistrar registrar_;
 
-  // This lock is for pid_to_oom_score_ and focus_tab_pid_.
-  base::Lock pid_to_oom_score_lock_;
-  // map maintaining the process - oom_score mapping.
-  typedef base::hash_map<base::ProcessHandle, int> ProcessScoreMap;
-  ProcessScoreMap pid_to_oom_score_;
-  base::ProcessHandle focused_tab_pid_;
+  // This lock is for |oom_score_map_| and |focused_tab_process_info_|.
+  base::Lock oom_score_lock_;
+  // Map maintaining the child process host id - oom_score mapping.
+  typedef base::hash_map<int, int> ProcessScoreMap;
+  ProcessScoreMap oom_score_map_;
+  // Holds the focused tab's child process host id.
+  ProcessInfo focused_tab_process_info_;
 
   // Observer for the kernel low memory signal.  NULL if tab discarding is
   // disabled.
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc b/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc
index 212e7e7..9280b9f 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc
+++ b/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc
@@ -43,6 +43,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
     OomPriorityManager::TabStats stats;
     stats.is_pinned = true;
     stats.renderer_handle = kPinned;
+    stats.child_process_host_id = kPinned;
     test_list.push_back(stats);
   }
 
@@ -50,6 +51,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
     OomPriorityManager::TabStats stats;
     stats.is_app = true;
     stats.renderer_handle = kApp;
+    stats.child_process_host_id = kApp;
     test_list.push_back(stats);
   }
 
@@ -57,6 +59,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
     OomPriorityManager::TabStats stats;
     stats.is_playing_audio = true;
     stats.renderer_handle = kPlayingAudio;
+    stats.child_process_host_id = kPlayingAudio;
     test_list.push_back(stats);
   }
 
@@ -64,6 +67,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
     OomPriorityManager::TabStats stats;
     stats.last_active = now - base::TimeDelta::FromSeconds(10);
     stats.renderer_handle = kRecent;
+    stats.child_process_host_id = kRecent;
     test_list.push_back(stats);
   }
 
@@ -71,6 +75,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
     OomPriorityManager::TabStats stats;
     stats.last_active = now - base::TimeDelta::FromMinutes(15);
     stats.renderer_handle = kOld;
+    stats.child_process_host_id = kOld;
     test_list.push_back(stats);
   }
 
@@ -78,6 +83,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
     OomPriorityManager::TabStats stats;
     stats.last_active = now - base::TimeDelta::FromDays(365);
     stats.renderer_handle = kReallyOld;
+    stats.child_process_host_id = kReallyOld;
     test_list.push_back(stats);
   }
 
@@ -86,6 +92,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
     stats.is_pinned = true;
     stats.last_active = now - base::TimeDelta::FromDays(365);
     stats.renderer_handle = kOldButPinned;
+    stats.child_process_host_id = kOldButPinned;
     test_list.push_back(stats);
   }
 
@@ -93,6 +100,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
     OomPriorityManager::TabStats stats;
     stats.is_reloadable_ui = true;
     stats.renderer_handle = kReloadableUI;
+    stats.child_process_host_id = kReloadableUI;
     test_list.push_back(stats);
   }
 
@@ -102,6 +110,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
     OomPriorityManager::TabStats stats;
     stats.is_selected = true;
     stats.renderer_handle = kSelected;
+    stats.child_process_host_id = kSelected;
     test_list.push_back(stats);
   }
 
@@ -119,6 +128,17 @@ TEST_F(OomPriorityManagerTest, Comparator) {
   EXPECT_EQ(kOld, test_list[index++].renderer_handle);
   EXPECT_EQ(kReallyOld, test_list[index++].renderer_handle);
   EXPECT_EQ(kReloadableUI, test_list[index++].renderer_handle);
+
+  index = 0;
+  EXPECT_EQ(kSelected, test_list[index++].child_process_host_id);
+  EXPECT_EQ(kPinned, test_list[index++].child_process_host_id);
+  EXPECT_EQ(kOldButPinned, test_list[index++].child_process_host_id);
+  EXPECT_EQ(kApp, test_list[index++].child_process_host_id);
+  EXPECT_EQ(kPlayingAudio, test_list[index++].child_process_host_id);
+  EXPECT_EQ(kRecent, test_list[index++].child_process_host_id);
+  EXPECT_EQ(kOld, test_list[index++].child_process_host_id);
+  EXPECT_EQ(kReallyOld, test_list[index++].child_process_host_id);
+  EXPECT_EQ(kReloadableUI, test_list[index++].child_process_host_id);
 }
 
 TEST_F(OomPriorityManagerTest, IsReloadableUI) {
@@ -144,44 +164,58 @@ TEST_F(OomPriorityManagerTest, IsReloadableUI) {
 
 TEST_F(OomPriorityManagerTest, GetProcessHandles) {
   OomPriorityManager::TabStats stats;
-  std::vector<base::ProcessHandle> handles;
+  std::vector<OomPriorityManager::ProcessInfo> process_id_pairs;
 
-  // Empty stats list gives empty handles list.
+  // Empty stats list gives empty |process_id_pairs| list.
   OomPriorityManager::TabStatsList empty_list;
-  handles = OomPriorityManager::GetProcessHandles(empty_list);
-  EXPECT_EQ(0u, handles.size());
+  process_id_pairs =
+      OomPriorityManager::GetChildProcessInfos(empty_list);
+  EXPECT_EQ(0u, process_id_pairs.size());
 
-  // Two tabs in two different processes generates two handles out.
+  // Two tabs in two different processes generates two
+  // |child_process_host_id| out.
   OomPriorityManager::TabStatsList two_list;
-  stats.renderer_handle = 100;
-  two_list.push_back(stats);
+  stats.child_process_host_id = 100;
   stats.renderer_handle = 101;
   two_list.push_back(stats);
-  handles = OomPriorityManager::GetProcessHandles(two_list);
-  EXPECT_EQ(2u, handles.size());
-  EXPECT_EQ(100, handles[0]);
-  EXPECT_EQ(101, handles[1]);
+  stats.child_process_host_id = 200;
+  stats.renderer_handle = 201;
+  two_list.push_back(stats);
+  process_id_pairs = OomPriorityManager::GetChildProcessInfos(two_list);
+  EXPECT_EQ(2u, process_id_pairs.size());
+  EXPECT_EQ(100, process_id_pairs[0].first);
+  EXPECT_EQ(101, process_id_pairs[0].second);
+  EXPECT_EQ(200, process_id_pairs[1].first);
+  EXPECT_EQ(201, process_id_pairs[1].second);
 
   // Zero handles are removed.
   OomPriorityManager::TabStatsList zero_handle_list;
+  stats.child_process_host_id = 100;
   stats.renderer_handle = 0;
   zero_handle_list.push_back(stats);
-  handles = OomPriorityManager::GetProcessHandles(zero_handle_list);
-  EXPECT_EQ(0u, handles.size());
+  process_id_pairs =
+      OomPriorityManager::GetChildProcessInfos(zero_handle_list);
+  EXPECT_EQ(0u, process_id_pairs.size());
 
   // Two tabs in the same process generates one handle out. When a duplicate
   // occurs the later instance is dropped.
   OomPriorityManager::TabStatsList same_process_list;
-  stats.renderer_handle = 100;
-  same_process_list.push_back(stats);
+  stats.child_process_host_id = 100;
   stats.renderer_handle = 101;
   same_process_list.push_back(stats);
-  stats.renderer_handle = 100;  // Duplicate.
+  stats.child_process_host_id = 200;
+  stats.renderer_handle = 201;
+  same_process_list.push_back(stats);
+  stats.child_process_host_id = 300;
+  stats.renderer_handle = 101;  // Duplicate.
   same_process_list.push_back(stats);
-  handles = OomPriorityManager::GetProcessHandles(same_process_list);
-  EXPECT_EQ(2u, handles.size());
-  EXPECT_EQ(100, handles[0]);
-  EXPECT_EQ(101, handles[1]);
+  process_id_pairs =
+      OomPriorityManager::GetChildProcessInfos(same_process_list);
+  EXPECT_EQ(2u, process_id_pairs.size());
+  EXPECT_EQ(100, process_id_pairs[0].first);
+  EXPECT_EQ(101, process_id_pairs[0].second);
+  EXPECT_EQ(200, process_id_pairs[1].first);
+  EXPECT_EQ(201, process_id_pairs[1].second);
 }
 
 }  // namespace chromeos
diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc
index 080ce03..0870e6c 100644
--- a/content/browser/child_process_launcher.cc
+++ b/content/browser/child_process_launcher.cc
@@ -429,7 +429,7 @@ void ChildProcessLauncher::Context::LaunchInternal(
     }
 
     if (launched)
-      broker->AddPlaceholderForPid(handle);
+      broker->AddPlaceholderForPid(handle, child_process_id);
 
     // After updating the broker, release the lock and let the child's
     // messasge be processed on the broker's thread.
diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc
index 11c3c8a..69eb043 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc
@@ -1405,7 +1405,6 @@ TEST_F(RenderFrameHostManagerTest, CleanUpSwappedOutRVHOnProcessCrash) {
 
   // Fake a process crash.
   RenderProcessHost::RendererClosedDetails details(
-      rvh1->GetProcess()->GetHandle(),
       base::TERMINATION_STATUS_PROCESS_CRASHED,
       0);
   NotificationService::current()->Notify(
diff --git a/content/browser/mach_broker_mac.h b/content/browser/mach_broker_mac.h
index 9088ca7..4aa34dc 100644
--- a/content/browser/mach_broker_mac.h
+++ b/content/browser/mach_broker_mac.h
@@ -5,11 +5,11 @@
 #ifndef CONTENT_BROWSER_MACH_BROKER_MAC_H_
 #define CONTENT_BROWSER_MACH_BROKER_MAC_H_
 
+#include <mach/mach.h>
+
 #include <map>
 #include <string>
 
-#include <mach/mach.h>
-
 #include "base/memory/singleton.h"
 #include "base/process/process_handle.h"
 #include "base/process/process_metrics.h"
@@ -61,7 +61,7 @@ class CONTENT_EXPORT MachBroker : public base::ProcessMetrics::PortProvider,
   // Callers are expected to later update the port with FinalizePid().  Callers
   // MUST acquire the lock given by GetLock() before calling this method (and
   // release the lock afterwards).
-  void AddPlaceholderForPid(base::ProcessHandle pid);
+  void AddPlaceholderForPid(base::ProcessHandle pid, int child_process_id);
 
   // Implement |ProcessMetrics::PortProvider|.
   mach_port_t TaskForPid(base::ProcessHandle process) const override;
@@ -90,8 +90,8 @@ class CONTENT_EXPORT MachBroker : public base::ProcessMetrics::PortProvider,
   // this method (and release the lock afterwards).
   void FinalizePid(base::ProcessHandle pid, mach_port_t task_port);
 
-  // Removes all mappings belonging to |pid| from the broker.
-  void InvalidatePid(base::ProcessHandle pid);
+  // Removes all mappings belonging to |child_process_id| from the broker.
+  void InvalidateChildProcessId(int child_process_id);
 
   // Returns the Mach port name to use when sending or receiving messages.
   // Does the Right Thing in the browser and in child processes.
@@ -110,7 +110,12 @@ class CONTENT_EXPORT MachBroker : public base::ProcessMetrics::PortProvider,
   typedef std::map<base::ProcessHandle, mach_port_t> MachMap;
   MachMap mach_map_;
 
-  // Mutex that guards |mach_map_|.
+  // Stores the Child process unique id (RenderProcessHost ID) for every
+  // process.
+  typedef std::map<int, base::ProcessHandle> ChildProcessIdMap;
+  ChildProcessIdMap child_process_id_map_;
+
+  // Mutex that guards |mach_map_| and |child_process_id_map_|.
   mutable base::Lock lock_;
 
   DISALLOW_COPY_AND_ASSIGN(MachBroker);
diff --git a/content/browser/mach_broker_mac.mm b/content/browser/mach_broker_mac.mm
index f2e5f98..0e0eec8 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,21 @@ 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);
+  if (it == child_process_id_map_.end())
     return;
 
-  kern_return_t kr = mach_port_deallocate(mach_task_self(),
-                                          it->second);
-  MACH_LOG_IF(WARNING, kr != KERN_SUCCESS, kr) << "mach_port_deallocate";
-  mach_map_.erase(it);
+  MachMap::iterator mach_it = mach_map_.find(it->second);
+  if (mach_it != mach_map_.end()) {
+    kern_return_t kr = mach_port_deallocate(mach_task_self(),
+                                            mach_it->second);
+    MACH_LOG_IF(WARNING, kr != KERN_SUCCESS, kr) << "mach_port_deallocate";
+    mach_map_.erase(mach_it);
+  }
+  child_process_id_map_.erase(it);
 }
 
 // static
diff --git a/content/browser/mach_broker_mac_unittest.cc b/content/browser/mach_broker_mac_unittest.cc
index a7eca4f..9ba50c3 100644
--- a/content/browser/mach_broker_mac_unittest.cc
+++ b/content/browser/mach_broker_mac_unittest.cc
@@ -12,13 +12,17 @@ namespace content {
 class MachBrokerTest : public testing::Test {
  public:
   // Helper function to acquire/release locks and call |PlaceholderForPid()|.
-  void AddPlaceholderForPid(base::ProcessHandle pid) {
+  void AddPlaceholderForPid(base::ProcessHandle pid, int child_process_id) {
     base::AutoLock lock(broker_.GetLock());
-    broker_.AddPlaceholderForPid(pid);
+    broker_.AddPlaceholderForPid(pid, child_process_id);
   }
 
-  void InvalidatePid(base::ProcessHandle pid) {
-    broker_.InvalidatePid(pid);
+  void InvalidateChildProcessId(int child_process_id) {
+    broker_.InvalidateChildProcessId(child_process_id);
+  }
+
+  int GetChildProcessCount(int child_process_id) {
+    return broker_.child_process_id_map_.count(child_process_id);
   }
 
   // Helper function to acquire/release locks and call |FinalizePid()|.
@@ -39,7 +43,7 @@ TEST_F(MachBrokerTest, Locks) {
 
 TEST_F(MachBrokerTest, AddPlaceholderAndFinalize) {
   // Add a placeholder for PID 1.
-  AddPlaceholderForPid(1);
+  AddPlaceholderForPid(1, 1);
   EXPECT_EQ(0u, broker_.TaskForPid(1));
 
   // Finalize PID 1.
@@ -50,15 +54,26 @@ TEST_F(MachBrokerTest, AddPlaceholderAndFinalize) {
   EXPECT_EQ(0u, broker_.TaskForPid(2));
 }
 
-TEST_F(MachBrokerTest, Invalidate) {
-  AddPlaceholderForPid(1);
+TEST_F(MachBrokerTest, InvalidateChildProcessId) {
+  // Add a placeholder for PID 1 and child process id 50.
+  AddPlaceholderForPid(1, 50);
   FinalizePid(1, 100u);
 
   EXPECT_EQ(100u, broker_.TaskForPid(1));
-  InvalidatePid(1u);
+  InvalidateChildProcessId(50);
   EXPECT_EQ(0u, broker_.TaskForPid(1));
 }
 
+TEST_F(MachBrokerTest, ValidateChildProcessIdMap) {
+  // Add a placeholder for PID 1 and child process id 50.
+  AddPlaceholderForPid(1, 50);
+  FinalizePid(1, 100u);
+
+  EXPECT_EQ(1, GetChildProcessCount(50));
+  InvalidateChildProcessId(50);
+  EXPECT_EQ(0, GetChildProcessCount(50));
+}
+
 TEST_F(MachBrokerTest, FinalizeUnknownPid) {
   // Finalizing an entry for an unknown pid should not add it to the map.
   FinalizePid(1u, 100u);
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index bb6a973..e683a1d 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -1947,7 +1947,7 @@ void RenderProcessHostImpl::ProcessDied(bool already_dead) {
                                                          &exit_code) :
       base::TERMINATION_STATUS_NORMAL_TERMINATION;
 
-  RendererClosedDetails details(GetHandle(), status, exit_code);
+  RendererClosedDetails details(status, exit_code);
   mojo_application_host_->WillDestroySoon();
 
   child_process_launcher_.reset();
diff --git a/content/public/browser/render_process_host.h b/content/public/browser/render_process_host.h
index 4ae7f3a..dcec7d8 100644
--- a/content/public/browser/render_process_host.h
+++ b/content/public/browser/render_process_host.h
@@ -42,14 +42,11 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Sender,
 
   // Details for RENDERER_PROCESS_CLOSED notifications.
   struct RendererClosedDetails {
-    RendererClosedDetails(base::ProcessHandle handle,
-                          base::TerminationStatus status,
+    RendererClosedDetails(base::TerminationStatus status,
                           int exit_code) {
-      this->handle = handle;
       this->status = status;
       this->exit_code = exit_code;
     }
-    base::ProcessHandle handle;
     base::TerminationStatus status;
     int exit_code;
   };