Replace the 1 in 10 grace period with an accumulating budget based on SES.

chrome/browser/push_messaging/background_budget_service_unittest.cc

Index: chrome/browser/push_messaging/background_budget_service_unittest.cc
diff --git a/chrome/browser/push_messaging/background_budget_service_unittest.cc b/chrome/browser/push_messaging/background_budget_service_unittest.cc
index 255dede4589ebc121c20e07507357c9b79027588..e396c950c49869186cc52b4c00c6222c458ec775 100644
--- a/chrome/browser/push_messaging/background_budget_service_unittest.cc
+++ b/chrome/browser/push_messaging/background_budget_service_unittest.cc
@@ -5,6 +5,9 @@
 #include <stdint.h>
 #include <string>
 
+#include "base/memory/ptr_util.h"
+#include "base/test/simple_test_clock.h"
+#include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/push_messaging/background_budget_service.h"
 #include "chrome/browser/push_messaging/background_budget_service_factory.h"
 #include "chrome/test/base/testing_profile.h"
@@ -14,7 +17,12 @@
 namespace {
 
 const char kTestOrigin[] = "https://example.com";
-const char kTestData[] = "1111101111";
+const double kTestBudget = 10.0;
+const double kTestSES = 24.0;
+const double kLowSES = 1.0;
+const double kMaxSES = 100.0;
+// Mirrors definition in BackgroundBudgetService, this is 10 days of seconds.
+const double kSecondsToAccumulate = 864000.0;
 
 }  // namespace
 
@@ -27,25 +35,155 @@ class BackgroundBudgetServiceTest : public testing::Test {
     return BackgroundBudgetServiceFactory::GetForProfile(&profile_);
   }
 
+  void SetSiteEngagementScore(const GURL& url, double score) {
+    SiteEngagementService* service = SiteEngagementService::Get(&profile_);
+    service->ResetScoreForURL(url, score);
+  }
+
+  Profile* profile() { return &profile_; }
+
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   TestingProfile profile_;
 };
 
