1004814 - console.timeEnd() always claims 0ms in web workers

Status: RESOLVED FIXED

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

Description

[Simon Lindholm]

Testcase:

data:text/html,<script>w=new Worker("data:,console.time('a'); t=Date.now()+1000; while (Date.now() < t); console.timeEnd('a')")</script>

The web console shows:
a: timer started
a: 0ms

Regressed in Firefox 30 - bisection points to https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=58eca03214a6&tochange=8abc76dedec2 and thus bug 965860.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Er, yes.  time() and timeEnd() only set mMonotonicTimer in the mWindow case, which is false on workers.

The old code seems like it was just taking the timings on the main thread, which also seems pretty useless.

Ideally we'd just have something like performance.now() in workers, but failing that, can we just record the TimeStamp when the worker starts and then have time() and timeEnd() send over millisecond durations from that TimeStamp?

Comment 2

[Andrea Marchesini [:baku]]

Attachment: timer.patch

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8416397 [details] [diff] [review]
timer.patch

r=me, but add a test?

Comment 4

[Andrea Marchesini [:baku]]

Attachment: timer.patch

Test included. It's on try: https://tbpl.mozilla.org/?tree=Try&rev=774f24ed45bd

Comment 5

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8e82e8257789

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8e82e8257789

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Please nominate for uplift so we can get this into this week's Beta.

Comment 8

[Andrea Marchesini [:baku]]

Comment on attachment 8417878 [details] [diff] [review]
timer.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 965860
User impact if declined: Console.time in workers is broken
Testing completed (on m-c, etc.): mochitest included in the patch.
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Andrea, you just nominated it for aurora and not beta. Is that on purpose?

Comment 10

[Andrea Marchesini [:baku]]

Comment on attachment 8417878 [details] [diff] [review]
timer.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 965860
User impact if declined: Console.time in workers is broken
Testing completed (on m-c, etc.): mochitest included in the patch.
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8417878 [details] [diff] [review]
timer.patch

Thanks
We also want it for aurora ;)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ed454a1a415c
https://hg.mozilla.org/releases/mozilla-beta/rev/aa9bc2b880a1

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/aa9bc2b880a1