Make RendererFrameManager a client of memory coordinator

content/browser/renderer_host/renderer_frame_manager.cc

 // Copyright 2013 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 "content/browser/renderer_host/renderer_frame_manager.h"
 
 #include <algorithm>
 
 #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 {
 
 const int kModeratePressurePercentage = 50;
 const int kCriticalPressurePercentage = 10;
 
 }  // namespace
 
 RendererFrameManager* RendererFrameManager::GetInstance() {
@@ skipping 38 matching lines @@
   if (locked_count > 1) {
     locked_frames_[frame]--;
   } else {
     RemoveFrame(frame);
     unlocked_frames_.push_front(frame);
     CullUnlockedFrames(GetMaxNumberOfSavedFrames());
   }
 }
 
 size_t RendererFrameManager::GetMaxNumberOfSavedFrames() const {
-  base::MemoryPressureMonitor* monitor = base::MemoryPressureMonitor::Get();
+  int percentage = 100;
+  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_;
+    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;
+    // 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;
 #else
       std::min(5, 2 + (base::SysInfo::AmountOfPhysicalMemoryMB() / 256));
 #endif
   max_handles_ = base::SharedMemory::GetHandleLimit() / 8.0f;
 }
 
 RendererFrameManager::~RendererFrameManager() {}
@@ skipping 13 matching lines @@
          unlocked_frames_.size() + locked_frames_.size() > saved_frame_limit) {
     size_t old_size = unlocked_frames_.size();
     // Should remove self from list.
     unlocked_frames_.back()->EvictCurrentFrame();
     DCHECK_EQ(unlocked_frames_.size() + 1, old_size);
   }
 }
 
 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));
 }
 
 }  // namespace content