From ae652b0b37b58200a1667fe9211ef4ba24600335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Oct 2017 21:08:50 +0200 Subject: [PATCH] Bug 1404324: Always remove out of flows from the placeholder frame, using its info. r=bz MozReview-Commit-ID: 3Dt0wF2XRAH --- layout/base/nsCSSFrameConstructor.cpp | 8 +++-- layout/base/nsLayoutUtils.cpp | 46 +++---------------------- layout/generic/nsPlaceholderFrame.cpp | 27 +++++++++++++-- layout/generic/nsPlaceholderFrame.h | 21 +++++------ layout/style/crashtests/1404324-1.html | 12 +++++++ layout/style/crashtests/1404324-2.html | 10 ++++++ layout/style/crashtests/1404324-3.html | 14 ++++++++ layout/style/crashtests/crashtests.list | 3 ++ 8 files changed, 83 insertions(+), 58 deletions(-) create mode 100644 layout/style/crashtests/1404324-1.html create mode 100644 layout/style/crashtests/1404324-2.html create mode 100644 layout/style/crashtests/1404324-3.html diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index ef8b2817f2da..0dbb53c26214 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -1260,8 +1260,10 @@ nsFrameConstructorState::GetOutOfFlowFrameItems(nsIFrame* aNewFrame, if (disp->mTopLayer != NS_STYLE_TOP_LAYER_NONE) { *aPlaceholderType = PLACEHOLDER_FOR_TOPLAYER; if (disp->mPosition == NS_STYLE_POSITION_FIXED) { + *aPlaceholderType |= PLACEHOLDER_FOR_FIXEDPOS; return &mTopLayerFixedItems; } + *aPlaceholderType |= PLACEHOLDER_FOR_ABSPOS; return &mTopLayerAbsoluteItems; } if (disp->mPosition == NS_STYLE_POSITION_ABSOLUTE) { @@ -1303,11 +1305,11 @@ nsFrameConstructorState::ConstructBackdropFrameFor(nsIContent* aContent, nsAbsoluteItems* frameItems = GetOutOfFlowFrameItems(backdropFrame, true, true, false, &placeholderType); - MOZ_ASSERT(placeholderType == PLACEHOLDER_FOR_TOPLAYER); + MOZ_ASSERT(placeholderType & PLACEHOLDER_FOR_TOPLAYER); nsIFrame* placeholder = nsCSSFrameConstructor:: CreatePlaceholderFrameFor(mPresShell, aContent, backdropFrame, - frame, nullptr, PLACEHOLDER_FOR_TOPLAYER); + frame, nullptr, placeholderType); nsFrameList temp(placeholder, placeholder); frame->SetInitialChildList(nsIFrame::kBackdropList, temp); @@ -1361,7 +1363,7 @@ nsFrameConstructorState::AddChild(nsIFrame* aNewFrame, // Add the placeholder frame to the flow aFrameItems.AddChild(placeholderFrame); - if (placeholderType == PLACEHOLDER_FOR_TOPLAYER) { + if (placeholderType & PLACEHOLDER_FOR_TOPLAYER) { ConstructBackdropFrameFor(aContent, aNewFrame); } } diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index e4b56a4f69d9..658329358386 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -1528,6 +1528,8 @@ nsLayoutUtils::GetChildListNameFor(nsIFrame* aChildFrame) { nsIFrame::ChildListID id = nsIFrame::kPrincipalList; + MOZ_DIAGNOSTIC_ASSERT(!(aChildFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW)); + if (aChildFrame->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER) { nsIFrame* pif = aChildFrame->GetPrevInFlow(); if (pif->GetParent() == aChildFrame->GetParent()) { @@ -1536,35 +1538,6 @@ nsLayoutUtils::GetChildListNameFor(nsIFrame* aChildFrame) else { id = nsIFrame::kOverflowContainersList; } - } - // See if the frame is moved out of the flow - else if (aChildFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) { - // Look at the style information to tell - const nsStyleDisplay* disp = aChildFrame->StyleDisplay(); - - if (NS_STYLE_POSITION_ABSOLUTE == disp->mPosition) { - id = nsIFrame::kAbsoluteList; - } else if (NS_STYLE_POSITION_FIXED == disp->mPosition) { - if (nsLayoutUtils::IsReallyFixedPos(aChildFrame)) { - id = nsIFrame::kFixedList; - } else { - id = nsIFrame::kAbsoluteList; - } -#ifdef MOZ_XUL - } else if (StyleDisplay::MozPopup == disp->mDisplay) { - // Out-of-flows that are DISPLAY_POPUP must be kids of the root popup set -#ifdef DEBUG - nsIFrame* parent = aChildFrame->GetParent(); - NS_ASSERTION(parent && parent->IsPopupSetFrame(), "Unexpected parent"); -#endif // DEBUG - - id = nsIFrame::kPopupList; -#endif // MOZ_XUL - } else { - NS_ASSERTION(aChildFrame->IsFloating(), "not a floated frame"); - id = nsIFrame::kFloatList; - } - } else { LayoutFrameType childType = aChildFrame->Type(); if (LayoutFrameType::MenuPopup == childType) { @@ -1599,19 +1572,8 @@ nsLayoutUtils::GetChildListNameFor(nsIFrame* aChildFrame) nsContainerFrame* parent = aChildFrame->GetParent(); bool found = parent->GetChildList(id).ContainsFrame(aChildFrame); if (!found) { - if (!(aChildFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW)) { - found = parent->GetChildList(nsIFrame::kOverflowList) - .ContainsFrame(aChildFrame); - } - else if (aChildFrame->IsFloating()) { - found = parent->GetChildList(nsIFrame::kOverflowOutOfFlowList) - .ContainsFrame(aChildFrame); - if (!found) { - found = parent->GetChildList(nsIFrame::kPushedFloatsList) - .ContainsFrame(aChildFrame); - } - } - // else it's positioned and should have been on the 'id' child list. + found = parent->GetChildList(nsIFrame::kOverflowList) + .ContainsFrame(aChildFrame); NS_POSTCONDITION(found, "not in child list"); } #endif diff --git a/layout/generic/nsPlaceholderFrame.cpp b/layout/generic/nsPlaceholderFrame.cpp index 7da2582ae526..4b2e1bf84b36 100644 --- a/layout/generic/nsPlaceholderFrame.cpp +++ b/layout/generic/nsPlaceholderFrame.cpp @@ -25,9 +25,9 @@ using namespace mozilla::gfx; nsIFrame* NS_NewPlaceholderFrame(nsIPresShell* aPresShell, nsStyleContext* aContext, - nsFrameState aTypeBit) + nsFrameState aTypeBits) { - return new (aPresShell) nsPlaceholderFrame(aContext, aTypeBit); + return new (aPresShell) nsPlaceholderFrame(aContext, aTypeBits); } NS_IMPL_FRAMEARENA_HELPERS(nsPlaceholderFrame) @@ -155,6 +155,26 @@ nsPlaceholderFrame::Reflow(nsPresContext* aPresContext, NS_FRAME_SET_TRUNCATION(aStatus, aReflowInput, aDesiredSize); } +static nsIFrame::ChildListID +ChildListIDForOutOfFlow(nsFrameState aPlaceholderState, nsIFrame* aChild) +{ + if (aPlaceholderState & PLACEHOLDER_FOR_FLOAT) { + return nsIFrame::kFloatList; + } + if (aPlaceholderState & PLACEHOLDER_FOR_POPUP) { + return nsIFrame::kPopupList; + } + if (aPlaceholderState & PLACEHOLDER_FOR_FIXEDPOS) { + return nsLayoutUtils::IsReallyFixedPos(aChild) + ? nsIFrame::kFixedList : nsIFrame::kAbsoluteList; + } + if (aPlaceholderState & PLACEHOLDER_FOR_ABSPOS) { + return nsIFrame::kAbsoluteList; + } + MOZ_DIAGNOSTIC_ASSERT(false, "unknown list"); + return nsIFrame::kFloatList; +} + void nsPlaceholderFrame::DestroyFrom(nsIFrame* aDestructRoot) { @@ -162,12 +182,13 @@ nsPlaceholderFrame::DestroyFrom(nsIFrame* aDestructRoot) if (oof) { mOutOfFlowFrame = nullptr; oof->DeleteProperty(nsIFrame::PlaceholderFrameProperty()); + // If aDestructRoot is not an ancestor of the out-of-flow frame, // then call RemoveFrame on it here. // Also destroy it here if it's a popup frame. (Bug 96291) if ((GetStateBits() & PLACEHOLDER_FOR_POPUP) || !nsLayoutUtils::IsProperAncestorFrame(aDestructRoot, oof)) { - ChildListID listId = nsLayoutUtils::GetChildListNameFor(oof); + ChildListID listId = ChildListIDForOutOfFlow(GetStateBits(), oof); nsFrameManager* fm = PresContext()->GetPresShell()->FrameManager(); fm->RemoveFrame(listId, oof); } diff --git a/layout/generic/nsPlaceholderFrame.h b/layout/generic/nsPlaceholderFrame.h index e270c76cdd31..2af9b4ccbc93 100644 --- a/layout/generic/nsPlaceholderFrame.h +++ b/layout/generic/nsPlaceholderFrame.h @@ -40,7 +40,7 @@ nsIFrame* NS_NewPlaceholderFrame(nsIPresShell* aPresShell, nsStyleContext* aContext, - nsFrameState aTypeBit); + nsFrameState aTypeBits); #define PLACEHOLDER_TYPE_MASK (PLACEHOLDER_FOR_FLOAT | \ PLACEHOLDER_FOR_ABSPOS | \ @@ -65,18 +65,19 @@ public: */ friend nsIFrame* NS_NewPlaceholderFrame(nsIPresShell* aPresShell, nsStyleContext* aContext, - nsFrameState aTypeBit); - nsPlaceholderFrame(nsStyleContext* aContext, nsFrameState aTypeBit) + nsFrameState aTypeBits); + nsPlaceholderFrame(nsStyleContext* aContext, nsFrameState aTypeBits) : nsFrame(aContext, kClassID) , mOutOfFlowFrame(nullptr) { - NS_PRECONDITION(aTypeBit == PLACEHOLDER_FOR_FLOAT || - aTypeBit == PLACEHOLDER_FOR_ABSPOS || - aTypeBit == PLACEHOLDER_FOR_FIXEDPOS || - aTypeBit == PLACEHOLDER_FOR_POPUP || - aTypeBit == PLACEHOLDER_FOR_TOPLAYER, - "Unexpected type bit"); - AddStateBits(aTypeBit); + MOZ_ASSERT(aTypeBits == PLACEHOLDER_FOR_FLOAT || + aTypeBits == PLACEHOLDER_FOR_ABSPOS || + aTypeBits == PLACEHOLDER_FOR_FIXEDPOS || + aTypeBits == PLACEHOLDER_FOR_POPUP || + aTypeBits == (PLACEHOLDER_FOR_TOPLAYER | PLACEHOLDER_FOR_ABSPOS) || + aTypeBits == (PLACEHOLDER_FOR_TOPLAYER | PLACEHOLDER_FOR_FIXEDPOS), + "Unexpected type bit"); + AddStateBits(aTypeBits); } // Get/Set the associated out of flow frame diff --git a/layout/style/crashtests/1404324-1.html b/layout/style/crashtests/1404324-1.html new file mode 100644 index 000000000000..574a5437cb1b --- /dev/null +++ b/layout/style/crashtests/1404324-1.html @@ -0,0 +1,12 @@ + + diff --git a/layout/style/crashtests/1404324-2.html b/layout/style/crashtests/1404324-2.html new file mode 100644 index 000000000000..797347d5c058 --- /dev/null +++ b/layout/style/crashtests/1404324-2.html @@ -0,0 +1,10 @@ + + + diff --git a/layout/style/crashtests/1404324-3.html b/layout/style/crashtests/1404324-3.html new file mode 100644 index 000000000000..3b06f12a2bf6 --- /dev/null +++ b/layout/style/crashtests/1404324-3.html @@ -0,0 +1,14 @@ + + + diff --git a/layout/style/crashtests/crashtests.list b/layout/style/crashtests/crashtests.list index 114d9e9fd3f6..ab90fe5eb4c6 100644 --- a/layout/style/crashtests/crashtests.list +++ b/layout/style/crashtests/crashtests.list @@ -241,3 +241,6 @@ load 1403615.html load 1403712.html load 1404180-1.html load 1404316.html +load 1404324-1.html +load 1404324-2.html +load 1404324-3.html