Removed the ProcessHandle from the RendererClosedDetails payload

chrome/browser/chromeos/memory/oom_priority_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/memory/oom_priority_manager.h"
 
 #include <algorithm>
 #include <set>
 #include <vector>
 
@@ skipping 153 matching lines @@
     is_selected(false),
     is_discarded(false),
     renderer_handle(0),
     tab_contents_id(0) {
 }
 
 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) {
   registrar_.Add(this,
       content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
       content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this,
       content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
       content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this,
@@ skipping 26 matching lines @@
 
 void OomPriorityManager::Stop() {
   timer_.Stop();
   recent_tab_discard_timer_.Stop();
   if (low_memory_observer_.get())
     low_memory_observer_->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;
     str += base::ASCIIToUTF16(it->is_app ? " app" : "");
     str += base::ASCIIToUTF16(it->is_reloadable_ui ? " reloadable_ui" : "");
     str += base::ASCIIToUTF16(it->is_playing_audio ? " playing_audio" : "");
     str += base::ASCIIToUTF16(it->is_pinned ? " pinned" : "");
     str += base::ASCIIToUTF16(it->is_discarded ? " discarded" : "");
     titles.push_back(str);
   }
@@ skipping 210 matching lines @@
   // with ad networks and statistics scripts.  Ideally we would like to check
   // for beforeUnload handlers, which are likely to present a dialog asking
   // if the user wants to discard state.  crbug.com/123049
 
   // Being more recently active is more important.
   return first.last_active > second.last_active;
 }
 
 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_);
   content::ZygoteHost::GetInstance()->AdjustRendererOOMScore(
-      focused_tab_pid_, chrome::kLowestRendererOomScore);
-  pid_to_oom_score_[focused_tab_pid_] = chrome::kLowestRendererOomScore;
+      focused_tab_process_info_.second, chrome::kLowestRendererOomScore);

[James Cook, 2014/12/01 19:25:58]

nit: Please pull this out into a local variable (like "pid" or "process_handle"
so the reader is reminded what "second" refers to.

[sramajay, 2014/12/02 15:36:57]

Done.

+  oom_score_map_[focused_tab_process_info_.first] =
+      chrome::kLowestRendererOomScore;
 }
 
 void OomPriorityManager::OnFocusTabScoreAdjustmentTimeout() {
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&OomPriorityManager::AdjustFocusedTabScoreOnFileThread,
                  base::Unretained(this)));
 }
 
 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);

[James Cook, 2014/12/01 19:25:58]

This is fine because the lines above show that first means.

[sramajay, 2014/12/02 15:36:57]

Acknowledged.

+        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
           // to the tab switching time.
           if (focus_tab_score_adjust_timer_.IsRunning())
             focus_tab_score_adjust_timer_.Reset();
           else
             focus_tab_score_adjust_timer_.Start(FROM_HERE,
               TimeDelta::FromMilliseconds(kFocusedTabScoreAdjustIntervalMs),
               this, &OomPriorityManager::OnFocusTabScoreAdjustmentTimeout);
@@ skipping 60 matching lines @@
         TabStats stats;
         stats.is_app = is_browser_for_app;
         stats.is_reloadable_ui =
             IsReloadableUI(contents->GetLastCommittedURL());
         stats.is_playing_audio = chrome::IsPlayingAudio(contents);
         stats.is_pinned = model->IsTabPinned(i);
         stats.is_selected = browser_active && model->IsTabSelected(i);
         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);
       }
     }
     // We process the active browser window in the first iteration.
     browser_active = false;
   }
   // Sort the data we collected so that least desirable to be
   // killed is first, most desirable is last.
   std::sort(stats_list.begin(), stats_list.end(), CompareTabStats);
   return stats_list;
 }
 
 // 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) {
     // stats_list contains entries for already-discarded tabs. If the PID
     // (renderer_handle) is zero, we don't need to adjust the oom_score.
     if (iterator->renderer_handle == 0)
       continue;
 
     bool inserted = already_seen.insert(iterator->renderer_handle).second;
     if (!inserted) {
       // We've already seen this process handle.
       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
   // kHighestRendererOomScore (defined in chrome_constants.h).
   // oom_score_adj takes values from -1000 to 1000.  Negative values
   // are reserved for system processes, and we want to give some room
   // below the range we're using to allow for things that want to be
   // above the renderers in priority, so the defined range gives us
   // some variation in priority without taking up the whole range.  In
   // the end, however, it's a pretty arbitrary range to use.  Higher
   // values are more likely to be killed by the OOM killer.
   float priority = chrome::kLowestRendererOomScore;
   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 (auto process_info : process_infos) {

[James Cook, 2014/12/01 19:25:58]

nit: consider "const auto&" here to avoid copying each pair.

[sramajay, 2014/12/02 15:36:57]

Done.

     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;
   }
 }
 
 }  // namespace chromeos