[DOMStorage] Do not try purging caches when no database is open

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 3ef365c2bb045af1bd4c4f3931d2fc9d5f7a5da9..425c45bee84d8e30482cc70c5028bfd75d7f425a 100644
--- a/content/browser/dom_storage/dom_storage_context_impl.cc
+++ b/content/browser/dom_storage/dom_storage_context_impl.cc
@@ -434,6 +434,8 @@ void DOMStorageContextImpl::PurgeMemory(PurgeOption purge_option) {
 
   DOMStorageNamespace::UsageStatistics initial_stats =
       GetTotalNamespaceStatistics(namespaces_);
+  if (!initial_stats.total_area_count)
+    return;
 
   // Track the total localStorage cache size.
   UMA_HISTOGRAM_CUSTOM_COUNTS("LocalStorage.BrowserLocalStorageCacheSizeInKB",
@@ -445,9 +447,6 @@ void DOMStorageContextImpl::PurgeMemory(PurgeOption purge_option) {
     // Purging is done based on the cache sizes without including the database
     // size since it can be expensive trying to estimate the sqlite usage for
     // all databases. For low end devices purge all inactive areas.
-    if (!initial_stats.inactive_area_count)
-      return;
-
     if (initial_stats.total_cache_size > kMaxCacheSize)
       purge_reason = "SizeLimitExceeded";
     else if (initial_stats.total_area_count > kMaxStorageAreaCount)
@@ -458,19 +457,22 @@ void DOMStorageContextImpl::PurgeMemory(PurgeOption purge_option) {
       return;
 
     purge_option = PURGE_UNOPENED;
+  } else {
+    if (purge_option == PURGE_AGGRESSIVE)
+      purge_reason = "AggressivePurgeTriggered";
+    else
+      purge_reason = "ModeratePurgeTriggered";
   }
 
+  // Return if no areas can be purged with the given option.
   bool aggressively = purge_option == PURGE_AGGRESSIVE;
+  if (!aggressively && !initial_stats.inactive_area_count) {
+    return;

[michaeln, 2016/06/27 22:59:02]

nit: you might be able to work this early return into the if else above too

if (PURGE_IF_NEEDED) {
 ...
} else if (PURGE_UNOPENED) {
  purge_reason = "Moderate"
  if (!initial_stats.inactive_area_count)
    return;
} else {
  purge_reason = "Aggressive"
}

[ssid, 2016/06/27 23:09:29]

Hm there are 2 cases where this helps.
1. When we decide purging is needed and set to PURGE_UNOPENED because size limit
or low end device. But, we do not actually check if there are unopened
databases.
2. When moderate memory signal is passed.
This will do an early return for both these cases. Previously we only used to
return for case (1). Now we do return for both these cases.

Isn't it better to do this check once here rather than having it twice in
different if conditions?

[michaeln, 2016/06/28 18:33:45]

still lgtm, good point about only having one early return call for the condition

+  }
   for (const auto& it : namespaces_)
     it.second->PurgeMemory(aggressively);
 
   // Track the size of cache purged.
-  if (!purge_reason) {
-    if (purge_option == PURGE_AGGRESSIVE)
-      purge_reason = "AggressivePurgeTriggered";
-    else
-      purge_reason = "ModeratePurgeTriggered";
-  }
   size_t purged_size_kib =
       (initial_stats.total_cache_size -
        GetTotalNamespaceStatistics(namespaces_).total_cache_size) /