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

This CL adds exceptions to the QuotaEvictionPolicy interface which are
used to prevent in-use origins from being evicted and implements handling
of origin exceptions in the SiteEngagementEvictionPolicy.

BUG=464234

Review URL: https://codereview.chromium.org/1354543002

Cr-Commit-Position: refs/heads/master@{#355236}

6 files changed, 81 insertions, 48 deletions

diff --git a/chrome/browser/engagement/site_engagement_eviction_policy.cc b/chrome/browser/engagement/site_engagement_eviction_policy.cc
index 79ebe1f..a078585 100644
--- a/chrome/browser/engagement/site_engagement_eviction_policy.cc
+++ b/chrome/browser/engagement/site_engagement_eviction_policy.cc
@@ -29,6 +29,7 @@ int64 GetSoftQuotaForOrigin(const GURL& origin,
 GURL DoCalculateEvictionOrigin(
     const scoped_refptr<storage::SpecialStoragePolicy>& special_storage_policy,
     SiteEngagementScoreProvider* score_provider,
+    const std::set<GURL>& exceptions,
     const std::map<GURL, int64>& usage_map,
     int64 global_quota) {
   // TODO(calamity): Integrate storage access frequency as an input to this
@@ -58,7 +59,7 @@ GURL DoCalculateEvictionOrigin(
     int64 overuse = usage.second - GetSoftQuotaForOrigin(
                                        origin, score_provider->GetScore(origin),
                                        total_engagement_points, global_quota);
-    if (overuse > max_overuse) {
+    if (overuse > max_overuse && !ContainsKey(exceptions, origin)) {
       max_overuse = overuse;
       origin_to_evict = origin;
     }
@@ -70,6 +71,7 @@ GURL DoCalculateEvictionOrigin(
 GURL GetSiteEngagementEvictionOriginOnUIThread(
     const scoped_refptr<storage::SpecialStoragePolicy>& special_storage_policy,
     content::BrowserContext* browser_context,
+    const std::set<GURL>& exceptions,
     const std::map<GURL, int64>& usage_map,
     int64 global_quota) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -84,7 +86,7 @@ GURL GetSiteEngagementEvictionOriginOnUIThread(
     return GURL();
 
   return DoCalculateEvictionOrigin(special_storage_policy, score_provider,
-                                   usage_map, global_quota);
+                                   exceptions, usage_map, global_quota);
 }
 
 }  // namespace
@@ -97,6 +99,7 @@ SiteEngagementEvictionPolicy::~SiteEngagementEvictionPolicy() {}
 
 void SiteEngagementEvictionPolicy::GetEvictionOrigin(
     const scoped_refptr<storage::SpecialStoragePolicy>& special_storage_policy,
+    const std::set<GURL>& exceptions,
     const std::map<GURL, int64>& usage_map,
     int64 global_quota,
     const storage::GetOriginCallback& callback) {
@@ -105,8 +108,8 @@ void SiteEngagementEvictionPolicy::GetEvictionOrigin(
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(&GetSiteEngagementEvictionOriginOnUIThread,
-                 special_storage_policy, browser_context_, usage_map,
-                 global_quota),
+                 special_storage_policy, browser_context_, exceptions,
+                 usage_map, global_quota),
       callback);
 }
 
@@ -114,8 +117,9 @@ void SiteEngagementEvictionPolicy::GetEvictionOrigin(
 GURL SiteEngagementEvictionPolicy::CalculateEvictionOriginForTests(
     const scoped_refptr<storage::SpecialStoragePolicy>& special_storage_policy,
     SiteEngagementScoreProvider* score_provider,
+    const std::set<GURL>& exceptions,
     const std::map<GURL, int64>& usage_map,
     int64 global_quota) {
   return DoCalculateEvictionOrigin(special_storage_policy, score_provider,
-                                   usage_map, global_quota);
+                                   exceptions, usage_map, global_quota);
 }
diff --git a/chrome/browser/engagement/site_engagement_eviction_policy.h b/chrome/browser/engagement/site_engagement_eviction_policy.h
index 60c8768..ae78603e 100644
--- a/chrome/browser/engagement/site_engagement_eviction_policy.h
+++ b/chrome/browser/engagement/site_engagement_eviction_policy.h
@@ -28,6 +28,7 @@ class SiteEngagementEvictionPolicy : public storage::QuotaEvictionPolicy {
   // Overridden from storage::QuotaEvictionPolicy:
   void GetEvictionOrigin(const scoped_refptr<storage::SpecialStoragePolicy>&
                              special_storage_policy,
+                         const std::set<GURL>& exceptions,
                          const std::map<GURL, int64>& usage_map,
                          int64 global_quota,
                          const storage::GetOriginCallback& callback) override;
@@ -39,6 +40,7 @@ class SiteEngagementEvictionPolicy : public storage::QuotaEvictionPolicy {
       const scoped_refptr<storage::SpecialStoragePolicy>&
           special_storage_policy,
       SiteEngagementScoreProvider* score_provider,
+      const std::set<GURL>& exceptions,
       const std::map<GURL, int64>& usage_map,
       int64 global_quota);
 
diff --git a/chrome/browser/engagement/site_engagement_eviction_policy_unittest.cc b/chrome/browser/engagement/site_engagement_eviction_policy_unittest.cc
index 19c948e..abef52d 100644
--- a/chrome/browser/engagement/site_engagement_eviction_policy_unittest.cc
+++ b/chrome/browser/engagement/site_engagement_eviction_policy_unittest.cc
@@ -57,9 +57,15 @@ class SiteEngagementEvictionPolicyTest : public testing::Test {
 
   ~SiteEngagementEvictionPolicyTest() override {}
 
-  GURL CalculateEvictionOrigin(const std::map<GURL, int64>& usage) {
+  GURL CalculateEvictionOriginWithExceptions(const std::map<GURL, int64>& usage,
+                                             const std::set<GURL>& exceptions) {
     return SiteEngagementEvictionPolicy::CalculateEvictionOriginForTests(
-        storage_policy_, score_provider_.get(), usage, kGlobalQuota);
+        storage_policy_, score_provider_.get(), exceptions, usage,
+        kGlobalQuota);
+  }
+
+  GURL CalculateEvictionOrigin(const std::map<GURL, int64>& usage) {
+    return CalculateEvictionOriginWithExceptions(usage, std::set<GURL>());
   }
 
   TestSiteEngagementScoreProvider* score_provider() {
@@ -137,3 +143,22 @@ TEST_F(SiteEngagementEvictionPolicyTest, SpecialStoragePolicy) {
   storage_policy()->AddUnlimited(url1);
   EXPECT_EQ(GURL(), CalculateEvictionOrigin(usage));
 }
+
+TEST_F(SiteEngagementEvictionPolicyTest, Exceptions) {
+  GURL url1("http://www.google.com");
+  GURL url2("http://www.example.com");
+
+  std::map<GURL, int64> usage;
+  usage[url1] = 10 * 1024;
+  usage[url2] = 10 * 1024;
+
+  score_provider()->SetScore(url1, 50);
+  score_provider()->SetScore(url2, 25);
+
+  EXPECT_EQ(url2, CalculateEvictionOrigin(usage));
+
+  // The policy should respect exceptions.
+  std::set<GURL> exceptions;
+  exceptions.insert(url2);
+  EXPECT_EQ(url1, CalculateEvictionOriginWithExceptions(usage, exceptions));
+}
diff --git a/content/browser/quota/quota_manager_unittest.cc b/content/browser/quota/quota_manager_unittest.cc
index b316648..6a7d1d9 100644
--- a/content/browser/quota/quota_manager_unittest.cc
+++ b/content/browser/quota/quota_manager_unittest.cc
@@ -74,6 +74,7 @@ class TestEvictionPolicy : public storage::QuotaEvictionPolicy {
   // Overridden from storage::QuotaEvictionPolicy:
   void GetEvictionOrigin(const scoped_refptr<storage::SpecialStoragePolicy>&
                              special_storage_policy,
+                         const std::set<GURL>& exceptions,
                          const std::map<GURL, int64>& usage_map,
                          int64 global_quota,
                          const storage::GetOriginCallback& callback) override {
diff --git a/storage/browser/quota/quota_manager.cc b/storage/browser/quota/quota_manager.cc
index 3187621..0052f2d 100644
--- a/storage/browser/quota/quota_manager.cc
+++ b/storage/browser/quota/quota_manager.cc
@@ -145,12 +145,12 @@ bool InitializeOnDBThread(int64* temporary_quota_override,
 }
 
 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;
 }
 
@@ -1510,6 +1510,36 @@ void QuotaManager::DidGetPersistentGlobalUsageForHistogram(
                        unlimited_origins);
 }
 
+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);
+  }
+
+  return exceptions;
+}
+
+void QuotaManager::DidGetEvictionOrigin(const GetOriginCallback& callback,
+                                        const GURL& 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 (ContainsKey(origins_in_use_, origin) ||
+      ContainsKey(access_notified_origins_, origin)) {
+    callback.Run(GURL());
+  } else {
+    callback.Run(origin);
+  }
+  access_notified_origins_.clear();
+
+  is_getting_eviction_origin_ = false;
+}
+
 void QuotaManager::GetEvictionOrigin(StorageType type,
                                      int64 global_quota,
                                      const GetOriginCallback& callback) {
@@ -1529,8 +1559,9 @@ void QuotaManager::GetEvictionOrigin(StorageType type,
     GetUsageTracker(kStorageTypeTemporary)->GetCachedOriginsUsage(&usage_map);
 
     temporary_storage_eviction_policy_->GetEvictionOrigin(
-        special_storage_policy_, usage_map, global_quota,
-        did_get_origin_callback);
+        special_storage_policy_, GetEvictionOriginExceptions(), usage_map,
+        global_quota, did_get_origin_callback);
+
     return;
   }
 
@@ -1538,13 +1569,6 @@ void QuotaManager::GetEvictionOrigin(StorageType type,
   GetLRUOrigin(type, did_get_origin_callback);
 }
 
-void QuotaManager::DidGetEvictionOrigin(const GetOriginCallback& callback,
-                                        const GURL& origin) {
-  callback.Run(origin);
-
-  is_getting_eviction_origin_ = false;
-}
-
 void QuotaManager::EvictOriginData(const GURL& origin,
                                    StorageType type,
                                    const EvictOriginDataCallback& callback) {
@@ -1589,31 +1613,12 @@ void QuotaManager::GetLRUOrigin(StorageType type,
     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)));
 }
 
@@ -1671,14 +1676,8 @@ void QuotaManager::DidInitialize(int64* temporary_quota_override,
 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();
 }
 
diff --git a/storage/browser/quota/quota_manager.h b/storage/browser/quota/quota_manager.h
index 5780cbb..bacf80b 100644
--- a/storage/browser/quota/quota_manager.h
+++ b/storage/browser/quota/quota_manager.h
@@ -82,6 +82,7 @@ class STORAGE_EXPORT QuotaEvictionPolicy {
   // are no evictable origins.
   virtual void GetEvictionOrigin(
       const scoped_refptr<SpecialStoragePolicy>& special_storage_policy,
+      const std::set<GURL>& exceptions,
       const std::map<GURL, int64>& usage_map,
       int64 global_quota,
       const GetOriginCallback& callback) = 0;
@@ -393,6 +394,7 @@ class STORAGE_EXPORT QuotaManager
   void DidGetPersistentGlobalUsageForHistogram(int64 usage,
                                                int64 unlimited_usage);
 
+  std::set<GURL> GetEvictionOriginExceptions();
   void DidGetEvictionOrigin(const GetOriginCallback& callback,
                             const GURL& origin);