Expand the functionality of the 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 "base/callback_forward.h"
+#include "base/containers/hash_tables.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "components/leveldb_proto/proto_database.h"
 
 namespace base {
 class SequencedTaskRunner;
 }
 
 namespace budget_service {
 class Budget;
 }
 
 class GURL;
 
 // A class used to asynchronously read and write details of the budget
 // assigned to an origin. The class uses an underlying LevelDB.
 class BudgetDatabase {
  public:
+  // Data structure for caching budget information.
+  typedef std::list<std::pair<double, double>> BudgetChunks;

[Peter Beverloo, 2016/07/22 16:14:52]

std::vector? I don't think any of the list-over-vector benefits applies.

[harkness, 2016/07/26 16:42:07]

It doesn't really matter for my purposes. Access will only be sequential, but I
can change it.

[Peter Beverloo, 2016/07/26 17:26:20]

std::vector is more popular in Chromium code by an order of magnitude, so let's
prefer consistency?

[harkness, 2016/07/27 10:59:02]

I changed BudgetChunks to be a vector, but because we're adding entries onto the
front of BudgetExpectation, that needed to stay a list.

+  typedef std::pair<double, BudgetChunks> BudgetInfo;

[Peter Beverloo, 2016/07/22 16:14:52]

It looks like both BudgetChunks and BudgetInfo can be private?

[harkness, 2016/07/26 16:42:07]

Ah yes, I'd planned to talk to you about this. You mentioned in a previous
review that all typedefs needed to be at the top of the class, which makes
sense, but I thought code style was also to try to have a single public block
and a single private block. Here I went ahead and added an extra block - is that
what I should be using for chromium style? 

[Peter Beverloo, 2016/07/26 17:26:20]

No, there are at most three blocks, one of each {public, protected, private}, in
that order. Within these blocks, however, declarations still have a common
declaration order (e.g. type defs on top).

These typedefs can simply be the first things in the private block below.

[harkness, 2016/07/27 10:59:02]

Done.

+
+  // Data structure for returing the budget decay expectations to the caller.
+  typedef std::list<std::pair<double, double>> BudgetExpectation;

[Peter Beverloo, 2016/07/22 16:14:52]

All three of these should be using `using`, as you do on lines 39, 44, 47 and
50.

[harkness, 2016/07/26 16:42:08]

Done.

+
   // Callback for the basic GetBudget call.
   using GetValueCallback =
       base::Callback<void(bool success,
                           std::unique_ptr<budget_service::Budget>)>;
 
   // Callback for setting a budget value.
   using SetValueCallback = base::Callback<void(bool success)>;
 
+  // Callback for getting a list of all budget chunks.
+  using GetBudgetDetailsCallback = base::Callback<
+      void(bool success, double debt, const BudgetExpectation& expectation)>;
+
+  using AddToCacheCallback = base::Callback<void(bool success)>;

[Peter Beverloo, 2016/07/22 16:14:52]

private

[harkness, 2016/07/26 16:42:07]

Done.

+
   // 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();
 
   void GetValue(const GURL& origin, const GetValueCallback& callback);
   void SetValue(const GURL& origin,
                 const budget_service::Budget& budget,
                 const SetValueCallback& callback);
 
+  // Get the full budget expectation for the origin. This will return any
+  // debt as well as a sequence of time points and the expected budget at
+  // those times.
+  void GetBudgetDetails(const GURL& origin,
+                        const GetBudgetDetailsCallback& callback);
+
  private:
   void OnDatabaseInit(bool success);
 
+  void AddToCache(const GURL& origin,
+                  const AddToCacheCallback& callback,
+                  bool success,
+                  std::unique_ptr<budget_service::Budget> budget);
+
+  void ReturnBudgetDetails(const GURL& origin,
+                           const GetBudgetDetailsCallback& callback,
+                           bool success);
+
   // 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.
+  base::hash_map<std::string, BudgetInfo> budget_map_;

[Peter Beverloo, 2016/07/22 16:14:52]

base::hash_map<> is deprecated, please use std::unordered_map<> instead.

[harkness, 2016/07/26 16:42:08]

Done.

+
   base::WeakPtrFactory<BudgetDatabase> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BudgetDatabase);
 };
 
 #endif  // CHROME_BROWSER_BUDGET_SERVICE_BUDGET_DATABASE_H_