799207 - Assertion failure: "using uninitialized baseline offset" with flexbox

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

With
  user_pref("layout.css.flexbox.enabled", true);
the testcase triggers:

Assertion failure: mCrossStartToFurthestBaseline != (-nscoord(1 << 30)) (using uninitialized baseline offset), at layout/generic/nsFlexContainerFrame.cpp:1659

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Daniel Holbert [:dholbert]]

/me starts writing on chalkboard:
> I will not write fatal assertions that assume reasonably-sized content.
> I will not write fatal assertions that assume reasonably-sized content.
> [etc]

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix

So -- our "mCrossStartToFurthestBaseline"-determination code starts out at negative infinity (nscoord_MIN), and then assumes that any baseline-aligned content will bring it up to a larger value, with a series of these for each baseline-aligned flex item:
>     mCrossStartToFurthestBaseline = NS_MAX(mCrossStartToFurthestBaseline,
>                                            crossStartToBaseline);

In this case, "crossStartToBaseline" is _more negative_ than nscoord_MIN (since nscoord_MIN isn't strictly the lowest representable nscoord value).  So we leave mCrossStartToFurthestBaseline at its initial value (representing "negative infinity").

I think it's fine to just warn in this case.  We could sprinkle some nscoord-saturating addition around, too, but I don't think it'd really help, since we'd still end up with nscoord_MIN.

Comment 4

[Daniel Holbert [:dholbert]]

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

Sorry, I had two debugging sessions mixed up.  I thought we were dealing with gigantic negative values, but we're actually dealing with gigantic positive values, which when added together, wrap into negative territory (to a value below nscoord_MIN).

So -- we do need to reduce the severity of the assertion, because we could legitimately get values as low as nscoord_MIN. But we also probably want some saturating addition to keep us from wrapping around from positive to negative.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: fix v2

OK, so this fix has a saturating-addition tweak, to keep us from overflowing into negative territory from adding an nscoord_MAX margin-top to an nscoord_MAX ascent.

With that, Jesse's testcase no longer triggers the assertion, so I added another crashtest that *does* still trigger it (with a giant negative margin-top), to justify still reducing the severity.

Comment 6

[Daniel Holbert [:dholbert]]

(FWIW, the "asserts(12)" annotation on the new crashtest is for a bunch of
  "ASSERTION: nscoord addition will reach or pass nscoord_MIN"
  "ASSERTION: NSCoordSaturatingAdd got nscoord_MIN as argument"
type assertions, for adding our margin-top (nscoord_MIN) to our ascent (0).)

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 669345 [details] [diff] [review]
fix v2

r=dbaron

Comment 8

[Daniel Holbert [:dholbert]]

Unprefixed testcase (per bug 801098), and pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/16adea14324f

Comment 9

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/16adea14324f