Fix tab backgrounding

This broke because an incompatible change was made to Process, and because there was no test that would catch it. I've fixed the underlying problem for both Linux and ChromeOS and made it testable.

BUG=102726
TEST=Added new RenderProcessHostTest.Backgrounding

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

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

6 files changed, 173 insertions, 106 deletions

diff --git a/base/process.h b/base/process.h
index 2670924..7869b88 100644
--- a/base/process.h
+++ b/base/process.h
@@ -34,37 +34,24 @@ const ProcessHandle kNullProcessHandle = 0;
 const ProcessId kNullProcessId = 0;
 #endif  // defined(OS_WIN)
 
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
-// saved_priority_ will be set to this to indicate that it's not holding
-// a valid value. -20 to 19 are valid process priorities.
-const int kUnsetProcessPriority = 256;
-#endif
-
 class BASE_EXPORT Process {
  public:
   Process() : process_(kNullProcessHandle) {
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
-    saved_priority_ = kUnsetProcessPriority;
-#endif
   }
 
   explicit Process(ProcessHandle handle) : process_(handle) {
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
-    saved_priority_ = kUnsetProcessPriority;
-#endif
   }
 
   // A handle to the current process.
   static Process Current();
 
+  static bool CanBackgroundProcesses();
+
   // Get/Set the handle for this process. The handle will be 0 if the process
   // is no longer running.
   ProcessHandle handle() const { return process_; }
   void set_handle(ProcessHandle handle) {
     process_ = handle;
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
-    saved_priority_ = kUnsetProcessPriority;
-#endif
   }
 
   // Get the PID for this process.
@@ -98,11 +85,6 @@ class BASE_EXPORT Process {
 
  private:
   ProcessHandle process_;
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
-  // Holds the priority that the process was set to when it was backgrounded.
-  // If the process wasn't backgrounded it will be kUnsetProcessPriority.
-  int saved_priority_;
-#endif
 };
 
 }  // namespace base
diff --git a/base/process_linux.cc b/base/process_linux.cc
index bfa1e4a..cb413e1 100644
--- a/base/process_linux.cc
+++ b/base/process_linux.cc
@@ -8,121 +8,128 @@
 #include <sys/resource.h>
 
 #include "base/file_util.h"
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/stringprintf.h"
+#include "base/string_split.h"
+#include "base/synchronization/lock.h"
 
