Backed out 3 changesets (bug 1344398) for assertion failures at Element.cpp a=backout

Backed out changeset d0e5a5ba01b5 (bug 1344398)
Backed out changeset d70f9de401d1 (bug 1344398)
Backed out changeset 647d0bb3714d (bug 1344398)

MozReview-Commit-ID: DTVWf28NcNb
This commit is contained in:
Wes Kocher 2017-05-10 17:43:50 -07:00
Родитель b7eda53134
Коммит 8f6058b583
19 изменённых файлов: 19 добавлений и 453 удалений

Просмотреть файл

@ -1805,24 +1805,6 @@ Element::UnbindFromTree(bool aDeep, bool aNullParent)
SetParentIsContent(false);
}
#ifdef DEBUG
// If we can get access to the PresContext, then we sanity-check that
// we're not leaving behind a pointer to ourselves as the PresContext's
// cached provider of the viewport's scrollbar styles.
if (document) {
nsIPresShell* presShell = document->GetShell();
if (presShell) {
nsPresContext* presContext = presShell->GetPresContext();
if (presContext) {
MOZ_ASSERT(this !=
presContext->GetViewportScrollbarStylesOverrideNode(),
"Leaving behind a raw pointer to this node (as having "
"propagated scrollbar styles) - that's dangerous...");
}
}
}
#endif
// Ensure that CSS transitions don't continue on an element at a
// different place in the tree (even if reinserted before next
// animation refresh).

Просмотреть файл

@ -454,7 +454,7 @@ RestyleManager::ChangeHintToString(nsChangeHint aHint)
"NeutralChange", "InvalidateRenderingObservers",
"ReflowChangesSizeOrPosition", "UpdateComputedBSize",
"UpdateUsesOpacity", "UpdateBackgroundPosition",
"AddOrRemoveTransform", "CSSOverflowChange",
"AddOrRemoveTransform"
};
static_assert(nsChangeHint_AllHints == (1 << ArrayLength(names)) - 1,
"Name list doesn't match change hints.");
@ -1348,62 +1348,6 @@ RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList)
FramePropertyTable* propTable = presContext->PropertyTable();
nsCSSFrameConstructor* frameConstructor = presContext->FrameConstructor();
// Handle nsChangeHint_CSSOverflowChange, by either updating the
// scrollbars on the viewport, or upgrading the change hint to frame-reconstruct.
for (nsStyleChangeData& data : aChangeList) {
if (data.mHint & nsChangeHint_CSSOverflowChange) {
data.mHint &= ~nsChangeHint_CSSOverflowChange;
bool doReconstruct = true; // assume the worst
// Only bother with this if we're html/body, since:
// (a) It'd be *expensive* to reframe these particular nodes. They're
// at the root, so reframing would mean rebuilding the world.
// (b) It's often *unnecessary* to reframe for "overflow" changes on
// these particular nodes. In general, the only reason we reframe
// for "overflow" changes is so we can construct (or destroy) a
// scrollframe & scrollbars -- and the html/body nodes often don't
// need their own scrollframe/scrollbars because they coopt the ones
// on the viewport (which always exist). So depending on whether
// that's happening, we can skip the reframe for these nodes.
if (data.mContent->IsAnyOfHTMLElements(nsGkAtoms::body,
nsGkAtoms::html)) {
// If the restyled element provided/provides the scrollbar styles for
// the viewport before and/or after this restyle, AND it's not coopting
// that responsibility from some other element (which would need
// reconstruction to make its own scrollframe now), THEN: we don't need
// to reconstruct - we can just reflow, because no scrollframe is being
// added/removed.
nsIContent* prevOverrideNode =
presContext->GetViewportScrollbarStylesOverrideNode();
nsIContent* newOverrideNode =
presContext->UpdateViewportScrollbarStylesOverride();
if (data.mContent == prevOverrideNode ||
data.mContent == newOverrideNode) {
// If we get here, the restyled element provided the scrollbar styles
// for viewport before this restyle, OR it will provide them after.
if (!prevOverrideNode || !newOverrideNode ||
prevOverrideNode == newOverrideNode) {
// If we get here, the restyled element is NOT replacing (or being
// replaced by) some other element as the viewport's
// scrollbar-styles provider. (If it were, we'd potentially need to
// reframe to create a dedicated scrollframe for whichever element
// is being booted from providing viewport scrollbar styles.)
//
// Under these conditions, we're OK to assume that this "overflow"
// change only impacts the root viewport's scrollframe, which
// already exists, so we can simply reflow instead of reframing.
data.mHint |= nsChangeHint_AllReflowHints;
doReconstruct = false;
}
}
}
if (doReconstruct) {
data.mHint |= nsChangeHint_ReconstructFrame;
}
}
}
// Make sure to not rebuild quote or counter lists while we're
// processing restyles
frameConstructor->BeginUpdate();

