Closed Bug 1466448 Opened Last year Closed Last year

remove ReflowOutput::mFlags

Categories

(Core :: Layout, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: heycam, Assigned: heycam)

Details

Attachments

(2 files)

ReflowOutput::mFlags is effectively unused, so let's remove it.
Comment on attachment 8982994 [details]
Bug 1466448 - Part 1: Remove unused NS_REFLOW_CALC_BOUNDING_METRICS flag.

https://reviewboard.mozilla.org/r/248836/#review254972
Attachment #8982994 - Flags: review?(jfkthame) → review+
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?
Attachment #8982995 - Flags: review?(mats) → review+
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.
Erh, without the aFlags of course. :-)
Assignee: nobody → cam
Status: NEW → ASSIGNED
Pushed by cam@mcc.id.au:
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
https://hg.mozilla.org/mozilla-central/rev/607a37a3553f
https://hg.mozilla.org/mozilla-central/rev/83ad729b0967
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.