Make BackgroundBudgetService calls asynchronous.

chrome/browser/budget_service/background_budget_service_unittest.cc

Index: chrome/browser/budget_service/background_budget_service_unittest.cc
diff --git a/chrome/browser/budget_service/background_budget_service_unittest.cc b/chrome/browser/budget_service/background_budget_service_unittest.cc
index 98ee3dffd1c31561276982627bb305b26d3d05aa..7d88d6c166dd047875f2efa697d90568c268641e 100644
--- a/chrome/browser/budget_service/background_budget_service_unittest.cc
+++ b/chrome/browser/budget_service/background_budget_service_unittest.cc
@@ -6,6 +6,7 @@
 #include <string>
 
 #include "base/memory/ptr_util.h"
+#include "base/run_loop.h"
 #include "base/test/simple_test_clock.h"
 #include "chrome/browser/budget_service/background_budget_service.h"
 #include "chrome/browser/budget_service/background_budget_service_factory.h"
@@ -27,11 +28,13 @@ const double kMaxSES = 100.0;
 // Mirrors definition in BackgroundBudgetService, this is 10 days of seconds.
 const double kSecondsToAccumulate = 864000.0;
 
+using base::RunLoop;

[Michael van Ouwerkerk, 2016/06/10 15:04:05]

Peter would probably ask you not to do this with things in the base namespace.
Personally, I'd just note that there's lots of using statements for things in
base, but not for RunLoop, which has a short name, and I don't think it would
cause any wrapping in this file.

[harkness, 2016/06/10 15:19:56]

Done.

