Make DOMStorageContextWrapper a client of memory coordinator

content/browser/dom_storage/dom_storage_context_wrapper.cc

Index: content/browser/dom_storage/dom_storage_context_wrapper.cc
diff --git a/content/browser/dom_storage/dom_storage_context_wrapper.cc b/content/browser/dom_storage/dom_storage_context_wrapper.cc
index 52e769cf72a1e7bc86c15b670d166ca0fb39e7e0..52dd43dfee9dde960a405e49575dc7e7051fe619 100644
--- a/content/browser/dom_storage/dom_storage_context_wrapper.cc
+++ b/content/browser/dom_storage/dom_storage_context_wrapper.cc
@@ -11,6 +11,7 @@
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/location.h"
+#include "base/memory/memory_coordinator_client_registry.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
@@ -23,9 +24,11 @@
 #include "content/browser/dom_storage/dom_storage_task_runner.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/leveldb_wrapper_impl.h"
+#include "content/browser/memory/memory_coordinator.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/local_storage_usage_info.h"
 #include "content/public/browser/session_storage_usage_info.h"
+#include "content/public/common/content_features.h"
 #include "mojo/common/common_type_converters.h"
 #include "services/file/public/cpp/constants.h"
 #include "services/file/public/interfaces/file_system.mojom.h"
@@ -284,8 +287,12 @@ DOMStorageContextWrapper::DOMStorageContextWrapper(
           worker_pool->GetNamedSequenceToken("dom_storage_commit"),
           BrowserThread::GetTaskRunnerForThread(BrowserThread::IO).get()));
 
-  memory_pressure_listener_.reset(new base::MemoryPressureListener(
-      base::Bind(&DOMStorageContextWrapper::OnMemoryPressure, this)));
+  if (base::FeatureList::IsEnabled(features::kMemoryCoordinator)) {
+    base::MemoryCoordinatorClientRegistry::GetInstance()->Register(this);
+  } else {
+    memory_pressure_listener_.reset(new base::MemoryPressureListener(
+        base::Bind(&DOMStorageContextWrapper::OnMemoryPressure, this)));
+  }
 }
 
 DOMStorageContextWrapper::~DOMStorageContextWrapper() {}
@@ -365,6 +372,10 @@ void DOMStorageContextWrapper::Shutdown() {
       FROM_HERE,
       DOMStorageTaskRunner::PRIMARY_SEQUENCE,
       base::Bind(&DOMStorageContextImpl::Shutdown, context_));
+  if (base::FeatureList::IsEnabled(features::kMemoryCoordinator)) {
+    base::MemoryCoordinatorClientRegistry::GetInstance()->Unregister(this);
+  }
+
 }
 
 void DOMStorageContextWrapper::Flush() {
@@ -390,6 +401,33 @@ void DOMStorageContextWrapper::OnMemoryPressure(
       base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL) {
     purge_option = DOMStorageContextImpl::PURGE_AGGRESSIVE;
   }
+  PurgeMemory(purge_option);
+}
+
+void DOMStorageContextWrapper::OnMemoryStateChange(base::MemoryState state) {
+  // TODO(hajimehoshi): As OnMemoryStateChange changes the state, we should
+  // adjust the limitation to the amount of cache, DomStroageContextImpl doesn't
+  // have such limitation so far though.
+  switch (state) {
+    case base::MemoryState::NORMAL:
+      // Don't have to purge memory here.
+      break;
+    case base::MemoryState::THROTTLED:

[ssid, 2016/09/15 21:58:54]

I fail to understand when THROTTLED gets called for browser. So, without memory
coordinator the pressure levels were moderate and critical. The DOMStorage does
non-aggressive and less aggressive purging in these respective cases.

What happens in browser when using memory coordinator and we hit moderate and
critical signals?
I am worried that if we hit a critical pressure signal and if we only purge
unopened, and not do an aggressive purge, then we might not be purging enough
here.

None of the levels could be used to differentiate between moderate and critical,
whereas it seems to make sense for renderers very well.
Also, I think in case where the foreground renderer is going to be throttled an
aggressive purge should be called. This would try to commit the databases that
foreground renderer has opened.

[bashi, 2016/09/15 23:09:29]

chrisha@ wrote a design doc for when to change memory state and I think he is
implementing the first version of memory coordinator in browser side.
https://docs.google.com/document/d/1GMSkij4qpbkK6T7MtCX9FcPlv39Q9J2lmW8qY8Wmf...
When memory coordinator is enabled, MemoryPressureMonitor will be disabled,
which means there is no critical/moderate notifications. Instead,
MemoryCoordinator will notify memory state changes to MemoryCoordinatorClient
(DOMStorageContextWrapper in this CL).

As hajimehoshi@ described in comments, we only have normal/throttled states in
the browser side and there is no levels in throttling for now. It might not be
enough but I think we can start here then improve this if this could be a
problem.

(In my ideal world we should actually throttle memory allocation if possible,
maybe adding ThrottleMemory())

[ssid, 2016/09/15 23:39:56]

Yes, but on Android we get critical and moderate signals from the OS and for
both these cases the coordinator would be sending a state of throttled. So, we
would never do an aggressive purge.
I am afraid if we don't add a level now, we might not notice that later we could
have released more memory on critical signals from OS. So, this would go
unnoticed.

Why not always purge aggressively and if we notice an issue, lets introduce
lower levels?

[bashi, 2016/09/15 23:57:42]

+1 to this idea. For now, purging aggressively when THROTTLED is received makes
sense to me.

[hajimehoshi, 2016/09/20 05:26:18]

+1. Done.

+      // TOOD(hajimehoshi): We don't have throttling 'level' so far. When we
+      // have such value, let's change the argument accroding to the value.
+      PurgeMemory(DOMStorageContextImpl::PURGE_UNOPENED);
+      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 DOMStorageContextWrapper::PurgeMemory(DOMStorageContextImpl::PurgeOption
+    purge_option) {
   context_->task_runner()->PostTask(
       FROM_HERE,
       base::Bind(&DOMStorageContextImpl::PurgeMemory, context_, purge_option));