Removed the ProcessHandle from the RendererClosedDetails payload

chrome/browser/chromeos/memory/oom_priority_manager.h

Index: chrome/browser/chromeos/memory/oom_priority_manager.h
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.h b/chrome/browser/chromeos/memory/oom_priority_manager.h
index fcf4d9288207da0c5b6e26b12b439c347413028e..fb7b2d4dcd4c91b4b81b136822db53126e2cdfb0 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager.h
+++ b/chrome/browser/chromeos/memory/oom_priority_manager.h
@@ -79,6 +79,7 @@ class OomPriorityManager : public content::NotificationObserver {
     bool is_discarded;
     base::TimeTicks last_active;
     base::ProcessHandle renderer_handle;
+    int child_process_id;

[James Cook, 2014/11/20 16:50:35]

I'm confused. This code only runs on Posix, where process_handle.h says that
base::ProcessHandle and base::ProcessId are the same thing. Is this
|child_process_id| referring to a different process than the one from
|renderer_handle|? Do you need both of these?

Or is |child_process_id| not a pid?  If that's the case then the name is
super-confusing.  Perhaps it should be something like child_host_id or
child_process_host_id.

[Charlie Reis, 2014/11/20 17:07:45]

Note: It's pretty common in the browser process to refer to the
RenderProcessHost ID as "process_id," for better or worse.  That said, in cases
like this where it's used alongside the ProcessHandle, it would be less
ambiguous to call it child_process_host_id.

[sramajay, 2014/11/21 12:18:47]

child_process_id is not a pid, it is just a unique auto generated number given
to all child processes including plugin process. 
Yes. It makes sense to rename it as child_process_host_id.

     base::string16 title;
     int64 tab_contents_id;  // unique ID per WebContents
   };
@@ -111,11 +112,11 @@ 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
-  // that the processes are selected based on their "most important" tab.
-  static std::vector<base::ProcessHandle> GetProcessHandles(
-      const TabStatsList& stats_list);
+  // Returns a list of child process ids from |stats_list| which unique pids.

[James Cook, 2014/11/20 16:50:35]

which -> with? which are?

[sramajay, 2014/11/21 12:18:47]

Done.

+  // If multiple tabs use the same process, returns the first child process id.
+  // This implies that the processes are selected based on their
+  // "most important" tab.
+  static std::vector<int> GetChildProcessIds(const TabStatsList& stats_list);
 
   // Called by AdjustOomPriorities.
   void AdjustOomPrioritiesOnFileThread(TabStatsList stats_list);
@@ -138,11 +139,11 @@ class OomPriorityManager : public content::NotificationObserver {
   content::NotificationRegistrar registrar_;
 
   // This lock is for pid_to_oom_score_ and focus_tab_pid_.

[James Cook, 2014/11/20 16:50:35]

Fix this comment, or comment on the individual member variables which lock
protects them.

[sramajay, 2014/11/21 12:18:47]

Done.

-  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_;
+  base::Lock oom_score_lock_;
+  // map maintaining the child process id - oom_score mapping.
+  typedef base::hash_map<int, int> ProcessScoreMap;
+  ProcessScoreMap oom_score_map_;
+  base::ProcessHandle focused_tab_process_id_;
 
   // Observer for the kernel low memory signal.  NULL if tab discarding is
   // disabled.