Using the new MemoryPressureListener instead of the LowMemoryObserver when the enhanced memory mana…

chrome/browser/chromeos/memory/oom_priority_manager.cc

Index: chrome/browser/chromeos/memory/oom_priority_manager.cc
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.cc b/chrome/browser/chromeos/memory/oom_priority_manager.cc
index d192680026aaa3e3f9d6607e0ecc7df353268b3d..12478c055bb985e9bc3be7216641ab4a2aa03a7f 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager.cc
+++ b/chrome/browser/chromeos/memory/oom_priority_manager.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2012 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.
 
@@ -13,6 +13,7 @@
 #include "ash/shell.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/chromeos/memory_pressure_observer_chromeos.h"
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
@@ -39,6 +40,7 @@
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/url_constants.h"
 #include "chromeos/chromeos_switches.h"
+#include "content/browser/browser_main_loop.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
@@ -103,6 +105,12 @@ void RecordLinearHistogram(const std::string& name,
   counter->Add(sample);
 }
 
+// Gets the MemoryPressureObserver - if it exists.
+base::MemoryPressureObserverChromeOS* memory_pressure_observer() {

[James Cook, 2014/12/19 19:38:08]

GetMemoryPressureObserver or similar

+  // TODO(skuhne): Before submitting this needs to move.

[James Cook, 2014/12/19 19:38:08]

Acknowledged.

+  return content::BrowserMainLoop::GetInstance()->memory_pressure_observer();
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -172,9 +180,13 @@ OomPriorityManager::TabStats::~TabStats() {
 
 OomPriorityManager::OomPriorityManager()
     : focused_tab_process_info_(std::make_pair(0, 0)),
-      low_memory_observer_(new LowMemoryObserver),
       discard_count_(0),
       recent_tab_discard_(false) {
+  // Use the old |LowMemoryObserver| when there is no
+  // |MemoryPressureObserverChromeOS|.
+  if (!memory_pressure_observer())
+    low_memory_observer_.reset(new LowMemoryObserver);
+
   registrar_.Add(this,
       content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
       content::NotificationService::AllBrowserContextsAndSources());
@@ -204,9 +216,23 @@ void OomPriorityManager::Start() {
         this,
         &OomPriorityManager::RecordRecentTabDiscard);
   }
+  start_time_ = TimeTicks::Now();
+  // If a |LowMemoryObserver| exists we use the old system, otherwise we create
+  // a |MemoryPressureListener| to listen for memory events.
   if (low_memory_observer_.get())

[James Cook, 2014/12/19 19:38:08]

I might write a little function like "bool UseOldMemoryObserver()" or "bool
UseNewMemoryListener()" to emphasize that there is an old way and a new way.

     low_memory_observer_->Start();

[James Cook, 2014/12/19 19:38:08]

just return after this line

-  start_time_ = TimeTicks::Now();
+  else {
+    base::MemoryPressureObserverChromeOS* observer = memory_pressure_observer();
+    if (observer) {
+      memory_pressure_listener_.reset(new base::MemoryPressureListener(

[James Cook, 2014/12/19 19:38:07]

optional: "using base::MemoryPressureListener;" might make some of this file
easier to read

+          base::Bind(&OomPriorityManager::OnMemoryPressure,
+                     base::Unretained(this))));
+      base::MemoryPressureListener::MemoryPressureLevel level =
+          observer->GetCurrentPressureLevel();
+      if (level == base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL)

[James Cook, 2014/12/19 19:38:08]

Do you really need this?  I think the manager is started pretty early in system
startup, so it's unlikely you'll be under memory pressure already.

+        OnMemoryPressure(level);
+    }
+  }
 }
 
 void OomPriorityManager::Stop() {
@@ -214,6 +240,8 @@ void OomPriorityManager::Stop() {
   recent_tab_discard_timer_.Stop();
   if (low_memory_observer_.get())
     low_memory_observer_->Stop();
+  else
+    memory_pressure_listener_.reset();
 }
 
 std::vector<base::string16> OomPriorityManager::GetTabTitles() {
@@ -401,13 +429,14 @@ void OomPriorityManager::PurgeBrowserMemory() {
   // have been too slow to use in OOM situations (V8 garbage collection) or
   // do not lead to persistent decreased usage (image/bitmap caches). This
   // function therefore only targets large blocks of memory in the browser.
+  // Note that other objects will listen to MemoryPressureListener events
+  // to release memory.
   for (TabContentsIterator it; !it.done(); it.Next()) {
     WebContents* web_contents = *it;
     // Screenshots can consume ~5 MB per web contents for platforms that do
     // touch back/forward.
     web_contents->GetController().ClearAllScreenshots();
   }
-  // TODO(jamescook): Are there other things we could flush? Drive metadata?

[James Cook, 2014/12/19 19:38:07]

Hooray for working on this!

 }
 
 int OomPriorityManager::GetTabCount() const {
@@ -481,6 +510,21 @@ void OomPriorityManager::Observe(int type,
       content::RenderProcessHost* host =
           content::Source<content::RenderProcessHost>(source).ptr();
       oom_score_map_.erase(host->GetID());
+      if (!low_memory_observer_.get()) {
+        // Coming here we know that a renderer was just killed and memory should
+        // come back into the pool. However - the memory pressure observer did
+        // not yet update its status and therefore we ask it to redo the
+        // measurement, calling us again if we have to release more.
+        // Note: We do not only accelerate the discarding speed by doing another
+        // check in short succession - we also accelerate it because the timer
+        // driven MemoryPressureObserver will continue to produce timed events
+        // on top. So as longer as the cleanup phase takes, as more tabs will
+        // get discarded in parallel.
+        base::MemoryPressureObserverChromeOS* observer =
+            memory_pressure_observer();
+        if (observer)

[James Cook, 2014/12/19 19:38:08]

Do you need this if()?

+          observer->ScheduleEarlyCheck();
+      }
       break;
     }
     case content::NOTIFICATION_RENDER_WIDGET_VISIBILITY_CHANGED: {
@@ -658,4 +702,15 @@ void OomPriorityManager::AdjustOomPrioritiesOnFileThread(
   }
 }
 
+void OomPriorityManager::OnMemoryPressure(
+    base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level) {
+  // For the moment we only do something when we reach a critical state.
+  if (memory_pressure_level ==
+      base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL) {
+    LogMemoryAndDiscardTab();
+  }
+  // TODO(skuhne): If more memory pressure levels are introduced, we might
+  // consider to call PurgeBrowserMemory() before CRITICAL is reached.
+}
+
 }  // namespace chromeos