Exclude in-use origins from storage evictions for all QuotaEvictionPolicies.

storage/browser/quota/quota_manager.cc

 // Copyright 2013 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 "storage/browser/quota/quota_manager.h"
 
 #include <algorithm>
 #include <functional>
 
 #include "base/bind.h"
@@ skipping 123 matching lines @@
                           QuotaDatabase* database) {
   DCHECK(database);
   database->GetQuotaConfigValue(QuotaDatabase::kTemporaryQuotaOverrideKey,
                                 temporary_quota_override);
   database->GetQuotaConfigValue(QuotaDatabase::kDesiredAvailableSpaceKey,
                                 desired_available_space);
   return true;
 }
 
 bool GetLRUOriginOnDBThread(StorageType type,
-                            std::set<GURL>* exceptions,
+                            const std::set<GURL>& exceptions,
                             SpecialStoragePolicy* policy,
                             GURL* url,
                             QuotaDatabase* database) {
   DCHECK(database);
-  database->GetLRUOrigin(type, *exceptions, policy, url);
+  database->GetLRUOrigin(type, exceptions, policy, url);
   return true;
 }
 
 bool DeleteOriginInfoOnDBThread(const GURL& origin,
                                 StorageType type,
                                 QuotaDatabase* database) {
   DCHECK(database);
   return database->DeleteOriginInfo(origin, type);
 }
 
@@ skipping 1291 matching lines @@
   LazyInitialize();
   is_getting_eviction_origin_ = true;
 
   GetOriginCallback did_get_origin_callback =
       base::Bind(&QuotaManager::DidGetEvictionOrigin,
                  weak_factory_.GetWeakPtr(), callback);
 
   auto eviction_policy = eviction_policy_map_.find(type);
   if (eviction_policy != eviction_policy_map_.end()) {
     eviction_policy->second->GetEvictionOrigin(special_storage_policy_,
+                                               GetEvictionOriginExceptions(),
                                                did_get_origin_callback);
   }
 
   // TODO(calamity): convert LRU origin retrieval into a QuotaEvictionPolicy.
   GetLRUOrigin(type, did_get_origin_callback);
 }
 
