Integrate SiteEngagementEvictionPolicy with QuotaManager.

chrome/browser/engagement/site_engagement_eviction_policy.cc

Index: chrome/browser/engagement/site_engagement_eviction_policy.cc
diff --git a/chrome/browser/engagement/site_engagement_eviction_policy.cc b/chrome/browser/engagement/site_engagement_eviction_policy.cc
index ba7a885fae5d8088978d2e9a18d8627894667c41..7a74197c36450d1bb827761f7aeb6b1288bee76e 100644
--- a/chrome/browser/engagement/site_engagement_eviction_policy.cc
+++ b/chrome/browser/engagement/site_engagement_eviction_policy.cc
@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "content/public/browser/browser_thread.h"
+#include "storage/browser/quota/usage_tracker.h"
 
 namespace {
 
@@ -67,43 +68,109 @@ GURL DoCalculateEvictionOrigin(
   return origin_to_evict;
 }
 
+SiteEngagementScoreProvider* GetSiteEngagementService(
+    content::BrowserContext* browser_context) {
+  Profile* profile = Profile::FromBrowserContext(browser_context);
+  return g_browser_process->profile_manager()->IsValidProfile(profile)
+             ? SiteEngagementService::Get(profile)
+             : nullptr;
+}
+
 GURL GetSiteEngagementEvictionOriginOnUIThread(
     const scoped_refptr<storage::SpecialStoragePolicy>& special_storage_policy,
-    content::BrowserContext* browser_context,
+    base::Callback<SiteEngagementScoreProvider*()> get_score_provider_callback,
     const std::map<GURL, int64>& usage_map,
     int64 global_quota) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  Profile* profile = Profile::FromBrowserContext(browser_context);
-  SiteEngagementService* service =
-      g_browser_process->profile_manager()->IsValidProfile(profile)
-          ? SiteEngagementService::Get(profile)
-          : nullptr;
-  if (!service)
+
+  SiteEngagementScoreProvider* score_provider =
+      get_score_provider_callback.Run();
+  if (!score_provider)
     return GURL();
 
-  return DoCalculateEvictionOrigin(special_storage_policy, service, usage_map,
-                                   global_quota);
+  return DoCalculateEvictionOrigin(special_storage_policy, score_provider,
+                                   usage_map, global_quota);
 }
 
 }  // namespace
 
+// static
+CR_DEFINE_STATIC_LOCAL(

[raymes, 2015/09/17 04:14:30]

I think this should only be called from within a function. You could have a
Get...() but I'm wondering if there is something better we can do to pass the
ScoreProvider though?

[calamity, 2015/09/17 06:06:46]

Cleaned this up as discussed.

+    base::Callback<SiteEngagementScoreProvider*(content::BrowserContext*)>,
+    score_provider_callback_,

[raymes, 2015/09/17 04:14:31]

I believe the trailing underscore style is only used for member variables.

[calamity, 2015/09/17 06:06:46]

Fixed.

+    ());
+
 SiteEngagementEvictionPolicy::SiteEngagementEvictionPolicy(
+    storage::StorageType type,
+    storage::QuotaManager* manager,
     content::BrowserContext* browser_context)
-    : browser_context_(browser_context) {}
+    : type_(type),
+      manager_(manager),
+      browser_context_(browser_context),
+      global_quota_(0),
+      remaining_tasks_(0),
+      weak_factory_(this) {}
 
 SiteEngagementEvictionPolicy::~SiteEngagementEvictionPolicy() {}
 
 void SiteEngagementEvictionPolicy::GetEvictionOrigin(
     const scoped_refptr<storage::SpecialStoragePolicy>& special_storage_policy,
-    const std::map<GURL, int64>& usage_map,
-    int64 global_quota,
     const storage::GetOriginCallback& callback) {

[raymes, 2015/09/17 04:14:30]

Sorry I can't recall which threads this stuff gets called on. I assume this gets
called on the IO thread?

[calamity, 2015/09/17 06:06:46]

Yep, added DCHECKs to document.

Tests now use ThreadBundle because that's what the cool kids are into.

+  DCHECK_EQ(0, remaining_tasks_);
+
+  remaining_tasks_ = 2;
+  eviction_origin_callback_ = callback;
+
+  // This will populate cached hosts and usage info.
+  manager_->GetUsageTracker(type_)->GetGlobalUsage(
+      base::Bind(&SiteEngagementEvictionPolicy::DidGetGlobalUsage,
+                 weak_factory_.GetWeakPtr(), type_));
+  manager_->GetTemporaryGlobalQuota(
+      base::Bind(&SiteEngagementEvictionPolicy::DidGetGlobalQuota,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void SiteEngagementEvictionPolicy::DidGetGlobalUsage(storage::StorageType type,
+                                                     int64,
+                                                     int64) {
+  storage::UsageTracker* tracker = manager_->GetUsageTracker(type);
+  DCHECK(tracker);
+  tracker->GetCachedOriginsUsage(&usage_map_);
+
+  if (--remaining_tasks_ == 0)
+    OnQuotaTasksCompleted();
+}
+
+void SiteEngagementEvictionPolicy::DidGetGlobalQuota(
+    storage::QuotaStatusCode status,
+    int64 quota) {
+  if (status == storage::kQuotaStatusOk)
+    global_quota_ = quota;
+
+  if (--remaining_tasks_ == 0)
+    OnQuotaTasksCompleted();
+}
+
+void SiteEngagementEvictionPolicy::OnQuotaTasksCompleted() {
+  base::Callback<SiteEngagementScoreProvider*()> score_provider_callback =
+      base::Bind(&GetSiteEngagementService, browser_context_);
+  if (!score_provider_callback_.is_null())
+    score_provider_callback =
+        base::Bind(score_provider_callback_, browser_context_);

[raymes, 2015/09/17 04:14:30]

nit: {} for multiline if

[calamity, 2015/09/17 06:06:46]

Removed.

+
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(&GetSiteEngagementEvictionOriginOnUIThread,
-                 special_storage_policy, browser_context_, usage_map,
-                 global_quota),
-      callback);
+                 special_storage_policy_, score_provider_callback,
+                 usage_map_, global_quota_),
+      eviction_origin_callback_);
+}
+
+// static
+void SiteEngagementEvictionPolicy::SetSiteEngagementScoreProviderCallback(

[raymes, 2015/09/17 04:14:31]

I can't see where this is called from, besides in tests? I might be missing
something.

[calamity, 2015/09/17 06:06:46]

Yeah, this is only for tests. Updated the name to reflect that.

+    base::Callback<SiteEngagementScoreProvider*(content::BrowserContext*)>
+        callback) {
+  score_provider_callback_ = callback;
 }
 
 // static