Просмотреть файл

@ -8488,19 +8488,11 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer,
ServoRestyleManager::ClearServoDataFromSubtree(aChild->AsElement());
}
nsPresContext* presContext = mPresShell->GetPresContext();
MOZ_ASSERT(presContext, "Our presShell should have a valid presContext");
if (aChild->IsHTMLElement(nsGkAtoms::body) ||
(!aContainer && aChild->IsElement())) {
// We might be removing the element that we propagated viewport scrollbar
// styles from. Recompute those. (This clause covers two of the three
// possible scrollbar-propagation sources: the <body> [as aChild or a
// descendant] and the root node. The other possible scrollbar-propagation
// source is a fullscreen element, and we have code elsewhere to update
// scrollbars after fullscreen elements are removed -- specifically, it's
// part of the fullscreen cleanup code called by Element::UnbindFromTree.)
presContext->UpdateViewportScrollbarStylesOverride();
// This might be the element we propagated viewport scrollbar
// styles from. Recompute those.
mPresShell->GetPresContext()->UpdateViewportScrollbarStylesOverride();
}
// XXXldb Do we need to re-resolve style to handle the CSS2 + combinator and
@ -8562,6 +8554,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer,
ClearDisplayContentsIn(aChild, aContainer);
}
nsPresContext* presContext = mPresShell->GetPresContext();
#ifdef MOZ_XUL
if (NotifyListBoxBody(presContext, aContainer, aChild, aOldNextSibling,
childFrame, CONTENT_REMOVED)) {

Просмотреть файл

@ -222,16 +222,6 @@ enum nsChangeHint : uint32_t {
*/
nsChangeHint_AddOrRemoveTransform = 1 << 27,
/**
* Indicates that the overflow-x and/or overflow-y property changed.
*
* In most cases, this is equivalent to nsChangeHint_ReconstructFrame. But
* in some special cases where the change is really targeting the viewport's
* scrollframe, this is instead equivalent to nsChangeHint_AllReflowHints
* (because the viewport always has an associated scrollframe).
*/
nsChangeHint_CSSOverflowChange = 1 << 28,
// IMPORTANT NOTE: When adding a new hint, you will need to add it to
// one of:
//
@ -247,7 +237,7 @@ enum nsChangeHint : uint32_t {
/**
* Dummy hint value for all hints. It exists for compile time check.
*/
nsChangeHint_AllHints = (1 << 29) - 1,
nsChangeHint_AllHints = (1 << 28) - 1,
};
// Redefine these operators to return nothing. This will catch any use
@ -336,7 +326,6 @@ inline nsChangeHint operator^=(nsChangeHint& aLeft, nsChangeHint aRight)
#define nsChangeHint_Hints_NeverHandledForDescendants ( \
nsChangeHint_BorderStyleNoneChange | \
nsChangeHint_ChildrenOnlyTransform | \
nsChangeHint_CSSOverflowChange | \
nsChangeHint_InvalidateRenderingObservers | \
nsChangeHint_RecomputePosition | \
nsChangeHint_UpdateBackgroundPosition | \

Просмотреть файл

@ -231,7 +231,6 @@ nsPresContext::nsPresContext(nsIDocument* aDocument, nsPresContextType aType)
mFocusBackgroundColor(mBackgroundColor),
mFocusTextColor(mDefaultColor),
mBodyTextColor(mDefaultColor),
mViewportScrollbarOverrideNode(nullptr),
mViewportStyleScrollbar(NS_STYLE_OVERFLOW_AUTO, NS_STYLE_OVERFLOW_AUTO),
mFocusRingWidth(1),
mExistThrottledUpdates(false),
@ -1497,10 +1496,10 @@ nsPresContext::UpdateViewportScrollbarStylesOverride()
// Start off with our default styles, and then update them as needed.
mViewportStyleScrollbar = ScrollbarStyles(NS_STYLE_OVERFLOW_AUTO,
NS_STYLE_OVERFLOW_AUTO);
mViewportScrollbarOverrideNode = nullptr;
nsIContent* propagatedFrom = nullptr;
// Don't propagate the scrollbar state in printing or print preview.
if (!IsPaginated()) {
mViewportScrollbarOverrideNode =
propagatedFrom =
GetPropagatedScrollbarStylesForViewport(this, &mViewportStyleScrollbar);
}
@ -1512,12 +1511,13 @@ nsPresContext::UpdateViewportScrollbarStylesOverride()
// the styles are from, so that the state of those elements is not
// affected across fullscreen change.
if (fullscreenElement != document->GetRootElement() &&
fullscreenElement != mViewportScrollbarOverrideNode) {
fullscreenElement != propagatedFrom) {
mViewportStyleScrollbar = ScrollbarStyles(NS_STYLE_OVERFLOW_HIDDEN,
NS_STYLE_OVERFLOW_HIDDEN);
}
}
return mViewportScrollbarOverrideNode;
return propagatedFrom;
}
bool

