From df90a017dea05f6eabfdfeaef57fedcb701cd680 Mon Sep 17 00:00:00 2001 From: "mats.palmgren%bredband.net" Date: Wed, 24 Oct 2007 00:02:27 +0000 Subject: [PATCH] Call WillDestroyFrameTree() before destroying the frame tree in ReconstructDocElementHierarchyInternal() because we have cleared the placeholder map etc at this point and we don't want RemoveMappingsForFrameSubtree() to mess with it. Also, remove the RemoveMappingsForFrameSubtree() call that was added in bug 372685 which was wrong since all the frames on a popupset's ::popupList are out-of-flows that are reachable (directly or indirectly) from a normal flow placeholder. b=398982 r+sr=bzbarsky a=dsicore git-svn-id: svn://10.0.0.236/trunk@238055 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/layout/base/nsCSSFrameConstructor.cpp | 13 +- .../layout/xul/base/src/nsPopupSetFrame.cpp | 147 +++++++++++++++--- mozilla/layout/xul/base/src/nsPopupSetFrame.h | 2 +- mozilla/layout/xul/test/Makefile.in | 2 + mozilla/layout/xul/test/test_bug398982-1.xul | 32 ++++ mozilla/layout/xul/test/test_bug398982-2.xul | 34 ++++ 6 files changed, 205 insertions(+), 25 deletions(-) create mode 100644 mozilla/layout/xul/test/test_bug398982-1.xul create mode 100644 mozilla/layout/xul/test/test_bug398982-2.xul diff --git a/mozilla/layout/base/nsCSSFrameConstructor.cpp b/mozilla/layout/base/nsCSSFrameConstructor.cpp index eff9a4408d7..ff00ba0a9e6 100644 --- a/mozilla/layout/base/nsCSSFrameConstructor.cpp +++ b/mozilla/layout/base/nsCSSFrameConstructor.cpp @@ -7836,17 +7836,22 @@ nsCSSFrameConstructor::ReconstructDocElementHierarchyInternal() NS_ASSERTION(docElementFrame->GetParent() == mDocElementContainingBlock, "Unexpected doc element parent frame"); + // Notify self that we will destroy the entire frame tree, this blocks + // RemoveMappingsForFrameSubtree() which would otherwise lead to a + // crash since we cleared the placeholder map above (bug 398982). + PRBool wasDestroyingFrameTree = mIsDestroyingFrameTree; + WillDestroyFrameTree(); // Remove the old document element hieararchy rv = state.mFrameManager->RemoveFrame(mDocElementContainingBlock, nsnull, docElementFrame); + mIsDestroyingFrameTree = wasDestroyingFrameTree; if (NS_FAILED(rv)) { return rv; } - } // Create the new document element hierarchy - nsIFrame* newChild; + nsIFrame* newChild; rv = ConstructDocElementFrame(state, rootContent, mDocElementContainingBlock, &newChild); @@ -11273,8 +11278,8 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent) PRUint32 event; if (frame) { nsIFrame *newFrame = mPresShell->GetPrimaryFrameFor(aContent); - event = newFrame ? nsIAccessibleEvent::EVENT_ASYNCH_SIGNIFICANT_CHANGE : - nsIAccessibleEvent::EVENT_ASYNCH_HIDE; + event = newFrame ? PRUint32(nsIAccessibleEvent::EVENT_ASYNCH_SIGNIFICANT_CHANGE) : + PRUint32(nsIAccessibleEvent::EVENT_ASYNCH_HIDE); } else { event = nsIAccessibleEvent::EVENT_ASYNCH_SHOW; diff --git a/mozilla/layout/xul/base/src/nsPopupSetFrame.cpp b/mozilla/layout/xul/base/src/nsPopupSetFrame.cpp index a9694afceee..7a8e5122ca4 100644 --- a/mozilla/layout/xul/base/src/nsPopupSetFrame.cpp +++ b/mozilla/layout/xul/base/src/nsPopupSetFrame.cpp @@ -45,13 +45,8 @@ #include "nsIContent.h" #include "nsPresContext.h" #include "nsStyleContext.h" -#include "nsCSSRendering.h" -#include "nsINameSpaceManager.h" -#include "nsIDocument.h" #include "nsBoxLayoutState.h" #include "nsIScrollableFrame.h" -#include "nsCSSFrameConstructor.h" -#include "nsGUIEvent.h" #include "nsIRootBox.h" nsPopupFrameList::nsPopupFrameList(nsIContent* aPopupContent, nsPopupFrameList* aNext) @@ -139,13 +134,11 @@ nsPopupSetFrame::SetInitialChildList(nsIAtom* aListName, void nsPopupSetFrame::Destroy() { - nsCSSFrameConstructor* frameConstructor = - PresContext()->PresShell()->FrameConstructor(); // remove each popup from the list as we go. while (mPopupList) { - if (mPopupList->mPopupFrame) - DestroyPopupFrame(frameConstructor, mPopupList->mPopupFrame); - + if (mPopupList->mPopupFrame) { + mPopupList->mPopupFrame->Destroy(); + } nsPopupFrameList* temp = mPopupList; mPopupList = mPopupList->mNextPopup; delete temp; @@ -227,6 +220,9 @@ nsPopupSetFrame::RemovePopupFrame(nsIFrame* aPopup) { // This was called by the Destroy() method of the popup, so all we have to do is // get the popup out of our list, so we don't reflow it later. +#ifdef DEBUG + PRBool found = PR_FALSE; +#endif nsPopupFrameList* currEntry = mPopupList; nsPopupFrameList* temp = nsnull; while (currEntry) { @@ -237,13 +233,18 @@ nsPopupSetFrame::RemovePopupFrame(nsIFrame* aPopup) else mPopupList = currEntry->mNextPopup; + NS_ASSERTION((aPopup->GetStateBits() & NS_FRAME_OUT_OF_FLOW) && + aPopup->GetType() == nsGkAtoms::menuPopupFrame, + "found wrong type of frame in popupset's ::popupList"); // Destroy the frame. - DestroyPopupFrame(PresContext()->PresShell()->FrameConstructor(), - currEntry->mPopupFrame); + currEntry->mPopupFrame->Destroy(); // Delete the entry. currEntry->mNextPopup = nsnull; delete currEntry; +#ifdef DEBUG + found = PR_TRUE; +#endif // Break out of the loop. break; @@ -253,6 +254,7 @@ nsPopupSetFrame::RemovePopupFrame(nsIFrame* aPopup) currEntry = currEntry->mNextPopup; } + NS_ASSERTION(found, "frame to remove is not in our ::popupList"); return NS_OK; } @@ -269,10 +271,9 @@ nsPopupSetFrame::AddPopupFrameList(nsIFrame* aPopupFrameList) nsresult nsPopupSetFrame::AddPopupFrame(nsIFrame* aPopup) { - NS_ASSERTION(aPopup->GetType() == nsGkAtoms::menuPopupFrame, - "expected a menupopup frame to be added to a popupset"); - if (aPopup->GetType() != nsGkAtoms::menuPopupFrame) - return NS_ERROR_UNEXPECTED; + NS_ASSERTION((aPopup->GetStateBits() & NS_FRAME_OUT_OF_FLOW) && + aPopup->GetType() == nsGkAtoms::menuPopupFrame, + "adding wrong type of frame in popupset's ::popupList"); // The entry should already exist, but might not (if someone decided to make their // popup visible straightaway, e.g., the autocomplete widget). @@ -297,9 +298,115 @@ nsPopupSetFrame::AddPopupFrame(nsIFrame* aPopup) return NS_OK; } -void -nsPopupSetFrame::DestroyPopupFrame(nsCSSFrameConstructor* aFc, nsIFrame* aPopup) +#ifdef DEBUG +NS_IMETHODIMP +nsPopupSetFrame::List(FILE* out, PRInt32 aIndent) const { - aFc->RemoveMappingsForFrameSubtree(aPopup); - aPopup->Destroy(); + IndentBy(out, aIndent); + ListTag(out); +#ifdef DEBUG_waterson + fprintf(out, " [parent=%p]", static_cast(mParent)); +#endif + if (HasView()) { + fprintf(out, " [view=%p]", static_cast(GetView())); + } + if (nsnull != mNextSibling) { + fprintf(out, " next=%p", static_cast(mNextSibling)); + } + if (nsnull != GetPrevContinuation()) { + fprintf(out, " prev-continuation=%p", static_cast(GetPrevContinuation())); + } + if (nsnull != GetNextContinuation()) { + fprintf(out, " next-continuation=%p", static_cast(GetNextContinuation())); + } + fprintf(out, " {%d,%d,%d,%d}", mRect.x, mRect.y, mRect.width, mRect.height); + if (0 != mState) { + fprintf(out, " [state=%08x]", mState); + } + fprintf(out, " [content=%p]", static_cast(mContent)); + nsPopupSetFrame* f = const_cast(this); + nsRect* overflowArea = f->GetOverflowAreaProperty(PR_FALSE); + if (overflowArea) { + fprintf(out, " [overflow=%d,%d,%d,%d]", overflowArea->x, overflowArea->y, + overflowArea->width, overflowArea->height); + } + fprintf(out, " [sc=%p]", static_cast(mStyleContext)); + nsIAtom* pseudoTag = mStyleContext->GetPseudoType(); + if (pseudoTag) { + nsAutoString atomString; + pseudoTag->ToString(atomString); + fprintf(out, " pst=%s", + NS_LossyConvertUTF16toASCII(atomString).get()); + } + + // Output the children + nsIAtom* listName = nsnull; + PRInt32 listIndex = 0; + PRBool outputOneList = PR_FALSE; + do { + nsIFrame* kid = GetFirstChild(listName); + if (nsnull != kid) { + if (outputOneList) { + IndentBy(out, aIndent); + } + outputOneList = PR_TRUE; + nsAutoString tmp; + if (nsnull != listName) { + listName->ToString(tmp); + fputs(NS_LossyConvertUTF16toASCII(tmp).get(), out); + } + fputs("<\n", out); + while (nsnull != kid) { + // Verify the child frame's parent frame pointer is correct + NS_ASSERTION(kid->GetParent() == (nsIFrame*)this, "bad parent frame pointer"); + + // Have the child frame list + nsIFrameDebug* frameDebug; + if (NS_SUCCEEDED(kid->QueryInterface(NS_GET_IID(nsIFrameDebug), (void**)&frameDebug))) { + frameDebug->List(out, aIndent + 1); + } + kid = kid->GetNextSibling(); + } + IndentBy(out, aIndent); + fputs(">\n", out); + } + listName = GetAdditionalChildListName(listIndex++); + } while(nsnull != listName); + + // XXXmats the above is copy-pasted from nsContainerFrame::List which is lame, + // clean this up after bug 399111 is implemented. + + if (mPopupList) { + fputs("<\n", out); + ++aIndent; + IndentBy(out, aIndent); + nsAutoString tmp; + nsGkAtoms::popupList->ToString(tmp); + fputs(NS_LossyConvertUTF16toASCII(tmp).get(), out); + fputs(" for ", out); + ListTag(out); + fputs(" <\n", out); + ++aIndent; + for (nsPopupFrameList* l = mPopupList; l; l = l->mNextPopup) { + nsIFrameDebug* frameDebug; + if (l->mPopupFrame && + NS_SUCCEEDED(CallQueryInterface(l->mPopupFrame, &frameDebug))) { + frameDebug->List(out, aIndent); + } + } + --aIndent; + IndentBy(out, aIndent); + fputs(">\n", out); + --aIndent; + IndentBy(out, aIndent); + fputs(">\n", out); + outputOneList = PR_TRUE; + } + + if (!outputOneList) { + fputs("<>\n", out); + } + + return NS_OK; } +#endif diff --git a/mozilla/layout/xul/base/src/nsPopupSetFrame.h b/mozilla/layout/xul/base/src/nsPopupSetFrame.h index 901b578ed6e..92a65bb8468 100644 --- a/mozilla/layout/xul/base/src/nsPopupSetFrame.h +++ b/mozilla/layout/xul/base/src/nsPopupSetFrame.h @@ -91,6 +91,7 @@ public: virtual nsIAtom* GetType() const; #ifdef DEBUG + NS_IMETHOD List(FILE* out, PRInt32 aIndent) const; NS_IMETHOD GetFrameName(nsAString& aResult) const { return MakeFrameName(NS_LITERAL_STRING("PopupSet"), aResult); @@ -102,7 +103,6 @@ protected: nsresult AddPopupFrameList(nsIFrame* aPopupFrameList); nsresult AddPopupFrame(nsIFrame* aPopup); nsresult RemovePopupFrame(nsIFrame* aPopup); - void DestroyPopupFrame(nsCSSFrameConstructor* aFc, nsIFrame* aPopup); nsPopupFrameList* mPopupList; diff --git a/mozilla/layout/xul/test/Makefile.in b/mozilla/layout/xul/test/Makefile.in index fefc1ae355d..f258d73f1b0 100644 --- a/mozilla/layout/xul/test/Makefile.in +++ b/mozilla/layout/xul/test/Makefile.in @@ -47,6 +47,8 @@ include $(topsrcdir)/config/rules.mk _TEST_FILES = test_bug372685.xul \ test_bug386386.html \ test_bug394800.xhtml \ + test_bug398982-1.xul \ + test_bug398982-2.xul \ $(NULL) libs:: $(_TEST_FILES) diff --git a/mozilla/layout/xul/test/test_bug398982-1.xul b/mozilla/layout/xul/test/test_bug398982-1.xul new file mode 100644 index 00000000000..d41c4e81bf0 --- /dev/null +++ b/mozilla/layout/xul/test/test_bug398982-1.xul @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + + diff --git a/mozilla/layout/xul/test/test_bug398982-2.xul b/mozilla/layout/xul/test/test_bug398982-2.xul new file mode 100644 index 00000000000..6a7bfe5cd3f --- /dev/null +++ b/mozilla/layout/xul/test/test_bug398982-2.xul @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + + + +