Make BackgroundBudgetService calls asynchronous.

chrome/browser/budget_service/background_budget_service.cc

Index: chrome/browser/budget_service/background_budget_service.cc
diff --git a/chrome/browser/budget_service/background_budget_service.cc b/chrome/browser/budget_service/background_budget_service.cc
index ec36dfd5e362bc6ffb1792805d858845f68f3368..cf0e77eaafa197020a87c8d4a70dade9a6c34a26 100644
--- a/chrome/browser/budget_service/background_budget_service.cc
+++ b/chrome/browser/budget_service/background_budget_service.cc
@@ -22,6 +22,9 @@
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
 
 namespace {
 
@@ -109,7 +112,8 @@ double BackgroundBudgetService::GetCost(CostType type) {
   return SiteEngagementScore::kMaxPoints + 1.0;
 }
 
-double BackgroundBudgetService::GetBudget(const GURL& origin) {
+void BackgroundBudgetService::GetBudget(const GURL& origin,
+                                        const GetBudgetCallback& callback) {
   DCHECK_EQ(origin, origin.GetOrigin());
 
   // Get the current SES score, which we'll use to set a new budget.
@@ -123,7 +127,8 @@ double BackgroundBudgetService::GetBudget(const GURL& origin) {
                               &last_updated_msec)) {
     // If there is no stored data or the data can't be parsed, just return the

[Michael van Ouwerkerk, 2016/06/10 12:10:19]

So this currently does not return, it just keeps running the rest of the method.
You might be able to update your test to detect this kind of bug, where the
callback would run more than once. Though... not sure how to wait for such a
condition that should not happen.

[harkness, 2016/06/10 14:52:56]

Ack. The good news is that it did get caught by the browser test. I missed this
when I was pulling out the extra async layer I had added that I decided to defer
for the level DB patch. I've updated it now.

     // SES.
-    return ses_score;
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::Bind(callback, ses_score));
   }
 
   base::Time now = clock_->Now();
@@ -135,7 +140,8 @@ double BackgroundBudgetService::GetBudget(const GURL& origin) {
   // TODO(harkness): Consider what to do if the clock jumps forward by a
   // significant amount.
   if (elapsed.InMicroseconds() < 0)
-    return old_budget;
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,

[Michael van Ouwerkerk, 2016/06/10 12:10:19]

nit: use curly braces for this if statement as this now wraps to multiple lines

[harkness, 2016/06/10 14:52:56]

Done.

+                            base::Bind(callback, old_budget));
 
   // For each time period that elapses, calculate the carryover ratio as the
   // ratio of time remaining in our max period to the total period.
@@ -159,10 +165,13 @@ double BackgroundBudgetService::GetBudget(const GURL& origin) {
   double budget = budget_carryover + ses_component;
   DCHECK_GE(budget, 0.0);
 
-  return budget;
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(callback, budget));
 }
 
-void BackgroundBudgetService::StoreBudget(const GURL& origin, double budget) {
+void BackgroundBudgetService::StoreBudget(const GURL& origin,
+                                          double budget,
+                                          const base::Closure& closure) {
   DCHECK_EQ(origin, origin.GetOrigin());
   DCHECK_GE(budget, 0.0);
   DCHECK_LE(budget, SiteEngagementService::GetMaxPoints());
@@ -173,6 +182,8 @@ void BackgroundBudgetService::StoreBudget(const GURL& origin, double budget) {
 
   base::Time time = clock_->Now();
   SetBudgetDataInPrefs(profile_, origin, time.ToDoubleT(), budget, ses_score);
+
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(closure));
 }
 
 // Override the default clock with the specified clock. Only used for testing.