-namespace base {
+namespace {
+const int kForegroundPriority = 0;
 
 #if defined(OS_CHROMEOS)
 // We are more aggressive in our lowering of background process priority
 // for chromeos as we have much more control over other processes running
 // on the machine.
-const int kPriorityAdjustment = 19;
-#else
-const int kPriorityAdjustment = 5;
-#endif
-
-bool Process::IsProcessBackgrounded() const {
-  DCHECK(process_);
-  return saved_priority_ == kUnsetProcessPriority;
-}
-
-bool Process::SetProcessBackgrounded(bool background) {
-  DCHECK(process_);
-
-#if defined(OS_CHROMEOS)
-  static bool cgroups_inited = false;
-  static bool use_cgroups = false;
+//
+// TODO(davemoore) Refactor this by adding support for higher levels to set
+// the foregrounding / backgrounding process so we don't have to keep
+// chrome / chromeos specific logic here.
+const int kBackgroundPriority = 19;
+const char kControlPath[] = "/tmp/cgroup/cpu%s/cgroup.procs";
+const char kForeground[] = "/chrome_renderers/foreground";
+const char kBackground[] = "/chrome_renderers/background";
+const char kProcPath[] = "/proc/%d/cgroup";
 
+struct CGroups {
   // Check for cgroups files. ChromeOS supports these by default. It creates
   // a cgroup mount in /tmp/cgroup and then configures two cpu task groups,
   // one contains at most a single foreground renderer and the other contains
   // all background renderers. This allows us to limit the impact of background
   // renderers on foreground ones to a greater level than simple renicing.
-  FilePath foreground_tasks(
-      "/tmp/cgroup/cpu/chrome_renderers/foreground/tasks");
-  FilePath background_tasks(
-      "/tmp/cgroup/cpu/chrome_renderers/background/tasks");
-  if (!cgroups_inited) {
-    cgroups_inited = true;
+  bool enabled;
+  FilePath foreground_file;
+  FilePath background_file;
+
+  CGroups() {
+    foreground_file = FilePath(StringPrintf(kControlPath, kForeground));
+    background_file = FilePath(StringPrintf(kControlPath, kBackground));
     file_util::FileSystemType foreground_type;
     file_util::FileSystemType background_type;
-    use_cgroups =
-        file_util::GetFileSystemType(foreground_tasks, &foreground_type) &&
-        file_util::GetFileSystemType(background_tasks, &background_type) &&
+    enabled =
+        file_util::GetFileSystemType(foreground_file, &foreground_type) &&
+        file_util::GetFileSystemType(background_file, &background_type) &&
         foreground_type == file_util::FILE_SYSTEM_CGROUP &&
         background_type == file_util::FILE_SYSTEM_CGROUP;
   }
+};
 
-  if (use_cgroups) {
-    if (background) {
-      std::string pid = StringPrintf("%d", process_);
-      if (file_util::WriteFile(background_tasks, pid.c_str(), pid.size()) > 0) {
-        // With cgroups there's no real notion of priority as an int, but this
-        // will ensure we only move renderers back to the foreground group
-        // if we've ever put them in the background one.
-        saved_priority_ = 0;
-        return true;
-      } else {
-        return false;
-      }
+base::LazyInstance<CGroups> cgroups = LAZY_INSTANCE_INITIALIZER;
+#else
+const int kBackgroundPriority = 5;
+#endif
+}
+
+namespace base {
+
+bool Process::IsProcessBackgrounded() const {
+  DCHECK(process_);
+
+#if defined(OS_CHROMEOS)
+  if (cgroups.Get().enabled) {
+    std::string proc;
+    if (file_util::ReadFileToString(
+        FilePath(StringPrintf(kProcPath, process_)),
+        &proc)) {
+      std::vector<std::string> proc_parts;
+      base::SplitString(proc, ':', &proc_parts);
+      DCHECK(proc_parts.size() == 3);
+      bool ret = proc_parts[2] == std::string(kBackground);
+      return ret;
     } else {
-      if (saved_priority_ == kUnsetProcessPriority) {
-        // Can't restore if we were never backgrounded.
-        return false;
-      }
-      std::string pid = StringPrintf("%d", process_);
-      if (file_util::WriteFile(foreground_tasks, pid.c_str(), pid.size()) > 0) {
-        saved_priority_ = kUnsetProcessPriority;
-        return true;
-      } else {
-        return false;
-      }
+      return false;
     }
   }
+#endif
+  return GetPriority() == kBackgroundPriority;
+}
+
+bool Process::SetProcessBackgrounded(bool background) {
+  DCHECK(process_);
+
+#if defined(OS_CHROMEOS)
+  if (cgroups.Get().enabled) {
+    std::string pid = StringPrintf("%d", process_);
+    const FilePath file =
+        background ?
+            cgroups.Get().background_file : cgroups.Get().foreground_file;
+    return file_util::WriteFile(file, pid.c_str(), pid.size()) > 0;
+  }
 #endif // OS_CHROMEOS
 
-  if (background) {
+  if (!CanBackgroundProcesses())
+    return false;
+
+  int priority = background ? kBackgroundPriority : kForegroundPriority;
+  int result = setpriority(PRIO_PROCESS, process_, priority);
+  DPCHECK(result == 0);
+  return result == 0;
+}
+
+struct CheckForNicePermission {
+  bool can_reraise_priority;
+
+  CheckForNicePermission() {
     // We won't be able to raise the priority if we don't have the right rlimit.
     // The limit may be adjusted in /etc/security/limits.conf for PAM systems.
     struct rlimit rlim;
-    if (getrlimit(RLIMIT_NICE, &rlim) != 0) {
-      // Call to getrlimit failed, don't background.
-      return false;
-    }
-    errno = 0;
-    int current_priority = GetPriority();
-    if (errno) {
-      // Couldn't get priority.
-      return false;
+    if ((getrlimit(RLIMIT_NICE, &rlim) == 0) &&
+        (20 - kForegroundPriority) <= static_cast<int>(rlim.rlim_cur)) {
+        can_reraise_priority = true;
     }
-    // {set,get}priority values are in the range -20 to 19, where -1 is higher
-    // priority than 0. But rlimit's are in the range from 0 to 39 where
-    // 1 is higher than 0.
-    if ((20 - current_priority) > static_cast<int>(rlim.rlim_cur)) {
-      // User is not allowed to raise the priority back to where it is now.
-      return false;
-    }
-    int result =
-        setpriority(
-            PRIO_PROCESS, process_, current_priority + kPriorityAdjustment);
-    if (result == -1) {
-      DLOG(ERROR) << "Failed to lower priority, errno: " << errno;
-      return false;
-    }
-    saved_priority_ = current_priority;
-    return true;
-  } else {
-    if (saved_priority_ == kUnsetProcessPriority) {
-      // Can't restore if we were never backgrounded.
-      return false;
-    }
-    int result = setpriority(PRIO_PROCESS, process_, saved_priority_);
-    // If we can't restore something has gone terribly wrong.
-    DPCHECK(result == 0);
-    saved_priority_ = kUnsetProcessPriority;
+  };
+};
+
+// static
+bool Process::CanBackgroundProcesses() {
+#if defined(OS_CHROMEOS)
+  if (cgroups.Get().enabled)
     return true;
-  }
+#endif
+
+  static LazyInstance<CheckForNicePermission> check_for_nice_permission =
+      LAZY_INSTANCE_INITIALIZER;
+  return check_for_nice_permission.Get().can_reraise_priority;
 }
 
 }  // namespace base
diff --git a/base/process_posix.cc b/base/process_posix.cc
index 6e65ebf..20e623c 100644
--- a/base/process_posix.cc
+++ b/base/process_posix.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -57,6 +57,12 @@ bool Process::SetProcessBackgrounded(bool value) {
   // priority.
   return false;
 }
+
+// static
+bool Process::CanBackgroundProcesses() {
+  return false;
+}
+
 #endif
 
 int Process::GetPriority() const {
diff --git a/base/process_win.cc b/base/process_win.cc
index 8b4c2b0..3f683ab 100644
--- a/base/process_win.cc
+++ b/base/process_win.cc
@@ -79,6 +79,11 @@ Process Process::Current() {
   return Process(::GetCurrentProcess());
 }
 
+// static
+bool Process::CanBackgroundProcesses() {
+  return true;
+}
+
 int Process::GetPriority() const {
   DCHECK(process_);
   return ::GetPriorityClass(process_);
diff --git a/content/browser/renderer_host/render_process_host_browsertest.cc b/content/browser/renderer_host/render_process_host_browsertest.cc
index 941371c..467dccd 100644
--- a/content/browser/renderer_host/render_process_host_browsertest.cc
+++ b/content/browser/renderer_host/render_process_host_browsertest.cc
@@ -4,12 +4,15 @@
 
 #include "content/browser/renderer_host/render_process_host_browsertest.h"
 
+#include "base/bind.h"
 #include "base/command_line.h"
+#include "base/process.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/common/test_url_constants.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
 
 RenderProcessHostTest::RenderProcessHostTest() {
@@ -28,6 +31,29 @@ int RenderProcessHostTest::RenderProcessHostCount() {
   return count;
 }
 
+void PostQuit(MessageLoop* loop) {
+  loop->PostTask(FROM_HERE, new MessageLoop::QuitTask);
+}
+
+void DoNothing() {}
+
+// Show a tab, activating the current one if there is one, and wait for
+// the renderer process to be created or foregrounded, returning the process
+// handle.
+base::ProcessHandle RenderProcessHostTest::ShowSingletonTab(const GURL& page) {
+  browser()->ShowSingletonTab(page);
+  TabContents* tc = browser()->GetSelectedTabContents();
+  CHECK(tc->GetURL() == page);
+
+  // Ensure that the backgrounding / foregrounding gets a chance to run.
+  content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
+      base::Bind(DoNothing), MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
+
+  return tc->GetRenderProcessHost()->GetHandle();
+}
+
 IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, ProcessPerTab) {
   // Set max renderers to 1 to force running out of processes.
   content::RenderProcessHost::SetMaxRendererProcessCountForTest(1);
@@ -84,6 +110,42 @@ IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, ProcessPerTab) {
   EXPECT_EQ(host_count, RenderProcessHostCount());
 }
 
+// We don't change process priorities on Mac or Posix because the user lacks the
+// permission to raise a process' priority even after lowering it.
+#if defined(OS_WIN) || defined(OS_LINUX)
+IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, Backgrounding) {
+  if (!base::Process::CanBackgroundProcesses()) {
+    LOG(ERROR) << "Can't background processes";
+    return;
+  }
+  CommandLine& parsed_command_line = *CommandLine::ForCurrentProcess();
+  parsed_command_line.AppendSwitch(switches::kProcessPerTab);
+
+  // Change the first tab to be the new tab page (TYPE_WEBUI).
+  GURL newtab(chrome::kTestNewTabURL);
+  ui_test_utils::NavigateToURL(browser(), newtab);
+
+  // Create a new tab. It should be foreground.
+  GURL page1("data:text/html,hello world1");
+  base::ProcessHandle pid1 = ShowSingletonTab(page1);
+  EXPECT_FALSE(base::Process(pid1).IsProcessBackgrounded());
+
+  // Create another tab. It should be foreground, and the first tab should
+  // now be background.
+  GURL page2("data:text/html,hello world2");
+  base::ProcessHandle pid2 = ShowSingletonTab(page2);
+  EXPECT_NE(pid1, pid2);
+  EXPECT_TRUE(base::Process(pid1).IsProcessBackgrounded());
+  EXPECT_FALSE(base::Process(pid2).IsProcessBackgrounded());
+
+  // Navigate back to first page. It should be foreground again, and the second
+  // tab should be background.
+  EXPECT_EQ(pid1, ShowSingletonTab(page1));
+  EXPECT_FALSE(base::Process(pid1).IsProcessBackgrounded());
+  EXPECT_TRUE(base::Process(pid2).IsProcessBackgrounded());
+}
+#endif
+
 // When we hit the max number of renderers, verify that the way we do process
 // sharing behaves correctly.  In particular, this test is verifying that even
 // when we hit the max process limit, that renderers of each type will wind up
diff --git a/content/browser/renderer_host/render_process_host_browsertest.h b/content/browser/renderer_host/render_process_host_browsertest.h
index 9b7e173..58d9f51 100644
--- a/content/browser/renderer_host/render_process_host_browsertest.h
+++ b/content/browser/renderer_host/render_process_host_browsertest.h
@@ -6,13 +6,18 @@
 #define CONTENT_BROWSER_RENDERER_HOST_RENDER_PROCESS_HOST_BROWSERTEST_H_
 #pragma once
 
+#include "base/process.h"
 #include "chrome/test/base/in_process_browser_test.h"
 
+
+class GURL;
+
 class RenderProcessHostTest : public InProcessBrowserTest {
  public:
   RenderProcessHostTest();
 
   int RenderProcessHostCount();
+  base::ProcessHandle ShowSingletonTab(const GURL& page);
 };
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_RENDER_PROCESS_HOST_BROWSERTEST_H_