Просмотреть файл

@ -737,18 +737,7 @@ public:
* it was propagated from.
*/
nsIContent* UpdateViewportScrollbarStylesOverride();
/**
* Returns the cached result from the last call to
* UpdateViewportScrollbarStylesOverride() -- i.e. return the node
* whose scrollbar styles we have propagated to the viewport (or nullptr if
* there is no such node).
*/
nsIContent* GetViewportScrollbarStylesOverrideNode() const {
return mViewportScrollbarOverrideNode;
}
const ScrollbarStyles& GetViewportScrollbarStylesOverride() const
const ScrollbarStyles& GetViewportScrollbarStylesOverride()
{
return mViewportStyleScrollbar;
}
@ -1396,16 +1385,7 @@ protected:
nscolor mBodyTextColor;
// This is a non-owning pointer. May be null. If non-null, it's guaranteed
// to be pointing to a node that's still alive, because we'll reset it in
// UpdateViewportScrollbarStylesOverride() as part of the cleanup code
// when this node is removed from the document. (For <body> and the root node,
// this call happens in nsCSSFrameConstructor::ContentRemoved(). For
// fullscreen elements, it happens in the fullscreen-specific cleanup
// invoked by Element::UnbindFromTree().)
nsIContent* MOZ_NON_OWNING_REF mViewportScrollbarOverrideNode;
ScrollbarStyles mViewportStyleScrollbar;
uint8_t mFocusRingWidth;
bool mExistThrottledUpdates;

Просмотреть файл

