Bug 1299736 - Remove unsafe optimizations from FrameHasPositionedPlaceholderDescendants. r?bzbarsky
MozReview-Commit-ID: 4DYtXqNYfPq
--- a/layout/base/RestyleManagerBase.cpp
+++ b/layout/base/RestyleManagerBase.cpp
@@ -673,26 +673,24 @@ FrameHasPositionedPlaceholderDescendants
// they ignore their position style ... but they can't.
NS_ASSERTION(!outOfFlow->IsSVGText(),
"SVG text frames can't be out of flow");
if (aPositionMask & (1 << outOfFlow->StyleDisplay()->mPosition)) {
return true;
}
}
uint32_t positionMask = aPositionMask;
- // Is it faster to check aPositionMask & (1 << NS_STYLE_POSITION_ABSOLUTE)
- // before we check IsAbsPosContainingBlock? Not clear....
- if (f->IsAbsPosContainingBlock()) {
- if (f->IsFixedPosContainingBlock()) {
- continue;
- }
- // We don't care about absolutely positioned things inside f, because
- // they will still use f as their containing block.
- positionMask = (1 << NS_STYLE_POSITION_FIXED);
- }
+ // NOTE: It's tempting to check f->IsAbsPosContainingBlock() or
+ // f->IsFixedPosContainingBlock() here. However, that would only
+ // be testing the *new* style of the frame, which might exclude
+ // descendants that currently have this frame as an abs-pos
+ // containing block. Taking the codepath where we don't reframe
+ // could lead to an unsafe call to
+ // cont->MarkAsNotAbsoluteContainingBlock() before we've reframed
+ // the descendant and taken it off the absolute list.
if (FrameHasPositionedPlaceholderDescendants(f, positionMask)) {
return true;
}
}
}
return false;
}
@@ -1130,17 +1128,27 @@ RestyleManagerBase::ProcessRestyledFrame
// descendants).
if (cont->IsAbsPosContainingBlock()) {
if (!cont->IsAbsoluteContainer() &&
(cont->GetStateBits() & NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN)) {
cont->MarkAsAbsoluteContainingBlock();
}
} else {
if (cont->IsAbsoluteContainer()) {
- cont->MarkAsNotAbsoluteContainingBlock();
+ if (cont->HasAbsolutelyPositionedChildren()) {
+ // If |cont| still has absolutely positioned children,
+ // we can't call MarkAsNotAbsoluteContainingBlock. This
+ // will remove a frame list that still has children in
+ // it that we need to keep track of.
+ // The optimization of removing it isn't particularly
+ // important, although it does mean we skip some tests.
+ NS_WARNING("skipping removal of absolute containing block");
+ } else {
+ cont->MarkAsNotAbsoluteContainingBlock();
+ }
}
}
}
}
}
if ((hint & nsChangeHint_AddOrRemoveTransform) && frame &&
!(hint & nsChangeHint_ReconstructFrame)) {
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1299736-1.html
@@ -0,0 +1,15 @@
+<!DOCTYPE HTML>
+<title>Testcase, bug 1299736</title>
+
+<div id="A" style="transform: translateX(50px)">
+ <div id="B">
+ <div id="C" style="position: fixed">
+ </div>
+ </div>
+</div>
+
+<script>
+ document.getElementById("C").offsetLeft; // flush
+ document.getElementById("B").style.transform = "translateX(50px)";
+ document.getElementById("A").style.transform = "";
+</script>
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -473,8 +473,9 @@ load 1163583.html
load 1234622-1.html
load 1235467-1.html
pref(dom.webcomponents.enabled,true) load 1261351.html
load 1270797-1.html
load 1278455-1.html
load 1286889.html
load 1297835.html
load 1288608.html
+load 1299736-1.html
