1888535 - Consider removing DISPLAY_REFLOW macros and DR_Cookie

Status: RESOLVED FIXED

(Core :: Layout, task)

Description

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

DISPLAY_RREFLOW macros [1] and DR_Cookie are old tool for debugging reflow. Nowadays, I feel their can be replaced by pernosco, rr, and MOZ_LOG, and we don't support it well in modern frame types like flex and grid. I propose we get rid of them to lower the maintain burden of layout.

Daniel suggested in private slack discussion that we can transform some of them to MOZ_LOG-logging.

[1] https://searchfox.org/mozilla-central/rev/f8258e49f277c1c881c5c0da95f463dd12b7397e/layout/generic/nsContainerFrame.h#1153-1176

Comment 1

[Daniel Holbert [:dholbert]]

--> Adding dependency on the bug that (I think) added this logging utility code, >20 years ago: bug 103925

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I give the tool a try by following http://devdoc.net/web/developer.mozilla.org/en-US/docs/Debugging_Frame_Reflow.html

reflow_rules.txt is just a line * 1, and running this command

GECKO_DISPLAY_REFLOW_RULES_FILE=reflow_rules.txt ./mach run --layoutdebug "data:text/html,hello"

The log is really spammy because it dumps all the reflow log for frontend UI, too ... I feel it's easier to debug via recording with rr, dumping frame tree, and starting backwards from the recording.

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1888535 Part 1 - Remove four unused display reflow debugging macros. r?#layout

DISPLAY_LAYOUT, DISPLAY_PREF_SIZE, DISPLAY_MIN_SIZE, and
DISPLAY_MAX_SIZE are not used in layout at all.

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1888535 Part 2 - Remove DISPLAY_INIT_CONSTRAINTS and DISPLAY_INIT_OFFSETS. r?#layout

The two macros are only used in ReflowInput.cpp, which are to print margin,
border, padding, available sizes, and other computed sizes in ReflowInput. The
information can be replaced by adding local printf or via rr if there are
debug needs.

Note: ReflowInput::DisplayInitFrameTypeEnter and
ReflowInput::DisplayInitFrameTypeExit have been removed in
https://hg.mozilla.org/mozilla-central/rev/429a4d022ade

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1888535 Part 3 - Remove DISPLAY_REFLOW_CHANGE used only in nsTableCellFrame. r?#layout

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1888535 Part 4 - Remove DISPLAY_PREF_INLINE_SIZE and DISPLAY_MIN_INLINE_SIZE. r?#layout

The two macros are used in GetPrefISize() and GetMinISize() implementations.
After removing them, we could further simplify some implementations because we
don't need a result variable in many cases.

This patch doesn't change behavior.

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1888535 Part 5 - Remove DISPLAY_REFLOW. r?#layout

It is used in Reflow() implementations.

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1888535 Part 6 - Unhook display reflow debugging from nsLayoutStatics, and remove display reflow debugging. r?#layout

This final patch completely removes display reflow debugging mechanism.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ccd4c30ad693
Part 1 - Remove four unused display reflow debugging macros. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/9f4f5c0809cb
Part 2 - Remove DISPLAY_INIT_CONSTRAINTS and DISPLAY_INIT_OFFSETS. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/4f809b006b38
Part 3 - Remove DISPLAY_REFLOW_CHANGE used only in nsTableCellFrame. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/b0a163af22bd
Part 4 - Remove DISPLAY_PREF_INLINE_SIZE and DISPLAY_MIN_INLINE_SIZE. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/6d308d64893c
Part 5 - Remove DISPLAY_REFLOW. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/298ae4f7d47b
Part 6 - Unhook display reflow debugging from nsLayoutStatics, and remove display reflow debugging. r=layout-reviewers,emilio

Comment 10

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/ccd4c30ad693
https://hg.mozilla.org/mozilla-central/rev/9f4f5c0809cb
https://hg.mozilla.org/mozilla-central/rev/4f809b006b38
https://hg.mozilla.org/mozilla-central/rev/b0a163af22bd
https://hg.mozilla.org/mozilla-central/rev/6d308d64893c
https://hg.mozilla.org/mozilla-central/rev/298ae4f7d47b