965897 - Make nsHTMLReflowState verify its "aPresContext Must be equal to aFrame->PresContext()" precondition

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

nsHTMLReflowState has this documentation on its constructor and its ::Init method:
> 468    * @param aPresContext Must be equal to aFrame->PresContext().
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsHTMLReflowState.h#468
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsHTMLReflowState.h#486

But we don't actually verify that anywhere, aside from a NS_PRECONDITION that checks that it's non-null.

Might as well add an explicit NS_PRECONDITION to enforce that documentation.

Comment 1

[Daniel Holbert [:dholbert]]

Sorry, by "on its constructor and its ::Init method" I meant "on its constructors".

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 3

[Daniel Holbert [:dholbert]]

Sanity-check try run, to be sure we don't violate this (at least, not in crashtests/reftests):
 https://tbpl.mozilla.org/?tree=Try&rev=622c501e0070

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 8368072 [details] [diff] [review]
fix

This is such a fundamental precondition that it should use MOZ_ASSERT.
r=mats with that.

The null-checks before it could also use MOZ_ASSERT but I suspect you
wouldn't get very far anyway :-)

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Mats Palmgren (:mats) from comment #4)
> The null-checks before it could also use MOZ_ASSERT but I suspect you
> wouldn't get very far anyway :-)

I took the liberty of converting the aFrame and aPresContext null-checks to be MOZ_ASSERTS, because we literally wouldn't be able to make it past my new MOZ_ASSERT if either of those values were null.

Landed, with that change:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/84b6f0aba30d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/84b6f0aba30d