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

chrome/browser/chromeos/memory/oom_priority_manager.cc

-// 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.
 
 #include "chrome/browser/chromeos/memory/oom_priority_manager.h"
 
 #include <algorithm>
 #include <set>
 #include <vector>
 
 #include "ash/multi_profile_uma.h"
 #include "ash/session/session_state_delegate.h"
 #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"
 #include "base/process/process.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part_chromeos.h"
 #include "chrome/browser/chromeos/memory/low_memory_observer.h"
 #include "chrome/browser/memory_details.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_iterator.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_iterator.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/tabs/tab_utils.h"
 #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"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/zygote_host_linux.h"
 #include "ui/base/text/bytes_formatting.h"
 
 using base::TimeDelta;
@@ skipping 44 matching lines @@
   // instance and thus only work if |name| is constant.
   base::HistogramBase* counter = base::LinearHistogram::FactoryGet(
       name,
       1,  // Minimum. The 0 bin for underflow is automatically added.
       maximum + 1,  // Ensure bucket size of |maximum| / |bucket_count|.
       bucket_count + 2,  // Account for the underflow and overflow bins.
       base::Histogram::kUmaTargetedHistogramFlag);
   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
 
 ////////////////////////////////////////////////////////////////////////////////
 // OomMemoryDetails logs details about all Chrome processes during an out-of-
 // memory event in an attempt to identify the culprit, then discards a tab and
 // deletes itself.
 class OomMemoryDetails : public MemoryDetails {
  public:
   OomMemoryDetails();
 
@@ skipping 49 matching lines @@
     is_discarded(false),
     renderer_handle(0),
     tab_contents_id(0) {
 }
 
 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());
   registrar_.Add(this,
       content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
       content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this,
       content::NOTIFICATION_RENDER_WIDGET_VISIBILITY_CHANGED,
       content::NotificationService::AllBrowserContextsAndSources());
 }
 
 OomPriorityManager::~OomPriorityManager() {
   Stop();
 }
 
 void OomPriorityManager::Start() {
   if (!timer_.IsRunning()) {
     timer_.Start(FROM_HERE,
                  TimeDelta::FromSeconds(kAdjustmentIntervalSeconds),
                  this,
                  &OomPriorityManager::AdjustOomPriorities);
   }
   if (!recent_tab_discard_timer_.IsRunning()) {
     recent_tab_discard_timer_.Start(
         FROM_HERE,
         TimeDelta::FromSeconds(kRecentTabDiscardIntervalSeconds),
         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() {
   timer_.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() {
   TabStatsList stats = GetTabStatsOnUIThread();
   base::AutoLock oom_score_autolock(oom_score_lock_);
   std::vector<base::string16> titles;
   titles.reserve(stats.size());
   TabStatsList::iterator it = stats.begin();
   for ( ; it != stats.end(); ++it) {
     base::string16 str;
@@ skipping 167 matching lines @@
                         recent_tab_discard_);
   // Reset for the next interval.
   recent_tab_discard_ = false;
 }
 
 void OomPriorityManager::PurgeBrowserMemory() {
   // Based on experimental evidence, attempts to free memory from renderers
   // 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 {
   int tab_count = 0;
   for (chrome::BrowserIterator it; !it.done(); it.Next())
     tab_count += it->tab_strip_model()->count();
   return tab_count;
 }
 
 // Returns true if |first| is considered less desirable to be killed
@@ skipping 53 matching lines @@
 void OomPriorityManager::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
   base::AutoLock oom_score_autolock(oom_score_lock_);
   switch (type) {
     case content::NOTIFICATION_RENDERER_PROCESS_CLOSED:
     case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED: {
       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: {
       bool visible = *content::Details<bool>(details).ptr();
       if (visible) {
         content::RenderProcessHost* render_host =
             content::Source<content::RenderWidgetHost>(source).ptr()->
             GetProcess();
         focused_tab_process_info_ = std::make_pair(render_host->GetID(),
                                                    render_host->GetHandle());
@@ skipping 157 matching lines @@
     // do not set it.
     if (it == oom_score_map_.end() || it->second != score) {
       content::ZygoteHost::GetInstance()->AdjustRendererOOMScore(
           process_info.second, score);
       oom_score_map_[process_info.first] = score;
     }
     priority += priority_increment;
   }
 }
 
+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