Reinstate proper usage of mReflowCause

RESOLVED FIXED in Firefox 53

Status

()

Core
Layout
RESOLVED FIXED
a month ago
a month ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla55
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox53 fixed, firefox54 fixed, firefox55 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a month ago
Bug 1322553 introduced GeckoProfilerTracingRAII. Part 2 converted a bunch of
places to use it, but messed one of them up:

> -          profiler_tracing("Paint", "Reflow", Move(mReflowCause), TRACING_INTERVAL_START);
> +          tracingLayoutFlush.emplace("Paint", "Reflow");

The Move(mReflowCause) was erroneously deleted. (There was a similar case with
Move(mStyleCause) that was not deleted.)

This patch reinstates the Move(mReflowCause).
(Assignee)

Comment 1

a month ago
Created attachment 8850352 [details] [diff] [review]
Reinstate proper usage of mReflowCause
Attachment #8850352 - Flags: review?(nfroyd)
(Assignee)

Comment 2

a month ago
Assuming this is valid, it should be backported to Aurora (54) and Beta (53).
It's valid, but the impact is limited because the perf.html UI doesn't expose this piece of information yet.
Attachment #8850352 - Flags: review?(nfroyd) → review+
(Assignee)

Comment 4

a month ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/98c248833c4da778bea843ada7c666f54430952c
Bug 1349856 - Reinstate proper usage of mReflowCause. r=froydnj.

Comment 5

a month ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/98c248833c4d
Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Comment 6

a month ago
Comment on attachment 8850352 [details] [diff] [review]
Reinstate proper usage of mReflowCause

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1322553

[User impact if declined]: Future versions of the profiler might be lacking information. (Current versions don't display the info collected by the buggy line of code.)

[Is this code covered by automated tests?]: Sort of? Not sure.

[Has the fix been verified in Nightly?]: The fixed code is in Nightly, but the line is not really used at the moment.

[Needs manual test from QE? If yes, steps to reproduce]: No.

[List of other uplifts needed for the feature/fix]: None.

[Is the change risky?]: No.

[Why is the change risky/not risky?]: Tiny change, obviously correct.

[String changes made/needed]: None.
Attachment #8850352 - Flags: approval-mozilla-beta?
Attachment #8850352 - Flags: approval-mozilla-aurora?

Updated

a month ago
status-firefox53: --- → affected
status-firefox54: --- → affected
Comment on attachment 8850352 [details] [diff] [review]
Reinstate proper usage of mReflowCause

This patch can help get correct information. Aurora54+ & Beta53+.
Attachment #8850352 - Flags: approval-mozilla-beta?
Attachment #8850352 - Flags: approval-mozilla-beta+
Attachment #8850352 - Flags: approval-mozilla-aurora?
Attachment #8850352 - Flags: approval-mozilla-aurora+

Comment 8

a month ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/58fe3ca31f9e
status-firefox54: affected → fixed

Comment 9

a month ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/a48c5cd45340
status-firefox53: affected → fixed
Setting qe-verify- based on Nicholas' assessment on manual testing needs (see Comment 6).
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.