Refactor budget computation to be more tuneable.

chrome/browser/budget_service/budget_manager_browsertest.cc

Index: chrome/browser/budget_service/budget_manager_browsertest.cc
diff --git a/chrome/browser/budget_service/budget_manager_browsertest.cc b/chrome/browser/budget_service/budget_manager_browsertest.cc
index aef3cd4c4fd680ee19be200cca65ab3b373c4f34..06d36b0bcf15ac21ad57e4aa785d94080b168c10 100644
--- a/chrome/browser/budget_service/budget_manager_browsertest.cc
+++ b/chrome/browser/budget_service/budget_manager_browsertest.cc
@@ -94,20 +94,22 @@ class BudgetManagerBrowserTest : public InProcessBrowserTest {
 IN_PROC_BROWSER_TEST_F(BudgetManagerBrowserTest, BudgetInDocument) {
   std::string script_result;
 
-  // The page will have been loaded once, which gives a budget of 3.
+  LoadTestPage();  // Reload to build site engagement.
+
+  // The page will have been loaded twice, which gives a budget of 2.
   ASSERT_TRUE(RunScript("documentGetBudget()", &script_result));
-  ASSERT_EQ("ok - budget returned value of 3", script_result);
+  EXPECT_EQ("ok - budget returned value of 2", script_result);
 
   ASSERT_TRUE(RunScript("documentReserveBudget()", &script_result));
-  ASSERT_EQ("ok - reserved budget", script_result);
+  EXPECT_EQ("ok - reserved budget", script_result);
 
-  // After reserving budget, the new budget should be at 1.
+  // After reserving budget, the new budget should be at 0.
   ASSERT_TRUE(RunScript("documentGetBudget()", &script_result));
-  ASSERT_EQ("ok - budget returned value of 1", script_result);
+  EXPECT_EQ("ok - budget returned value of 0", script_result);
 
   // A second reserve should fail because there is not enough budget.
   ASSERT_TRUE(RunScript("documentReserveBudget()", &script_result));
-  ASSERT_EQ("failed - not able to reserve budget", script_result);
+  EXPECT_EQ("failed - not able to reserve budget", script_result);
 
   // Consume should succeed because there is an existing reservation.
   ConsumeReservation();
@@ -122,32 +124,35 @@ IN_PROC_BROWSER_TEST_F(BudgetManagerBrowserTest, BudgetInWorker) {
   std::string script_result;
 
   ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
-  ASSERT_EQ("ok - service worker registered", script_result);
+  EXPECT_EQ("ok - service worker registered", script_result);
 
   LoadTestPage();  // Reload to become controlled.
+  LoadTestPage();  // Reload to build site engagement.
+  LoadTestPage();  // Reload to build site engagement.
+  LoadTestPage();  // Reload to build site engagement.
+  LoadTestPage();  // Reload to build site engagement.

[Peter Beverloo, 2017/01/12 14:40:04]

This will likely contribute to making this test flaky. Can we fake this somehow
without reloading the page?

[harkness, 2017/01/12 17:44:15]

I don't know of a way to do it. Browser tests are supposed to test the way a
test is used by a developer and there's no tool we have to bump up the site
engagement other than normal use. Is there a way to inject user input or playing
media into the test?

[Peter Beverloo, 2017/01/12 17:49:41]

Probably, but that will also need a fair bit of plumbing of its own. I was
hoping that we might be able to use SiteEngagementScore::SiteEngagementService()
here.

[harkness, 2017/01/13 11:54:45]

Done!

 
   ASSERT_TRUE(RunScript("isControlled()", &script_result));
-  ASSERT_EQ("true - is controlled", script_result);
+  EXPECT_EQ("true - is controlled", script_result);

[Peter Beverloo, 2017/01/12 14:40:04]

This should be an ASSERT.

We use ASSERT_* when it's a requirement. If isControlled() says that the page is
*not* controlled, then workerGetBudget() won't be able to do anything. That
means it makes no sense to continue.

[harkness, 2017/01/12 17:44:15]

Done.

 
-  // The page will have been loaded twice and a service worker was registered,
-  // which gives a budget of 4.5.
+  // The page will have been loaded five times, giving a budget of 5.
   ASSERT_TRUE(RunScript("workerGetBudget()", &script_result));
-  ASSERT_EQ("ok - budget returned value of 4.5", script_result);
+  EXPECT_EQ("ok - budget returned value of 5", script_result);
 
-  // With a budget of 4.5, two reservations should succeed.
+  // With a budget of 5, two reservations should succeed.
   ASSERT_TRUE(RunScript("workerReserveBudget()", &script_result));
-  ASSERT_EQ("ok - reserved budget", script_result);
+  EXPECT_EQ("ok - reserved budget", script_result);
 
   ASSERT_TRUE(RunScript("workerReserveBudget()", &script_result));
-  ASSERT_EQ("ok - reserved budget", script_result);
+  EXPECT_EQ("ok - reserved budget", script_result);
 
-  // After reserving budget, the new budget should be at 0.5.
+  // After reserving budget, the new budget should be at 1.
   ASSERT_TRUE(RunScript("workerGetBudget()", &script_result));
-  ASSERT_EQ("ok - budget returned value of 0.5", script_result);
+  EXPECT_EQ("ok - budget returned value of 1", script_result);
 
   // A second reserve should fail because there is not enough budget.
   ASSERT_TRUE(RunScript("workerReserveBudget()", &script_result));
-  ASSERT_EQ("failed - not able to reserve budget", script_result);
+  EXPECT_EQ("failed - not able to reserve budget", script_result);
 
   // Two consumes should succeed because there are existing reservations.
   ConsumeReservation();