From e79a69e85a04502f8f5109c9982a9a63c6fe3856 Mon Sep 17 00:00:00 2001 From: Timothy Nikkel Date: Fri, 28 May 2010 14:34:50 -0500 Subject: [PATCH] Bug 561981. Guard against nsFrameLoader::Show flushing and re-entering or destroying us. r=smaug --- content/base/src/nsFrameLoader.cpp | 53 +++++++++++++++++++++++--- content/base/src/nsFrameLoader.h | 15 ++++++-- layout/generic/nsFrameFrame.cpp | 53 +++++++++++++++++--------- layout/reftests/bugs/561981-1-ref.html | 7 ++++ layout/reftests/bugs/561981-1.html | 16 ++++++++ layout/reftests/bugs/561981-2-ref.html | 7 ++++ layout/reftests/bugs/561981-2.html | 16 ++++++++ layout/reftests/bugs/561981-3-ref.html | 7 ++++ layout/reftests/bugs/561981-3.html | 16 ++++++++ layout/reftests/bugs/561981-4-ref.html | 7 ++++ layout/reftests/bugs/561981-4.html | 16 ++++++++ layout/reftests/bugs/561981-5-ref.html | 7 ++++ layout/reftests/bugs/561981-5.html | 12 ++++++ layout/reftests/bugs/561981-6-ref.html | 7 ++++ layout/reftests/bugs/561981-6.html | 12 ++++++ layout/reftests/bugs/561981-7-ref.html | 7 ++++ layout/reftests/bugs/561981-7.html | 12 ++++++ layout/reftests/bugs/561981-8-ref.html | 7 ++++ layout/reftests/bugs/561981-8.html | 12 ++++++ layout/reftests/bugs/reftest.list | 8 ++++ 20 files changed, 270 insertions(+), 27 deletions(-) create mode 100644 layout/reftests/bugs/561981-1-ref.html create mode 100644 layout/reftests/bugs/561981-1.html create mode 100644 layout/reftests/bugs/561981-2-ref.html create mode 100644 layout/reftests/bugs/561981-2.html create mode 100644 layout/reftests/bugs/561981-3-ref.html create mode 100644 layout/reftests/bugs/561981-3.html create mode 100644 layout/reftests/bugs/561981-4-ref.html create mode 100644 layout/reftests/bugs/561981-4.html create mode 100644 layout/reftests/bugs/561981-5-ref.html create mode 100644 layout/reftests/bugs/561981-5.html create mode 100644 layout/reftests/bugs/561981-6-ref.html create mode 100644 layout/reftests/bugs/561981-6.html create mode 100644 layout/reftests/bugs/561981-7-ref.html create mode 100644 layout/reftests/bugs/561981-7.html create mode 100644 layout/reftests/bugs/561981-8-ref.html create mode 100644 layout/reftests/bugs/561981-8.html diff --git a/content/base/src/nsFrameLoader.cpp b/content/base/src/nsFrameLoader.cpp index 9806beb4b357..c3e953d1a0e5 100644 --- a/content/base/src/nsFrameLoader.cpp +++ b/content/base/src/nsFrameLoader.cpp @@ -89,6 +89,7 @@ #include "nsIContentViewer.h" #include "nsIDOMChromeWindow.h" #include "nsInProcessTabChildGlobal.h" +#include "mozilla/AutoRestore.h" class nsAsyncDocShellDestroyer : public nsRunnable { @@ -514,25 +515,50 @@ AllDescendantsOfType(nsIDocShellTreeItem* aParentItem, PRInt32 aType) return PR_TRUE; } -bool +/** + * A class that automatically sets mInShow to false when it goes + * out of scope. + */ +class NS_STACK_CLASS AutoResetInShow { + private: + nsFrameLoader* mFrameLoader; + MOZILLA_DECL_USE_GUARD_OBJECT_NOTIFIER + public: + AutoResetInShow(nsFrameLoader* aFrameLoader MOZILLA_GUARD_OBJECT_NOTIFIER_PARAM) + : mFrameLoader(aFrameLoader) + { + MOZILLA_GUARD_OBJECT_NOTIFIER_INIT; + } + ~AutoResetInShow() { mFrameLoader->mInShow = PR_FALSE; } +}; + + +PRBool nsFrameLoader::Show(PRInt32 marginWidth, PRInt32 marginHeight, PRInt32 scrollbarPrefX, PRInt32 scrollbarPrefY, nsIFrameFrame* frame) { + if (mInShow) { + return PR_FALSE; + } + // Reset mInShow if we exit early. + AutoResetInShow resetInShow(this); + mInShow = PR_TRUE; + nsContentType contentType; nsresult rv = EnsureDocShell(); if (NS_FAILED(rv)) { - return false; + return PR_FALSE; } if (!mDocShell) - return false; + return PR_FALSE; nsCOMPtr presShell; mDocShell->GetPresShell(getter_AddRefs(presShell)); if (presShell) - return true; + return PR_TRUE; mDocShell->SetMarginWidth(marginWidth); mDocShell->SetMarginHeight(marginHeight); @@ -563,7 +589,7 @@ nsFrameLoader::Show(PRInt32 marginWidth, PRInt32 marginHeight, nsIView* view = frame->CreateViewAndWidget(contentType); if (!view) - return false; + return PR_FALSE; nsCOMPtr baseWindow = do_QueryInterface(mDocShell); NS_ASSERTION(baseWindow, "Found a nsIDocShell that isn't a nsIBaseWindow."); @@ -594,12 +620,26 @@ nsFrameLoader::Show(PRInt32 marginWidth, PRInt32 marginHeight, } } - return true; + mInShow = PR_FALSE; + if (mHideCalled) { + mHideCalled = PR_FALSE; + Hide(); + return PR_FALSE; + } + return PR_TRUE; } void nsFrameLoader::Hide() { + if (mHideCalled) { + return; + } + if (mInShow) { + mHideCalled = PR_TRUE; + return; + } + if (!mDocShell) return; @@ -623,6 +663,7 @@ nsFrameLoader::SwapWithOtherLoader(nsFrameLoader* aOther, NS_PRECONDITION((aFirstToSwap == this && aSecondToSwap == aOther) || (aFirstToSwap == aOther && aSecondToSwap == this), "Swapping some sort of random loaders?"); + NS_ENSURE_STATE(!mInShow && !aOther->mInShow); nsIContent* ourContent = mOwnerContent; nsIContent* otherContent = aOther->mOwnerContent; diff --git a/content/base/src/nsFrameLoader.h b/content/base/src/nsFrameLoader.h index 49e86281dc44..3ac504dcb86c 100644 --- a/content/base/src/nsFrameLoader.h +++ b/content/base/src/nsFrameLoader.h @@ -54,9 +54,12 @@ class nsIContent; class nsIURI; class nsIFrameFrame; class nsIInProcessContentFrameMessageManager; +class AutoResetInShow; class nsFrameLoader : public nsIFrameLoader { + friend class AutoResetInShow; + protected: nsFrameLoader(nsIContent *aOwner) : mOwnerContent(aOwner), @@ -64,7 +67,9 @@ protected: mIsTopLevelContent(PR_FALSE), mDestroyCalled(PR_FALSE), mNeedsAsyncDestroy(PR_FALSE), - mInSwap(PR_FALSE) + mInSwap(PR_FALSE), + mInShow(PR_FALSE), + mHideCalled(PR_FALSE) {} public: @@ -89,9 +94,9 @@ public: * Called from the layout frame associated with this frame loader; * this notifies us to hook up with the widget and view. */ - bool Show(PRInt32 marginWidth, PRInt32 marginHeight, - PRInt32 scrollbarPrefX, PRInt32 scrollbarPrefY, - nsIFrameFrame* frame); + PRBool Show(PRInt32 marginWidth, PRInt32 marginHeight, + PRInt32 scrollbarPrefX, PRInt32 scrollbarPrefY, + nsIFrameFrame* frame); /** * Called from the layout frame associated with this frame loader, when @@ -130,6 +135,8 @@ private: PRPackedBool mDestroyCalled : 1; PRPackedBool mNeedsAsyncDestroy : 1; PRPackedBool mInSwap : 1; + PRPackedBool mInShow : 1; + PRPackedBool mHideCalled : 1; }; #endif diff --git a/layout/generic/nsFrameFrame.cpp b/layout/generic/nsFrameFrame.cpp index 521c6fca946a..895bdb68219b 100644 --- a/layout/generic/nsFrameFrame.cpp +++ b/layout/generic/nsFrameFrame.cpp @@ -221,14 +221,16 @@ protected: nsIView* mInnerView; PRPackedBool mIsInline; PRPackedBool mPostedReflowCallback; - bool mDidCreateDoc; + PRPackedBool mDidCreateDoc; + PRPackedBool mCallingShow; }; nsSubDocumentFrame::nsSubDocumentFrame(nsStyleContext* aContext) : nsLeafFrame(aContext) , mIsInline(PR_FALSE) , mPostedReflowCallback(PR_FALSE) - , mDidCreateDoc(false) + , mDidCreateDoc(PR_FALSE) + , mCallingShow(PR_FALSE) { } @@ -327,19 +329,31 @@ inline PRInt32 ConvertOverflow(PRUint8 aOverflow) void nsSubDocumentFrame::ShowViewer() { + if (mCallingShow) { + return; + } + if (!PresContext()->IsDynamic()) { // We let the printing code take care of loading the document; just // create a widget for it to use (void) CreateViewAndWidget(eContentTypeContent); } else { - nsFrameLoader* frameloader = FrameLoader(); + nsRefPtr frameloader = FrameLoader(); if (frameloader) { nsIntSize margin = GetMarginAttributes(); const nsStyleDisplay* disp = GetStyleDisplay(); - mDidCreateDoc = frameloader->Show(margin.width, margin.height, - ConvertOverflow(disp->mOverflowX), - ConvertOverflow(disp->mOverflowY), - this); + nsWeakFrame weakThis(this); + mCallingShow = PR_TRUE; + PRBool didCreateDoc = + frameloader->Show(margin.width, margin.height, + ConvertOverflow(disp->mOverflowX), + ConvertOverflow(disp->mOverflowY), + this); + if (!weakThis.IsAlive()) { + return; + } + mCallingShow = PR_FALSE; + mDidCreateDoc = didCreateDoc; } } } @@ -825,7 +839,7 @@ nsSubDocumentFrame::DestroyFrom(nsIFrame* aDestructRoot) void nsSubDocumentFrame::HideViewer() { - if (mFrameLoader && mDidCreateDoc) + if (mFrameLoader && (mDidCreateDoc || mCallingShow)) mFrameLoader->Hide(); } @@ -882,8 +896,8 @@ nsSubDocumentFrame::BeginSwapDocShells(nsIFrame* aOther) } nsSubDocumentFrame* other = static_cast(aOther); - if (!mFrameLoader || !mDidCreateDoc || !other->mFrameLoader || - !other->mDidCreateDoc) { + if (!mFrameLoader || !mDidCreateDoc || mCallingShow || + !other->mFrameLoader || !other->mDidCreateDoc) { return NS_ERROR_NOT_IMPLEMENTED; } @@ -898,20 +912,25 @@ void nsSubDocumentFrame::EndSwapDocShells(nsIFrame* aOther) { nsSubDocumentFrame* other = static_cast(aOther); + nsWeakFrame weakThis(this); + nsWeakFrame weakOther(aOther); ShowViewer(); other->ShowViewer(); // Now make sure we reflow both frames, in case their contents // determine their size. - PresContext()->PresShell()-> - FrameNeedsReflow(this, nsIPresShell::eTreeChange, NS_FRAME_IS_DIRTY); - other->PresContext()->PresShell()-> - FrameNeedsReflow(other, nsIPresShell::eTreeChange, NS_FRAME_IS_DIRTY); - // And repaint them, for good measure, in case there's nothing // interesting that happens during reflow. - InvalidateOverflowRect(); - other->InvalidateOverflowRect(); + if (weakThis.IsAlive()) { + PresContext()->PresShell()-> + FrameNeedsReflow(this, nsIPresShell::eTreeChange, NS_FRAME_IS_DIRTY); + InvalidateOverflowRect(); + } + if (weakOther.IsAlive()) { + other->PresContext()->PresShell()-> + FrameNeedsReflow(other, nsIPresShell::eTreeChange, NS_FRAME_IS_DIRTY); + other->InvalidateOverflowRect(); + } } nsIView* diff --git a/layout/reftests/bugs/561981-1-ref.html b/layout/reftests/bugs/561981-1-ref.html new file mode 100644 index 000000000000..1f2d3710a288 --- /dev/null +++ b/layout/reftests/bugs/561981-1-ref.html @@ -0,0 +1,7 @@ + + + + +