Purge browser cache for dom storage in a smarter way

content/browser/dom_storage/dom_storage_context_impl.cc

Index: content/browser/dom_storage/dom_storage_context_impl.cc
diff --git a/content/browser/dom_storage/dom_storage_context_impl.cc b/content/browser/dom_storage/dom_storage_context_impl.cc
index dad99c8462873eab07bc6e1dd36da7e62687eaac..57339445794fec2b820572957e5a3faad711fddf 100644
--- a/content/browser/dom_storage/dom_storage_context_impl.cc
+++ b/content/browser/dom_storage/dom_storage_context_impl.cc
@@ -35,6 +35,41 @@ static const int kSessionStoraceScavengingSeconds = 60;
 // to help identify when an id from one is mistakenly used in another.
 static int g_session_id_offset_sequence = 1;
 
+// Helper class that posts the memory pressure notification on dom storage task
+// runner.
+class DOMStorageContextImpl::DOMStorageMemoryPressureListener {
+ public:
+  DOMStorageMemoryPressureListener(DOMStorageTaskRunner* task_runner,
+                                   DOMStorageContextImpl* context)
+      : task_runner_(task_runner),
+        context_(context),
+        listener_(
+            base::Bind(&DOMStorageMemoryPressureListener::OnMemoryPressure,
+                       base::Unretained(this))) {}
+
+  void OnMemoryPressure(
+      base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level) {
+    base::AutoLock lock(lock_);
+    if (task_runner_) {
+      task_runner_->PostTask(
+          FROM_HERE, base::Bind(&DOMStorageContextImpl::PurgeMemory, context_,
+                                memory_pressure_level));
+    }
+  }
+
+  void Shutdown() {
+    base::AutoLock lock(lock_);

[michaeln, 2016/05/09 21:50:31]

Lets avoid leaks and locks for this. Having a separate helper with a
MemoryPressureListener to eves drop on OnMemoryPressure in a well known thread
looks good, but leaking and interlocking on shutdown does not.

What if...

A) We have the DOMStorageContextWrapper be responsible for setting up and
tearing down the MemoryPressureListener. The purpose of the wrapper class is to
help plug into other parts of chrome. The base::Listener could be created in the
ctor (called on the UI thread) and deleted in
DOMStorageContextWrapper::Shutdow() (aldo called on the UI thread). That might
be the most straightforward way of hooking into OnMemoryPressure() which whould
then PostTask on the sequence runner to the context impl.

There's probably other ways to avoid leaks and locks.

B) Another idea, lazily construct this inner Listener class.  PostTask to the
BrowserThread::IO to initialize it. We'll have to construct the
base::MemoryPressureListener on the IO thread. Then in
DOMStorageContextImpl::Shutdown() we call
BrowserThread::DeleteSoon(BrowserThread::IO, mem_listner_.release()). That
sounds more complicated that option A though.

[ssid, 2016/05/10 02:04:16]

listener in DOMStorageContextWrapper sounds like a good idea. I should have
looked into the code little more.

+    task_runner_ = nullptr;
+    context_ = nullptr;
+  }
+
+ private:
+  scoped_refptr<DOMStorageTaskRunner> task_runner_;
+  scoped_refptr<DOMStorageContextImpl> context_;
+  base::MemoryPressureListener listener_;
+  base::Lock lock_;
+};
+
 DOMStorageContextImpl::DOMStorageContextImpl(
     const base::FilePath& localstorage_directory,
     const base::FilePath& sessionstorage_directory,
@@ -55,6 +90,11 @@ DOMStorageContextImpl::DOMStorageContextImpl(
 
   // Tests may run without task runners.
   if (task_runner_) {
+    // Created for each DOMStorageContextImpl object and is leaked since
+    // destruction is not thread safe.
+    leaked_memory_pressure_listener_ =
+        new DOMStorageMemoryPressureListener(task_runner, this);
+
     // Registering dump provider is safe even outside the task runner.
     base::trace_event::MemoryDumpManager::GetInstance()
         ->RegisterDumpProviderWithSequencedTaskRunner(
@@ -193,6 +233,8 @@ void DOMStorageContextImpl::Shutdown() {
   for (; it != namespaces_.end(); ++it)
     it->second->Shutdown();
 
+  leaked_memory_pressure_listener_->Shutdown();
+
   base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
       this);
 
@@ -374,6 +416,19 @@ void DOMStorageContextImpl::StartScavengingUnusedSessionStorage() {
   }
 }
 
+void DOMStorageContextImpl::PurgeMemory(
+    base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level) {
+  if (is_shutdown_)
+    return;
+
+  DOMStorageNamespace::PurgeOption option = DOMStorageNamespace::PURGE_UNOPENED;
+  if (memory_pressure_level ==
+      base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL)
+    option = DOMStorageNamespace::PURGE_AGGRESSIVE;
+  for (const auto& it : namespaces_)
+    it.second->PurgeMemory(option);
+}
+
 bool DOMStorageContextImpl::OnMemoryDump(
     const base::trace_event::MemoryDumpArgs& args,
     base::trace_event::ProcessMemoryDump* pmd) {