Domain Reliability: More security review.

components/domain_reliability/dispatcher.cc

Index: components/domain_reliability/dispatcher.cc
diff --git a/components/domain_reliability/dispatcher.cc b/components/domain_reliability/dispatcher.cc
index ceb4da09c8b5e6ca76129831eacb696cb93af012..d951cd2b8e842a62ef16da2f5b3e5b90c46ed78a 100644
--- a/components/domain_reliability/dispatcher.cc
+++ b/components/domain_reliability/dispatcher.cc
@@ -12,6 +12,20 @@
 
 namespace domain_reliability {
 
+struct DomainReliabilityDispatcher::Task {
+  Task(const base::Closure& closure,
+       scoped_ptr<MockableTime::Timer> timer,
+       base::TimeDelta min_delay,
+       base::TimeDelta max_delay);
+  ~Task();
+
+  base::Closure closure;
+  scoped_ptr<MockableTime::Timer> timer;
+  base::TimeDelta min_delay;
+  base::TimeDelta max_delay;
+  bool eligible;
+};
+
 DomainReliabilityDispatcher::DomainReliabilityDispatcher(MockableTime* time)
     : time_(time) {}
 
@@ -72,13 +86,11 @@ void DomainReliabilityDispatcher::MakeTaskWaiting(Task* task) {
   DCHECK(task);
   DCHECK(!task->eligible);
   DCHECK(!task->timer->IsRunning());
-  task->timer->Start(
-      FROM_HERE,
-      task->min_delay,
-      base::Bind(
-          &DomainReliabilityDispatcher::MakeTaskEligible,
-          base::Unretained(this),
-          task));
+  task->timer->Start(FROM_HERE,
+                     task->min_delay,
+                     base::Bind(&DomainReliabilityDispatcher::MakeTaskEligible,
+                                base::Unretained(this),
+                                task));
 }
 
 void
@@ -87,13 +99,11 @@ DomainReliabilityDispatcher::MakeTaskEligible(Task* task) {
   DCHECK(!task->eligible);
   task->eligible = true;
   eligible_tasks_.insert(task);
-  task->timer->Start(
-      FROM_HERE,
-      task->max_delay - task->min_delay,
-      base::Bind(
-          &DomainReliabilityDispatcher::RunAndDeleteTask,
-          base::Unretained(this),
-          task));
+  task->timer->Start(FROM_HERE,
+                     task->max_delay - task->min_delay,
+                     base::Bind(&DomainReliabilityDispatcher::RunAndDeleteTask,
+                                base::Unretained(this),
+                                task));

[Ryan Sleevi, 2014/04/30 23:58:11]

DESIGN: Not something for now, but I'm wondering if there is a better way to
aggregate these timers. MessageLoop is fairly efficient in using a scan and mark
here, and it seems like as long as the timer was the lowest possible timeout,
you could implement it as such.

Then again, this seems like it's re-implementing a lot of the existing
MessageLoop task pump stuff. Why wouldn't PostDelayedTask suffice here?

[Deprecated (see juliatuttle), 2014/05/02 19:23:40]

I added two layers of indirection:
MockableTime: Lets me test time-based interactions in unittests.
Dispatcher: Lets me schedule tasks with a range of times instead of a single
time.

 }
 
 void DomainReliabilityDispatcher::RunAndDeleteTask(Task* task) {