Closed Bug 1004814 Opened 6 years ago Closed 6 years ago

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

Categories

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

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32
Tracking Status
firefox30 + fixed
firefox31 + fixed
firefox32 + fixed
b2g-v1.4 --- fixed

People

(Reporter: simon.lindholm10, Assigned: baku)

References

Details

(Keywords: regression, Whiteboard: [qa-] )

Attachments

(1 file, 1 obsolete file)

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.
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?
Flags: needinfo?(amarchesini)
Attached patch timer.patch (obsolete) — Splinter Review
Attachment #8416397 - Flags: review?(bzbarsky)
Flags: needinfo?(amarchesini)
Assignee: nobody → amarchesini
Comment on attachment 8416397 [details] [diff] [review]
timer.patch

r=me, but add a test?
Attachment #8416397 - Flags: review?(bzbarsky) → review+
Attached patch timer.patchSplinter Review
Test included. It's on try: https://tbpl.mozilla.org/?tree=Try&rev=774f24ed45bd
Attachment #8416397 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/8e82e8257789
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Please nominate for uplift so we can get this into this week's Beta.
Flags: needinfo?(amarchesini)
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
Attachment #8417878 - Flags: approval-mozilla-aurora?
Flags: needinfo?(amarchesini)
Andrea, you just nominated it for aurora and not beta. Is that on purpose?
Flags: needinfo?(amarchesini)
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
Attachment #8417878 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Flags: needinfo?(amarchesini)
Comment on attachment 8417878 [details] [diff] [review]
timer.patch

Thanks
We also want it for aurora ;)
Attachment #8417878 - Flags: approval-mozilla-beta?
Attachment #8417878 - Flags: approval-mozilla-beta+
Attachment #8417878 - Flags: approval-mozilla-aurora+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.