Changing setTimeout to not clamp delays to 1ms minimum, since a 1ms clamp is not in the spec.

Changing setTimeout to not clamp delays to 1ms minimum, since a 1ms clamp is not in the spec.

ATTN Sheriffs: There is good probability that that this patch may have unintended side effects. For example there may be some JS tests that use setTimeout(...0) and hope that layout will occur.  With this patch that's much less likely to be the case and is actually a bug in the test.  I tried to fix some of those here https://codereview.chromium.org/644093003/

Another possibility is that something using setInterval(... 0) may suddenly show a perf regression due to running much more frequently.

BUG=402694
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185372

[Sami, 2014-08-13 15:20:38]

+jamesr

[jamesr, 2014-08-13 15:29:48]

lgtm

[alexclarke, 2014-08-14 10:15:05]

The CQ bit was checked by alexclarke@google.com

[commit-bot: I haz the power, 2014-08-14 10:15:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/alexclarke@chromium.org/467153003/40001

[commit-bot: I haz the power, 2014-08-14 13:36:04]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19827)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22768)

[commit-bot: I haz the power, 2014-08-14 14:15:46]

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

[commit-bot: I haz the power, 2014-08-14 14:15:47]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19831)

[alexclarke, 2014-08-26 14:35:34]

It's common for layout tests to use setTimeout(..., 0) and expect some
reapinting to happen.  With the timeout genuinely being zero, that's no longer a
valid assumption so I've changed the affected tests.

PTAL

[Sami, 2014-08-26 16:21:25]

I wonder if some of these should be using requestAnimationFrame instead?

[alexclarke, 2014-08-26 16:26:51]

On 2014/08/26 16:21:25, Sami wrote:
> I wonder if some of these should be using requestAnimationFrame instead?

That's a possibility, although it didn't seem to work for the repaint tests when
I tried it.

[alexclarke, 2014-09-29 16:50:33]

I've changed a number of the tests to use RAF instead of setTimeout.  PTAL

[jamesr, 2014-09-29 18:16:35]

Land the test changes first and separately to make sure they're stable.

[alexclarke, 2014-09-30 15:03:01]

On 2014/09/29 18:16:35, jamesr wrote:
> Land the test changes first and separately to make sure they're stable.

OK sounds like a plan.  I've split them of into
https://codereview.chromium.org/615033002/

[eseidel, 2014-10-14 16:20:21]

eseidel@chromium.org changed reviewers:
+ eseidel@chromium.org

[eseidel, 2014-10-14 16:20:22]

You might just try changing to 1ms everywhere and leave a FIXME.

[eseidel, 2014-10-21 16:23:10]

I discussed this patch some with Hixie (html5 editor) and abarth and we agreed
that it's unfortunate that it's not currently possible to "post a task" to the
Chromium runloop from JS.  That setTimeout may run before exiting back to the
chromium runloop.  It was thrown around the idea that we should make
setTimeout(0) be that way to get to the chromium runloop.  That we actually
still schedule with 0, but we don't run more than one Blink timer each time the
shared timer wakes up.  I  suspect that would be a larger and more difficult
change, but offering that to you as an eventual goal.

[alexclarke, 2014-10-21 16:34:43]

On 2014/10/21 16:23:10, eseidel wrote:
> I discussed this patch some with Hixie (html5 editor) and abarth and we agreed
> that it's unfortunate that it's not currently possible to "post a task" to the
> Chromium runloop from JS.  That setTimeout may run before exiting back to the
> chromium runloop.  It was thrown around the idea that we should make
> setTimeout(0) be that way to get to the chromium runloop.  That we actually
> still schedule with 0, but we don't run more than one Blink timer each time
the
> shared timer wakes up.  I  suspect that would be a larger and more difficult
> change, but offering that to you as an eventual goal.

I think it would be great if setTimeout(0) did eventually post onto the chromium
runloop.

I'm hoping this patch will flush out any things that broken by removing the
clamping first :)

[alex clarke (OOO till 29th), 2014-11-10 13:19:04]

The CQ bit was checked by alexclarke@chromium.org

[commit-bot: I haz the power, 2014-11-10 13:19:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/467153003/390001

[commit-bot: I haz the power, 2014-11-10 13:23:00]

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

[commit-bot: I haz the power, 2014-11-10 13:23:02]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/1...)

[alex clarke (OOO till 29th), 2014-11-13 11:10:12]

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

[alex clarke (OOO till 29th), 2014-11-13 11:10:34]

+jochen@  Eric suggested I ask you to review this :)

[jochen (gone - plz use gerrit), 2014-11-13 15:14:57]

can you explain why this won't go back to the message loop in any case?

[alex clarke (OOO till 29th), 2014-11-14 12:49:49]

On 2014/11/13 15:14:57, jochen (slow) wrote:
> can you explain why this won't go back to the message loop in any case?

Actually it should, the real problem for layout tests is ones that expect
something like layout to occur.  I've updated the description slightly.

[jochen (gone - plz use gerrit), 2014-11-14 12:58:40]

lgtm

[alex clarke (OOO till 29th), 2014-11-14 13:08:23]

The CQ bit was checked by alexclarke@chromium.org

[commit-bot: I haz the power, 2014-11-14 13:08:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/467153003/390001

[commit-bot: I haz the power, 2014-11-14 13:12:39]

Committed patchset #21 (id:390001) as 185372

[alex clarke (OOO till 29th), 2014-11-19 12:18:25]

A revert of this CL (patchset #21 id:390001) has been created in
https://codereview.chromium.org/739813003/ by alexclarke@chromium.org.

The reason for reverting is: It broke setInterval(0) see
https://code.google.com/p/chromium/issues/detail?id=434601

We need to decide what to do with setInterval(0) I can see some legitimate uses
of that (e.g. implementing a scheduler in JS) but I can also see a lot of ways
existing code may break.

Perhaps we still need the clamp for setInterval :(.