Integrate SiteEngagementEvictionPolicy with QuotaManager.

chrome/browser/engagement/site_engagement_eviction_policy.cc

 // Copyright 2015 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 "base/barrier_closure.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/engagement/site_engagement_eviction_policy.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #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 {
 
 const int kExpectedEngagementSites = 200;
 
 // Gets the quota that an origin deserves based on its site engagement.
 int64 GetSoftQuotaForOrigin(const GURL& origin,
                             int score,
                             int total_engagement_points,
                             int64 global_quota) {
@@ skipping 38 matching lines @@
                                        origin, score_provider->GetScore(origin),
                                        total_engagement_points, global_quota);
     if (overuse > max_overuse) {
       max_overuse = overuse;
       origin_to_evict = origin;
     }
   }
   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
 GURL SiteEngagementEvictionPolicy::CalculateEvictionOrigin(

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

We should probably actually name this CalculateEvictionOriginForTest

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

Done.

     const scoped_refptr<storage::SpecialStoragePolicy>& special_storage_policy,
     SiteEngagementScoreProvider* score_provider,
     const std::map<GURL, int64>& usage_map,
     int64 global_quota) {
   return DoCalculateEvictionOrigin(special_storage_policy, score_provider,
                                    usage_map, global_quota);
 }