-TEST_F(BackgroundBudgetServiceTest, GetBudgetFailure) {
+TEST_F(BackgroundBudgetServiceTest, GetBudgetNoBudgetOrSES) {
   const GURL origin(kTestOrigin);
 
-  std::string budget_string = service()->GetBudget(origin);
+  double budget = service()->GetBudget(origin);
+
+  EXPECT_DOUBLE_EQ(budget, 0.0);
+}
+
+TEST_F(BackgroundBudgetServiceTest, GetBudgetNoBudgetSESExists) {
+  const GURL origin(kTestOrigin);
+
+  // Set a starting SES for the url but no stored budget info.
+  SetSiteEngagementScore(origin, kTestSES);
+
+  double budget = service()->GetBudget(origin);
 
-  EXPECT_EQ(std::string(), budget_string);
+  EXPECT_DOUBLE_EQ(budget, kTestSES);
 }
 
-TEST_F(BackgroundBudgetServiceTest, GetBudgetSuccess) {
+TEST_F(BackgroundBudgetServiceTest, GetBudgetNoElapsedTime) {
+  const GURL origin(kTestOrigin);
+
+  // Manually construct a BackgroundBudgetServie with a clock that the test

[Peter Beverloo, 2016/05/04 15:41:52]

nit: s/BackgroundBudgetServie/BackgroundBudgetService/g (3 occurrences)

[harkness, 2016/05/09 15:33:27]

Done.

+  // can control so that we can fast forward in time.
+  base::SimpleTestClock* clock = new base::SimpleTestClock();
+  std::unique_ptr<BackgroundBudgetService> service(
+      new BackgroundBudgetService(profile(), base::WrapUnique(clock)));
+
+  service->StoreBudget(origin, kTestBudget);
+
+  double budget = service->GetBudget(origin);
+
+  EXPECT_NEAR(budget, kTestBudget, kTestBudget);
+}
+
+TEST_F(BackgroundBudgetServiceTest, GetBudgetElapsedTime) {
+  // Manually construct a BackgroundBudgetServie with a clock that the test
+  // can control so that we can fast forward in time.
+  base::SimpleTestClock* clock = new base::SimpleTestClock();
+  base::Time starting_time = clock->Now();
+  std::unique_ptr<BackgroundBudgetService> service(
+      new BackgroundBudgetService(profile(), base::WrapUnique(clock)));
+
+  // Set initial SES and budget values.
   const GURL origin(kTestOrigin);
+  SetSiteEngagementScore(origin, kTestSES);
+  service->StoreBudget(origin, kTestBudget);
 
-  service()->StoreBudget(origin, kTestData);
+  double budget = service->GetBudget(origin);
+  EXPECT_DOUBLE_EQ(budget, kTestBudget);
 
-  std::string budget_string = service()->GetBudget(origin);
+  // 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);

[Peter Beverloo, 2016/05/04 15:41:52]

Heh, you're using EXPECT_NEAR as sort of a EXPECT_INCREASE :)

While I'm not against this, one thing to keep in mind is that this would pass on
*decreases* as well. (Although the check on line 123 would then fail.)

[harkness, 2016/05/09 15:33:27]

I added extra GT checks to avoid that case.

+
+  // 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);
+
+  // 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;
+
+  // Query for the budget after 11 days have passed. By this point, the budget
+  // should converge to the SES score.
+  clock->SetNow(starting_time + base::TimeDelta::FromDays(11));

[Peter Beverloo, 2016/05/04 15:41:52]

Why 11 days instead of 10? (kSecondsToAccumulate)

[harkness, 2016/05/09 15:33:27]

Originally, I was using Advance() instead of SetNow() so I advanced 10 days,
which put me around 11. Now that I'm using SetNow(), I reduced the days down to
10, which should be enough to stabilize the budget.

+  budget = service->GetBudget(origin);
+  EXPECT_DOUBLE_EQ(budget, kTestSES);
+
+  // Now, change the SES score to the maximum amount and reinitialize budget.
+  SetSiteEngagementScore(origin, kMaxSES);
+  service->StoreBudget(origin, kTestBudget);
+  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);
+
+  // 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);
+
+  // 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);
+
+  // Now, change the SES score to a low amount and reinitialize budget.
+  SetSiteEngagementScore(origin, kLowSES);
+  service->StoreBudget(origin, kTestBudget);
+  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);
+}
+
+TEST_F(BackgroundBudgetServiceTest, GetBudgetConsumedOverTime) {
+  // Manually construct a BackgroundBudgetServie with a clock that the test
+  // can control so that we can fast forward in time.
+  base::SimpleTestClock* clock = new base::SimpleTestClock();
+  std::unique_ptr<BackgroundBudgetService> service(
+      new BackgroundBudgetService(profile(), base::WrapUnique(clock)));
+
+  // Set initial SES and budget values.
+  const GURL origin(kTestOrigin);
+  SetSiteEngagementScore(origin, kTestSES);
+  service->StoreBudget(origin, kTestBudget);
+  double budget = 0.0;
+
+  // 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);
+
+    if (i % 10 == 0) {
+      service->StoreBudget(origin, budget - 1.0);
+    }
+  }
 
-  EXPECT_EQ(kTestData, budget_string);
+  // With a SES of 24.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.

[Peter Beverloo, 2016/05/04 15:41:52]

We should definitely measure the site's budget for *every* incoming message. Do
you think we can still get that done in time for M52? (~May 20)

[harkness, 2016/05/09 15:33:27]

Do you want to measure for every message, or only for when the budget is
consumed? 

Right now, the algorithm has the very nice property that we only query SES and
calculate the budget if a notification was needed but not shown. That's one of
the changes that let me clean up PushMessagingNotificationManager so much.

As for the UMA, I've got a branch I'm working on that now, I think we should be
able to get it for 52.

[Peter Beverloo, 2016/05/09 17:12:06]

Since it's all synchronous, I think we can just query the budget whenever we
want without introducing new methods or thread jumps, minimizing overhead?

+  EXPECT_GT(budget, 0.0);
+  EXPECT_LT(budget, kTestBudget);
 }