Revert 67175 - This change implements OOM priority management for ChromeOS

(Build failure, probably debug/release warning difference.)

This adds periodic OOM score adjustment, based on the last access time of the tab, whether or not it is pinned, and (of course) how much memory it is using.

BUG=http://crosbug.com/8990
TEST=Ran some ui_tests, ran on device.

Review URL: http://codereview.chromium.org/4498001

TBR=gspencer@chromium.org
Review URL: http://codereview.chromium.org/5284003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67183 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 12 insertions, 279 deletions

diff --git a/base/task.h b/base/task.h
index b21ccd8..28d15fc 100644
--- a/base/task.h
+++ b/base/task.h
@@ -274,7 +274,7 @@ struct RunnableMethodTraits {
 //   };
 //   }  // namespace foo
 //
-//   DISABLE_RUNNABLE_METHOD_REFCOUNT(foo::Bar);
+//   DISABLE_RUNNABLE_METHOD_REFCOUNT(foo::Bar)
 //
 // This is different from DISALLOW_COPY_AND_ASSIGN which is declared inside the
 // class.
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 3986163..e7afaff 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -113,7 +113,6 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/boot_times_loader.h"
-#include "chrome/browser/oom_priority_manager.h"
 #endif
 
 // TODO(port): several win-only methods have been pulled out of this, but
@@ -1558,9 +1557,9 @@ int BrowserMain(const MainFunctionParams& parameters) {
   bool record_search_engine = is_first_run && !profile->IsOffTheRecord();
 #endif
 
-  // ChildProcess:: is a misnomer unless you consider context.  Use
-  // of --wait-for-debugger only makes sense when Chrome itself is a
-  // child process (e.g. when launched by PyAuto).
+    // ChildProcess:: is a misnomer unless you consider context.  Use
+    // of --wait-for-debugger only makes sense when Chrome itself is a
+    // child process (e.g. when launched by PyAuto).
   if (parsed_command_line.HasSwitch(switches::kWaitForDebugger)) {
     ChildProcess::WaitForDebugger(L"Browser");
   }
@@ -1578,17 +1577,6 @@ int BrowserMain(const MainFunctionParams& parameters) {
     }
   }
 
-#if defined(OS_CHROMEOS)
-  // Run the Out of Memory priority manager while in this scope.  Wait
-  // until here to start so that we give the most amount of time for
-  // the other services to start up before we start adjusting the oom
-  // priority.  In reality, it doesn't matter much where in this scope
-  // this is started, but it must be started in this scope so it will
-  // also be terminated when this scope exits.
-  scoped_ptr<browser::OomPriorityManager> oom_priority_manager(
-      new browser::OomPriorityManager);
-#endif
-
   // Create the instance of the cloud print proxy service so that it can launch
   // the service process if needed. This is needed because the service process
   // might have shutdown because an update was available.
diff --git a/chrome/browser/oom_priority_manager.cc b/chrome/browser/oom_priority_manager.cc
deleted file mode 100644
index f841570..0000000
--- a/chrome/browser/oom_priority_manager.cc
+++ /dev/null
@@ -1,176 +0,0 @@
-// Copyright (c) 2010 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/oom_priority_manager.h"
-
-#include <list>
-
-#include "base/process.h"
-#include "base/process_util.h"
-#include "base/thread.h"
-#include "build/build_config.h"
-#include "chrome/browser/browser_list.h"
-#include "chrome/browser/browser_thread.h"
-#include "chrome/browser/renderer_host/render_process_host.h"
-#include "chrome/browser/tab_contents/tab_contents.h"
-#include "chrome/browser/tab_contents_wrapper.h"
-#include "chrome/browser/tabs/tab_strip_model.h"
-#include "chrome/browser/zygote_host_linux.h"
-
-#if !defined(OS_CHROMEOS)
-#error This file only meant to be compiled on ChromeOS
-#endif
-
-using base::TimeDelta;
-using base::TimeTicks;
-using base::ProcessHandle;
-using base::ProcessMetrics;
-
-namespace browser {
-
-// The default interval in seconds after which to adjust the oom_adj
-// value.
-#define ADJUSTMENT_INTERVAL_SECONDS 10
-
-// The default interval in minutes for considering activation times
-// "equal".
-#define BUCKET_INTERVAL_MINUTES 10
-
-OomPriorityManager::OomPriorityManager() {
-  StartTimer();
-}
-
-OomPriorityManager::~OomPriorityManager() {
-  StopTimer();
-}
-
-void OomPriorityManager::StartTimer() {
-  if (!timer_.IsRunning()) {
-    timer_.Start(TimeDelta::FromSeconds(ADJUSTMENT_INTERVAL_SECONDS),
-                 this,
-                 &OomPriorityManager::AdjustOomPriorities);
-  }
-}
-
-void OomPriorityManager::StopTimer() {
-  timer_.Stop();
-}
-
-// Returns true if |first| is considered less desirable to be killed
-// than |second|.
-bool OomPriorityManager::CompareRendererStats(RendererStats first,
-                                              RendererStats second) {
-  // The size of the slop in comparing activation times.  [This is
-  // allocated here to avoid static initialization at startup time.]
-  static const int64 kTimeBucketInterval =
-      TimeDelta::FromMinutes(BUCKET_INTERVAL_MINUTES).ToInternalValue();
-
-  // Being pinned is most important.
-  if (first.is_pinned != second.is_pinned)
-    return first.is_pinned == true;
-
-  // We want to be a little "fuzzy" when we compare these, because
-  // it's not really possible for the times to be identical, but if
-  // the user selected two tabs at about the same time, we still want
-  // to take the one that uses more memory.
-  if (abs((first.last_selected - second.last_selected).ToInternalValue()) <
-      kTimeBucketInterval)
-    return first.last_selected < second.last_selected;
-
-  return first.memory_used < second.memory_used;
-}
-
-// Here we collect most of the information we need to sort the
-// existing renderers in priority order, and hand out oom_adj scores
-// based on that sort order.
-//
-// Things we need to collect on the browser thread (because
-// TabStripModel isn't thread safe):
-// 1) whether or not a tab is pinned
-// 2) last time a tab was selected
-//
-// We also need to collect:
-// 3) size in memory of a tab
-// But we do that in DoAdjustOomPriorities on the FILE thread so that
-// we avoid jank, because it accesses /proc.
-void OomPriorityManager::AdjustOomPriorities() {
-  if (BrowserList::size() == 0)
-    return;
-
-  StatsList renderer_stats;
-  for (BrowserList::const_iterator browser_iterator = BrowserList::begin();
-       browser_iterator != BrowserList::end(); ++browser_iterator) {
-    Browser* browser = *browser_iterator;
-    const TabStripModel* model = browser->tabstrip_model();
-    for (int i = 0; i < model->count(); i++) {
-      TabContents* contents = model->GetTabContentsAt(i)->tab_contents();
-      RendererStats stats;
-      stats.last_selected = contents->last_selected_time();
-      stats.renderer_handle = contents->GetRenderProcessHost()->GetHandle();
-      stats.is_pinned = model->IsTabPinned(i);
-      renderer_stats.push_back(stats);
-    }
-  }
-
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &OomPriorityManager::DoAdjustOomPriorities,
-                        renderer_stats));
-}
-
-void OomPriorityManager::DoAdjustOomPriorities(StatsList renderer_stats) {
-  for (StatsList::iterator stats_iter = renderer_stats.begin();
-       stats_iter != renderer_stats.end(); ++stats_iter) {
-    scoped_ptr<ProcessMetrics> metrics(ProcessMetrics::CreateProcessMetrics(
-        stats_iter->renderer_handle));
-
-    base::WorkingSetKBytes working_set_kbytes;
-    if (metrics->GetWorkingSetKBytes(&working_set_kbytes)) {
-      // We use the proportional set size (PSS) to calculate memory
-      // usage "badness" on Linux.
-      stats_iter->memory_used = working_set_kbytes.shared * 1024;
-    } else {
-      // and if for some reason we can't get PSS, we revert to using
-      // resident set size (RSS).  This will be zero if the process
-      // has already gone away, but we can live with that, since the
-      // process is gone anyhow.
-      stats_iter->memory_used = metrics->GetWorkingSetSize();
-    }
-  }
-
-  // Now we sort the data we collected so that least desirable to be
-  // killed is first, most desirable is last.
-  renderer_stats.sort(OomPriorityManager::CompareRendererStats);
-
-  // Now we assign priorities based on the sorted list.  We're
-  // assigning priorities in the range of 5 to 10.  oom_adj takes
-  // values from -17 to 15.  Negative values are reserved for system
-  // processes, and we want to give some room on either side of the
-  // range we're using to allow for things that want to be above or
-  // below the renderers in priority, so 5 to 10 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.  We also
-  // remove any duplicate PIDs, leaving the most important of the
-  // duplicates.
-  const int kMinPriority = 5;
-  const int kMaxPriority = 10;
-  const int kPriorityRange = kMaxPriority - kMinPriority;
-  float priority_increment =
-      static_cast<float>(kPriorityRange) / renderer_stats.size();
-  float priority = kMinPriority;
-  std::set<base::ProcessHandle> already_seen;
-  for (StatsList::iterator iterator = renderer_stats.begin();
-       iterator != renderer_stats.end(); ++iterator) {
-    if (already_seen.find(iterator->renderer_handle) == already_seen.end()) {
-      already_seen.insert(iterator->renderer_handle);
-      Singleton<ZygoteHost>::get()->AdjustRendererOOMScore(
-          iterator->renderer_handle,
-          static_cast<int>(priority + 0.5f));
-      priority += priority_increment;
-    }
-  }
-}
-
-}  // namespace browser
diff --git a/chrome/browser/oom_priority_manager.h b/chrome/browser/oom_priority_manager.h
deleted file mode 100644
index 55ce733..0000000
--- a/chrome/browser/oom_priority_manager.h
+++ /dev/null
@@ -1,61 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef CHROME_BROWSER_OOM_PRIORITY_MANAGER_H_
-#define CHROME_BROWSER_OOM_PRIORITY_MANAGER_H_
-
-#include <list>
-
-#include "base/timer.h"
-#include "base/process.h"
-
-namespace browser {
-
-// The OomPriorityManager periodically checks (see
-// ADJUSTMENT_INTERVAL_SECONDS in the source) the status of renderers
-// and adjusts the out of memory (OOM) adjustment value (in
-// /proc/<pid>/oom_adj) of the renderers so that they match the
-// algorithm embedded here for priority in being killed upon OOM
-// conditions.
-//
-// The algorithm used favors killing tabs that are not pinned, have
-// been idle for longest, and take up the most memory, in that order
-// of priority.  We round the idle times to the nearest few minutes
-// (see BUCKET_INTERVAL_MINUTES in the source) so that we can bucket
-// them, as no two tabs will have exactly the same idle time.
-class OomPriorityManager {
- public:
-  OomPriorityManager();
-  ~OomPriorityManager();
-
- private:
-  struct RendererStats {
-    bool is_pinned;
-    base::TimeTicks last_selected;
-    size_t memory_used;
-    base::ProcessHandle renderer_handle;
-  };
-  typedef std::list<RendererStats> StatsList;
-
-  void StartTimer();
-  void StopTimer();
-
-  // Posts DoAdjustOomPriorities task to the file thread.  Called when
-  // the timer fires.
-  void AdjustOomPriorities();
-
-  // Called by AdjustOomPriorities.  Runs on the file thread.
-  void DoAdjustOomPriorities(StatsList list);
-
-  static bool CompareRendererStats(RendererStats first, RendererStats second);
-
-  base::RepeatingTimer<OomPriorityManager> timer_;
-
-  DISALLOW_COPY_AND_ASSIGN(OomPriorityManager);
-};
-}  // namespace browser
-
-DISABLE_RUNNABLE_METHOD_REFCOUNT(browser::OomPriorityManager);
-
-#endif  // CHROME_BROWSER_OOM_PRIORITY_MANAGER_H_
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 4a746cc..cfa1ac0 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -797,7 +797,6 @@ void TabContents::DidBecomeSelected() {
   }
 
   WebCacheManager::GetInstance()->ObserveActivity(GetRenderProcessHost()->id());
