Bug 1728319 - Tweak an assertion to recognize integer overflow due to huge margin/border/padding. r=dholbert

We could fix the testcase in this patch by promoting
`FlexLine::mTotalItemMBP` to `AuCoord64`, but then
`layout/generic/crashtests/1488762-1.html` will start crashing. It is
because `FlexItem::OuterMainSize()` can return negative size due to
integer overflow, and the negative sizes will accumulate in
`mTotalOuterHypotheticalMainSize`.

Instead of promoting more variables to `AuCoord64` to workaround huge
margin/border/padding that we cannot handle gracefully, this patch
tweaks the assertion to check only if `mTotalOuterHypotheticalMainSize`
and `mTotalItemMBP` have valid values.

Differential Revision: https://phabricator.services.mozilla.com/D124409
This commit is contained in:
Ting-Yu Lin 2021-09-03 21:23:52 +00:00
parent d60e22aec0
commit 30a1f1e161
3 changed files with 29 additions and 6 deletions

View File

@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
<head>
<style>
* {
padding-right: 2589893033.302024Q;
display: inline-flex;
}
</style>
</head>
</html>

View File

@ -797,3 +797,4 @@ load 1697262-1.html
pref(layout.css.aspect-ratio.enabled,true) load 1682032.html
pref(layout.css.aspect-ratio.enabled,true) load 1699263.html
pref(layout.css.aspect-ratio.enabled,true) load 1699468.html
load 1728319.html

View File

@ -3059,14 +3059,25 @@ void FlexLine::ResolveFlexibleLengths(nscoord aFlexContainerMainSize,
availableFreeSpace -= item.MainSize();
}
FLEX_LOG(" available free space = %" PRId64, availableFreeSpace.value);
FLEX_LOG(" available free space: %" PRId64 "; flex items should \"%s\"",
availableFreeSpace.value, isUsingFlexGrow ? "grow" : "shrink");
// The sign of our free space should agree with the type of flexing
// (grow/shrink) that we're doing (except if we've had integer overflow,
// which is unlikely given we've used 64-bit arithmetic). Any disagreement
// should've made us use the other type of flexing, or should've been
// resolved in FreezeItemsEarly.
MOZ_ASSERT((isUsingFlexGrow && availableFreeSpace >= 0) ||
// (grow/shrink) that we're doing. Any disagreement should've made us use
// the other type of flexing, or should've been resolved in
// FreezeItemsEarly.
//
// Note: it's possible that an individual flex item has huge
// margin/border/padding that makes either its
// MarginBorderPaddingSizeInMainAxis() or OuterMainSize() negative due to
// integer overflow. If that happens, the accumulated
// mTotalOuterHypotheticalMainSize or mTotalItemMBP could be negative due to
// that one item's negative (overflowed) size. In that case, we throw up our
// hands and don't require isUsingFlexGrow to agree with availableFreeSpace.
// Luckily, we won't get stuck in the algorithm below, and just distribute
// the wrong availableFreeSpace with the wrong grow/shrink factors.
MOZ_ASSERT(!(mTotalOuterHypotheticalMainSize >= 0 && mTotalItemMBP >= 0) ||
(isUsingFlexGrow && availableFreeSpace >= 0) ||
(!isUsingFlexGrow && availableFreeSpace <= 0),
"availableFreeSpace's sign should match isUsingFlexGrow");