Reinstate proper usage of mReflowCause

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox53 fixed, firefox54 fixed, firefox55 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years 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 2

2 years 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+

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/98c248833c4d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee

Comment 6

2 years 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?
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.