+
 }  // namespace
 
 class BackgroundBudgetServiceTest : public testing::Test {
  public:
-  BackgroundBudgetServiceTest() {}
+  BackgroundBudgetServiceTest() : budget_(0.0) {}
   ~BackgroundBudgetServiceTest() override {}
 
   BackgroundBudgetService* GetService() {
@@ -52,6 +55,16 @@ class BackgroundBudgetServiceTest : public testing::Test {
     return clock;
   }
 
+  void GotBudget(base::Closure run_loop_closure, double budget) {
+    budget_ = budget;
+    run_loop_closure.Run();
+  }
+
+  void StoredBudget(base::Closure run_loop_closure) { run_loop_closure.Run(); }
+
+  // Budget for callbacks to set.
+  double budget_;
+
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   TestingProfile profile_;
@@ -60,9 +73,13 @@ class BackgroundBudgetServiceTest : public testing::Test {
 TEST_F(BackgroundBudgetServiceTest, GetBudgetNoBudgetOrSES) {
   const GURL origin(kTestOrigin);
 
-  double budget = GetService()->GetBudget(origin);
+  std::unique_ptr<RunLoop> run_loop(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
 
-  EXPECT_DOUBLE_EQ(budget, 0.0);
+  EXPECT_DOUBLE_EQ(budget_, 0.0);
 }
 
 TEST_F(BackgroundBudgetServiceTest, GetBudgetNoBudgetSESExists) {
@@ -71,9 +88,13 @@ TEST_F(BackgroundBudgetServiceTest, GetBudgetNoBudgetSESExists) {
   // Set a starting SES for the url but no stored budget info.
   SetSiteEngagementScore(origin, kTestSES);
 
-  double budget = GetService()->GetBudget(origin);
+  std::unique_ptr<RunLoop> run_loop(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
 
-  EXPECT_DOUBLE_EQ(budget, kTestSES);
+  EXPECT_DOUBLE_EQ(budget_, kTestSES);
 }
 
 TEST_F(BackgroundBudgetServiceTest, GetBudgetNoElapsedTime) {
@@ -82,11 +103,20 @@ TEST_F(BackgroundBudgetServiceTest, GetBudgetNoElapsedTime) {
   std::unique_ptr<BackgroundBudgetService> service(
       new BackgroundBudgetService(profile()));
 
-  service->StoreBudget(origin, kTestBudget);
+  std::unique_ptr<RunLoop> run_loop(new RunLoop());
+  service->StoreBudget(
+      origin, kTestBudget,
+      base::Bind(&BackgroundBudgetServiceTest::StoredBudget,
+                 base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
 
-  double budget = service->GetBudget(origin);
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
 
-  EXPECT_NEAR(budget, kTestBudget, kTestBudget);
+  EXPECT_NEAR(budget_, kTestBudget, kTestBudget);
 }
 
 TEST_F(BackgroundBudgetServiceTest, GetBudgetElapsedTime) {
@@ -99,69 +129,117 @@ TEST_F(BackgroundBudgetServiceTest, GetBudgetElapsedTime) {
   // Set initial SES and budget values.
   const GURL origin(kTestOrigin);
   SetSiteEngagementScore(origin, kTestSES);
-  service->StoreBudget(origin, kTestBudget);
-
-  double budget = service->GetBudget(origin);
-  EXPECT_DOUBLE_EQ(budget, kTestBudget);
+  std::unique_ptr<RunLoop> run_loop(new RunLoop());
+  service->StoreBudget(
+      origin, kTestBudget,
+      base::Bind(&BackgroundBudgetServiceTest::StoredBudget,
+                 base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_DOUBLE_EQ(budget_, kTestBudget);
 
   // Query for the budget after 1 second has passed.
   clock->SetNow(starting_time + base::TimeDelta::FromSeconds(1));
-  budget = service->GetBudget(origin);
-  EXPECT_NEAR(budget, kTestBudget, kTestSES * 1.0 / kSecondsToAccumulate);
-  EXPECT_GT(budget, kTestBudget);
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_NEAR(budget_, kTestBudget, kTestSES * 1.0 / kSecondsToAccumulate);
+  EXPECT_GT(budget_, kTestBudget);
 
   // Query for the budget after 1 hour has passed.
   clock->SetNow(starting_time + base::TimeDelta::FromHours(1));
-  budget = service->GetBudget(origin);
-  EXPECT_NEAR(budget, kTestBudget, kTestSES * 3600.0 / kSecondsToAccumulate);
-  EXPECT_GT(budget, kTestBudget);
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_NEAR(budget_, kTestBudget, kTestSES * 3600.0 / kSecondsToAccumulate);
+  EXPECT_GT(budget_, kTestBudget);
 
   // Query for the budget after 5 days have passed. The budget should be
   // increasing, but not up the SES score.
   clock->SetNow(starting_time + base::TimeDelta::FromDays(5));
-  budget = service->GetBudget(origin);
-  EXPECT_GT(budget, kTestBudget);
-  EXPECT_LT(budget, kTestSES);
-  double moderate_ses_budget = budget;
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_GT(budget_, kTestBudget);
+  EXPECT_LT(budget_, kTestSES);
+  double moderate_ses_budget = budget_;
 
   // Query for the budget after 10 days have passed. By this point, the budget
   // should converge to the SES score.
   clock->SetNow(starting_time + base::TimeDelta::FromDays(10));
-  budget = service->GetBudget(origin);
-  EXPECT_DOUBLE_EQ(budget, kTestSES);
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_DOUBLE_EQ(budget_, kTestSES);
 
   // Now, change the SES score to the maximum amount and reinitialize budget.
   SetSiteEngagementScore(origin, kMaxSES);
-  service->StoreBudget(origin, kTestBudget);
+  run_loop.reset(new RunLoop());
+  service->StoreBudget(
+      origin, kTestBudget,
+      base::Bind(&BackgroundBudgetServiceTest::StoredBudget,
+                 base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+
   starting_time = clock->Now();
 
   // Query for the budget after 1 second has passed.
   clock->SetNow(starting_time + base::TimeDelta::FromSeconds(1));
-  budget = service->GetBudget(origin);
-  EXPECT_NEAR(budget, kTestBudget, kMaxSES * 1.0 / kSecondsToAccumulate);
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_NEAR(budget_, kTestBudget, kMaxSES * 1.0 / kSecondsToAccumulate);
 
   // Query for the budget after 5 days have passed. Again, the budget should be
   // approaching the SES, but not have reached it.
   clock->SetNow(starting_time + base::TimeDelta::FromDays(5));
-  budget = service->GetBudget(origin);
-  EXPECT_GT(budget, kTestBudget);
-  EXPECT_LT(budget, kMaxSES);
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_GT(budget_, kTestBudget);
+  EXPECT_LT(budget_, kMaxSES);
 
   // The budget after 5 days with max SES should be greater than the budget
   // after 5 days with moderate SES.
-  EXPECT_GT(budget, moderate_ses_budget);
+  EXPECT_GT(budget_, moderate_ses_budget);
 
   // Now, change the SES score to a low amount and reinitialize budget.
   SetSiteEngagementScore(origin, kLowSES);
-  service->StoreBudget(origin, kTestBudget);
+  run_loop.reset(new RunLoop());
+  service->StoreBudget(
+      origin, kTestBudget,
+      base::Bind(&BackgroundBudgetServiceTest::StoredBudget,
+                 base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
   starting_time = clock->Now();
 
   // Query for the budget after 5 days have passed. Again, the budget should be
   // approaching the SES, this time decreasing, but not have reached it.
   clock->SetNow(starting_time + base::TimeDelta::FromDays(5));
-  budget = service->GetBudget(origin);
-  EXPECT_LT(budget, kTestBudget);
-  EXPECT_GT(budget, kLowSES);
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_LT(budget_, kTestBudget);
+  EXPECT_GT(budget_, kLowSES);
 }
 
 TEST_F(BackgroundBudgetServiceTest, GetBudgetConsumedOverTime) {
@@ -173,28 +251,41 @@ TEST_F(BackgroundBudgetServiceTest, GetBudgetConsumedOverTime) {
   // Set initial SES and budget values.
   const GURL origin(kTestOrigin);
   SetSiteEngagementScore(origin, kTestSES);
-  service->StoreBudget(origin, kTestBudget);
-  double budget = 0.0;
+  std::unique_ptr<RunLoop> run_loop(new RunLoop());
+  service->StoreBudget(
+      origin, kTestBudget,
+      base::Bind(&BackgroundBudgetServiceTest::StoredBudget,
+                 base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
 
   // Measure over 200 hours. In each hour a message is received, and for 1 in
   // 10, budget is consumed.
   for (int i = 0; i < 200; i++) {
     // Query for the budget after 1 hour has passed.
     clock->Advance(base::TimeDelta::FromHours(1));
-    budget = service->GetBudget(origin);
+    run_loop.reset(new RunLoop());
+    GetService()->GetBudget(
+        origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                           base::Unretained(this), run_loop->QuitClosure()));
+    run_loop->Run();
 
     if (i % 10 == 0) {
       double cost = BackgroundBudgetService::GetCost(
           BackgroundBudgetService::CostType::SILENT_PUSH);
-      service->StoreBudget(origin, budget - cost);
+      run_loop.reset(new RunLoop());
+      service->StoreBudget(
+          origin, budget_ - cost,
+          base::Bind(&BackgroundBudgetServiceTest::StoredBudget,
+                     base::Unretained(this), run_loop->QuitClosure()));
+      run_loop->Run();
     }
   }
 
   // With a SES of 48.0, the origin will get a budget of 2.4 per day, but the
   // old budget will also decay. At the end, we expect the budget to be lower
   // than the starting budget.
-  EXPECT_GT(budget, 0.0);
-  EXPECT_LT(budget, kTestBudget);
+  EXPECT_GT(budget_, 0.0);
+  EXPECT_LT(budget_, kTestBudget);
 }
 
 TEST_F(BackgroundBudgetServiceTest, GetBudgetInvalidBudget) {
@@ -210,9 +301,13 @@ TEST_F(BackgroundBudgetServiceTest, GetBudgetInvalidBudget) {
   update_map->SetStringWithoutPathExpansion(origin.spec(), "20#2.0");
 
   // Get the budget, expect that it will return SES.
-  double budget = GetService()->GetBudget(origin);
+  std::unique_ptr<RunLoop> run_loop(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
 
-  EXPECT_DOUBLE_EQ(budget, kTestSES);
+  EXPECT_DOUBLE_EQ(budget_, kTestSES);
 }
 
 TEST_F(BackgroundBudgetServiceTest, GetBudgetNegativeTime) {
@@ -225,17 +320,38 @@ TEST_F(BackgroundBudgetServiceTest, GetBudgetNegativeTime) {
   // Set initial SES and budget values.
   const GURL origin(kTestOrigin);
   SetSiteEngagementScore(origin, kTestSES);
-  service->StoreBudget(origin, kTestBudget);
+  std::unique_ptr<RunLoop> run_loop(new RunLoop());
+  service->StoreBudget(
+      origin, kTestBudget,
+      base::Bind(&BackgroundBudgetServiceTest::StoredBudget,
+                 base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
 
   // Move time forward an hour and get the budget.
   clock->SetNow(starting_time + base::TimeDelta::FromHours(1));
-  double budget = service->GetBudget(origin);
-  service->StoreBudget(origin, budget);
-  EXPECT_NE(kTestBudget, budget);
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+
+  // Store the updated budget.
+  run_loop.reset(new RunLoop());
+  service->StoreBudget(
+      origin, budget_,
+      base::Bind(&BackgroundBudgetServiceTest::StoredBudget,
+                 base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_NE(kTestBudget, budget_);
+  double original_budget = budget_;
 
   // Now move time backwards a day and make sure that the current
   // budget matches the budget of the most foward time.
   clock->SetNow(starting_time - base::TimeDelta::FromDays(1));
-  double back_budget = service->GetBudget(origin);
-  EXPECT_NEAR(budget, back_budget, 0.01);
+  run_loop.reset(new RunLoop());
+  GetService()->GetBudget(
+      origin, base::Bind(&BackgroundBudgetServiceTest::GotBudget,
+                         base::Unretained(this), run_loop->QuitClosure()));
+  run_loop->Run();
+  EXPECT_NEAR(original_budget, budget_, 0.01);
 }