+std::set<GURL> QuotaManager::GetEvictionOriginExceptions() {
+  std::set<GURL> exceptions;
+  for (const auto& p : origins_in_use_) {
+    if (p.second > 0)
+      exceptions.insert(p.first);
+  }
+
+  for (const auto& p : origins_in_error_) {
+    if (p.second > QuotaManager::kThresholdOfErrorsToBeBlacklisted)
+      exceptions.insert(p.first);
+  }

[raymes, 2015/09/22 07:16:10]

I'm confused a bit by this stuff - but I see you've just copied it from below!

e.g. why do we consider access_notified_origins_ below but not here. Also why
don't we consider origins_in_error_ below? =\

[calamity, 2015/09/23 01:55:21]

Good question. Okay, so, access_notified_origins_ is populated during the
eviction origin task and accounts for origins accessed during the task. ie These
lists represent the exceptions before the task is launched and
access_notified_origins_ are exceptions that arise during the task. (I'm not
sure why origins_in_use_ isn't just checked again below. Maybe it was deemed not
performant?)

I'm not sure why something similar isn't done for the origins in error, or in
fact why origins in error need to excepted from eviction.

+
+  return exceptions;
+}
+
 void QuotaManager::DidGetEvictionOrigin(const GetOriginCallback& callback,
                                         const GURL& origin) {
-  callback.Run(origin);
+  // Make sure the returned origin is (still) not in the origin_in_use_ set
+  // and has not been accessed since we posted the task.
+  if (origins_in_use_.find(origin) != origins_in_use_.end() ||
+      access_notified_origins_.find(origin) != access_notified_origins_.end()) {
+    callback.Run(GURL());
+  } else {
+    callback.Run(origin);
+  }
+  access_notified_origins_.clear();
 
   is_getting_eviction_origin_ = false;
 }
 
 void QuotaManager::EvictOriginData(const GURL& origin,
                                    StorageType type,
                                    const EvictOriginDataCallback& callback) {
   DCHECK(io_thread_->BelongsToCurrentThread());
   DCHECK_EQ(type, kStorageTypeTemporary);
 
@@ skipping 25 matching lines @@
   LazyInitialize();
   // This must not be called while there's an in-flight task.
   DCHECK(lru_origin_callback_.is_null());
   lru_origin_callback_ = callback;
   if (db_disabled_) {
     lru_origin_callback_.Run(GURL());
     lru_origin_callback_.Reset();
     return;
   }
 
-  // TODO(calamity): make all QuotaEvictionPolicies aware of these exceptions.
-  std::set<GURL>* exceptions = new std::set<GURL>;
-  for (std::map<GURL, int>::const_iterator p = origins_in_use_.begin();
-       p != origins_in_use_.end();
-       ++p) {
-    if (p->second > 0)
-      exceptions->insert(p->first);
-  }
-  for (std::map<GURL, int>::const_iterator p = origins_in_error_.begin();
-       p != origins_in_error_.end();
-       ++p) {
-    if (p->second > QuotaManager::kThresholdOfErrorsToBeBlacklisted)
-      exceptions->insert(p->first);
-  }
-
   GURL* url = new GURL;
   PostTaskAndReplyWithResultForDBThread(
       FROM_HERE,
-      base::Bind(&GetLRUOriginOnDBThread,
-                 type,
-                 base::Owned(exceptions),
-                 special_storage_policy_,
-                 base::Unretained(url)),
-      base::Bind(&QuotaManager::DidGetLRUOrigin,
-                 weak_factory_.GetWeakPtr(),
+      base::Bind(&GetLRUOriginOnDBThread, type, GetEvictionOriginExceptions(),
+                 special_storage_policy_, base::Unretained(url)),
+      base::Bind(&QuotaManager::DidGetLRUOrigin, weak_factory_.GetWeakPtr(),
                  base::Owned(url)));
 }
 
 void QuotaManager::DidSetTemporaryGlobalOverrideQuota(
     const QuotaCallback& callback,
     const int64* new_quota,
     bool success) {
   QuotaStatusCode status = kQuotaErrorInvalidAccess;
   DidDatabaseWork(success);
   if (success) {
@@ skipping 37 matching lines @@
 
   db_initialization_callbacks_.Run();
   GetTemporaryGlobalQuota(
       base::Bind(&QuotaManager::DidGetInitialTemporaryGlobalQuota,
                  weak_factory_.GetWeakPtr()));
 }
 
 void QuotaManager::DidGetLRUOrigin(const GURL* origin,
                                    bool success) {
   DidDatabaseWork(success);
-  // Make sure the returned origin is (still) not in the origin_in_use_ set
-  // and has not been accessed since we posted the task.
-  if (origins_in_use_.find(*origin) != origins_in_use_.end() ||
-      access_notified_origins_.find(*origin) != access_notified_origins_.end())
-    lru_origin_callback_.Run(GURL());
-  else
-    lru_origin_callback_.Run(*origin);
-  access_notified_origins_.clear();
+
+  lru_origin_callback_.Run(*origin);
   lru_origin_callback_.Reset();
 }
 
 void QuotaManager::DidGetInitialTemporaryGlobalQuota(
     QuotaStatusCode status, int64 quota_unused) {
   if (eviction_disabled_)
     return;
 
   std::set<GURL>* origins = new std::set<GURL>;
   temporary_usage_tracker_->GetCachedOrigins(origins);
@@ skipping 37 matching lines @@
   // |database_|, therefore we can be sure that database_ is alive when this
   // task runs.
   base::PostTaskAndReplyWithResult(
       db_thread_.get(),
       from_here,
       base::Bind(task, base::Unretained(database_.get())),
       reply);
 }
 
 }  // namespace storage