Create SystemMemoryPressureEvaluator from ChromeOS MemoryPressureMonitor.

This CL creates the SystemMemoryPressureEvaluator for ChromeOS, as part
of a refactoring of the MemoryPressureMonitor to use the
Voter-Aggregator design described here:
https://docs.google.com/document/d/1W3FPDyjIAKBcFGNYsHA3EKR1FHrJlbBaqT4_RUnxzq0/edit?ts=5d3f5714#heading=h.7nki9mck5t64
This Evaluator calculates memory pressure and emits notifications
identically to the previous Monitor, but does so by casting its votes
for memory pressure through a MemoryPressureVoter.

Bug: 980965
Change-Id: I0b3d9caa19ede5474ef1bae04be43fd5a9334869
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1756765
Commit-Queue: Ryan Powell <ryanpow@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688684}

base/BUILD.gn

@@ -416,8 +416,6 @@ jumbo_component("base") {
     "memory/memory_pressure_listener.h",
     "memory/memory_pressure_monitor.cc",
     "memory/memory_pressure_monitor.h",
-    "memory/memory_pressure_monitor_chromeos.cc",
-    "memory/memory_pressure_monitor_chromeos.h",
     "memory/platform_shared_memory_region.cc",
     "memory/platform_shared_memory_region.h",
     "memory/protected_memory.cc",
@@ -2576,7 +2574,6 @@ test("base_unittests") {
     "memory/aligned_memory_unittest.cc",
     "memory/discardable_shared_memory_unittest.cc",
     "memory/memory_pressure_listener_unittest.cc",
-    "memory/memory_pressure_monitor_chromeos_unittest.cc",
     "memory/memory_pressure_monitor_unittest.cc",
     "memory/platform_shared_memory_region_unittest.cc",
     "memory/protected_memory_unittest.cc",

base/util/memory_pressure/BUILD.gn

@@ -10,6 +10,8 @@ source_set("memory_pressure") {
     "multi_source_memory_pressure_monitor.h",
     "system_memory_pressure_evaluator.cc",
     "system_memory_pressure_evaluator.h",
+    "system_memory_pressure_evaluator_chromeos.cc",
+    "system_memory_pressure_evaluator_chromeos.h",
     "system_memory_pressure_evaluator_mac.cc",
     "system_memory_pressure_evaluator_mac.h",
     "system_memory_pressure_evaluator_win.cc",
@@ -26,6 +28,7 @@ source_set("unittests") {
   sources = [
     "memory_pressure_voter_unittest.cc",
     "multi_source_memory_pressure_monitor_unittest.cc",
+    "system_memory_pressure_evaluator_chromeos_unittest.cc",
     "system_memory_pressure_evaluator_mac_unittest.cc",
     "system_memory_pressure_evaluator_win_unittest.cc",
   ]

base/util/memory_pressure/system_memory_pressure_evaluator.cc

@@ -6,7 +6,9 @@
 
 #include "build/build_config.h"
 
-#if defined(OS_MACOSX) && !defined(OS_IOS)
+#if defined(OS_CHROMEOS)
+#include "base/util/memory_pressure/system_memory_pressure_evaluator_chromeos.h"
+#elif defined(OS_MACOSX) && !defined(OS_IOS)
 #include "base/util/memory_pressure/system_memory_pressure_evaluator_mac.h"
 #elif defined(OS_WIN)
 #include "base/util/memory_pressure/system_memory_pressure_evaluator_win.h"
@@ -18,7 +20,15 @@ namespace util {
 std::unique_ptr<SystemMemoryPressureEvaluator>
 SystemMemoryPressureEvaluator::CreateDefaultSystemEvaluator(
     MultiSourceMemoryPressureMonitor* monitor) {
-#if defined(OS_MACOSX) && !defined(OS_IOS)
+#if defined(OS_CHROMEOS)
+  if (util::chromeos::SystemMemoryPressureEvaluator::
+          SupportsKernelNotifications()) {
+    return std::make_unique<util::chromeos::SystemMemoryPressureEvaluator>(
+        monitor->CreateVoter());
+  }
+  LOG(ERROR) << "No MemoryPressureMonitor created because the kernel does "
+                "not support notifications.";
+#elif defined(OS_MACOSX) && !defined(OS_IOS)
   return std::make_unique<util::mac::SystemMemoryPressureEvaluator>(
       monitor->CreateVoter());
 #elif defined(OS_WIN)

base/memory/memory_pressure_monitor_chromeos.h → base/util/memory_pressure/system_memory_pressure_evaluator_chromeos.h

 // Copyright 2019 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 BASE_MEMORY_MEMORY_PRESSURE_MONITOR_CHROMEOS_H_
-#define BASE_MEMORY_MEMORY_PRESSURE_MONITOR_CHROMEOS_H_
+#ifndef BASE_UTIL_MEMORY_PRESSURE_SYSTEM_MEMORY_PRESSURE_EVALUATOR_CHROMEOS_H_
+#define BASE_UTIL_MEMORY_PRESSURE_SYSTEM_MEMORY_PRESSURE_EVALUATOR_CHROMEOS_H_
 
 #include <vector>
 
@@ -10,35 +10,33 @@
 #include "base/files/scoped_file.h"
 #include "base/macros.h"
 #include "base/memory/memory_pressure_listener.h"
-#include "base/memory/memory_pressure_monitor.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "base/util/memory_pressure/memory_pressure_voter.h"
+#include "base/util/memory_pressure/system_memory_pressure_evaluator.h"
 
-namespace base {
+namespace util {
 namespace chromeos {
 
 ////////////////////////////////////////////////////////////////////////////////
-// MemoryPressureMonitor
+// SystemMemoryPressureEvaluator
 //
 // A class to handle the observation of our free memory. It notifies the
 // MemoryPressureListener of memory fill level changes, so that it can take
 // action to reduce memory resources accordingly.
-class BASE_EXPORT MemoryPressureMonitor : public base::MemoryPressureMonitor {
+class SystemMemoryPressureEvaluator
+    : public util::SystemMemoryPressureEvaluator {
  public:
-  // The MemoryPressureMonitor reads the pressure levels from the
+  // The SystemMemoryPressureEvaluator reads the pressure levels from the
   // /sys/kernel/mm/chromeos-low_mem/margin and does not need to be configured.
   //
   // NOTE: You should check that the kernel supports notifications by calling
   // SupportsKernelNotifications() before constructing a new instance of this
   // class.
-  MemoryPressureMonitor();
-  ~MemoryPressureMonitor() override;
-
-  // Get the current memory pressure level.
-  MemoryPressureListener::MemoryPressureLevel GetCurrentPressureLevel()
-      const override;
-  void SetDispatchCallback(const DispatchCallback& callback) override;
+  explicit SystemMemoryPressureEvaluator(
+      std::unique_ptr<MemoryPressureVoter> voter);
+  ~SystemMemoryPressureEvaluator() override;
 
   // GetMarginFileParts returns a vector of the configured margin file values.
   // The margin file contains two or more values, but we're only concerned with
@@ -65,17 +63,22 @@ class BASE_EXPORT MemoryPressureMonitor : public base::MemoryPressureMonitor {
     return critical_pressure_threshold_mb_;
   }
 
-  // Returns a type-casted version of the current memory pressure monitor. A
-  // simple wrapper to base::MemoryPressureMonitor::Get.
-  static MemoryPressureMonitor* Get();
+  // Returns the current system memory pressure evaluator.
+  static SystemMemoryPressureEvaluator* Get();
+
+  base::MemoryPressureListener::MemoryPressureLevel current_vote_for_testing()
+      const {
+    return current_vote_;
+  }
 
  protected:
   // This constructor is only used for testing.
-  MemoryPressureMonitor(
+  SystemMemoryPressureEvaluator(
       const std::string& margin_file,
       const std::string& available_file,
       base::RepeatingCallback<bool(int)> kernel_waiting_callback,
-      bool enable_metrics);
+      bool enable_metrics,
+      std::unique_ptr<MemoryPressureVoter> voter);
 
   static std::vector<int> GetMarginFileParts(const std::string& margin_file);
   void CheckMemoryPressure();
@@ -96,14 +99,12 @@ class BASE_EXPORT MemoryPressureMonitor : public base::MemoryPressureMonitor {
   // Memory.PressureLevel metric.
   base::TimeTicks last_pressure_level_report_;
 
-  MemoryPressureListener::MemoryPressureLevel current_memory_pressure_level_ =
-      MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE;
+  base::MemoryPressureListener::MemoryPressureLevel current_vote_ =
+      base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE;
 
   // File descriptor used to read and poll(2) available memory from sysfs,
   // In /sys/kernel/mm/chromeos-low_mem/available.
-  ScopedFD available_mem_file_;
-
-  DispatchCallback dispatch_callback_;
+  base::ScopedFD available_mem_file_;
 
   // A periodic timer which will be used to report a UMA metric on the current
   // memory pressure level as theoretically we could go a very long time without
@@ -115,11 +116,17 @@ class BASE_EXPORT MemoryPressureMonitor : public base::MemoryPressureMonitor {
   // configurable to make testing easier.
   base::RepeatingCallback<bool()> kernel_waiting_callback_;
 
-  base::WeakPtrFactory<MemoryPressureMonitor> weak_ptr_factory_{this};
+  // In charge of forwarding votes from here to the
+  // MemoryPressureVoteAggregator.
+  std::unique_ptr<MemoryPressureVoter> voter_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+
+  base::WeakPtrFactory<SystemMemoryPressureEvaluator> weak_ptr_factory_;
 
-  DISALLOW_COPY_AND_ASSIGN(MemoryPressureMonitor);
+  DISALLOW_COPY_AND_ASSIGN(SystemMemoryPressureEvaluator);
 };
 
 }  // namespace chromeos
-}  // namespace base
-#endif  // BASE_MEMORY_MEMORY_PRESSURE_MONITOR_CHROMEOS_H_
+}  // namespace util
+#endif  // BASE_UTIL_MEMORY_PRESSURE_SYSTEM_MEMORY_PRESSURE_EVALUATOR_CHROMEOS_H_

chrome/browser/BUILD.gn

@@ -3509,6 +3509,7 @@ jumbo_split_static_library("browser") {
     ]
     deps += [
       ":theme_properties",
+      "//base/util/memory_pressure",
       "//chrome/app/vector_icons",
       "//chrome/browser/policy:path_parser",
       "//chrome/browser/profile_resetter:profile_reset_report_proto",

chrome/browser/resource_coordinator/tab_manager_delegate_chromeos.cc

@@ -16,7 +16,6 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
-#include "base/memory/memory_pressure_monitor_chromeos.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/process/memory.h"
 #include "base/process/process_handle.h"  // kNullProcessHandle.
@@ -26,6 +25,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "base/util/memory_pressure/system_memory_pressure_evaluator_chromeos.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/memory/memory_kills_monitor.h"
 #include "chrome/browser/resource_coordinator/lifecycle_unit.h"
@@ -241,7 +241,7 @@ int TabManagerDelegate::MemoryStat::LowMemoryMarginKB() {
   // A margin file can contain multiple values but the first one
   // represents the critical memory threshold.
   std::vector<int> margin_parts =
-      base::chromeos::MemoryPressureMonitor::GetMarginFileParts();
+      util::chromeos::SystemMemoryPressureEvaluator::GetMarginFileParts();
   if (!margin_parts.empty()) {
     return margin_parts[0] * 1024;
   }
@@ -450,7 +450,7 @@ void TabManagerDelegate::Observe(int type,
       // on top. So the longer the cleanup phase takes, the more tabs will
       // get discarded in parallel.
 
-      auto* monitor = base::chromeos::MemoryPressureMonitor::Get();
+      auto* monitor = util::chromeos::SystemMemoryPressureEvaluator::Get();
       if (monitor) {
         monitor->ScheduleEarlyCheck();
       }

content/browser/browser_main_loop.cc

@@ -191,7 +191,6 @@
 #endif
 
 #if defined(OS_CHROMEOS)
-#include "base/memory/memory_pressure_monitor_chromeos.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "services/data_decoder/public/mojom/constants.mojom.h"
@@ -369,23 +368,14 @@ std::unique_ptr<base::MemoryPressureMonitor> CreateMemoryPressureMonitor(
   if (command_line.HasSwitch(switches::kBrowserTest))
     return nullptr;
 
-// TODO(chrisha): Simplify this code once MemoryPressureMonitor is made a
-// concrete class.
 #if defined(OS_CHROMEOS)
-  if (chromeos::switches::MemoryPressureHandlingEnabled()) {
-    if (base::chromeos::MemoryPressureMonitor::SupportsKernelNotifications()) {
-      return std::make_unique<base::chromeos::MemoryPressureMonitor>();
-    }
-    LOG(ERROR) << "No MemoryPressureMonitor created because the kernel does "
-                  "not support notifications.";
-  }
-  return nullptr;
+  if (chromeos::switches::MemoryPressureHandlingEnabled())
+    return std::make_unique<util::MultiSourceMemoryPressureMonitor>();
 #elif defined(OS_MACOSX) || defined(OS_WIN)
   return std::make_unique<util::MultiSourceMemoryPressureMonitor>();
-#else
+#endif
   // No memory monitor on other platforms...
   return nullptr;
-#endif
 }
 
 #if defined(OS_CHROMEOS)