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

content/browser/dom_storage/dom_storage_context_impl.cc

 // Copyright (c) 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 "content/browser/dom_storage/dom_storage_context_impl.h"
 
 #include <inttypes.h>
 #include <stddef.h>
 #include <stdlib.h>
 
@@ skipping 416 matching lines @@
         base::TimeDelta::FromSeconds(kSessionStoraceScavengingSeconds));
   }
 }
 
 void DOMStorageContextImpl::PurgeMemory(PurgeOption purge_option) {
   if (is_shutdown_)
     return;
 
   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",
                               initial_stats.total_cache_size / 1024, 1, 100000,
                               50);
 
   const char* purge_reason = nullptr;
   if (purge_option == PURGE_IF_NEEDED) {
     // 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)
       purge_reason = "AreaCountLimitExceeded";
     else if (is_low_end_device_)
       purge_reason = "InactiveOnLowEndDevice";
     if (!purge_reason)
       return;
 
     purge_option = PURGE_UNOPENED;
-  }
-
-  bool aggressively = purge_option == PURGE_AGGRESSIVE;
-  for (const auto& it : namespaces_)
-    it.second->PurgeMemory(aggressively);
-
-  // Track the size of cache purged.
-  if (!purge_reason) {
+  } 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.
   size_t purged_size_kib =
       (initial_stats.total_cache_size -
        GetTotalNamespaceStatistics(namespaces_).total_cache_size) /
       1024;
   std::string full_histogram_name =
       std::string("LocalStorage.BrowserLocalStorageCachePurgedInKB.") +
       purge_reason;
   base::HistogramBase* histogram = base::Histogram::FactoryGet(
       full_histogram_name, 1, 100000, 50,
       base::HistogramBase::kUmaTargetedHistogramFlag);
@@ skipping 95 matching lines @@
   if (!deletable_persistent_namespace_ids_.empty()) {
     task_runner_->PostDelayedTask(
         FROM_HERE, base::Bind(
             &DOMStorageContextImpl::DeleteNextUnusedNamespace,
             this),
         base::TimeDelta::FromSeconds(kSessionStoraceScavengingSeconds));
   }
 }
 
 }  // namespace content