1400421 - nsLeafFrame::Reflow / DoReflow seem to be dead code and want to be deleted

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Daniel Holbert [:dholbert]]

nsLeafFrame (an abstract superclass for some non-container frames) has its own override of nsIFrame::Reflow. It also adds a virtual helper function "DoReflow", which its Reflow implementation calls (and is the only caller of).


However, it seems both of these are effectively unused!
 * Firstly, no subclasses override ::DoReflow, as far as I can tell, so it has no reason to exist (it could conceptually be merged into Reflow).

 * More importantly: each subclass seems to provide its own Reflow impl, so nsLeafFrame's reflow method is just completely unused.  This means DoReflow is completely unused as well (on top of being needlessly virtual).

So, both methods are dead code and might as well just be deleted.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: Bug 1400421 part 1: Annotate class 'nsPageBreakFrame' as final, to reflect reality.

(This was the only concrete nsLeafBoxFrame subclass that was missing this
annotation.  So, after this patch, all concrete nsLeafBoxFrame subclasses will
have this annotation. Nice!)

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

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: Bug 1400421 part 2: Delete unused nsLeafFrame::Reflow override (and its DoReflow helper).

nsLeafFrame only exists as an abstract superclass, and every subclass actually
brings along its own Reflow impl, so nsLeafFrame's own impl is just dead code.

This patch makes sure it's really dead code by leaving in the Reflow() decl
with " = 0;" so the compiler will enforce that all concrete subclasses *do*
have a more specific override (and that they don't just inherit the stub
nsFrame impl from further up the ancestor chain, for example).

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

Comment 3

[Mats Palmgren (inactive)]

Comment on attachment 8908862 [details]
Bug 1400421 part 1: Annotate class 'nsPageBreakFrame' as final, to reflect reality.

https://reviewboard.mozilla.org/r/180476/#review185734

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 8908863 [details]
Bug 1400421 part 2: Delete unused nsLeafFrame::Reflow override (and its DoReflow helper).

https://reviewboard.mozilla.org/r/180480/#review185736

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c20a1a0d01ff
part 1: Annotate class 'nsPageBreakFrame' as final, to reflect reality. r=mats
https://hg.mozilla.org/integration/autoland/rev/5393e4f98858
part 2: Delete unused nsLeafFrame::Reflow override (and its DoReflow helper). r=mats

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c20a1a0d01ff
https://hg.mozilla.org/mozilla-central/rev/5393e4f98858