-  last_selected_time_ = base::TimeTicks::Now();
 }
 
 void TabContents::WasHidden() {
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index f0e4666..2a69a14 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -328,9 +328,6 @@ class TabContents : public PageNavigator,
   // Invoked when the tab contents becomes selected. If you override, be sure
   // and invoke super's implementation.
   virtual void DidBecomeSelected();
-  base::TimeTicks last_selected_time() const {
-    return last_selected_time_;
-  }
 
   // Invoked when the tab contents becomes hidden.
   // NOTE: If you override this, call the superclass version too!
@@ -1281,9 +1278,6 @@ class TabContents : public PageNavigator,
   // The time that we started to close the tab.
   base::TimeTicks tab_close_start_time_;
 
-  // The time that this tab was last selected.
-  base::TimeTicks last_selected_time_;
-
   // Information about the language the page is in and has been translated to.
   LanguageState language_state_;
 
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 26665e1..1341578 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -476,7 +476,6 @@ void TabStripModel::SetTabPinned(int index, bool pinned) {
 }
 
 bool TabStripModel::IsTabPinned(int index) const {
-  DCHECK(ContainsIndex(index));
   return contents_data_[index]->pinned;
 }
 
diff --git a/chrome/browser/zygote_host_linux.cc b/chrome/browser/zygote_host_linux.cc
index b6115f1..5536eaf 100644
--- a/chrome/browser/zygote_host_linux.cc
+++ b/chrome/browser/zygote_host_linux.cc
@@ -242,13 +242,6 @@ pid_t ZygoteHost::ForkRenderer(
       return base::kNullProcessHandle;
   }
 
-  const int kRendererScore = 5;
-  AdjustRendererOOMScore(pid, kRendererScore);
-
-  return pid;
-}
-
-void ZygoteHost::AdjustRendererOOMScore(base::ProcessHandle pid, int score) {
   // 1) You can't change the oom_adj of a non-dumpable process (EPERM) unless
   //    you're root. Because of this, we can't set the oom_adj from the browser
   //    process.
@@ -281,23 +274,27 @@ void ZygoteHost::AdjustRendererOOMScore(base::ProcessHandle pid, int score) {
     selinux_valid = true;
   }
 
+  const int kRendererScore = 5;
   if (using_suid_sandbox_ && !selinux) {
     base::ProcessHandle sandbox_helper_process;
+    base::file_handle_mapping_vector dummy_map;
     std::vector<std::string> adj_oom_score_cmdline;
 
     adj_oom_score_cmdline.push_back(sandbox_binary_);
     adj_oom_score_cmdline.push_back(base::kAdjustOOMScoreSwitch);
     adj_oom_score_cmdline.push_back(base::Int64ToString(pid));
-    adj_oom_score_cmdline.push_back(base::IntToString(score));
+    adj_oom_score_cmdline.push_back(base::IntToString(kRendererScore));
     CommandLine adj_oom_score_cmd(adj_oom_score_cmdline);
-    if (base::LaunchApp(adj_oom_score_cmd, false, true,
+    if (base::LaunchApp(adj_oom_score_cmdline, dummy_map, false,
                         &sandbox_helper_process)) {
       ProcessWatcher::EnsureProcessGetsReaped(sandbox_helper_process);
     }
   } else if (!using_suid_sandbox_) {
-    if (!base::AdjustOOMScore(pid, score))
-      PLOG(ERROR) << "Failed to adjust OOM score of renderer with pid " << pid;
+    if (!base::AdjustOOMScore(pid, kRendererScore))
+      LOG(ERROR) << "Failed to adjust OOM score of renderer";
   }
+
+  return pid;
 }
 
 void ZygoteHost::EnsureProcessTerminated(pid_t process) {
diff --git a/chrome/browser/zygote_host_linux.h b/chrome/browser/zygote_host_linux.h
index ffb7964..dd00336 100644
--- a/chrome/browser/zygote_host_linux.h
+++ b/chrome/browser/zygote_host_linux.h
@@ -67,9 +67,6 @@ class ZygoteHost {
     return 0;
   }
 
-  // Adjust the OOM score of the given renderer's PID.
-  void AdjustRendererOOMScore(base::ProcessHandle process_handle, int score);
-
  private:
   friend struct DefaultSingletonTraits<ZygoteHost>;
   ZygoteHost();
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index f88083d..3c8c36f 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2332,8 +2332,6 @@
         'browser/ntp_background_util.h',
         'browser/omnibox_search_hint.cc',
         'browser/omnibox_search_hint.h',
-        'browser/oom_priority_manager.cc',
-        'browser/oom_priority_manager.h',
         'browser/options_page_base.cc',
         'browser/options_page_base.h',
         'browser/options_util.cc',
@@ -3421,8 +3419,6 @@
             ['exclude', 'browser/dom_ui/mediaplayer_ui.cc'],
             ['exclude', 'browser/dom_ui/slideshow_ui.cc'],
             ['exclude', 'browser/extensions/extension_tts_api_chromeos.cc'],
-            ['exclude', 'browser/oom_priority_manager.cc'],
-            ['exclude', 'browser/oom_priority_manager.h'],
             ['exclude', 'browser/renderer_host/offline_resource_handler.cc'],
             ['exclude', 'browser/renderer_host/offline_resource_handler.h'],
           ],