[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 420 matching lines @@
 void DOMStorageContextImpl::PurgeMemory(PurgeOption purge_option) {
   if (is_shutdown_)
     return;
 
   DOMStorageNamespace::UsageStatistics initial_stats =
       GetTotalNamespaceStatistics(namespaces_);
 
   // Track the total localStorage cache size.
   UMA_HISTOGRAM_CUSTOM_COUNTS("LocalStorage.BrowserLocalStorageCacheSizeInKB",
                               initial_stats.total_cache_size / 1024, 1, 100000,
                               50);

[michaeln, 2016/06/27 21:51:15]

if (!stats.total_area_count)
  return;

I think the change makes sense. Would hoisting this test for no areas up here
may make the significant early return more obvious? This is the significant
change, is that right?

[ssid, 2016/06/27 22:00:37]

Thanks for pointing out, i have moved this condition up and before the uma since
we don't want to keep recording 0s after purging all databases.

 
   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;
   }
 
+  // Return if no areas can be purged with the given option.
   bool aggressively = purge_option == PURGE_AGGRESSIVE;
+  if ((aggressively && !initial_stats.total_area_count) ||
+      (!aggressively && !initial_stats.inactive_area_count)) {
+    return;
+  }
   for (const auto& it : namespaces_)
     it.second->PurgeMemory(aggressively);
 
   // Track the size of cache purged.
   if (!purge_reason) {

[michaeln, 2016/06/27 21:51:15]

nit: it might be nice to set purge_reason in all cases up above where your
figuring out the options, before calling ->PurgeMemory

[ssid, 2016/06/27 22:00:37]

Done.

-    if (purge_option == PURGE_AGGRESSIVE)
+    if (aggressively)
       purge_reason = "AggressivePurgeTriggered";
     else
       purge_reason = "ModeratePurgeTriggered";
   }
   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.") +
@@ skipping 99 matching lines @@
   if (!deletable_persistent_namespace_ids_.empty()) {
     task_runner_->PostDelayedTask(
         FROM_HERE, base::Bind(
             &DOMStorageContextImpl::DeleteNextUnusedNamespace,
             this),
         base::TimeDelta::FromSeconds(kSessionStoraceScavengingSeconds));
   }
 }
 
 }  // namespace content