From c82427eb2450e4573e4c51e6743a36df7bf03174 Mon Sep 17 00:00:00 2001 From: "bzbarsky%mit.edu" Date: Mon, 1 Oct 2007 02:46:15 +0000 Subject: [PATCH] Fire XBL constructors asynchronously after binding attachment, unless we're in the middle of an update. In that case, fire them at the end of the update. Bugs 267833, 373756, 394676, 394014. r+sr=sicking, a=dveditz git-svn-id: svn://10.0.0.236/branches/MOZILLA_1_8_BRANCH@236965 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/content/base/src/nsDocument.cpp | 21 +++- mozilla/content/base/src/nsDocument.h | 3 + .../html/document/src/nsHTMLContentSink.cpp | 3 + mozilla/content/xbl/src/nsBindingManager.cpp | 98 ++++++++++++++++++- mozilla/content/xbl/src/nsBindingManager.h | 36 ++++++- .../content/xbl/src/nsXBLResourceLoader.cpp | 9 +- .../xul/templates/src/nsXULContentBuilder.cpp | 4 + mozilla/layout/base/nsCSSFrameConstructor.cpp | 18 +--- mozilla/layout/base/nsCSSFrameConstructor.h | 13 +++ mozilla/layout/base/nsPresShell.cpp | 20 ++++ 10 files changed, 195 insertions(+), 30 deletions(-) diff --git a/mozilla/content/base/src/nsDocument.cpp b/mozilla/content/base/src/nsDocument.cpp index 95ca73851f2..6040564ec59 100644 --- a/mozilla/content/base/src/nsDocument.cpp +++ b/mozilla/content/base/src/nsDocument.cpp @@ -985,12 +985,14 @@ nsDocument::Init() mLinkMap.Init(); // Force initialization. - nsBindingManager *bindingManager = new nsBindingManager(); + nsBindingManager *bindingManager = new nsBindingManager(this); NS_ENSURE_TRUE(bindingManager, NS_ERROR_OUT_OF_MEMORY); mBindingManager = bindingManager; // The binding manager must always be the first observer of the document. - mObservers.PrependElement(bindingManager); + if (!mObservers.PrependElement(bindingManager)) { + return NS_ERROR_OUT_OF_MEMORY; + } mOnloadBlocker = new nsOnloadBlocker(); NS_ENSURE_TRUE(mOnloadBlocker, NS_ERROR_OUT_OF_MEMORY); @@ -2237,6 +2239,12 @@ nsDocument::RemoveObserver(nsIDocumentObserver* aObserver) void nsDocument::BeginUpdate(nsUpdateType aUpdateType) { + if (mUpdateNestLevel == 0) { + nsIBindingManager* bm = mBindingManager; + NS_STATIC_CAST(nsBindingManager*, bm)->BeginOutermostUpdate(); + } + + ++mUpdateNestLevel; if (mScriptLoader) { NS_STATIC_CAST(nsScriptLoader*, NS_STATIC_CAST(nsIScriptLoader*, @@ -2249,6 +2257,15 @@ void nsDocument::EndUpdate(nsUpdateType aUpdateType) { NS_DOCUMENT_NOTIFY_OBSERVERS(EndUpdate, (this, aUpdateType)); + + --mUpdateNestLevel; + if (mUpdateNestLevel == 0) { + // This set of updates may have created XBL bindings. Let the + // binding manager know we're done. + nsIBindingManager* bm = mBindingManager; + NS_STATIC_CAST(nsBindingManager*, bm)->EndOutermostUpdate(); + } + if (mScriptLoader) { NS_STATIC_CAST(nsScriptLoader*, NS_STATIC_CAST(nsIScriptLoader*, diff --git a/mozilla/content/base/src/nsDocument.h b/mozilla/content/base/src/nsDocument.h index 2a3ea9d8cf8..71f90fd2066 100644 --- a/mozilla/content/base/src/nsDocument.h +++ b/mozilla/content/base/src/nsDocument.h @@ -900,6 +900,9 @@ private: nsTHashtable mLinkMap; // URIs whose visitedness has changed while we were hidden nsCOMArray mVisitednessChangedURIs; + + // Our update nesting level + PRUint32 mUpdateNestLevel; }; diff --git a/mozilla/content/html/document/src/nsHTMLContentSink.cpp b/mozilla/content/html/document/src/nsHTMLContentSink.cpp index a2744afe3c5..4dbf12dd39d 100644 --- a/mozilla/content/html/document/src/nsHTMLContentSink.cpp +++ b/mozilla/content/html/document/src/nsHTMLContentSink.cpp @@ -1644,6 +1644,9 @@ SinkContext::AddText(const nsAString& aText) nsresult SinkContext::FlushTags(PRBool aNotify) { + // Not starting an update here, unlike trunk. We'll get XBL + // constructors firing async of the stuff we flush right now. + // Don't release last text node in case we need to add to it again FlushText(); diff --git a/mozilla/content/xbl/src/nsBindingManager.cpp b/mozilla/content/xbl/src/nsBindingManager.cpp index ca05a9b1ecf..c41ad41b451 100644 --- a/mozilla/content/xbl/src/nsBindingManager.cpp +++ b/mozilla/content/xbl/src/nsBindingManager.cpp @@ -305,8 +305,11 @@ SetOrRemoveObject(PLDHashTable& table, nsISupports* aKey, nsISupports* aValue) NS_IMPL_ISUPPORTS3(nsBindingManager, nsIBindingManager, nsIStyleRuleSupplier, nsIDocumentObserver) // Constructors/Destructors -nsBindingManager::nsBindingManager(void) -: mProcessingAttachedStack(PR_FALSE) +nsBindingManager::nsBindingManager(nsIDocument* aDocument) + : mProcessingAttachedStack(PR_FALSE), + mProcessOnEndUpdate(PR_FALSE), + mProcessAttachedQueueEvent(nsnull), + mDocument(aDocument) { mContentListTable.ops = nsnull; mAnonymousNodesTable.ops = nsnull; @@ -786,6 +789,28 @@ nsBindingManager::AddToAttachedQueue(nsXBLBinding* aBinding) return NS_ERROR_OUT_OF_MEMORY; NS_ADDREF(aBinding); + + // If we're in the middle of processing our queue already, don't + // bother posting the event. + if (!mProcessingAttachedStack && !mProcessAttachedQueueEvent) { + nsCOMPtr eventQueueService = + do_GetService(NS_EVENTQUEUESERVICE_CONTRACTID); + nsCOMPtr eventQueue; + if (eventQueueService) { + eventQueueService-> + GetSpecialEventQueue(nsIEventQueueService::UI_THREAD_EVENT_QUEUE, + getter_AddRefs(eventQueue)); + } + if (eventQueue) { + ProcessAttachedQueueEvent* ev = new ProcessAttachedQueueEvent(this); + if (ev && NS_FAILED(eventQueue->PostEvent(ev))) { + PL_DestroyEvent(ev); + } else { + mProcessAttachedQueueEvent = ev; + } + } + } + return NS_OK; } @@ -797,10 +822,21 @@ nsBindingManager::ClearAttachedQueue() return NS_OK; } +void +nsBindingManager::DoProcessAttachedQueue() +{ + ProcessAttachedQueue(); + + NS_ASSERTION(mAttachedStack.Count() == 0, + "Shouldn't have pending bindings!"); + + mProcessAttachedQueueEvent = nsnull; +} + NS_IMETHODIMP nsBindingManager::ProcessAttachedQueue() { - if (mProcessingAttachedStack) + if (mProcessingAttachedStack || mAttachedStack.Count() == 0) return NS_OK; mProcessingAttachedStack = PR_TRUE; @@ -817,7 +853,7 @@ nsBindingManager::ProcessAttachedQueue() } mProcessingAttachedStack = PR_FALSE; - ClearAttachedQueue(); + NS_ASSERTION(mAttachedStack.Count() == 0, "How did we get here?"); return NS_OK; } @@ -1309,3 +1345,57 @@ nsBindingManager::ContentRemoved(nsIDocument* aDocument, } } } + +void +nsBindingManager::DocumentWillBeDestroyed(nsIDocument* aDocument) +{ + // Make sure to not run any more XBL constructors + mProcessingAttachedStack = PR_TRUE; + + mDocument = nsnull; +} + +void +nsBindingManager::BeginOutermostUpdate() +{ + mProcessOnEndUpdate = (mAttachedStack.Count() == 0); +} + +void +nsBindingManager::EndOutermostUpdate() +{ + if (mProcessOnEndUpdate) { + mProcessOnEndUpdate = PR_FALSE; + ProcessAttachedQueue(); + } +} + +static void PR_CALLBACK +HandlePLEvent(nsBindingManager::ProcessAttachedQueueEvent* aEvent) +{ + aEvent->HandleEvent(); +} + +static void PR_CALLBACK +DestroyPLEvent(nsBindingManager::ProcessAttachedQueueEvent* aEvent) +{ + delete aEvent; +} + +nsBindingManager::ProcessAttachedQueueEvent::ProcessAttachedQueueEvent(nsBindingManager* aBindingManager) + : mBindingManager(aBindingManager) +{ + PL_InitEvent(this, aBindingManager, + (PLHandleEventProc) ::HandlePLEvent, + (PLDestroyEventProc) ::DestroyPLEvent); + if (aBindingManager->mDocument) { + aBindingManager->mDocument->BlockOnload(); + } +} + +nsBindingManager::ProcessAttachedQueueEvent::~ProcessAttachedQueueEvent() +{ + if (mBindingManager->mDocument) { + mBindingManager->mDocument->UnblockOnload(); + } +} diff --git a/mozilla/content/xbl/src/nsBindingManager.h b/mozilla/content/xbl/src/nsBindingManager.h index 5d0fcf8f14d..01e840a40f6 100755 --- a/mozilla/content/xbl/src/nsBindingManager.h +++ b/mozilla/content/xbl/src/nsBindingManager.h @@ -47,6 +47,7 @@ #include "nsInterfaceHashtable.h" #include "nsRefPtrHashtable.h" #include "nsURIHashKey.h" +#include "plevent.h" class nsIContent; class nsIXPConnectWrappedJS; @@ -66,7 +67,7 @@ class nsBindingManager : public nsIBindingManager, NS_DECL_ISUPPORTS public: - nsBindingManager(); + nsBindingManager(nsIDocument* aDocument); ~nsBindingManager(); virtual nsXBLBinding* GetBinding(nsIContent* aContent); @@ -141,6 +142,26 @@ public: nsIContent* aContainer, nsIContent* aChild, PRInt32 aIndexInContainer); + virtual void DocumentWillBeDestroyed(nsIDocument* aDocument); + + struct ProcessAttachedQueueEvent; + friend struct ProcessAttachedQueueEvent; + + struct ProcessAttachedQueueEvent : public PLEvent { + ProcessAttachedQueueEvent(nsBindingManager* aBindingManager); + ~ProcessAttachedQueueEvent(); + + void HandleEvent() { + mBindingManager->DoProcessAttachedQueue(); + } + + nsRefPtr mBindingManager; + }; + + // Notify the binding manager when an outermost update begins and + // ends. The end method can execute script. + void BeginOutermostUpdate(); + void EndOutermostUpdate(); protected: nsresult GetXBLChildNodesInternal(nsIContent* aContent, @@ -156,6 +177,10 @@ protected: nsresult GetNestedInsertionPoint(nsIContent* aParent, nsIContent* aChild, nsIContent** aResult); + // Same as ProcessAttachedQueue, but also nulls out + // mProcessAttachedQueueEvent + void DoProcessAttachedQueue(); + // MEMBER VARIABLES protected: // A mapping from nsIContent* to the nsXBLBinding* that is @@ -203,7 +228,14 @@ protected: // A queue of binding attached event handlers that are awaiting execution. nsVoidArray mAttachedStack; - PRBool mProcessingAttachedStack; + PRPackedBool mProcessingAttachedStack; + PRPackedBool mProcessOnEndUpdate; + + // Our posted event to process the attached queue, if any + ProcessAttachedQueueEvent* mProcessAttachedQueueEvent; + + // Our document. This is a weak ref; the document owns us + nsIDocument* mDocument; }; PRBool PR_CALLBACK ReleaseInsertionPoint(void* aElement, void* aData); diff --git a/mozilla/content/xbl/src/nsXBLResourceLoader.cpp b/mozilla/content/xbl/src/nsXBLResourceLoader.cpp index 11c70570576..2e56ad0246c 100644 --- a/mozilla/content/xbl/src/nsXBLResourceLoader.cpp +++ b/mozilla/content/xbl/src/nsXBLResourceLoader.cpp @@ -238,12 +238,6 @@ nsXBLResourceLoader::NotifyBoundElements() // Flush first to make sure we can get the frame for content doc->FlushPendingNotifications(Flush_Frames); - // Notify - nsIContent* parent = content->GetParent(); - PRInt32 index = 0; - if (parent) - index = parent->IndexOf(content); - // If |content| is (in addition to having binding |mBinding|) // also a descendant of another element with binding |mBinding|, // then we might have just constructed it due to the @@ -263,8 +257,7 @@ nsXBLResourceLoader::NotifyBoundElements() shell->FrameManager()->GetUndisplayedContent(content); if (!sc) { - nsCOMPtr obs(do_QueryInterface(shell)); - obs->ContentInserted(doc, parent, content, index); + shell->RecreateFramesFor(content); } } } diff --git a/mozilla/content/xul/templates/src/nsXULContentBuilder.cpp b/mozilla/content/xul/templates/src/nsXULContentBuilder.cpp index 603e0a63204..93e828351da 100644 --- a/mozilla/content/xul/templates/src/nsXULContentBuilder.cpp +++ b/mozilla/content/xul/templates/src/nsXULContentBuilder.cpp @@ -1773,6 +1773,8 @@ nsXULContentBuilder::OpenContainer(nsIContent* aElement) if (! doc) return NS_ERROR_UNEXPECTED; + mozAutoDocUpdate upd(container->GetCurrentDoc(), UPDATE_CONTENT_MODEL, + PR_TRUE); doc->ContentAppended(container, newIndex); } @@ -1907,6 +1909,8 @@ nsXULContentBuilder::RebuildAll() if (! doc) return NS_ERROR_UNEXPECTED; + mozAutoDocUpdate upd(container->GetCurrentDoc(), UPDATE_CONTENT_MODEL, + PR_TRUE); doc->ContentAppended(container, newIndex); } diff --git a/mozilla/layout/base/nsCSSFrameConstructor.cpp b/mozilla/layout/base/nsCSSFrameConstructor.cpp index 884400e7a86..c1b493f4789 100644 --- a/mozilla/layout/base/nsCSSFrameConstructor.cpp +++ b/mozilla/layout/base/nsCSSFrameConstructor.cpp @@ -8901,9 +8901,6 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, } } - // We built some new frames. Initialize any newly-constructed bindings. - mDocument->BindingManager()->ProcessAttachedQueue(); - // process the current pseudo frame state if (!state.mPseudoFrames.IsEmpty()) { ProcessPseudoFrames(state, frameItems); @@ -9334,8 +9331,6 @@ nsCSSFrameConstructor::ContentInserted(nsIContent* aContainer, #endif } - mDocument->BindingManager()->ProcessAttachedQueue(); - // otherwise this is not a child of the root element, and we // won't let it have a frame. return NS_OK; @@ -9577,10 +9572,6 @@ nsCSSFrameConstructor::ContentInserted(nsIContent* aContainer, } } - // Now that we've created frames, run the attach queue. - //XXXwaterson should we do this after we've processed pseudos, too? - mDocument->BindingManager()->ProcessAttachedQueue(); - // process the current pseudo frame state if (!state.mPseudoFrames.IsEmpty()) ProcessPseudoFrames(state, frameItems); @@ -13216,8 +13207,6 @@ nsCSSFrameConstructor::CreateListBoxContent(nsPresContext* aPresContext, *aNewFrame = newFrame; if (NS_SUCCEEDED(rv) && (nsnull != newFrame)) { - mDocument->BindingManager()->ProcessAttachedQueue(); - // Notify the parent frame if (aIsAppend) rv = ((nsListBoxBodyFrame*)aParentFrame)->ListBoxAppendFrames(newFrame); @@ -14234,11 +14223,11 @@ nsCSSFrameConstructor::ProcessPendingRestyles() currentRestyle->mChangeHint); } + delete [] restylesToProcess; + EndUpdate(); viewManager->EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC); - - delete [] restylesToProcess; } void @@ -14283,7 +14272,7 @@ nsCSSFrameConstructor::PostRestyleEvent(nsIContent* aContent, void nsCSSFrameConstructor::RestyleEvent::HandleEvent() { nsCSSFrameConstructor* constructor = NS_STATIC_CAST(nsCSSFrameConstructor*, owner); - nsIViewManager* viewManager = + nsCOMPtr viewManager = constructor->mDocument->GetShellAt(0)->GetPresContext()->GetViewManager(); NS_ASSERTION(viewManager, "Must have view manager for update"); @@ -14299,6 +14288,7 @@ void nsCSSFrameConstructor::RestyleEvent::HandleEvent() { constructor->mRestyleEventQueue = nsnull; constructor->ProcessPendingRestyles(); + constructor->mDocument->BindingManager()->ProcessAttachedQueue(); viewManager->EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC); } diff --git a/mozilla/layout/base/nsCSSFrameConstructor.h b/mozilla/layout/base/nsCSSFrameConstructor.h index 76bd191c1f4..2cb9060a23f 100644 --- a/mozilla/layout/base/nsCSSFrameConstructor.h +++ b/mozilla/layout/base/nsCSSFrameConstructor.h @@ -138,11 +138,24 @@ public: // Note: It's the caller's responsibility to make sure to wrap a // ProcessRestyledFrames call in a view update batch. + // This function does not call ProcessAttachedQueue() on the binding manager. + // If the caller wants that to happen synchronously, it needs to handle that + // itself. nsresult ProcessRestyledFrames(nsStyleChangeList& aRestyleArray); +private: + // This function does not call ProcessAttachedQueue() on the binding manager. + // If the caller wants that to happen synchronously, it needs to handle that + // itself. void ProcessOneRestyle(nsIContent* aContent, nsReStyleHint aRestyleHint, nsChangeHint aChangeHint); + +public: + // This function does not call ProcessAttachedQueue() on the binding manager. + // If the caller wants that to happen synchronously, it needs to handle that + // itself. void ProcessPendingRestyles(); + void PostRestyleEvent(nsIContent* aContent, nsReStyleHint aRestyleHint, nsChangeHint aMinChangeHint); diff --git a/mozilla/layout/base/nsPresShell.cpp b/mozilla/layout/base/nsPresShell.cpp index 69daba67c0d..3cc808ca4ef 100644 --- a/mozilla/layout/base/nsPresShell.cpp +++ b/mozilla/layout/base/nsPresShell.cpp @@ -2985,6 +2985,9 @@ PresShell::InitialReflow(nscoord aWidth, nscoord aHeight) } } + // Run the XBL binding constructors for any new frames we've constructed + mDocument->BindingManager()->ProcessAttachedQueue(); + return NS_OK; //XXX this needs to be real. MMP } @@ -5387,7 +5390,24 @@ PresShell::FlushPendingNotifications(mozFlushType aType) mViewManager->BeginUpdateViewBatch(); if (aType & Flush_StyleReresolves) { + // Processing pending restyles can kill us, and some callers only + // hold weak refs when calling FlushPendingNotifications(). :( + nsCOMPtr kungFuDeathGrip(this); mFrameConstructor->ProcessPendingRestyles(); + if (mIsDestroying) { + // We no longer have a view manager and all that. + // XXX FIXME: Except we're in the middle of a view update batch... We + // need to address that somehow. See bug 369165. + return NS_OK; + } + + mDocument->BindingManager()->ProcessAttachedQueue(); + if (mIsDestroying) { + // We no longer have a view manager and all that. + // XXX FIXME: Except we're in the middle of a view update batch... We + // need to address that somehow. See bug 369165. + return NS_OK; + } } if (aType & Flush_OnlyReflow) {