AddEngagementBudget and SpendBudget added to BudgetDatabase.

chrome/browser/budget_service/budget_database.h

 // 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.
 
 #ifndef CHROME_BROWSER_BUDGET_SERVICE_BUDGET_DATABASE_H_
 #define CHROME_BROWSER_BUDGET_SERVICE_BUDGET_DATABASE_H_
 
 #include <list>
 #include <memory>
 #include <unordered_map>
@@ skipping 27 matching lines @@
     BudgetStatus(double budget_at, base::Time time)
         : budget_at(budget_at), time(time) {}
     BudgetStatus(const BudgetStatus& other)
         : budget_at(other.budget_at), time(other.time) {}
 
     double budget_at;
     base::Time time;
   };
 
   // Data structure for returing the budget decay expectations to the caller.
   using BudgetExpectation = std::list<BudgetStatus>;

[johnme, 2016/08/19 18:48:36]

Nit: Can we rename this to BudgetPrediction (in a separate patch if necessary)?
Expectation sounds like it's for testing only.

[harkness, 2016/08/22 13:46:26]

Done.

 
   // Callback for setting a budget value.
   using StoreBudgetCallback = base::Callback<void(bool success)>;
 
   // Callback for getting a list of all budget chunks.
   using GetBudgetDetailsCallback =
       base::Callback<void(bool success, const BudgetExpectation& expectation)>;
 
   // The database_dir specifies the location of the budget information on
   // disk. The task_runner is used by the ProtoDatabase to handle all blocking
   // calls and disk access.
   BudgetDatabase(const base::FilePath& database_dir,
                  const scoped_refptr<base::SequencedTaskRunner>& task_runner);
   ~BudgetDatabase();
 
   // Get the full budget expectation for the origin. This will return a
   // sequence of time points and the expected budget at those times.
   void GetBudgetDetails(const GURL& origin,
                         const GetBudgetDetailsCallback& callback);
 
   // Add budget for an origin. The caller specifies the amount, and the method
   // adds the amount to the cache with the correct expiration. Callback is
   // invoked only after the newly cached value is written to storage. This
   // should only be called after the budget has been read from the database.
   void AddBudget(const GURL& origin,

[johnme, 2016/08/19 18:48:36]

What's the use case for adding non-engagement budget?

[harkness, 2016/08/22 13:46:26]

There are a few of these. One would be a bootstrapping amount of budget for new
origins. Another would be refunding budget if a scheduled timer didn't use it.
In the short term, it gives a very useful way to test the infrastructure and
doesn't add much extra code.

                  double amount,
                  const StoreBudgetCallback& callback);
 
+  // Add budget based on engagement with an origin. The caller specifies the
+  // engagement score of the origin, and the method calculates when engagement
+  // budget was last awarded and awards a portion of the score based on that.
+  // Callback is invoked after the value is written to storage. This should
+  // only be called after the budget has been read from the database.
+  void AddEngagementBudget(const GURL& origin,
+                           double engagementScore,
+                           const StoreBudgetCallback& callback);
+
+  // Spend a particular amount of budget for an origin. The callback specifies

[johnme, 2016/08/19 18:48:36]

Nit: s/specifies/takes/

[harkness, 2016/08/22 13:46:26]

Done.

+  // a boolean which indicates whether the origin had enough budget to spend.
+  // If it returns success, then the budget was deducted and the result written
+  // to the database. This should only be called after the budget has been read.

[johnme, 2016/08/19 18:48:36]

It seems a slightly awkward constraint that "This should only be called after
the budget has been read". I'm not even sure what that means - do you have to
call GetBudgetDetails and wait for its callback? Could you avoid that using a
queue of pending tasks to perform asynchronously once loaded (in the style of
GCMDelayedTaskController)?

[harkness, 2016/08/22 13:46:26]

Peter and I had a long discussion about how much work we needed to do to make
this code safe, and came to the conclusion that as the only consumer was the
code in BudgetManager, we could rely on the caller being well behaved, rather
than building in a lot of structure to enforce that.

[johnme, 2016/08/22 14:50:31]

Doesn't that just mean you'll have to add equivalent complexity to
BudgetManager? If you keep it this way, please make the comments more explicit
anyway.

+  void SpendBudget(const GURL& origin,
+                   double amount,
+                   const StoreBudgetCallback& callback);
+
  private:
   friend class BudgetDatabaseTest;
 
   // Used to allow tests to change time for testing.
   void SetClockForTesting(std::unique_ptr<base::Clock> clock);
 
   // Holds information about individual pieces of awarded budget. There is a
   // one-to-one mapping of these to the chunks in the underlying database.
   struct BudgetChunk {
     BudgetChunk(double amount, base::Time expiration)
         : amount(amount), expiration(expiration) {}
     BudgetChunk(const BudgetChunk& other)
         : amount(other.amount), expiration(other.expiration) {}
 
     double amount;
     base::Time expiration;
   };
 
   // Data structure for caching budget information.
   using BudgetChunks = std::vector<BudgetChunk>;
 
+  // Holds information about the overall budget for a site. This includes the
+  // time the budget was last incremented, as well as a list of budget chunks
+  // which have been awarded.
+  struct BudgetInfo {
+    BudgetInfo();
+    BudgetInfo(const BudgetInfo& other);
+    ~BudgetInfo();
+
+    base::Time last_engagement_award;
+    BudgetChunks chunks;
+  };
+
   using AddToCacheCallback = base::Callback<void(bool success)>;
 
   void OnDatabaseInit(bool success);
 
   bool IsCached(const GURL& origin) const;
 
   void AddToCache(const GURL& origin,
                   const AddToCacheCallback& callback,
                   bool success,
                   std::unique_ptr<budget_service::Budget> budget);
 
   void DidGetBudget(const GURL& origin,
                     const GetBudgetDetailsCallback& callback,
                     bool success);
 
   void WriteCachedValuesToDatabase(const GURL& origin,
                                    const StoreBudgetCallback& callback);
 
   void CleanupExpiredBudget(const GURL& origin);
 
   // The database for storing budget information.
   std::unique_ptr<leveldb_proto::ProtoDatabase<budget_service::Budget>> db_;
 
   // Cached data for the origins which have been loaded.
-  std::unordered_map<std::string, BudgetChunks> budget_map_;
+  std::unordered_map<std::string, BudgetInfo> budget_map_;
 
   // The clock used to vend times.
   std::unique_ptr<base::Clock> clock_;
 
   base::WeakPtrFactory<BudgetDatabase> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BudgetDatabase);
 };
 
 #endif  // CHROME_BROWSER_BUDGET_SERVICE_BUDGET_DATABASE_H_