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

chrome/browser/push_messaging/background_budget_service_unittest.cc

 // Copyright 2016 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 <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"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 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
 
 class BackgroundBudgetServiceTest : public testing::Test {
  public:
   BackgroundBudgetServiceTest() {}
   ~BackgroundBudgetServiceTest() override {}
 
   BackgroundBudgetService* service() {
     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_EQ(std::string(), budget_string);
+  EXPECT_DOUBLE_EQ(budget, 0.0);
 }
 
-TEST_F(BackgroundBudgetServiceTest, GetBudgetSuccess) {
+TEST_F(BackgroundBudgetServiceTest, GetBudgetNoBudgetSESExists) {
   const GURL origin(kTestOrigin);
 
-  service()->StoreBudget(origin, kTestData);
+  // Set a starting SES for the url but no stored budget info.
+  SetSiteEngagementScore(origin, kTestSES);
 
-  std::string budget_string = service()->GetBudget(origin);
+  double budget = service()->GetBudget(origin);
 
-  EXPECT_EQ(kTestData, budget_string);
+  EXPECT_DOUBLE_EQ(budget, kTestSES);
 }
+
+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);
+
+  double budget = service->GetBudget(origin);
+  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);

[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);
+    }
+  }
+
+  // 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);
+}