1008917 - Make nsIFrame::Reflow and friends return 'void'

Status: RESOLVED FIXED

(Core :: Layout, defect, P5)

Description

[Mats Palmgren (inactive)]

The return value isn't used for anything nowadays.  It used to report
OOM failures but those are all infallible now.  The only other use that
I know of was MathML which used it to propagate markup errors and do
some error handling, but someone told me that's not the case anymore.

Comment 1

[Mats Palmgren (inactive)]

Attachment: part 1, stop using the nsresult from Reflow methods in MathML code

This patch singles out the use of nsresult in MathML, and changes it
to NS_OK.  Later patches will remove it altogether.  Fred, does this
look OK to you?  (it's green on Try, fwiw)

Comment 2

[Mats Palmgren (inactive)]

Attachment: part 2, make Reflow() return type 'void' in header files (automated change)

Comment 3

[Mats Palmgren (inactive)]

Attachment: part 3, make Reflow() return type 'void' in .cpp files (automated change)

Comment 4

[Mats Palmgren (inactive)]

Attachment: part 4, fix Reflow() return statements and remove uses of Reflow() return values, and make a few reflow related helper methods 'void' too

Comment 5

[Frédéric Wang Nélar]

Comment on attachment 8420956 [details] [diff] [review]
part 1, stop using the nsresult from Reflow methods in MathML code

Review of attachment 8420956 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not aware of any reason why the nsresult was needed for MathML.
This looks good to me.

::: layout/mathml/nsMathMLTokenFrame.cpp
@@ +161,5 @@
>                                      childDesiredSize.mBoundingMetrics);
>  
>      childFrame = childFrame->GetNextSibling();
>    }
>  

whitespace change

Comment 6

[Mats Palmgren (inactive)]

Attachment: part 5, make ReflowChild() return type 'void' (automated change)

Comment 7

[Mats Palmgren (inactive)]

Attachment: part 6, make FinishReflowChild() return type 'void' (automated change)

Comment 8

[Mats Palmgren (inactive)]

Attachment: part 7, fix for ReflowChild() and FinishReflowChild() uses, and making a few related helper methods 'void' too

Comment 9

[Mats Palmgren (inactive)]

Attachment: part 8, make nsLeafFrame::DoRewflow() return type 'void'

Comment 10

[Mats Palmgren (inactive)]

Attachment: part 9, make DidReflow() return type 'void'

Comment 11

[Mats Palmgren (inactive)]

Attachment: part 10, fix uses of DidReflow() return values

Comment 12

[Mats Palmgren (inactive)]

Attachment: part 11, make nsBlockFrame::ReflowDirtyLines() and nsLineLayout::ReflowFrame() return types 'void'

Comment 13

[Mats Palmgren (inactive)]

Attachment: part 12, make nsFrame::WillReflow() return type 'void'

Comment 14

[Mats Palmgren (inactive)]

Attachment: part 13, make nsContainerFrame::ReflowOverflowContainerChildren() return type 'void'

Comment 15

[Mats Palmgren (inactive)]

Attachment: part 14, make nsAbsoluteContainingBlock::Reflow() return type 'void'

Comment 16

[Mats Palmgren (inactive)]

Attachment: part 15, make RestyleManager::StyleChangeReflow() return type 'void'

Comment 17

[Mats Palmgren (inactive)]

Attachment: part 16, make nsFrame::BoxReflow() return type 'void'

Comment 18

[Mats Palmgren (inactive)]

(In reply to Frédéric Wang (:fredw) from comment #5)
> This looks good to me.

Thanks for the quick review.

> whitespace change

Intentional - there was two lines of separation there and I couldn't
resist the temptation to fix it :-)

Comment 19

[Mats Palmgren (inactive)]

(That's all the patches I have for this bug)

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Please squash together the patches that need to land together to not break the build, so every hg changeset builds. Thanks! This is good stuff :-)

Comment 21

[Mats Palmgren (inactive)]

No problem, I folded parts 2+3+4, and 5+6+7, and 9+10, and verified it built properly
after each patch:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c380c21351f
https://hg.mozilla.org/integration/mozilla-inbound/rev/9afa5e7715e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8000b31277c
https://hg.mozilla.org/integration/mozilla-inbound/rev/25b2475d2368
https://hg.mozilla.org/integration/mozilla-inbound/rev/161c41991d46
https://hg.mozilla.org/integration/mozilla-inbound/rev/10ed89a302e9
https://hg.mozilla.org/integration/mozilla-inbound/rev/80304bb9a572
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7a38df1d44f
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ab5b01da4de
https://hg.mozilla.org/integration/mozilla-inbound/rev/b52154d8d900
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2ab653f688a

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

All backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/157af79fbc88 for apparently introducing an intermittent B2G crashtest-2 failure:

https://tbpl.mozilla.org/php/getParsedLog.php?id=39505889&tree=Mozilla-Inbound

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Wes Kocher (:KWierso) from comment #22)
> All backed out in
> https://hg.mozilla.org/integration/mozilla-inbound/rev/157af79fbc88 for
> apparently introducing an intermittent B2G crashtest-2 failure:
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=39505889&tree=Mozilla-
> Inbound

Retriggers on earlier builds had this failure, so this can probably safely reland.

Comment 24

[Mats Palmgren (inactive)]

No problem, will do so as soon as the tree reopens.

Comment 25

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/30d055013319
https://hg.mozilla.org/integration/mozilla-inbound/rev/edfbdecd9e82
https://hg.mozilla.org/integration/mozilla-inbound/rev/b37e7eabd9ef
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a5c32341b95
https://hg.mozilla.org/integration/mozilla-inbound/rev/8db1cef6a53d
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e2dead825d9
https://hg.mozilla.org/integration/mozilla-inbound/rev/a03df0fff293
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a854a7051ab
https://hg.mozilla.org/integration/mozilla-inbound/rev/32e8633c7cc0
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d161e4c59fe
https://hg.mozilla.org/integration/mozilla-inbound/rev/a13c7393880d

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/30d055013319
https://hg.mozilla.org/mozilla-central/rev/edfbdecd9e82
https://hg.mozilla.org/mozilla-central/rev/b37e7eabd9ef
https://hg.mozilla.org/mozilla-central/rev/4a5c32341b95
https://hg.mozilla.org/mozilla-central/rev/8db1cef6a53d
https://hg.mozilla.org/mozilla-central/rev/7e2dead825d9
https://hg.mozilla.org/mozilla-central/rev/a03df0fff293
https://hg.mozilla.org/mozilla-central/rev/6a854a7051ab
https://hg.mozilla.org/mozilla-central/rev/32e8633c7cc0
https://hg.mozilla.org/mozilla-central/rev/0d161e4c59fe
https://hg.mozilla.org/mozilla-central/rev/a13c7393880d