Closed Bug 1008917 Opened 6 years ago Closed 6 years ago

Make nsIFrame::Reflow and friends return 'void'

Categories

(Core :: Layout, defect, P5, trivial)

defect

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: mats, Assigned: mats)

References

Details

Attachments

(16 files)

17.45 KB, patch
fredw
: review+
Details | Diff | Splinter Review
50.56 KB, patch
roc
: review+
Details | Diff | Splinter Review
66.61 KB, patch
roc
: review+
Details | Diff | Splinter Review
216.92 KB, patch
roc
: review+
Details | Diff | Splinter Review
4.41 KB, patch
roc
: review+
Details | Diff | Splinter Review
2.41 KB, patch
roc
: review+
Details | Diff | Splinter Review
35.44 KB, patch
roc
: review+
Details | Diff | Splinter Review
3.88 KB, patch
roc
: review+
Details | Diff | Splinter Review
12.29 KB, patch
roc
: review+
Details | Diff | Splinter Review
7.95 KB, patch
roc
: review+
Details | Diff | Splinter Review
4.54 KB, patch
roc
: review+
Details | Diff | Splinter Review
4.06 KB, patch
roc
: review+
Details | Diff | Splinter Review
4.94 KB, patch
roc
: review+
Details | Diff | Splinter Review
3.79 KB, patch
roc
: review+
Details | Diff | Splinter Review
2.71 KB, patch
roc
: review+
Details | Diff | Splinter Review
6.27 KB, patch
roc
: review+
Details | Diff | Splinter Review
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.
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)
Attachment #8420956 - Flags: review?(fred.wang)
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
Attachment #8420956 - Flags: review?(fred.wang) → review+
Attachment #8420989 - Attachment description: make nsBlockFrame::ReflowDirtyLines() and nsLineLayout::ReflowFrame() return types 'void' → part 11, make nsBlockFrame::ReflowDirtyLines() and nsLineLayout::ReflowFrame() return types 'void'
(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 :-)
(That's all the patches I have for this bug)
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 :-)
(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.
No problem, will do so as soon as the tree reopens.
Flags: needinfo?(matspal)
You need to log in before you can comment on or make changes to this bug.