1466448 - remove ReflowOutput::mFlags

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P3)

Description

[Cameron McCormack (:heycam)]

ReflowOutput::mFlags is effectively unused, so let's remove it.

Comment 1

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=487f5c346eb952703ad1fc854c898077eb680f14

Comment 2

[Cameron McCormack (:heycam)]

Attachment: Bug 1466448 - Part 1: Remove unused NS_REFLOW_CALC_BOUNDING_METRICS flag.

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

Comment 3

[Cameron McCormack (:heycam)]

Attachment: Bug 1466448 - Part 2: Remove unused ReflowOutput::mFlags.

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

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 8982994 [details]
Bug 1466448 - Part 1: Remove unused NS_REFLOW_CALC_BOUNDING_METRICS flag.

https://reviewboard.mozilla.org/r/248836/#review254972

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 8982995 [details]
Bug 1466448 - Part 2: Remove unused ReflowOutput::mFlags.

https://reviewboard.mozilla.org/r/248838/#review255060

::: layout/generic/ReflowOutput.h:203
(Diff revision 1)
>    // XXX width/height/ascent are OUT parameters and so they shouldn't
>    // have to be initialized, but there are some bad frame classes that
>    // aren't properly setting them when returning from Reflow()...

I think we should remove this comment too.
Leaving members uninitialized is generally bad.

We could perhaps initialize them to NS_UNCONSTRAINEDSIZE to hopefully catch
the bad code the comment is talking about, if we think that's important.

::: layout/generic/ReflowOutput.h:206
(Diff revision 1)
> -  // Some places do it, and some don't.  |aFlags| should perhaps go away
> -  // entirely.
>    // XXX width/height/ascent are OUT parameters and so they shouldn't
>    // have to be initialized, but there are some bad frame classes that
>    // aren't properly setting them when returning from Reflow()...
> -  explicit ReflowOutput(mozilla::WritingMode aWritingMode, uint32_t aFlags = 0)
> +  explicit ReflowOutput(mozilla::WritingMode aWritingMode)

I wonder if we can remove one of these ctors?
Most of the callers seems to pass aReflowInput.GetWritingMode() anyway and
the ones that don't are probably wrong.
Can you file a follow-up bug on this please?

Comment 6

[Mats Palmgren (inactive)]

Comment on attachment 8982995 [details]
Bug 1466448 - Part 2: Remove unused ReflowOutput::mFlags.

https://reviewboard.mozilla.org/r/248838/#review255072

::: layout/generic/ReflowOutput.cpp:42
(Diff revision 1)
> -ReflowOutput::ReflowOutput(const ReflowInput& aReflowInput,
> +ReflowOutput::ReflowOutput(const ReflowInput& aReflowInput)
> -                                         uint32_t aFlags)
>    : mISize(0)
>    , mBSize(0)
>    , mBlockStartAscent(ASK_FOR_BASELINE)
> -  , mFlags(aFlags)
>    , mWritingMode(aReflowInput.GetWritingMode())

Actually, what you can do in this bug is to change the above to:
  : ReflowOutput(aReflowInput.GetWritingMode(), aFlags)

to at least avoid the code duplication.

Comment 7

[Mats Palmgren (inactive)]

Erh, without the aFlags of course. :-)

Comment 8

[Cameron McCormack (:heycam)]

Comment on attachment 8982995 [details]
Bug 1466448 - Part 2: Remove unused ReflowOutput::mFlags.

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/607a37a3553f
Part 1: Remove unused NS_REFLOW_CALC_BOUNDING_METRICS flag. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/83ad729b0967
Part 2: Remove unused ReflowOutput::mFlags. r=mats

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/607a37a3553f
https://hg.mozilla.org/mozilla-central/rev/83ad729b0967