Fix computation of runtime for throttled tasks

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
Committed: https://crrev.com/b56bbe4ce3e7276a130a00d83cf00e5672cd6be1
Cr-Commit-Position: refs/heads/master@{#376993}

[alex clarke (OOO till 29th), 2016-02-22 18:39:38]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[alex clarke (OOO till 29th), 2016-02-22 18:39:48]

alexclarke@chromium.org changed reviewers:
+ skyostil@chromium.org

[commit-bot: I haz the power, 2016-02-22 18:40:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1718233002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1718233002/1

[commit-bot: I haz the power, 2016-02-22 19:54:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-22 19:54:47]

Dry run: This issue passed the CQ dry run.

[Sami, 2016-02-23 11:41:22]

Looks pretty neat. Just one question.

https://codereview.chromium.org/1718233002/diff/1/components/scheduler/base/t...
File components/scheduler/base/time_domain.h (right):

https://codereview.chromium.org/1718233002/diff/1/components/scheduler/base/t...
components/scheduler/base/time_domain.h:54: // when scomputing the task run
time.
typo: computing

Could you add a note here saying this is thread safe?

https://codereview.chromium.org/1718233002/diff/1/components/scheduler/render...
File components/scheduler/renderer/throttled_time_domain.cc (right):

https://codereview.chromium.org/1718233002/diff/1/components/scheduler/render...
components/scheduler/renderer/throttled_time_domain.cc:24: return
tick_clock_->NowTicks() + delay;
Do we need a lock for |tick_clock_|?

[alex clarke (OOO till 29th), 2016-02-23 14:02:31]

PTAL

https://codereview.chromium.org/1718233002/diff/1/components/scheduler/base/t...
File components/scheduler/base/time_domain.h (right):

https://codereview.chromium.org/1718233002/diff/1/components/scheduler/base/t...
components/scheduler/base/time_domain.h:54: // when scomputing the task run
time.
On 2016/02/23 11:41:22, Sami wrote:
> typo: computing
> 
> Could you add a note here saying this is thread safe?

Done.

https://codereview.chromium.org/1718233002/diff/1/components/scheduler/render...
File components/scheduler/renderer/throttled_time_domain.cc (right):

https://codereview.chromium.org/1718233002/diff/1/components/scheduler/render...
components/scheduler/renderer/throttled_time_domain.cc:24: return
tick_clock_->NowTicks() + delay;
On 2016/02/23 11:41:22, Sami wrote:
> Do we need a lock for |tick_clock_|?

No because it's only set via the constructor and then it can't be changed.  I've
marked it as const to try and reinforce that.

[Sami, 2016-02-23 14:07:42]

lgtm.

https://codereview.chromium.org/1718233002/diff/20001/components/scheduler/ba...
File components/scheduler/base/time_domain.h (right):

https://codereview.chromium.org/1718233002/diff/20001/components/scheduler/ba...
components/scheduler/base/time_domain.h:54: // when scomputing the task run
time.  This can be called from any thread.
nit: looks like you missed s/scomputing/computing/.

[alex clarke (OOO till 29th), 2016-02-23 14:39:08]

The CQ bit was checked by alexclarke@chromium.org

[alex clarke (OOO till 29th), 2016-02-23 14:39:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/1718233002/#ps40001
(title: "Fix typo")

[alex clarke (OOO till 29th), 2016-02-23 14:39:18]

Thanks.

https://codereview.chromium.org/1718233002/diff/20001/components/scheduler/ba...
File components/scheduler/base/time_domain.h (right):

https://codereview.chromium.org/1718233002/diff/20001/components/scheduler/ba...
components/scheduler/base/time_domain.h:54: // when scomputing the task run
time.  This can be called from any thread.
On 2016/02/23 14:07:42, Sami wrote:
> nit: looks like you missed s/scomputing/computing/.

Done.

[commit-bot: I haz the power, 2016-02-23 14:39:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1718233002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1718233002/40001

[commit-bot: I haz the power, 2016-02-23 15:48:16]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-23 15:50:11]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/b56bbe4ce3e7276a130a00d83cf00e5672cd6be1
Cr-Commit-Position: refs/heads/master@{#376993}
==========

[commit-bot: I haz the power, 2016-02-23 15:50:12]

Patchset 3 (id:??) landed as
https://crrev.com/b56bbe4ce3e7276a130a00d83cf00e5672cd6be1
Cr-Commit-Position: refs/heads/master@{#376993}

[jaydasika, 2016-02-26 20:02:56]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1741053002/ by jaydasika@chromium.org.

The reason for reverting is: Speculative revert for Compile failure on Win, Mac
& Precise 64 Beta Bots (crbug.com/590286) .