Closed Bug 1004814 Opened 11 years ago Closed 11 years ago

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

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla32
Tracking Flags:
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)

timer.patch
(deleted), patch
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review
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) (deleted) — — 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.patch (deleted) — — Splinter 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: 11 years ago
status-firefox32: affected → fixed
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.

Attachment

General

Creator:
Created:
Updated:
Size: