1663822 - Simply the flag manipulation (ReflowInputFlags, ComputeSizeFlags) in ReflowInput

Status: RESOLVED FIXED

(Core :: Layout, task)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

While investigating bug 1316534, I notice we may need to add a flag to ComputeSizeFlags to control whether flex-basis can influence a flex-item's main-size property (either width or height).

However, our current flag passing mechanism from ReflowInput's constructor to nsIFrame::ComputeSize is not very ergonomic. For example, if we want to use shrink-wrap behavior, we need to pass an anonymous enum on ReflowInput's constructor. Then ReflowInput sets the relevant ReflowInputFlags in its constructor. Finally, when ReflowInput is about to call nsIFrame::ComputeSize, it then constructs ComputeSizeFlags based on ReflowInputFlags.

So in this bug, I'm planning to fix this with the following goals in mind.

1. Make nsIFrame::ComputeSizeFlags an EnumSet so that it's easier to use.
2. Let ReflowInput's constructor take ComputeSizeFlags to avoid the flag handover described above. This can eliminate parts of the anonymous enum values that use to construct ComputeSizeFlags. The relevant bits in ReflowInputFlags can also be removed.
3. The rest of the values in the anynoums enum are controlling ReflowInput's behavior. We can convert it into an EnumSet.

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1663822 Part 1 - Drop the "e" prefix from the enum values in ComputeSizeFlags.

This change shouldn't change behavior.

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1663822 Part 2 - Change ComputeSizeFlags into an EnumSet.

This patch does:

1. Rename the original ComputeSizeFlags to ComputeSizeFlag (dropping the
   "s"), and make it an enum class.
2. Make ComputeSizeFlags can an EnumSet.
3. Adapt the users to use EnumSet's APIs.

Default enum value in ComputeSizeFlag is not needed. It equals to an
empty ComputeSizeFlags.

This change shouldn't change behavior.

Depends on D89540

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1663822 Part 3 - Move ComputeSizeFlags to LayoutConstants.h, and put it under mozilla namespace.

In the next part, I'm going to use ComputeSizeFlags as the arguments in
some ReflowInput's methods. Because nsIFrame.h includes ReflowInput.h,
to solve the circular dependency, ComputeSizeFlags needs to be moved to
somewhere else.

This change shouldn't change behavior.

Depends on D89541

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1663822 Part 4 - Add a ComputeSizeFlags parameter to ReflowInput's constructor, and store it in ReflowInput.

Currently, to set ComputeSizeFlags, the caller uses an anonymous enum as
a parameter to ReflowInput constructor to set ReflowInputFlags fields,
and then ReflowInput creates a ComputeSizeFlags from the relevant
ReflowInputFlags fields in Init().

This patch simplifies this flags handover by adding ComputeSizeFlags
parameter to ReflowInput so that the caller can create the flags and
pass it to ReflowInput directly. The can also simplifies the process
needed to add a new ComputeSizeFlag.

We still need to store ComputeSizeFlags in ReflowInput since there's one
caller in nsBlockFrame::ComputeFinalSize wanting to checking
ComputeSizeFlag::BClampMarginBoxMinSize.

Note 1: ComputeSizeFlags is added only to the ReflowInput's constructor
that also takes parent ReflowInput. The other constructor's existing
callers don't need it.

Note 2: I don't bother adjust the value of DUMMY_PARENT_REFLOW_INPUT,
CALLER_WILL_INIT, and STATIC_POS_IS_CB_ORIGIN because they are going to
be converted into a enum class in a later patch.

This change shouldn't change behavior.

Depends on D89542

Comment 5

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1663822 Part 5 - Move ReflowInputFlags from SizeComputationInput to ReflowInput.

In previous part, we changed SizeComputationInput::InitOffsets to take a
ComputeSizeFlags parameter instead of ReflowInputFlags. Now there's no
reason to put ReflowInputFlags under SizeComputationInput.

Also rename it to Flags because it now lives in ReflowInput. We'll
use the name more in the next part (in ReflowInput's constructor).

This change shouldn't change behavior.

Depends on D89543

Comment 6

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1663822 Part 6 - Convert the anonymous enum controlling ReflowInput's behavior into an EnumSet.

This change shouldn't change behavior.

Depends on D89544

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1663822 Part 7 - Use constructor delegation for SizeComputationInput.

Depends on D89545

Comment 8

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1663822 Part 8 - Revise nsIFrame::ComputeSize()'s documentation.

Depends on D89546

Comment 9

[Daniel Holbert [:dholbert]]

This is great - thanks for doing this cleanup & for splitting it out so nicely!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/070f9ba9109a
Part 1 - Drop the "e" prefix from the enum values in ComputeSizeFlags. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/8e587833e629
Part 2 - Change ComputeSizeFlags into an EnumSet. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/fbc1eaa18e51
Part 3 - Move ComputeSizeFlags to LayoutConstants.h, and put it under mozilla namespace. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/bfd617a93523
Part 4 - Add a ComputeSizeFlags parameter to ReflowInput's constructor, and store it in ReflowInput. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/bcc431e93c8e
Part 5 - Move ReflowInputFlags from SizeComputationInput to ReflowInput. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/ff505e83d042
Part 6 - Convert the anonymous enum controlling ReflowInput's behavior into an EnumSet. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/c7547d0ef4be
Part 7 - Use constructor delegation for SizeComputationInput. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/c97d502f76a2
Part 8 - Revise nsIFrame::ComputeSize()'s documentation. r=dholbert

Comment 11

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/070f9ba9109a
https://hg.mozilla.org/mozilla-central/rev/8e587833e629
https://hg.mozilla.org/mozilla-central/rev/fbc1eaa18e51
https://hg.mozilla.org/mozilla-central/rev/bfd617a93523
https://hg.mozilla.org/mozilla-central/rev/bcc431e93c8e
https://hg.mozilla.org/mozilla-central/rev/ff505e83d042
https://hg.mozilla.org/mozilla-central/rev/c7547d0ef4be
https://hg.mozilla.org/mozilla-central/rev/c97d502f76a2