Skip to content

Commit

Permalink
Ensure we always have a float manager during block-frame reflow.
Browse files Browse the repository at this point in the history 
Normally we shouldn't be starting reflow from a block that is not a BFC,
and so BlockNeedsFloatManager would return true for the reflow root.

What happens in the testcase here, though, is that we call PresShell::FrameNeedsReflow
for the <article> block (because we've just added a child to it), which is fine:
it's a BFC at that moment.

Then we restyle, and during restyling we remove the NS_BLOCK_BFC flag because the
newly-added element means that the <article> no longer matches ::only-child, and
so we lose the `contain` property that was previously set. So <article> is no longer
a BFC, but we've already recorded it in the PresShell's mDirtyRoots.

We then call PresShell::FrameNeedsReflow for the foreignObject, which adds that
to mDirtyRoots as well.

And then we flush layout, reflowing first the foreignObject (because it is the
shallower of the two dirty-roots) and then the <article> block. We might expect
that the reflow of the SVGForeignObject would fix things, because it would reflow
all its descendants (including <article>) safely, but there's an early-return in
SVGForeignObjectFrame::Reflow in the case where it is zero-sized (which it is here).

So the <article> block remains dirty, and PresShell::ProcessReflowCommands tries
to reflow it directly even though it is no longer a BFC, and that's when we crash
due to not having a float manager.

Removing that early-return from SVGForeignObjectFrame::Reflow would avoid the crash,
but in any case I think we should make nsBlockFrame::Reflow handle this situation
as there may be other code-paths that potentially set up a similar scenario of
attempting to reflow from a root that has lost its BFC-ness.

Differential Revision: https://phabricator.services.mozilla.com/D228778

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1928724
gecko-commit: 421d7bf240078ff30871183289ce376441fadb09
gecko-reviewers: layout-reviewers, emilio
  • Loading branch information
@jfkthame @moz-wptsync-bot
jfkthame authored and moz-wptsync-bot committed Nov 14, 2024
1 parent ae8ec92 commit d0fa23b
Showing 1 changed file with 25 additions and 0 deletions.
25 changes: 25 additions & 0 deletions css/css-contain/crashtests/contain-dynamic-change-crash.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1928724">

<style>
*:only-child {
contain: size layout paint;
}
</style>

<script>
document.addEventListener("DOMContentLoaded", () => {
a.getBoundingClientRect();
a.appendChild(c);
b.scrollBy();
})
</script>

<body>
<svg>
<animateMotion id="b">
</animateMotion>
<foreignObject id="a">
<article>
<data id="c">

0 comments on commit d0fa23b

Please sign in to comment.