1464947 - display value logging in ReflowInput::DisplayInitFrameTypeExit is wrong

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Cameron McCormack (:heycam)]

It's become out of sync with the StyleDisplay enum values.

Comment 1

[Cameron McCormack (:heycam)]

Attachment: Bug 1464947 - Fix ReflowInput::DisplayInitFrameTypeExit display type logging.

Review commit: https://reviewboard.mozilla.org/r/247394/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/247394/

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8981291 [details]
Bug 1464947 - Fix ReflowInput::DisplayInitFrameTypeExit display type logging.

https://reviewboard.mozilla.org/r/247394/#review253410

r+

Optional nits: (Could be done in a follow-up bug)

::: layout/style/nsStyleConsts.h:411
(Diff revision 1)
>             NS_STYLE_WRITING_MODE_SIDEWAYS_MASK)
>  
>  // See nsStyleDisplay
>  //
>  // NOTE: Order is important! If you change it, make sure to take a look at
>  // the FrameConstructorDataByDisplay stuff (both the XUL and non-XUL version),

While you're here: It should be "FrameConstructionDataByDisplay" ('ion' instead of 'or').

::: layout/style/nsStyleConsts.h:412
(Diff revision 1)
>  
>  // See nsStyleDisplay
>  //
>  // NOTE: Order is important! If you change it, make sure to take a look at
>  // the FrameConstructorDataByDisplay stuff (both the XUL and non-XUL version),
> -// and ensure it's still correct!
> +// and the displayTypes array in ReflowInput::DisplayInitFrameTypeExit,

That's scary!
Anyway there could be compile-time checks (at least that sizes match, it should catch most changes quickly), or a unit test to more thoroughly link all guilty parties?

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8981291 [details]
Bug 1464947 - Fix ReflowInput::DisplayInitFrameTypeExit display type logging.

https://reviewboard.mozilla.org/r/247394/#review253410

> That's scary!
> Anyway there could be compile-time checks (at least that sizes match, it should catch most changes quickly), or a unit test to more thoroughly link all guilty parties?

I meant "Any way"

Comment 4

[Cameron McCormack (:heycam)]

The reflow logging is not performance critical, so I suppose we could just be looking up nsCSSProps::kDisplayKTable instead.  Let me switch to that and put another patch up.

Comment 5

[Cameron McCormack (:heycam)]

Comment on attachment 8981291 [details]
Bug 1464947 - Fix ReflowInput::DisplayInitFrameTypeExit display type logging.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247394/diff/1-2/

Comment 6

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8981291 [details]
Bug 1464947 - Fix ReflowInput::DisplayInitFrameTypeExit display type logging.

https://reviewboard.mozilla.org/r/247394/#review253418

Thanks!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f79ea1a6633d
Fix ReflowInput::DisplayInitFrameTypeExit display type logging. r=gerald

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/f79ea1a6633d