Closed Bug 1349856 Opened 7 years ago Closed 7 years ago

Reinstate proper usage of mReflowCause


(Core :: Layout, defect)

Not set



Tracking Status
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- fixed


(Reporter: n.nethercote, Assigned: n.nethercote)




(1 file)

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).
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+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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?
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+
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.