If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

TraceLogger: Fix start/stop messages

RESOLVED FIXED in Firefox 37

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: h4writer, Assigned: h4writer)

Tracking

(Blocks: 1 bug)

unspecified
mozilla38
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 unaffected, firefox36 unaffected, firefox37 fixed, firefox38 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
The start/stop messages don't always correspond anymore, due to a change a made to fix a windows bug.

This fixes it again.

(found it by a new check I added. Will open a bug to include that eventually)
(Assignee)

Comment 1

3 years ago
Created attachment 8545905 [details] [diff] [review]
Patch

Give the IonMonkey start events before doing the arguments checks, which could bail.
Assignee: nobody → hv1989
Attachment #8545905 - Flags: review?(benj)
(Assignee)

Updated

3 years ago
Blocks: 1065722
(Assignee)

Comment 2

3 years ago
Created attachment 8546535 [details] [diff] [review]
Patch

Forgot to update the other architectures.
Attachment #8545905 - Attachment is obsolete: true
Attachment #8545905 - Flags: review?(benj)
Attachment #8546535 - Flags: review?(benj)
Comment on attachment 8546535 [details] [diff] [review]
Patch

Review of attachment 8546535 [details] [diff] [review]:
-----------------------------------------------------------------

OK to not block on this particular one, but please open a follow up for erasing all this code duplication in the future... (and as said on irc, code duplication the generateEpilogue() could be avoided as well)
Attachment #8546535 - Flags: review?(benj) → review+
(Assignee)

Updated

3 years ago
Depends on: 1120924
(Assignee)

Comment 4

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/335c09bb1c95
status-firefox37: --- → affected
https://hg.mozilla.org/mozilla-central/rev/335c09bb1c95
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(Assignee)

Comment 6

3 years ago
Comment on attachment 8546535 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]:
Bug 1071546

[User impact if declined]:
Not much impact for regular users. It might annoy few people that they need to wait another release before tracelogger is working well. This just missed the merge, so a bit of a bummer.

[Describe test coverage new/current, TBPL]:
1 day on Inbound.

[Risks and why]: 
Very small risk. Small patch and only used by select public

[String/UUID change made/needed]:
/
Attachment #8546535 - Flags: approval-mozilla-aurora?
status-firefox35: --- → unaffected
status-firefox36: --- → unaffected
status-firefox38: --- → fixed
Comment on attachment 8546535 [details] [diff] [review]
Patch

Aurora+
Attachment #8546535 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/305d94d1ba09
status-firefox37: affected → fixed
You need to log in before you can comment on or make changes to this bug.