512645 - Only clamp nested timeouts

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

chrome uses setTimeout(0) often to just wait for an event loop cycle or similar; we should not enforce the 10ms minimum for chrome windows.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Booyah.

Comment 2

[Peter Van der Beken [:peterv]]

I'm going to morph this, we just discussed this yesterday. We shouldn't special-case chrome, we should follow HTML5 and only clamp for nested timeouts. HTML5 says to clamp for nesting level >= 1, Webkit seems to clamp for nesting level >= 5. iBench chains 3 timeouts, so following Webkit would also help our score there.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Whatever we do here, we should make sure to tweak the worker setTimeout code as well as the normal Window one.

Comment 4

[Peter Van der Beken [:peterv]]

Attachment: v1

Still trying to write a testcase for this, but it's not easy to make a reliable one.

Comment 5

[Peter Van der Beken [:peterv]]

I copied WebKit for now, using 5 for the nesting level at which to start clamping.

(In reply to comment #3)
> Whatever we do here, we should make sure to tweak the worker setTimeout code as
> well as the normal Window one.

Workers don't seem to clamp timeouts at all. Want to file a separate bug for adding clamping?

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Actually, would it make sense to not do what webkit does and instead simply follow the spec, and then point to this as for why iBench is totally broken in its current state?

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Filed bug 518406 on worker timeouts.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #7)
> Filed bug 518406 on worker timeouts.

Punks! ;)

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

(In reply to comment #6)
> Actually, would it make sense to not do what webkit does and instead simply
> follow the spec, and then point to this as for why iBench is totally broken in
> its current state?

I don't think it would.

I don't see how allowing some nesting this way is worse for the web, since spec-compliant code still runs fine, and complaining about iBench is unlikely to have any effect on the world (it'll never get changed).

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I agree iBench is unlikely to get fixed. And that this fix isn't bad for the web.

However my understanding was that it was so broken that it's unlikely that we'll ever get any useful data out of it, so we'd really like Apple to quit using it. Having glaring bugs like this to point at would give us a strong case for that Apple is not acting in good faith if they keep using it.

Comment 11

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

iBench has the bug regardless of what we do here, so point away!

Comment 12

[Dietrich Ayala (:dietrich)]

are there open questions here or just waiting on review? if the broadening of the change suggested in comment #0 is slowing things down, can we just do the minimal change first?

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I think we're just waiting for jsts review.

Comment 14

[Peter Van der Beken [:peterv]]

Sorry for the delay in checking this in.

http://hg.mozilla.org/mozilla-central/rev/350ffc1d793a