Fix tab backgrounding on Linux / ChromeOS

base/process_linux.cc

Index: base/process_linux.cc
diff --git a/base/process_linux.cc b/base/process_linux.cc
index bfa1e4a03cc4b2caf6f741c554e701dbe6062804..cb413e153b6d02cbeb36d0bbe7745f118a21e001 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(

[willchan no longer on Chromium, 2011/12/01 22:26:03]

Do we need to worry about this file I/O here? I assume you're calling this on
the IO thread or something. I don't have a good solution, but just wanted to see
what you thought.

+        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