Refactor budget computation to be more tuneable.

chrome/browser/budget_service/budget_database.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 "chrome/browser/budget_service/budget_database.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "chrome/browser/budget_service/budget.pb.h"
 #include "chrome/browser/engagement/site_engagement_score.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/leveldb_proto/proto_database_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
 using content::BrowserThread;
 
 namespace {
 
 // UMA are logged for the database with this string as part of the name.
 // They will be LevelDB.*.BudgetManager. Changes here should be synchronized
 // with histograms.xml.
 const char kDatabaseUMAName[] = "BudgetManager";
 
 // The default amount of time during which a budget will be valid.
-// This is 4 days = 96 hours.
-constexpr double kBudgetDurationInHours = 96;
+constexpr int kBudgetDurationInDays = 4;
+
+// The amount of budget that a maximally engaged site should receive per hour.
+// This allows 6 silent push messages a day for a maximal site.

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

Can we make this comment slightly clearer, since it assumes knowledge that the
cost of a silent push is 2.0, which is defined in another file?

// The amount of budget that a maximally engaged site should receive per hour.
// This value allows for up to 6 silent push messages, which have a cost of 2,
// per day for a fully engaged site.

(Even better if these values could be defined in the same file.)

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

I updated the comment here. I'd like to keep the cost values in BudgetManager,
since the database shouldn't have to know how much various actions cost.

+constexpr double kMaximumHourlyBudget = 12.0 / 24.0;
 
 }  // namespace
 
 BudgetDatabase::BudgetInfo::BudgetInfo() {}
 
 BudgetDatabase::BudgetInfo::BudgetInfo(const BudgetInfo&& other)
     : last_engagement_award(other.last_engagement_award) {
   chunks = std::move(other.chunks);
 }
 
@@ skipping 115 matching lines @@
   // expiration times going forward.
   const BudgetChunks& chunks = budget_map_[origin].chunks;
   for (const auto& chunk : chunks) {
     blink::mojom::BudgetStatePtr prediction(blink::mojom::BudgetState::New());
     total -= chunk.amount;
     prediction->budget_at = total;
     prediction->time = chunk.expiration.ToDoubleT();
     predictions.push_back(std::move(prediction));
   }
 
-  DCHECK_EQ(0, total);
-
   callback.Run(blink::mojom::BudgetServiceErrorType::NONE,
                std::move(predictions));
 }
 
 void BudgetDatabase::SpendBudgetAfterSync(const url::Origin& origin,
                                           double amount,
                                           const SpendBudgetCallback& callback,
                                           bool success) {
   if (!success) {
     callback.Run(blink::mojom::BudgetServiceErrorType::DATABASE_ERROR,
@@ skipping 118 matching lines @@
   // Get the SES score and add engagement budget for the site.
   AddEngagementBudget(origin);
 
   if (needs_write)
     WriteCachedValuesToDatabase(origin, callback);
   else
     callback.Run(success);
 }
 
 void BudgetDatabase::AddEngagementBudget(const url::Origin& origin) {
-  // Get the current SES score, which we'll use to set a new budget.
+  // Calculate how much budget should be awarded. The award depends on the
+  // time elapsed since the last award and the SES score.
+  base::TimeDelta elapsed = base::TimeDelta::FromDays(kBudgetDurationInDays);

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

nit: I think the "By default..." comment would still be very useful to explain
why we use kBudgetDurationInDays for uncached origins.

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

Done.

+  if (IsCached(origin)) {
+    elapsed = clock_->Now() - budget_map_[origin].last_engagement_award;
+    // Don't give engagement awards for periods less than an hour.
+    if (elapsed.InHours() < 1)
+      return;
+    // Cap elapsed time to the budget duration.
+    if (elapsed.InDays() > kBudgetDurationInDays)
+      elapsed = base::TimeDelta::FromDays(kBudgetDurationInDays);
+  }
+
+  // Get the current SES score, and calculate the hourly budget for that score.
   SiteEngagementService* service = SiteEngagementService::Get(profile_);
-  double score = service->GetScore(origin.GetURL());
-
-  // By default we award the "full" award. Then that ratio is decreased if
-  // there have been other awards recently.
-  double ratio = 1.0;
-
-  // Calculate how much budget should be awarded. If there is no entry in the
-  // cache then we award a full amount.
-  if (IsCached(origin)) {
-    base::TimeDelta elapsed =
-        clock_->Now() - budget_map_[origin].last_engagement_award;
-    int elapsed_hours = elapsed.InHours();
-    // Don't give engagement awards for periods less than an hour.
-    if (elapsed_hours < 1)
-      return;
-    if (elapsed_hours < kBudgetDurationInHours)
-      ratio = elapsed_hours / kBudgetDurationInHours;
-  }
+  double hourly_budget = kMaximumHourlyBudget *
+                         service->GetScore(origin.GetURL()) /
+                         service->GetMaxPoints();
 
   // Update the last_engagement_award to the current time. If the origin wasn't
   // already in the map, this adds a new entry for it.
   budget_map_[origin].last_engagement_award = clock_->Now();
 
   // Add a new chunk of budget for the origin at the default expiration time.
   base::Time expiration =
-      clock_->Now() + base::TimeDelta::FromHours(kBudgetDurationInHours);
-  budget_map_[origin].chunks.emplace_back(ratio * score, expiration);
+      clock_->Now() + base::TimeDelta::FromDays(kBudgetDurationInDays);
+  budget_map_[origin].chunks.emplace_back(elapsed.InHours() * hourly_budget,
+                                          expiration);
 
   // Any time we award engagement budget, which is done at most once an hour
   // whenever any budget action is taken, record the budget.
   double budget = GetBudget(origin);
   UMA_HISTOGRAM_COUNTS_100("PushMessaging.BackgroundBudget", budget);
 }
 
 // Cleans up budget in the cache. Relies on the caller eventually writing the
 // cache back to the database.
 bool BudgetDatabase::CleanupExpiredBudget(const url::Origin& origin) {
   if (!IsCached(origin))
     return false;
 
   base::Time now = clock_->Now();
   BudgetChunks& chunks = budget_map_[origin].chunks;
   auto cleanup_iter = chunks.begin();
 
   // This relies on the list of chunks being in timestamp order.
   while (cleanup_iter != chunks.end() && cleanup_iter->expiration <= now)
     cleanup_iter = chunks.erase(cleanup_iter);
 
   // If the entire budget is empty now AND there have been no engagements
-  // in the last kBudgetDurationInHours hours, remove this from the cache.
+  // in the last kBudgetDurationInDays days, remove this from the cache.
   if (chunks.empty() &&
       budget_map_[origin].last_engagement_award <
-          clock_->Now() - base::TimeDelta::FromHours(kBudgetDurationInHours)) {
+          clock_->Now() - base::TimeDelta::FromDays(kBudgetDurationInDays)) {
     budget_map_.erase(origin);
     return true;
   }
 
   // Although some things may have expired, there are some chunks still valid.
   // Don't write to the DB now, write either when all chunks expire or when the
   // origin spends some budget.
   return false;
 }