Skip to content

Commit

Permalink
Backed out 4 changesets (bug 1799343) for causing multiple crashes. C…
Browse files Browse the repository at this point in the history
…LOSED TREE

Backed out changeset 3150074bccfd (bug 1799343)
Backed out changeset 7f6358a0b692 (bug 1799343)
Backed out changeset 34f040f379b1 (bug 1799343)
Backed out changeset a609c8c27ca8 (bug 1799343)
  • Loading branch information
Sandor Molnar committed Nov 8, 2022
1 parent 1ae11a4 commit 8e6fe2d
Show file tree
Hide file tree
Showing 25 changed files with 562 additions and 145 deletions.
2 changes: 2 additions & 0 deletions accessible/tests/mochitest/attributes/test_obj_css.xhtml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
// CSS display
testCSSAttrs("display_mozbox");
testCSSAttrs("display_mozinlinebox");
testCSSAttrs("display_mozpopup");

SimpleTest.finish();
}
Expand All @@ -48,6 +49,7 @@

<vbox id="display_mozbox" style="display: -moz-box;" role="img"/>
<vbox id="display_mozinlinebox" style="display: -moz-inline-box;" role="img"/>
<vbox id="display_mozpopup" style="display: -moz-popup;" role="img"/>

</hbox>
</window>
2 changes: 1 addition & 1 deletion browser/base/content/test/static/browser_parsable_css.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ let whitelist = [
isFromDevTools: false,
},
{
sourceName: /\b(xul|minimal-xul|html|mathml|ua|forms|svg|manageDialog|autocomplete-item-shared|formautofill)\.css$/i,
sourceName: /\b(minimal-xul|html|mathml|ua|forms|svg|manageDialog|autocomplete-item-shared|formautofill)\.css$/i,
errorMessage: /Unknown property.*-moz-/i,
isFromDevTools: false,
},
Expand Down
29 changes: 29 additions & 0 deletions layout/base/crashtests/401589-1.xhtml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
xmlns:html="http://www.w3.org/1999/xhtml"
xmlns:math="http://www.w3.org/1998/Math/MathML"
class="reftest-wait"
onload="boom();">

<html:style type="text/css">
[class="mp"] { display: -moz-popup; }
</html:style>

<script>

function boom()
{
document.getElementById("mtd").setAttribute("class", "mp");
setTimeout(boom2, 30);
}

function boom2()
{
document.getElementById("mtd").setAttribute("class", "");
document.documentElement.removeAttribute("class");
}

</script>

<math:mtd id="mtd" />

</window>
4 changes: 2 additions & 2 deletions layout/base/crashtests/405049-1.xhtml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" style="display: table;">
<panel/>
</window>
<box style="display: -moz-popup;"/>
</window>
1 change: 1 addition & 0 deletions layout/base/crashtests/crashtests.list
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ load 399951-1.html
load 399994-1.html
load 400445-1.xhtml
load 400904-1.xhtml
load chrome://reftest/content/crashtests/layout/base/crashtests/401589-1.xhtml
load 401734-1.html
load 401734-2.html
needs-focus pref(accessibility.browsewithcaret,true) load 403048.html
Expand Down
117 changes: 105 additions & 12 deletions layout/base/nsCSSFrameConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ static FrameCtorDebugFlags gFlags[] = {
#endif

#include "nsMenuFrame.h"
#include "nsPopupSetFrame.h"
#include "nsTreeColFrame.h"

//------------------------------------------------------------------
Expand All @@ -220,6 +221,8 @@ nsIFrame* NS_NewSplitterFrame(PresShell* aPresShell, ComputedStyle* aStyle);

nsIFrame* NS_NewMenuPopupFrame(PresShell* aPresShell, ComputedStyle* aStyle);

nsIFrame* NS_NewPopupSetFrame(PresShell* aPresShell, ComputedStyle* aStyle);

nsIFrame* NS_NewMenuFrame(PresShell* aPresShell, ComputedStyle* aStyle,
uint32_t aFlags);

Expand Down Expand Up @@ -601,6 +604,9 @@ class MOZ_STACK_CLASS nsFrameConstructorState {
PresShell* mPresShell;
nsFrameManager* mFrameManager;

// Frames destined for the kPopupList.
AbsoluteFrameList mPopupList;

// Containing block information for out-of-flow frames.
AbsoluteFrameList mFixedList;
AbsoluteFrameList mAbsoluteList;
Expand Down Expand Up @@ -632,6 +638,11 @@ class MOZ_STACK_CLASS nsFrameConstructorState {
// abs-pos lists together.
bool mFixedPosIsAbsPos;

// A boolean to indicate whether we have a "pending" popupgroup. That is, we
// have already created the FrameConstructionItem for the root popupgroup but
// we have not yet created the relevant frame.
bool mHavePendingPopupgroup;

// If false (which is the default) then call SetPrimaryFrame() as needed
// during frame construction. If true, don't make any SetPrimaryFrame()
// calls, except for generated content which doesn't have a primary frame
Expand Down Expand Up @@ -734,11 +745,13 @@ class MOZ_STACK_CLASS nsFrameConstructorState {
* positioned
* @param aCanBeFloated pass false if the frame isn't allowed to be
* floated
* @param aIsOutOfFlowPopup pass true if the frame is an out-of-flow popup
* (XUL-only)
*/
void AddChild(nsIFrame* aNewFrame, nsFrameList& aFrameList,
nsIContent* aContent, nsContainerFrame* aParentFrame,
bool aCanBePositioned = true, bool aCanBeFloated = true,
bool aInsertAfter = false,
bool aIsOutOfFlowPopup = false, bool aInsertAfter = false,
nsIFrame* aInsertAfterFrame = nullptr);

/**
Expand Down Expand Up @@ -776,6 +789,7 @@ class MOZ_STACK_CLASS nsFrameConstructorState {
AbsoluteFrameList* GetOutOfFlowFrameList(nsIFrame* aNewFrame,
bool aCanBePositioned,
bool aCanBeFloated,
bool aIsOutOfFlowPopup,
nsFrameState* aPlaceholderType);

void ConstructBackdropFrameFor(nsIContent* aContent, nsIFrame* aFrame);
Expand All @@ -789,6 +803,7 @@ nsFrameConstructorState::nsFrameConstructorState(
: mPresContext(aPresShell->GetPresContext()),
mPresShell(aPresShell),
mFrameManager(aPresShell->FrameConstructor()),
mPopupList(nullptr),
mFixedList(aFixedContainingBlock),
mAbsoluteList(aAbsoluteContainingBlock),
mFloatedList(aFloatContainingBlock),
Expand All @@ -805,8 +820,14 @@ nsFrameConstructorState::nsFrameConstructorState(
// block, use the abs-pos containing block's abs-pos list for fixed-pos
// frames.
mFixedPosIsAbsPos(aFixedContainingBlock == aAbsoluteContainingBlock),
mHavePendingPopupgroup(false),
mCreatingExtraFrames(false),
mHasRenderedLegend(false) {
nsIPopupContainer* popupContainer =
nsIPopupContainer::GetPopupContainer(aPresShell);
if (popupContainer) {
mPopupList.mContainingBlock = popupContainer->GetPopupSetFrame();
}
MOZ_COUNT_CTOR(nsFrameConstructorState);
}

Expand All @@ -833,6 +854,7 @@ void nsFrameConstructorState::ProcessFrameInsertionsForAllLists() {
ProcessFrameInsertions(mFloatedList, nsIFrame::kFloatList);
ProcessFrameInsertions(mAbsoluteList, nsIFrame::kAbsoluteList);
ProcessFrameInsertions(mFixedList, nsIFrame::kFixedList);
ProcessFrameInsertions(mPopupList, nsIFrame::kPopupList);
}

void nsFrameConstructorState::PushAbsoluteContainingBlock(
Expand Down Expand Up @@ -1032,7 +1054,12 @@ void nsFrameConstructorState::ReparentFloats(nsContainerFrame* aNewParent) {

AbsoluteFrameList* nsFrameConstructorState::GetOutOfFlowFrameList(
nsIFrame* aNewFrame, bool aCanBePositioned, bool aCanBeFloated,
nsFrameState* aPlaceholderType) {
bool aIsOutOfFlowPopup, nsFrameState* aPlaceholderType) {
if (MOZ_UNLIKELY(aIsOutOfFlowPopup)) {
MOZ_ASSERT(mPopupList.mContainingBlock, "Must have a popup set frame!");
*aPlaceholderType = PLACEHOLDER_FOR_POPUP;
return &mPopupList;
}
const nsStyleDisplay* disp = aNewFrame->StyleDisplay();
if (aCanBeFloated && disp->IsFloatingStyle()) {
*aPlaceholderType = PLACEHOLDER_FOR_FLOAT;
Expand Down Expand Up @@ -1084,7 +1111,7 @@ void nsFrameConstructorState::ConstructBackdropFrameFor(nsIContent* aContent,

nsFrameState placeholderType;
AbsoluteFrameList* frameList =
GetOutOfFlowFrameList(backdropFrame, true, true, &placeholderType);
GetOutOfFlowFrameList(backdropFrame, true, true, false, &placeholderType);
MOZ_ASSERT(placeholderType & PLACEHOLDER_FOR_TOPLAYER);

nsIFrame* placeholder = nsCSSFrameConstructor::CreatePlaceholderFrameFor(
Expand All @@ -1098,13 +1125,13 @@ void nsFrameConstructorState::ConstructBackdropFrameFor(nsIContent* aContent,
void nsFrameConstructorState::AddChild(
nsIFrame* aNewFrame, nsFrameList& aFrameList, nsIContent* aContent,
nsContainerFrame* aParentFrame, bool aCanBePositioned, bool aCanBeFloated,
bool aInsertAfter, nsIFrame* aInsertAfterFrame) {
bool aIsOutOfFlowPopup, bool aInsertAfter, nsIFrame* aInsertAfterFrame) {
MOZ_ASSERT(!aNewFrame->GetNextSibling(), "Shouldn't happen");

nsFrameState placeholderType;
AbsoluteFrameList* outOfFlowFrameList =
GetOutOfFlowFrameList(aNewFrame, aCanBePositioned, aCanBeFloated,
&placeholderType);
aIsOutOfFlowPopup, &placeholderType);

// The comments in GetGeometricParent regarding root table frames
// all apply here, unfortunately. Thus, we need to check whether
Expand Down Expand Up @@ -1161,7 +1188,8 @@ MOZ_NEVER_INLINE void nsFrameConstructorState::ProcessFrameInsertions(
aChildListID == nsIFrame::kAbsoluteList) || \
((&aFrameList == &mFixedList || &aFrameList == &mTopLayerFixedList) && \
aChildListID == nsIFrame::kFixedList)
MOZ_ASSERT(NS_NONXUL_LIST_TEST,
MOZ_ASSERT(NS_NONXUL_LIST_TEST || (&aFrameList == &mPopupList &&
aChildListID == nsIFrame::kPopupList),
"Unexpected aFrameList/aChildListID combination");

if (aFrameList.IsEmpty()) {
Expand Down Expand Up @@ -2510,6 +2538,9 @@ nsIFrame* nsCSSFrameConstructor::ConstructDocElementFrame(
mDocElementContainingBlock->AppendFrames(kPrincipalList,
std::move(frameList));

MOZ_ASSERT(!state.mHavePendingPopupgroup,
"Should have proccessed pending popup group by now");

return newFrame;
}

Expand Down Expand Up @@ -3654,11 +3685,15 @@ void nsCSSFrameConstructor::ConstructFrameFromItemInternal(
} else {
newFrame = (*data->mFunc.mCreationFunc)(mPresShell, computedStyle);

const bool allowOutOfFlow = !(bits & FCDATA_DISALLOW_OUT_OF_FLOW);
const bool isPopup = aItem.mIsPopup;
bool allowOutOfFlow = !(bits & FCDATA_DISALLOW_OUT_OF_FLOW);
bool isPopup = aItem.mIsPopup;
NS_ASSERTION(
!isPopup || (aState.mPopupList.mContainingBlock &&
aState.mPopupList.mContainingBlock->IsPopupSetFrame()),
"Should have a containing block here!");

nsContainerFrame* geometricParent =
isPopup ? aState.mTopLayerAbsoluteList.mContainingBlock
isPopup ? aState.mPopupList.mContainingBlock
: (allowOutOfFlow
? aState.GetGeometricParent(*display, aParentFrame)
: aParentFrame);
Expand Down Expand Up @@ -3746,6 +3781,17 @@ void nsCSSFrameConstructor::ConstructFrameFromItemInternal(

nsContainerFrame* newFrameAsContainer = do_QueryFrame(newFrame);
if (newFrameAsContainer) {
// Icky XUL stuff, sadly

if (aItem.mIsRootPopupgroup) {
NS_ASSERTION(nsIPopupContainer::GetPopupContainer(mPresShell) &&
nsIPopupContainer::GetPopupContainer(mPresShell)
->GetPopupSetFrame() == newFrame,
"Unexpected PopupSetFrame");
aState.mPopupList.mContainingBlock = newFrameAsContainer;
aState.mHavePendingPopupgroup = false;
}

// Process the child content if requested
nsFrameList childList;
nsFrameConstructorSaveState absoluteSaveState;
Expand Down Expand Up @@ -4043,6 +4089,7 @@ nsCSSFrameConstructor::FindXULTagData(const Element& aElement,
#else
SIMPLE_XUL_CREATE(menubar, NS_NewMenuBarFrame),
#endif /* XP_MACOSX */
SIMPLE_TAG_CHAIN(popupgroup, nsCSSFrameConstructor::FindPopupGroupData),
SIMPLE_XUL_CREATE(iframe, NS_NewSubDocumentFrame),
SIMPLE_XUL_CREATE(editor, NS_NewSubDocumentFrame),
SIMPLE_XUL_CREATE(browser, NS_NewSubDocumentFrame),
Expand All @@ -4054,6 +4101,19 @@ nsCSSFrameConstructor::FindXULTagData(const Element& aElement,
return FindDataByTag(aElement, aStyle, sXULTagData, ArrayLength(sXULTagData));
}

/* static */
const nsCSSFrameConstructor::FrameConstructionData*
nsCSSFrameConstructor::FindPopupGroupData(const Element& aElement,
ComputedStyle&) {
if (!aElement.IsRootOfNativeAnonymousSubtree()) {
return nullptr;
}

static constexpr FrameConstructionData sPopupSetData =
SIMPLE_XUL_FCDATA(NS_NewPopupSetFrame);
return &sPopupSetData;
}

/* static */
const nsCSSFrameConstructor::FrameConstructionData*
nsCSSFrameConstructor::FindXULButtonData(const Element& aElement,
Expand Down Expand Up @@ -4461,7 +4521,8 @@ nsCSSFrameConstructor::FindDisplayData(const nsStyleDisplay& aDisplay,
}
case StyleDisplayInside::MozPopup: {
static constexpr FrameConstructionData data(
NS_NewMenuPopupFrame, FCDATA_IS_POPUP | FCDATA_SKIP_ABSPOS_PUSH);
NS_NewMenuPopupFrame, FCDATA_DISALLOW_OUT_OF_FLOW | FCDATA_IS_POPUP |
FCDATA_SKIP_ABSPOS_PUSH);
return &data;
}
default:
Expand Down Expand Up @@ -5322,6 +5383,11 @@ void nsCSSFrameConstructor::AddFrameConstructionItemsInternal(
(data->mBits & FCDATA_IS_POPUP) && (!aParentFrame || // Parent is inline
!aParentFrame->IsMenuFrame());

if (isPopup && !aState.mPopupList.mContainingBlock &&
!aState.mHavePendingPopupgroup) {
return;
}

const uint32_t bits = data->mBits;

// Inside colgroups, suppress everything except columns.
Expand Down Expand Up @@ -5355,6 +5421,11 @@ void nsCSSFrameConstructor::AddFrameConstructionItemsInternal(
// This corresponds to the Release in ConstructFramesFromItem.
item->mContent->AddRef();
}
item->mIsRootPopupgroup = aContent->IsRootOfNativeAnonymousSubtree() &&
aContent->IsXULElement(nsGkAtoms::popupgroup);
if (item->mIsRootPopupgroup) {
aState.mHavePendingPopupgroup = true;
}
item->mIsPopup = isPopup;

if (canHavePageBreak && display.BreakAfter()) {
Expand Down Expand Up @@ -5526,6 +5597,16 @@ void nsCSSFrameConstructor::ConstructFramesFromItem(
}
}

void nsCSSFrameConstructor::ReconstructDocElementHierarchy(
InsertionKind aInsertionKind) {
Element* rootElement = mDocument->GetRootElement();
if (!rootElement) {
/* nothing to do */
return;
}
RecreateFramesForContent(rootElement, aInsertionKind);
}

nsContainerFrame* nsCSSFrameConstructor::GetAbsoluteContainingBlock(
nsIFrame* aFrame, ContainingBlockType aType) {
// Starting with aFrame, look for a frame that is absolutely positioned or
Expand Down Expand Up @@ -8283,6 +8364,16 @@ bool nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(
return true;
}

if (aFrame->IsPopupSetFrame()) {
nsIPopupContainer* popupContainer =
nsIPopupContainer::GetPopupContainer(mPresShell);
if (popupContainer && popupContainer->GetPopupSetFrame() == aFrame) {
TRACE("PopupSet");
ReconstructDocElementHierarchy(InsertionKind::Async);
return true;
}
}

// Reconstruct if inflowFrame is parent's only child, and parent is, or has,
// a non-fluid continuation, i.e. it was split by bidi resolution
if (!inFlowFrame->GetPrevSibling() && !inFlowFrame->GetNextSibling() &&
Expand Down Expand Up @@ -9590,7 +9681,9 @@ void nsCSSFrameConstructor::ProcessChildren(
itemsToConstruct.SetLineBoundaryAtEnd(true);
}

// Create any anonymous frames we need here.
// Create any anonymous frames we need here. This must happen before the
// non-anonymous children are processed to ensure that popups are never
// constructed before the popupset.
AutoTArray<nsIAnonymousContentCreator::ContentInfo, 4> anonymousItems;
GetAnonymousContent(aContent, aPossiblyLeafFrame, anonymousItems);
#ifdef DEBUG
Expand Down Expand Up @@ -9953,7 +10046,7 @@ nsFirstLetterFrame* nsCSSFrameConstructor::CreateFloatingLetterFrame(
}

aState.AddChild(letterFrame, aResult, letterContent, aParentFrame, false,
true, true, prevSibling);
true, false, true, prevSibling);

if (nextTextFrame) {
aResult.AppendFrame(nullptr, nextTextFrame);
Expand Down
Loading

0 comments on commit 8e6fe2d

Please sign in to comment.