[scheduler] Untangle BudgetPool from TaskQueueThrottler.

third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h

 // Copyright 2015 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 THIRD_PARTY_WEBKIT_SOURCE_PLATFORM_SCHEDULER_RENDERER_TASK_QUEUE_THROTTLER_H_
 #define THIRD_PARTY_WEBKIT_SOURCE_PLATFORM_SCHEDULER_RENDERER_TASK_QUEUE_THROTTLER_H_
 
 #include <set>
 #include <unordered_map>
 
@@ skipping 91 matching lines @@
       base::Optional<base::TimeDelta> max_throttling_duration);
 
   // Accounts for given task for cpu-based throttling needs.
   void OnTaskRunTimeReported(TaskQueue* task_queue,
                              base::TimeTicks start_time,
                              base::TimeTicks end_time);
 
   void AsValueInto(base::trace_event::TracedValue* state,
                    base::TimeTicks now) const;
 
+  // Methods for BudgetPools to interact with TaskQueueThrottler.
+
+  // To be used by BudgetPool only, use BudgetPool::{Add,Remove}Queue
+  // methods instead.
+  void AddQueueToBudgetPool(TaskQueue* queue, BudgetPool* budget_pool);

[alex clarke (OOO till 29th), 2017/03/14 18:22:39]

bike-shed:  Should we try to decouple even further?  E.g. put an interface
between the BudgetPool and TaskQueueThrottler.  The eventual goal would be to be
able to reuse the BudgetPool elsewhere, and potentially the throttling with
lucky luke?

[altimin, 2017/03/14 18:26:44]

I like the idea in general, but that I think that's premature at the moment.
Lucky Luke integration will require some effort and direct copy-paste will be
impossible, so let's wait and do the refactoring then the need arises.

[Sami, 2017/03/14 18:43:01]

I think I'd also like to suggest turning these methods into an explicit abstract
interface. It doesn't need to be perfect, but will help testing the budget pools
in isolation and makes it clear what the dependencies and responsibilities
between the two class hierarchies are (and would avoid a circular dependency
too). Doing that later will be much harder as more dependencies creep in.

[altimin, 2017/03/14 20:04:28]

Done.

+  void RemoveQueueFromBudgetPool(TaskQueue* queue, BudgetPool* budget_pool);
+
+  // Deletes the budget pool.
+  void UnregisterBudgetPool(BudgetPool* budget_pool);
+
+  // Schedule a call to PumpThrottledTasks to unblock a queue at an appropriate
+  // moment.
+  void SchedulePumpQueue(const tracked_objects::Location& from_here,
+                         base::TimeTicks now,
+                         TaskQueue* queue);
+
+  // Insert a fence to prevent tasks from running and schedule a wakeup at
+  // an appropriate time.
+  void BlockThrottledQueue(base::TimeTicks now, TaskQueue* queue);
+
  private:
-  friend class BudgetPool;
-  friend class CPUTimeBudgetPool;
-
   struct Metadata {
-    Metadata() : throttling_ref_count(0), time_budget_pool(nullptr) {}
+    Metadata() : throttling_ref_count(0), budget_pool(nullptr) {}
 
     size_t throttling_ref_count;
 
-    CPUTimeBudgetPool* time_budget_pool;
+    BudgetPool* budget_pool;
   };
   using TaskQueueMap = std::unordered_map<TaskQueue*, Metadata>;
 
   void PumpThrottledTasks();
 
   // Note |unthrottled_runtime| might be in the past. When this happens we
   // compute the delay to the next runtime based on now rather than
   // unthrottled_runtime.
   void MaybeSchedulePumpThrottledTasks(
       const tracked_objects::Location& from_here,
       base::TimeTicks now,
       base::TimeTicks runtime);
 
-  CPUTimeBudgetPool* GetTimeBudgetPoolForQueue(TaskQueue* queue);
-
-  // Schedule pumping because of given task queue.
-  void MaybeSchedulePumpQueue(
-      const tracked_objects::Location& from_here,
-      base::TimeTicks now,
-      TaskQueue* queue,
-      base::Optional<base::TimeTicks> next_possible_run_time);
+  BudgetPool* GetBudgetPoolForQueue(TaskQueue* queue);
 
   // Return next possible time when queue is allowed to run in accordance
   // with throttling policy.
   base::TimeTicks GetNextAllowedRunTime(base::TimeTicks now, TaskQueue* queue);
 
   void MaybeDeleteQueueMetadata(TaskQueueMap::iterator it);
 
   TaskQueueMap queue_details_;
   base::Callback<void(TaskQueue*)> forward_immediate_work_callback_;
   scoped_refptr<TaskQueue> task_runner_;
   RendererSchedulerImpl* renderer_scheduler_;  // NOT OWNED
   base::TickClock* tick_clock_;                // NOT OWNED
   const char* tracing_category_;               // NOT OWNED
   std::unique_ptr<ThrottledTimeDomain> time_domain_;
 
   CancelableClosureHolder pump_throttled_tasks_closure_;
   base::Optional<base::TimeTicks> pending_pump_throttled_tasks_runtime_;
   bool allow_throttling_;
 
-  std::unordered_map<BudgetPool*, std::unique_ptr<BudgetPool>>
-      time_budget_pools_;
+  std::unordered_map<BudgetPool*, std::unique_ptr<BudgetPool>> budget_pools_;
 
   base::WeakPtrFactory<TaskQueueThrottler> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(TaskQueueThrottler);
 };
 
 }  // namespace scheduler
 }  // namespace blink
 
 #endif  // THIRD_PARTY_WEBKIT_SOURCE_PLATFORM_SCHEDULER_RENDERER_TASK_QUEUE_THROTTLER_H_