[Cherrypick try #2] Fix computation of runtime for throttled tasks

The computation of the delayed task runtime was computing the (virtual)
TimeDomain's now + delay.  That doesn't take into account the skew
between the real time and the virtual time, which meant timers could
run sooner than expected for background tabs.  Timers associated with
foreground tabs are unaffected.

This patch makes the computation of delayed runtime  the responsibility
of the TimeDomain and introduces a ThrottledTimeDomain which computes
the delayed runtime as realtime + delay.

This means timers will no longer run sooner than expected.  Under normal
circumstancs this only timers whose delay modulo 1000ms is less than 100
ms are less likely to be affected.  Timers whose whose delay modulo
1000ms is greater than 900 are highly likely to be affected (there's a
good chance their delay was too small before).

BUG=587074

Review URL: https://codereview.chromium.org/1718233002

Cr-Commit-Position: refs/heads/master@{#376993}
(cherry picked from commit b56bbe4ce3e7276a130a00d83cf00e5672cd6be1)

R=skyostil@chromium.org

Review URL: https://codereview.chromium.org/1751953002 .

Cr-Commit-Position: refs/branch-heads/2623@{#550}
Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907}

13 files changed, 134 insertions, 8 deletions

diff --git a/components/scheduler/base/real_time_domain.cc b/components/scheduler/base/real_time_domain.cc
index 5c92648..33c0dff 100644
--- a/components/scheduler/base/real_time_domain.cc
+++ b/components/scheduler/base/real_time_domain.cc
@@ -26,6 +26,12 @@ LazyNow RealTimeDomain::CreateLazyNow() {
   return task_queue_manager_->CreateLazyNow();
 }
 
+base::TimeTicks RealTimeDomain::ComputeDelayedRunTime(
+    base::TimeTicks time_domain_now,
+    base::TimeDelta delay) const {
+  return time_domain_now + delay;
+}
+
 void RealTimeDomain::RequestWakeup(LazyNow* lazy_now, base::TimeDelta delay) {
   // NOTE this is only called if the scheduled runtime is sooner than any
   // previously scheduled runtime, or there is no (outstanding) previously
@@ -55,5 +61,4 @@ void RealTimeDomain::AsValueIntoInternal(
 const char* RealTimeDomain::GetName() const {
   return "RealTimeDomain";
 }
-
 }  // namespace scheduler
diff --git a/components/scheduler/base/real_time_domain.h b/components/scheduler/base/real_time_domain.h
index ab20d80..579e57b 100644
--- a/components/scheduler/base/real_time_domain.h
+++ b/components/scheduler/base/real_time_domain.h
@@ -20,6 +20,8 @@ class SCHEDULER_EXPORT RealTimeDomain : public TimeDomain {
 
   // TimeDomain implementation:
   LazyNow CreateLazyNow() override;
+  base::TimeTicks ComputeDelayedRunTime(base::TimeTicks time_domain_now,
+                                        base::TimeDelta delay) const override;
   bool MaybeAdvanceTime() override;
   const char* GetName() const override;
 
diff --git a/components/scheduler/base/task_queue_impl.cc b/components/scheduler/base/task_queue_impl.cc
index b449e2d..0d79b0d 100644
--- a/components/scheduler/base/task_queue_impl.cc
+++ b/components/scheduler/base/task_queue_impl.cc
@@ -149,8 +149,11 @@ bool TaskQueueImpl::PostDelayedTaskImpl(
     return false;
   LazyNow lazy_now(any_thread().time_domain->CreateLazyNow());
   base::TimeTicks desired_run_time;
-  if (delay > base::TimeDelta())
-    desired_run_time = lazy_now.Now() + delay;
+  if (delay > base::TimeDelta()) {
+    base::TimeTicks time_domain_now = lazy_now.Now();
+    desired_run_time =
+        any_thread().time_domain->ComputeDelayedRunTime(time_domain_now, delay);
+  }
   return PostDelayedTaskLocked(&lazy_now, from_here, task, desired_run_time,
                                task_type);
 }
diff --git a/components/scheduler/base/time_domain.h b/components/scheduler/base/time_domain.h
index dd82a13..4e4fbad 100644
--- a/components/scheduler/base/time_domain.h
+++ b/components/scheduler/base/time_domain.h
@@ -46,6 +46,13 @@ class SCHEDULER_EXPORT TimeDomain {
   // TODO(alexclarke): Make this main thread only.
   virtual LazyNow CreateLazyNow() = 0;
 
+  // Computes a runtime which is >= |time_domain_now| + |delay|. This is used to
+  // allow the TimeDomain to decide if the real or virtual time should be used
+  // when computing the task run time.  This can be called from any thread.
+  virtual base::TimeTicks ComputeDelayedRunTime(
+      base::TimeTicks time_domain_now,
+      base::TimeDelta delay) const = 0;
+
   // Some TimeDomains support virtual time, this method tells us to advance time
   // if possible and return true if time was advanced.
   virtual bool MaybeAdvanceTime() = 0;
diff --git a/components/scheduler/base/time_domain_unittest.cc b/components/scheduler/base/time_domain_unittest.cc
index efcd3fc..111b726 100644
--- a/components/scheduler/base/time_domain_unittest.cc
+++ b/components/scheduler/base/time_domain_unittest.cc
@@ -39,6 +39,11 @@ class MockTimeDomain : public TimeDomain {
   // TimeSource implementation:
   LazyNow CreateLazyNow() override { return LazyNow(now_); }
 
+  base::TimeTicks ComputeDelayedRunTime(base::TimeTicks time_domain_now,
+                                        base::TimeDelta delay) const override {
+    return time_domain_now + delay;
+  }
+
   void AsValueIntoInternal(
       base::trace_event::TracedValue* state) const override {}
 
diff --git a/components/scheduler/base/virtual_time_domain.cc b/components/scheduler/base/virtual_time_domain.cc
index 0abea94..97c63da 100644
--- a/components/scheduler/base/virtual_time_domain.cc
+++ b/components/scheduler/base/virtual_time_domain.cc
@@ -28,6 +28,12 @@ LazyNow VirtualTimeDomain::CreateLazyNow() {
   return LazyNow(now_);
 }
 
+base::TimeTicks VirtualTimeDomain::ComputeDelayedRunTime(
+    base::TimeTicks time_domain_now,
+    base::TimeDelta delay) const {
+  return time_domain_now + delay;
+}
+
 void VirtualTimeDomain::RequestWakeup(LazyNow* lazy_now,
                                       base::TimeDelta delay) {
   // We don't need to do anything here because the caller of AdvanceTo is
diff --git a/components/scheduler/base/virtual_time_domain.h b/components/scheduler/base/virtual_time_domain.h
index 6091b31..59f615f 100644
--- a/components/scheduler/base/virtual_time_domain.h
+++ b/components/scheduler/base/virtual_time_domain.h
@@ -20,6 +20,8 @@ class SCHEDULER_EXPORT VirtualTimeDomain : public TimeDomain {
 
   // TimeDomain implementation:
   LazyNow CreateLazyNow() override;
+  base::TimeTicks ComputeDelayedRunTime(base::TimeTicks time_domain_now,
+                                        base::TimeDelta delay) const override;
   bool MaybeAdvanceTime() override;
   const char* GetName() const override;
 
diff --git a/components/scheduler/renderer/throttled_time_domain.cc b/components/scheduler/renderer/throttled_time_domain.cc
new file mode 100644
index 0000000..76a94a2
--- /dev/null
+++ b/components/scheduler/renderer/throttled_time_domain.cc
@@ -0,0 +1,31 @@
+// 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.
+
+#include "components/scheduler/renderer/throttled_time_domain.h"
+
+#include "base/time/tick_clock.h"
+
+namespace scheduler {
+
+ThrottledTimeDomain::ThrottledTimeDomain(TimeDomain::Observer* observer,
+                                         base::TickClock* tick_clock)
+    : VirtualTimeDomain(observer, tick_clock->NowTicks()),
+      tick_clock_(tick_clock) {}
+
+ThrottledTimeDomain::~ThrottledTimeDomain() {}
+
+base::TimeTicks ThrottledTimeDomain::ComputeDelayedRunTime(
+    base::TimeTicks,
+    base::TimeDelta delay) const {
+  // We ignore the |time_domain_now| parameter since its the virtual time but we
+  // need to use the current real time when computing the delayed runtime.  If
+  // don't do that, throttled timers may fire sooner than expected.
+  return tick_clock_->NowTicks() + delay;
+}
+
+const char* ThrottledTimeDomain::GetName() const {
+  return "ThrottledTimeDomain";
+}
+
+}  // namespace scheduler
diff --git a/components/scheduler/renderer/throttled_time_domain.h b/components/scheduler/renderer/throttled_time_domain.h
new file mode 100644
index 0000000..f274e8c
--- /dev/null
+++ b/components/scheduler/renderer/throttled_time_domain.h
@@ -0,0 +1,34 @@
+// 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 COMPONENTS_SCHEDULER_RENDERER_THROTTLED_TIME_DOMAIN_H_
+#define COMPONENTS_SCHEDULER_RENDERER_THROTTLED_TIME_DOMAIN_H_
+
+#include "base/macros.h"
+#include "components/scheduler/base/virtual_time_domain.h"
+
+namespace scheduler {
+
+// A time domain that's mostly a VirtualTimeDomain except that real time is used
+// when computing the delayed runtime.
+class SCHEDULER_EXPORT ThrottledTimeDomain : public VirtualTimeDomain {
+ public:
+  ThrottledTimeDomain(TimeDomain::Observer* observer,
+                      base::TickClock* tick_clock);
+  ~ThrottledTimeDomain() override;
+
+  // TimeDomain implementation:
+  base::TimeTicks ComputeDelayedRunTime(base::TimeTicks time_domain_now,
+                                        base::TimeDelta delay) const override;
+  const char* GetName() const override;
+
+ private:
+  base::TickClock* const tick_clock_;  // NOT OWNED
+
+  DISALLOW_COPY_AND_ASSIGN(ThrottledTimeDomain);
+};
+
+}  // namespace scheduler
+
+#endif  // COMPONENTS_SCHEDULER_RENDERER_THROTTLED_TIME_DOMAIN_H_
diff --git a/components/scheduler/renderer/throttling_helper.cc b/components/scheduler/renderer/throttling_helper.cc
index 9a99001..7683bd3 100644
--- a/components/scheduler/renderer/throttling_helper.cc
+++ b/components/scheduler/renderer/throttling_helper.cc
@@ -6,9 +6,9 @@
 
 #include "base/logging.h"
 #include "components/scheduler/base/real_time_domain.h"
-#include "components/scheduler/base/virtual_time_domain.h"
 #include "components/scheduler/child/scheduler_tqm_delegate.h"
 #include "components/scheduler/renderer/renderer_scheduler_impl.h"
+#include "components/scheduler/renderer/throttled_time_domain.h"
 #include "components/scheduler/renderer/web_frame_scheduler_impl.h"
 #include "third_party/WebKit/public/platform/WebFrameScheduler.h"
 
@@ -20,7 +20,7 @@ ThrottlingHelper::ThrottlingHelper(RendererSchedulerImpl* renderer_scheduler,
       renderer_scheduler_(renderer_scheduler),
       tick_clock_(renderer_scheduler->tick_clock()),
       tracing_category_(tracing_category),
-      time_domain_(new VirtualTimeDomain(this, tick_clock_->NowTicks())),
+      time_domain_(new ThrottledTimeDomain(this, tick_clock_)),
       weak_factory_(this) {
   suspend_timers_when_backgrounded_closure_.Reset(base::Bind(
       &ThrottlingHelper::PumpThrottledTasks, weak_factory_.GetWeakPtr()));
diff --git a/components/scheduler/renderer/throttling_helper.h b/components/scheduler/renderer/throttling_helper.h
index 69a4776..8c67338 100644
--- a/components/scheduler/renderer/throttling_helper.h
+++ b/components/scheduler/renderer/throttling_helper.h
@@ -16,7 +16,7 @@
 namespace scheduler {
 
 class RendererSchedulerImpl;
-class VirtualTimeDomain;
+class ThrottledTimeDomain;
 class WebFrameSchedulerImpl;
 
 class SCHEDULER_EXPORT ThrottlingHelper : public TimeDomain::Observer {
@@ -33,7 +33,7 @@ class SCHEDULER_EXPORT ThrottlingHelper : public TimeDomain::Observer {
   void Throttle(TaskQueue* task_queue);
   void Unthrottle(TaskQueue* task_queue);
 
-  const VirtualTimeDomain* time_domain() const { return time_domain_.get(); }
+  const ThrottledTimeDomain* time_domain() const { return time_domain_.get(); }
 
   static base::TimeTicks ThrottledRunTime(base::TimeTicks unthrottled_runtime);
 
@@ -52,7 +52,7 @@ class SCHEDULER_EXPORT ThrottlingHelper : public TimeDomain::Observer {
   RendererSchedulerImpl* renderer_scheduler_;  // NOT OWNED
   base::TickClock* tick_clock_;                // NOT OWNED
   const char* tracing_category_;               // NOT OWNED
-  scoped_ptr<VirtualTimeDomain> time_domain_;
+  scoped_ptr<ThrottledTimeDomain> time_domain_;
 
   CancelableClosureHolder suspend_timers_when_backgrounded_closure_;
   base::TimeTicks pending_pump_throttled_tasks_runtime_;
diff --git a/components/scheduler/renderer/throttling_helper_unittest.cc b/components/scheduler/renderer/throttling_helper_unittest.cc
index f52a605..7c7cb02 100644
--- a/components/scheduler/renderer/throttling_helper_unittest.cc
+++ b/components/scheduler/renderer/throttling_helper_unittest.cc
@@ -276,4 +276,33 @@ TEST_F(ThrottlingHelperTest,
                   base::TimeTicks() + base::TimeDelta::FromSeconds(16)));
 }
 
+TEST_F(ThrottlingHelperTest, TaskDelayIsBasedOnRealTime) {
+  std::vector<base::TimeTicks> run_times;
+  throttling_helper_->Throttle(timer_queue_.get());
+
+  // Post an initial task that should run at the first aligned time period.
+  timer_queue_->PostDelayedTask(FROM_HERE,
+                                base::Bind(&TestTask, &run_times, clock_.get()),
+                                base::TimeDelta::FromMilliseconds(900.0));
+
+  mock_task_runner_->RunUntilIdle();
+
+  // Advance realtime.
+  clock_->Advance(base::TimeDelta::FromMilliseconds(250));
+
+  // Post a task that due to real time + delay must run in the third aligned
+  // time period.
+  timer_queue_->PostDelayedTask(FROM_HERE,
+                                base::Bind(&TestTask, &run_times, clock_.get()),
+                                base::TimeDelta::FromMilliseconds(900.0));
+
+  mock_task_runner_->RunUntilIdle();
+
+  EXPECT_THAT(
+      run_times,
+      ElementsAre(
+          base::TimeTicks() + base::TimeDelta::FromMilliseconds(1000.0),
+          base::TimeTicks() + base::TimeDelta::FromMilliseconds(3000.0)));
+}
+
 }  // namespace scheduler
diff --git a/components/scheduler/scheduler.gypi b/components/scheduler/scheduler.gypi
index 8271fff..87e2be0 100644
--- a/components/scheduler/scheduler.gypi
+++ b/components/scheduler/scheduler.gypi
@@ -78,6 +78,8 @@
       'renderer/render_widget_signals.h',
       'renderer/task_cost_estimator.cc',
       'renderer/task_cost_estimator.h',
+      'renderer/throttled_time_domain.cc',
+      'renderer/throttled_time_domain.h',
       'renderer/throttling_helper.cc',
       'renderer/throttling_helper.h',
       'renderer/web_frame_scheduler_impl.cc',