Quota: Remove storage::{CallbackQueue, CallbackQueueMap} in UsageTracker.

storage::CallbackQueue and storage::CallbackQueueMap are non-trivial to
understand, as they use fairly involved template machinery. Most
importantly, their Run() methods appear to perfectly forward their
arguments, but actually end up copying them, in order to be able to pass
the same arguments to multiple callbacks.

This CL removes the custom types in favor of std::vector and std::map.
In return for slightly more lines of code (but not binary size), the
code is significantly easier to follow.

Change-Id: I563675c4aa6c4e771cd4979712996d878f9a8aa0
Reviewed-on: https://chromium-review.googlesource.com/c/1295593
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601918}

storage/browser/quota/usage_tracker.cc

@@ -56,13 +56,14 @@ ClientUsageTracker* UsageTracker::GetClientTracker(QuotaClient::ID client_id) {
 }
 
 void UsageTracker::GetGlobalLimitedUsage(UsageCallback callback) {
-  if (global_usage_callbacks_.HasCallbacks()) {
-    global_usage_callbacks_.Add(base::BindOnce(
+  if (!global_usage_callbacks_.empty()) {
+    global_usage_callbacks_.emplace_back(base::BindOnce(
         &DidGetGlobalUsageForLimitedGlobalUsage, std::move(callback)));
     return;
   }
 
-  if (!global_limited_usage_callbacks_.Add(std::move(callback)))
+  global_limited_usage_callbacks_.emplace_back(std::move(callback));
+  if (global_limited_usage_callbacks_.size() > 1)
     return;
 
   AccumulateInfo* info = new AccumulateInfo;
@@ -86,7 +87,8 @@ void UsageTracker::GetGlobalLimitedUsage(UsageCallback callback) {
 }
 
 void UsageTracker::GetGlobalUsage(GlobalUsageCallback callback) {
-  if (!global_usage_callbacks_.Add(std::move(callback)))
+  global_usage_callbacks_.emplace_back(std::move(callback));
+  if (global_usage_callbacks_.size() > 1)
     return;
 
   AccumulateInfo* info = new AccumulateInfo;
@@ -119,7 +121,10 @@ void UsageTracker::GetHostUsage(const std::string& host,
 void UsageTracker::GetHostUsageWithBreakdown(
     const std::string& host,
     UsageWithBreakdownCallback callback) {
-  if (!host_usage_callbacks_.Add(host, std::move(callback)))
+  std::vector<UsageWithBreakdownCallback>& host_callbacks =
+      host_usage_callbacks_[host];
+  host_callbacks.emplace_back(std::move(callback));
+  if (host_callbacks.size() > 1)
     return;
 
   AccumulateInfo* info = new AccumulateInfo;
@@ -194,9 +199,12 @@ void UsageTracker::AccumulateClientGlobalLimitedUsage(AccumulateInfo* info,
   if (--info->pending_clients)
     return;
 
-  // All the clients have returned their usage data.  Dispatch the
-  // pending callbacks.
-  global_limited_usage_callbacks_.Run(info->usage);
+  // Moving callbacks out of the original vector handles the case where a
+  // callback makes a new quota call.
+  std::vector<UsageCallback> pending_callbacks;
+  pending_callbacks.swap(global_limited_usage_callbacks_);
+  for (auto& callback : pending_callbacks)
+    std::move(callback).Run(info->usage);
 }
 
 void UsageTracker::AccumulateClientGlobalUsage(AccumulateInfo* info,
@@ -218,16 +226,20 @@ void UsageTracker::AccumulateClientGlobalUsage(AccumulateInfo* info,
   else if (info->unlimited_usage < 0)
     info->unlimited_usage = 0;
 
-  // All the clients have returned their usage data.  Dispatch the
-  // pending callbacks.
-  global_usage_callbacks_.Run(info->usage, info->unlimited_usage);
+  // Moving callbacks out of the original vector early handles the case where a
+  // callback makes a new quota call.
+  std::vector<GlobalUsageCallback> pending_callbacks;
+  pending_callbacks.swap(global_usage_callbacks_);
+  for (auto& callback : pending_callbacks)
+    std::move(callback).Run(info->usage, info->unlimited_usage);
 }
 
-void UsageTracker::AccumulateClientHostUsage(const base::Closure& barrier,
-                                             AccumulateInfo* info,
-                                             const std::string& host,
-                                             QuotaClient::ID client,
-                                             int64_t usage) {
+void UsageTracker::AccumulateClientHostUsage(
+    const base::RepeatingClosure& barrier,
+    AccumulateInfo* info,
+    const std::string& host,
+    QuotaClient::ID client,
+    int64_t usage) {
   info->usage += usage;
   // Defend against confusing inputs from clients.
   if (info->usage < 0)
@@ -239,8 +251,18 @@ void UsageTracker::AccumulateClientHostUsage(const base::Closure& barrier,
 
 void UsageTracker::FinallySendHostUsageWithBreakdown(AccumulateInfo* info,
                                                      const std::string& host) {
-  host_usage_callbacks_.Run(host, info->usage,
-                            std::move(info->usage_breakdown));
+  auto host_it = host_usage_callbacks_.find(host);
+  if (host_it == host_usage_callbacks_.end())
+    return;
+
+  std::vector<UsageWithBreakdownCallback> pending_callbacks;
+  pending_callbacks.swap(host_it->second);
+  DCHECK(pending_callbacks.size() > 0)
+      << "host_usage_callbacks_ should only have non-empty callback lists";
+  host_usage_callbacks_.erase(host_it);
+
+  for (auto& callback : pending_callbacks)
+    std::move(callback).Run(info->usage, info->usage_breakdown);
 }
 
 }  // namespace storage

storage/browser/quota/usage_tracker.h

@@ -57,8 +57,7 @@ class STORAGE_EXPORT UsageTracker : public QuotaTaskObserver {
       std::map<url::Origin, int64_t>* origin_usage) const;
   void GetCachedOrigins(std::set<url::Origin>* origins) const;
   bool IsWorking() const {
-    return global_usage_callbacks_.HasCallbacks() ||
-           host_usage_callbacks_.HasAnyCallbacks();
+    return !global_usage_callbacks_.empty() || !host_usage_callbacks_.empty();
   }
 
   void SetUsageCacheEnabled(QuotaClient::ID client_id,
@@ -93,13 +92,9 @@ class STORAGE_EXPORT UsageTracker : public QuotaTaskObserver {
   std::map<QuotaClient::ID, std::unique_ptr<ClientUsageTracker>>
       client_tracker_map_;
 
-  CallbackQueue<UsageCallback, int64_t> global_limited_usage_callbacks_;
-  CallbackQueue<GlobalUsageCallback, int64_t, int64_t> global_usage_callbacks_;
-
-  CallbackQueueMap<UsageWithBreakdownCallback,
-                   std::string,
-                   int64_t,
-                   base::flat_map<QuotaClient::ID, int64_t>>
+  std::vector<UsageCallback> global_limited_usage_callbacks_;
+  std::vector<GlobalUsageCallback> global_usage_callbacks_;
+  std::map<std::string, std::vector<UsageWithBreakdownCallback>>
       host_usage_callbacks_;
 
   StorageMonitor* storage_monitor_;