@ -1,18 +0,0 @@
<!DOCTYPE html>
<html>
<head>
<title>
Reference case with body and html *independently* scrollable.
</title>
<style>
html {
overflow: scroll;
}
body {
overflow: scroll;
}
</style>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -1,23 +0,0 @@
<!DOCTYPE html>
<html class="reftest-wait">
<head>
<title>
Testcase with body and html *independently* scrollable,
with body's "overflow" set dynamically.
</title>
<style>
html {
overflow: scroll;
}
</style>
<script>
function doTest() {
document.body.style.overflow = "scroll";
document.documentElement.removeAttribute("class");
}
window.addEventListener("MozReftestInvalidate", doTest);
</script>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -1,23 +0,0 @@
<!DOCTYPE html>
<html class="reftest-wait">
<head>
<title>
Testcase with body and html *independently* scrollable,
with html's "overflow" set dynamically.
</title>
<style>
body {
overflow: scroll;
}
</style>
<script>
function doTest() {
document.documentElement.style.overflow = "scroll";
document.documentElement.removeAttribute("class");
}
window.addEventListener("MozReftestInvalidate", doTest);
</script>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -1,19 +0,0 @@
<!DOCTYPE html>
<html class="reftest-wait">
<head>
<title>
Testcase with body and html *independently* scrollable,
with both html & body's "overflow" set dynamically.
</title>
<script>
function doTest() {
document.documentElement.style.overflow = "scroll";
document.body.style.overflow = "scroll";
document.documentElement.removeAttribute("class");
}
window.addEventListener("MozReftestInvalidate", doTest);
</script>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -1,15 +0,0 @@
<!DOCTYPE html>
<html>
<head>
<title>
Reference case with the root viewport scrollable, via styles on html node.
</title>
<style>
html {
overflow: scroll;
}
</style>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -1,26 +0,0 @@
<!DOCTYPE html>
<html class="reftest-wait">
<head>
<title>
Testcase with only one of [html,body] being scrollable,
after body's "overflow" is reset dynamically.
</title>
<style>
html {
overflow: scroll;
}
body {
overflow: scroll;
}
</style>
<script>
function doTest() {
document.body.style.overflow = "visible";
document.documentElement.removeAttribute("class");
}
window.addEventListener("MozReftestInvalidate", doTest);
</script>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -1,26 +0,0 @@
<!DOCTYPE html>
<html class="reftest-wait">
<head>
<title>
Testcase with only one of [html,body] being scrollable,
after html's "overflow" is reset dynamically.
</title>
<style>
html {
overflow: scroll;
}
body {
overflow: scroll;
}
</style>
<script>
function doTest() {
document.documentElement.style.overflow = "visible";
document.documentElement.removeAttribute("class");
}
window.addEventListener("MozReftestInvalidate", doTest);
</script>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -1,24 +0,0 @@
<!DOCTYPE html>
<html class="reftest-wait">
<head>
<title>
Testcase with only one of [html,body] being scrollable,
with their "overflow" styles being dynamically swapped.
</title>
<style>
html {
overflow: scroll;
}
</style>
<script>
function doTest() {
document.documentElement.style.overflow = "visible";
document.body.style.overflow = "scroll";
document.documentElement.removeAttribute("class");
}
window.addEventListener("MozReftestInvalidate", doTest);
</script>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -1,24 +0,0 @@
<!DOCTYPE html>
<html class="reftest-wait">
<head>
<title>
Testcase with only one of [html,body] being scrollable,
with their "overflow" styles being dynamically swapped.
</title>
<style>
body {
overflow: scroll;
}
</style>
<script>
function doTest() {
document.documentElement.style.overflow = "scroll";
document.body.style.overflow = "visible";
document.documentElement.removeAttribute("class");
}
window.addEventListener("MozReftestInvalidate", doTest);
</script>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -1,15 +0,0 @@
<!DOCTYPE html>
<html>
<head>
<title>
Testcase with the root viewport scrollable, via styles on body node.
</title>
<style>
body {
overflow: scroll;
}
</style>
</head>
<body>
</body>
</html>

Просмотреть файл

@ -86,13 +86,3 @@ fuzzy-if(asyncPan&&!layersGPUAccelerated,102,2420) == frame-scrolling-attr-2.htm
== fractional-scroll-area.html?top=0.4&outerBottom=99.6&innerBottom=200.4&scrollBefore=999 fractional-scroll-area.html?top=0&outerBottom=100&innerBottom=200&scrollBefore=999
== fractional-scroll-area.html?top=0.4&outerBottom=100.4&innerBottom=200.4&scrollBefore=999 fractional-scroll-area.html?top=0&outerBottom=100&innerBottom=200&scrollBefore=999
!= fractional-scroll-area-invalidation.html about:blank
# Tests for "overflow" styles that may be propagated to the viewport:
== propagated-overflow-style-1a.html propagated-overflow-style-1-ref.html
== propagated-overflow-style-1b.html propagated-overflow-style-1-ref.html
== propagated-overflow-style-1c.html propagated-overflow-style-1-ref.html
== propagated-overflow-style-2a.html propagated-overflow-style-2-ref.html
== propagated-overflow-style-2b.html propagated-overflow-style-2-ref.html
== propagated-overflow-style-2c.html propagated-overflow-style-2-ref.html
== propagated-overflow-style-2d.html propagated-overflow-style-2-ref.html
== propagated-overflow-style-2e.html propagated-overflow-style-2-ref.html

Просмотреть файл

