Make RendererFrameManager a client of memory coordinator

content/browser/renderer_host/renderer_frame_manager.cc

Index: content/browser/renderer_host/renderer_frame_manager.cc
diff --git a/content/browser/renderer_host/renderer_frame_manager.cc b/content/browser/renderer_host/renderer_frame_manager.cc
index 3f2281f6960ca78abbec6d917ffbc77fba98bcc7..58721ba1390d112ed7f5804e550051e345d246fa 100644
--- a/content/browser/renderer_host/renderer_frame_manager.cc
+++ b/content/browser/renderer_host/renderer_frame_manager.cc
@@ -8,12 +8,14 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/memory/memory_coordinator_client_registry.h"
 #include "base/memory/memory_pressure_listener.h"
 #include "base/memory/memory_pressure_monitor.h"
 #include "base/memory/shared_memory.h"
 #include "base/sys_info.h"
 #include "build/build_config.h"
 #include "content/common/host_shared_bitmap_manager.h"
+#include "content/public/common/content_features.h"
 
 namespace content {
 namespace {
@@ -72,35 +74,55 @@ void RendererFrameManager::UnlockFrame(RendererFrameManagerClient* frame) {
 }
 
 size_t RendererFrameManager::GetMaxNumberOfSavedFrames() const {
-  base::MemoryPressureMonitor* monitor = base::MemoryPressureMonitor::Get();
-
-  if (!monitor)
-    return max_number_of_saved_frames_;
-
-  // Until we have a global OnMemoryPressureChanged event we need to query the
-  // value from our specific pressure monitor.
   int percentage = 100;
-  switch (monitor->GetCurrentPressureLevel()) {
-    case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE:
-      percentage = 100;
-      break;
-    case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE:
-      percentage = kModeratePressurePercentage;
-      break;
-    case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL:
-      percentage = kCriticalPressurePercentage;
-      break;
+  if (base::FeatureList::IsEnabled(features::kMemoryCoordinator)) {
+    switch (memory_state_) {

[chrisha, 2016/09/26 14:26:22]

Presumably the thing that assigned the memory pressure (the
ChildMemoryCoordinatorImpl) knows the memory pressure that has been given to
this MCC. Worth adding an API to query for the current state, rather than having
each MCC cache it individually?

[hajimehoshi, 2016/09/28 07:31:10]

1. To which class should I add a new API? Sorry but I didn't understand.
2. Would this work even after we eliminate MemoryPressure(Listener)?

+      case base::MemoryState::NORMAL:
+        percentage = 100;
+        break;
+      case base::MemoryState::THROTTLED:
+        percentage = kCriticalPressurePercentage;
+        break;
+      case base::MemoryState::SUSPENDED:
+      case base::MemoryState::UNKNOWN:
+        NOTREACHED();
+        break;
+    }
+  } else {
+    base::MemoryPressureMonitor* monitor = base::MemoryPressureMonitor::Get();
+
+    if (!monitor)
+      return max_number_of_saved_frames_;
+
+    // Until we have a global OnMemoryPressureChanged event we need to query the
+    // value from our specific pressure monitor.
+    switch (monitor->GetCurrentPressureLevel()) {
+      case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE:
+        percentage = 100;
+        break;
+      case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE:
+        percentage = kModeratePressurePercentage;
+        break;
+      case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL:
+        percentage = kCriticalPressurePercentage;
+        break;
+    }
   }
   size_t frames = (max_number_of_saved_frames_ * percentage) / 100;
   return std::max(static_cast<size_t>(1), frames);
 }
 
 RendererFrameManager::RendererFrameManager()
-    : memory_pressure_listener_(
+  : memory_state_(base::MemoryState::NORMAL) {
+  if (base::FeatureList::IsEnabled(features::kMemoryCoordinator)) {
+    base::MemoryCoordinatorClientRegistry::GetInstance()->Register(this);

[sadrul, 2016/09/26 20:04:20]

nit: Can the MemoryCoordinatorClient parts be moved into a separate object as
well? which, like MemoryPressureListener, takes a callback to run in response to
OnMemoryStateChange()? That way, RendererFrameManager itself doesn't have to
become one, and instead delegates that work to one of
MemoryCoordinatorClient/MemoryPressureListener (depending on which feature is
enabled)

[hajimehoshi, 2016/09/28 07:31:10]

Hmm, as for interfaces vs callbacks, we are discussing.

+  } else {
+    memory_pressure_listener_.reset(new base::MemoryPressureListener(
         base::Bind(&RendererFrameManager::OnMemoryPressure,
-                   base::Unretained(this))) {
-  // Note: With the destruction of this class the |memory_pressure_listener_|
-  // gets destroyed and the observer will remove itself.
+                   base::Unretained(this))));
+    // Note: With the destruction of this class the |memory_pressure_listener_|
+    // gets destroyed and the observer will remove itself.
+  }
   max_number_of_saved_frames_ =
 #if defined(OS_ANDROID)
       1;
@@ -134,21 +156,43 @@ void RendererFrameManager::CullUnlockedFrames(size_t saved_frame_limit) {
 
 void RendererFrameManager::OnMemoryPressure(
     base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level) {
-  int saved_frame_limit = max_number_of_saved_frames_;
-  if (saved_frame_limit <= 1)
-    return;
-  int percentage = 100;
   switch (memory_pressure_level) {
     case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE:
-      percentage = kModeratePressurePercentage;
+      PurgeMemory(kModeratePressurePercentage);
       break;
     case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL:
-      percentage = kCriticalPressurePercentage;
+      PurgeMemory(kCriticalPressurePercentage);
       break;
     case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE:
       // No need to change anything when there is no pressure.
       return;
   }
+}
+
+void RendererFrameManager::OnMemoryStateChange(base::MemoryState state) {
+  memory_state_ = state;
+  switch (state) {
+    case base::MemoryState::NORMAL:
+      // Don't have to purge here.
+      break;
+    case base::MemoryState::THROTTLED:
+      // TOOD(hajimehoshi): We don't have throttling 'level' so far. When we
+      // have such value, let's change the argument accroding to the value.

[chrisha, 2016/09/26 14:26:22]

according*

[hajimehoshi, 2016/09/28 07:31:10]

Done.

+      PurgeMemory(kCriticalPressurePercentage);
+      break;
+    case base::MemoryState::SUSPENDED:
+      // Note that SUSPENDED never occurs in the main browser process so far.
+      // Fall through.
+    case base::MemoryState::UNKNOWN:
+      NOTREACHED();
+      break;
+  }
+}
+
+void RendererFrameManager::PurgeMemory(int percentage) {
+  int saved_frame_limit = max_number_of_saved_frames_;
+  if (saved_frame_limit <= 1)
+    return;
   CullUnlockedFrames(std::max(1, (saved_frame_limit * percentage) / 100));
 }