@ -3488,6 +3488,8 @@ nsStyleDisplay::CalcDifference(const nsStyleDisplay& aNewData) const
|| mDisplay != aNewData.mDisplay
|| mContain != aNewData.mContain
|| (mFloat == StyleFloat::None) != (aNewData.mFloat == StyleFloat::None)
|| mOverflowX != aNewData.mOverflowX
|| mOverflowY != aNewData.mOverflowY
|| mScrollBehavior != aNewData.mScrollBehavior
|| mScrollSnapTypeX != aNewData.mScrollSnapTypeX
|| mScrollSnapTypeY != aNewData.mScrollSnapTypeY
@ -3499,11 +3501,6 @@ nsStyleDisplay::CalcDifference(const nsStyleDisplay& aNewData) const
hint |= nsChangeHint_ReconstructFrame;
}
if (mOverflowX != aNewData.mOverflowX
|| mOverflowY != aNewData.mOverflowY) {
hint |= nsChangeHint_CSSOverflowChange;
}
/* Note: When mScrollBehavior, mScrollSnapTypeX, mScrollSnapTypeY,
* mScrollSnapPointsX, mScrollSnapPointsY, or mScrollSnapDestination are
* changed, nsChangeHint_NeutralChange is not sufficient to enter

Просмотреть файл

@ -95,90 +95,6 @@ const gTestcases = [
expectReflow: true,
},
// * Changing 'overflow' on <body> should cause reflow,
// but not frame reconstruction
{
elem: document.body,
/* beforeStyle: implicitly 'overflow:visible' */
afterStyle: "overflow: hidden",
expectConstruction: false,
expectReflow: true,
},
{
elem: document.body,
/* beforeStyle: implicitly 'overflow:visible' */
afterStyle: "overflow: scroll",
expectConstruction: false,
expectReflow: true,
},
{
elem: document.body,
beforeStyle: "overflow: hidden",
afterStyle: "overflow: auto",
expectConstruction: false,
expectReflow: true,
},
{
elem: document.body,
beforeStyle: "overflow: hidden",
afterStyle: "overflow: scroll",
expectConstruction: false,
expectReflow: true,
},
{
elem: document.body,
beforeStyle: "overflow: hidden",
afterStyle: "overflow: visible",
expectConstruction: false,
expectReflow: true,
},
{
elem: document.body,
beforeStyle: "overflow: auto",
afterStyle: "overflow: hidden",
expectConstruction: false,
expectReflow: true,
},
{
elem: document.body,
beforeStyle: "overflow: visible",
afterStyle: "overflow: hidden",
expectConstruction: false,
expectReflow: true,
},
// * Changing 'overflow' on <html> should cause reflow,
// but not frame reconstruction
{
elem: document.documentElement,
/* beforeStyle: implicitly 'overflow:visible' */
afterStyle: "overflow: auto",
expectConstruction: false,
expectReflow: true,
},
{
elem: document.documentElement,
beforeStyle: "overflow: visible",
afterStyle: "overflow: auto",
expectConstruction: false,
expectReflow: true,
},
// * Setting 'overflow' on arbitrary node should cause reflow as well as
// frame reconstruction
{
/* beforeStyle: implicitly 'overflow:visible' */
afterStyle: "overflow: auto",
expectConstruction: true,
expectReflow: true,
},
{
beforeStyle: "overflow: auto",
afterStyle: "overflow: visible",
expectConstruction: true,
expectReflow: true,
},
// * Changing 'display' should cause frame construction and reflow.
{
beforeStyle: "display: inline",
@ -219,35 +135,23 @@ function runOneTest(aTestcase)
return;
}
// Figure out which element we'll be tweaking (defaulting to gElem)
let elem = aTestcase.elem ?
aTestcase.elem : gElem;
// Verify that 'style' attribute is unset (avoid causing ourselves trouble):
if (elem.hasAttribute("style")) {
ok(false,
"test element has 'style' attribute already set! We're going to stomp " +
"on whatever's there when we clean up...");
}
// Set the "before" style, and compose the first part of the message
// to be used in our "is"/"isnot" invocations:
let msgPrefix = "Changing style ";
if (aTestcase.beforeStyle) {
elem.setAttribute("style", aTestcase.beforeStyle);
gElem.setAttribute("style", aTestcase.beforeStyle);
msgPrefix += "from '" + aTestcase.beforeStyle + "' ";
}
msgPrefix += "to '" + aTestcase.afterStyle + "' ";
msgPrefix += "on " + elem.nodeName + " ";
// Establish initial counts:
let unusedVal = elem.offsetHeight; // flush layout
let unusedVal = gElem.offsetHeight; // flush layout
let origFramesConstructed = gUtils.framesConstructed;
let origFramesReflowed = gUtils.framesReflowed;
// Make the change and flush:
elem.setAttribute("style", aTestcase.afterStyle);
unusedVal = elem.offsetHeight; // flush layout
gElem.setAttribute("style", aTestcase.afterStyle);
unusedVal = gElem.offsetHeight; // flush layout
// Make our is/isnot assertions about whether things should have changed:
checkFinalCount(gUtils.framesConstructed, origFramesConstructed,
@ -258,7 +162,7 @@ function runOneTest(aTestcase)
"reflow");
// Clean up!
elem.removeAttribute("style");
gElem.removeAttribute("style");
}
gTestcases.forEach(runOneTest);