From 419e8ff8639fb6ba6a52ab2a742581fefc9065ba Mon Sep 17 00:00:00 2001 From: "dbaron%dbaron.org" Date: Fri, 26 May 2006 01:00:21 +0000 Subject: [PATCH] Create a mechanism to allow GC participants to be marked as externally reachable due to network loads, make XMLHttpRequest a GC participant and use nsMarkedJSFunctionHolder to manage its event listeners just like DOM event listeners to avoid leaks due to cycles. b=206520 r=mrbkap, darin, bzbarsky sr=jst --- content/base/src/nsXMLHttpRequest.cpp | 210 +++++++++++++++++--------- content/base/src/nsXMLHttpRequest.h | 43 ++++-- dom/src/base/nsDOMClassInfo.cpp | 204 ++++++++++++++++++++++--- dom/src/base/nsDOMClassInfo.h | 34 ++++- dom/src/base/nsJSUtils.cpp | 7 +- dom/src/base/nsJSUtils.h | 4 + 6 files changed, 399 insertions(+), 103 deletions(-) diff --git a/content/base/src/nsXMLHttpRequest.cpp b/content/base/src/nsXMLHttpRequest.cpp index 177a15d5054..271da8f4931 100644 --- a/content/base/src/nsXMLHttpRequest.cpp +++ b/content/base/src/nsXMLHttpRequest.cpp @@ -79,6 +79,8 @@ #include "nsICachingChannel.h" #include "nsContentUtils.h" #include "nsEventDispatcher.h" +#include "nsCOMArray.h" +#include "nsDOMClassInfo.h" static const char* kLoadAsData = "loadAsData"; @@ -100,7 +102,7 @@ static NS_DEFINE_CID(kCharsetConverterManagerCID, NS_ICHARSETCONVERTERMANAGER_CI #define XML_HTTP_REQUEST_COMPLETED (1 << 4) // 4 #define XML_HTTP_REQUEST_SENT (1 << 5) // Internal, LOADING in IE and external view #define XML_HTTP_REQUEST_STOPPED (1 << 6) // Internal, INTERACTIVE in IE and external view -// The above states are mutually exclusing, change with ChangeState() only. +// The above states are mutually exclusive, change with ChangeState() only. // The states below can be combined. #define XML_HTTP_REQUEST_ABORTED (1 << 7) // Internal #define XML_HTTP_REQUEST_ASYNC (1 << 8) // Internal @@ -108,6 +110,7 @@ static NS_DEFINE_CID(kCharsetConverterManagerCID, NS_ICHARSETCONVERTERMANAGER_CI #define XML_HTTP_REQUEST_XSITEENABLED (1 << 10) // Internal #define XML_HTTP_REQUEST_SYNCLOOPING (1 << 11) // Internal #define XML_HTTP_REQUEST_MULTIPART (1 << 12) // Internal +#define XML_HTTP_REQUEST_ROOTED (1 << 13) // Internal #define XML_HTTP_REQUEST_LOADSTATES \ (XML_HTTP_REQUEST_UNINITIALIZED | \ @@ -286,6 +289,9 @@ nsXMLHttpRequest::~nsXMLHttpRequest() NS_ABORT_IF_FALSE(!(mState & XML_HTTP_REQUEST_SYNCLOOPING), "we rather crash than hang"); mState &= ~XML_HTTP_REQUEST_SYNCLOOPING; + + // Needed to free the listener arrays. + ClearEventListeners(); } @@ -301,6 +307,7 @@ NS_INTERFACE_MAP_BEGIN(nsXMLHttpRequest) NS_INTERFACE_MAP_ENTRY(nsIChannelEventSink) NS_INTERFACE_MAP_ENTRY(nsIProgressEventSink) NS_INTERFACE_MAP_ENTRY(nsIInterfaceRequestor) + NS_INTERFACE_MAP_ENTRY(nsIDOMGCParticipant) NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference) NS_INTERFACE_MAP_ENTRY_CONTENT_CLASSINFO(XMLHttpRequest) NS_INTERFACE_MAP_END @@ -319,11 +326,11 @@ nsXMLHttpRequest::AddEventListener(const nsAString& type, { NS_ENSURE_ARG(listener); + nsTArray *array; + #define IMPL_ADD_LISTENER(_type, _member) \ if (type.EqualsLiteral(_type)) { \ - if (!(_member).AppendObject(listener)) { \ - return NS_ERROR_OUT_OF_MEMORY; \ - } \ + array = &(_member); \ } else IMPL_ADD_LISTENER(LOAD_STR, mLoadEventListeners) @@ -334,6 +341,12 @@ nsXMLHttpRequest::AddEventListener(const nsAString& type, { return NS_ERROR_INVALID_ARG; } + + ListenerHolder *holder = new ListenerHolder; + NS_ENSURE_TRUE(holder, NS_ERROR_OUT_OF_MEMORY); + holder->Set(listener, this); + array->AppendElement(holder); + mScriptContext = GetCurrentContext(); #undef IMPL_ADD_LISTENER @@ -350,9 +363,10 @@ nsXMLHttpRequest::RemoveEventListener(const nsAString & type, { NS_ENSURE_ARG(listener); + nsTArray *array; #define IMPL_REMOVE_LISTENER(_type, _member) \ if (type.EqualsLiteral(_type)) { \ - (_member).RemoveObject(listener); \ + array = &(_member); \ } else IMPL_REMOVE_LISTENER(LOAD_STR, mLoadEventListeners) @@ -364,6 +378,14 @@ nsXMLHttpRequest::RemoveEventListener(const nsAString & type, return NS_ERROR_INVALID_ARG; } + // Allow a caller to remove O(N^2) behavior by removing end-to-start. + for (PRUint32 i = array->Length() - 1; i != PRUint32(-1); --i) { + if (nsCOMPtr(array->ElementAt(i)->Get()) == listener) { + array->RemoveElementAt(i); + break; + } + } + return NS_OK; } @@ -382,8 +404,7 @@ nsXMLHttpRequest::GetOnreadystatechange(nsIDOMEventListener * *aOnreadystatechan { NS_ENSURE_ARG_POINTER(aOnreadystatechange); - *aOnreadystatechange = mOnReadystatechangeListener; - NS_IF_ADDREF(*aOnreadystatechange); + mOnReadystatechangeListener.Get(aOnreadystatechange); return NS_OK; } @@ -391,7 +412,7 @@ nsXMLHttpRequest::GetOnreadystatechange(nsIDOMEventListener * *aOnreadystatechan NS_IMETHODIMP nsXMLHttpRequest::SetOnreadystatechange(nsIDOMEventListener * aOnreadystatechange) { - mOnReadystatechangeListener = aOnreadystatechange; + mOnReadystatechangeListener.Set(aOnreadystatechange, this); mScriptContext = GetCurrentContext(); @@ -405,8 +426,7 @@ nsXMLHttpRequest::GetOnload(nsIDOMEventListener * *aOnLoad) { NS_ENSURE_ARG_POINTER(aOnLoad); - *aOnLoad = mOnLoadListener; - NS_IF_ADDREF(*aOnLoad); + mOnLoadListener.Get(aOnLoad); return NS_OK; } @@ -414,7 +434,7 @@ nsXMLHttpRequest::GetOnload(nsIDOMEventListener * *aOnLoad) NS_IMETHODIMP nsXMLHttpRequest::SetOnload(nsIDOMEventListener * aOnLoad) { - mOnLoadListener = aOnLoad; + mOnLoadListener.Set(aOnLoad, this); mScriptContext = GetCurrentContext(); @@ -427,8 +447,7 @@ nsXMLHttpRequest::GetOnerror(nsIDOMEventListener * *aOnerror) { NS_ENSURE_ARG_POINTER(aOnerror); - *aOnerror = mOnErrorListener; - NS_IF_ADDREF(*aOnerror); + mOnErrorListener.Get(aOnerror); return NS_OK; } @@ -436,7 +455,7 @@ nsXMLHttpRequest::GetOnerror(nsIDOMEventListener * *aOnerror) NS_IMETHODIMP nsXMLHttpRequest::SetOnerror(nsIDOMEventListener * aOnerror) { - mOnErrorListener = aOnerror; + mOnErrorListener.Set(aOnerror, this); mScriptContext = GetCurrentContext(); @@ -449,8 +468,7 @@ nsXMLHttpRequest::GetOnprogress(nsIDOMEventListener * *aOnprogress) { NS_ENSURE_ARG_POINTER(aOnprogress); - *aOnprogress = mOnProgressListener; - NS_IF_ADDREF(*aOnprogress); + mOnProgressListener.Get(aOnprogress); return NS_OK; } @@ -458,7 +476,7 @@ nsXMLHttpRequest::GetOnprogress(nsIDOMEventListener * *aOnprogress) NS_IMETHODIMP nsXMLHttpRequest::SetOnprogress(nsIDOMEventListener * aOnprogress) { - mOnProgressListener = aOnprogress; + mOnProgressListener.Set(aOnprogress, this); mScriptContext = GetCurrentContext(); @@ -471,8 +489,7 @@ nsXMLHttpRequest::GetOnuploadprogress(nsIDOMEventListener * *aOnuploadprogress) { NS_ENSURE_ARG_POINTER(aOnuploadprogress); - *aOnuploadprogress = mOnUploadProgressListener; - NS_IF_ADDREF(*aOnuploadprogress); + mOnUploadProgressListener.Get(aOnuploadprogress); return NS_OK; } @@ -480,7 +497,7 @@ nsXMLHttpRequest::GetOnuploadprogress(nsIDOMEventListener * *aOnuploadprogress) NS_IMETHODIMP nsXMLHttpRequest::SetOnuploadprogress(nsIDOMEventListener * aOnuploadprogress) { - mOnUploadProgressListener = aOnuploadprogress; + mOnUploadProgressListener.Set(aOnuploadprogress, this); mScriptContext = GetCurrentContext(); @@ -832,8 +849,22 @@ nsXMLHttpRequest::CreateEvent(nsEvent* aEvent, const nsAString& aType, } void -nsXMLHttpRequest::NotifyEventListeners(nsIDOMEventListener* aHandler, - nsCOMArray& aListeners, +nsXMLHttpRequest::CopyEventListeners(ListenerHolder& aListener, + const nsTArray& aListenerArray, + nsCOMArray& aCopy) +{ + NS_PRECONDITION(aCopy.Count() == 0, "aCopy should start off empty"); + nsCOMPtr listener = aListener.Get(); + if (listener) + aCopy.AppendObject(listener); + + PRUint32 count = aListenerArray.Length(); + for (PRUint32 i = 0; i < count; ++i) + aCopy.AppendObject(nsCOMPtr(aListenerArray[i]->Get())); +} + +void +nsXMLHttpRequest::NotifyEventListeners(const nsCOMArray& aListeners, nsIDOMEvent* aEvent) { // XXXbz wouldn't it be easier to just have an actual nsEventListenerManager @@ -856,10 +887,6 @@ nsXMLHttpRequest::NotifyEventListeners(nsIDOMEventListener* aHandler, } } - if (aHandler) { - aHandler->HandleEvent(aEvent); - } - PRInt32 count = aListeners.Count(); for (PRInt32 index = 0; index < count; ++index) { nsIDOMEventListener* listener = aListeners[index]; @@ -877,16 +904,34 @@ nsXMLHttpRequest::NotifyEventListeners(nsIDOMEventListener* aHandler, void nsXMLHttpRequest::ClearEventListeners() { - mLoadEventListeners.Clear(); - mErrorEventListeners.Clear(); - mProgressEventListeners.Clear(); - mUploadProgressEventListeners.Clear(); - mReadystatechangeEventListeners.Clear(); - mOnLoadListener = nsnull; - mOnErrorListener = nsnull; - mOnProgressListener = nsnull; - mOnUploadProgressListener = nsnull; - mOnReadystatechangeListener = nsnull; + if (mState & XML_HTTP_REQUEST_ROOTED) { + nsDOMClassInfo::UnsetExternallyReferenced(this); + mState &= ~XML_HTTP_REQUEST_ROOTED; + } + + // This isn't *really* needed anymore now that we use + // nsMarkedJSFunctionHolder, but we may as well keep it for safety + // (against leaks) and compatibility, and also for the code to clear + // the first listener arrays (called from the destructor). + PRUint32 i, i_end; +#define CLEAR_ARRAY(member_) \ + for (i = 0, i_end = (member_).Length(); i < i_end; ++i) \ + delete (member_)[i]; \ + (member_).Clear(); + + CLEAR_ARRAY(mLoadEventListeners) + CLEAR_ARRAY(mErrorEventListeners) + CLEAR_ARRAY(mProgressEventListeners) + CLEAR_ARRAY(mUploadProgressEventListeners) + CLEAR_ARRAY(mReadystatechangeEventListeners) + +#undef CLEAR_ARRAY + + mOnLoadListener.Set(nsnull, this); + mOnErrorListener.Set(nsnull, this); + mOnProgressListener.Set(nsnull, this); + mOnUploadProgressListener.Set(nsnull, this); + mOnReadystatechangeListener.Set(nsnull, this); } already_AddRefed @@ -981,9 +1026,10 @@ nsXMLHttpRequest::OpenRequest(const nsACString& method, // a progress event handler we must load with nsIRequest::LOAD_NORMAL or // necko won't generate any progress notifications nsLoadFlags loadFlags; - if (mOnProgressListener || mOnUploadProgressListener || - mProgressEventListeners.Count() != 0 || - mUploadProgressEventListeners.Count() != 0) { + if (nsCOMPtr(mOnProgressListener.Get()) || + nsCOMPtr(mOnUploadProgressListener.Get()) || + mProgressEventListeners.Length() != 0 || + mUploadProgressEventListeners.Length() != 0) { loadFlags = nsIRequest::LOAD_NORMAL; } else { loadFlags = nsIRequest::LOAD_BACKGROUND; @@ -1435,10 +1481,15 @@ nsXMLHttpRequest::RequestCompleted() return NS_OK; } + // Grab hold of the event listeners we will need before we possibly clear + // them. + nsCOMArray loadEventListeners; + CopyEventListeners(mOnLoadListener, mLoadEventListeners, loadEventListeners); + // We need to create the event before nulling out mDocument nsEvent evt(PR_TRUE, NS_PAGE_LOAD); nsCOMPtr domevent; - if (mOnLoadListener || mLoadEventListeners.Count()) { + if (loadEventListeners.Count()) { rv = CreateEvent(&evt, EmptyString(), getter_AddRefs(domevent)); } @@ -1454,17 +1505,12 @@ nsXMLHttpRequest::RequestCompleted() } } - // Grab hold of the event listeners we will need before we possibly clear - // them. - nsCOMPtr onLoadListener = mOnLoadListener; - nsCOMArray loadEventListeners(mLoadEventListeners); - // Clear listeners here unless we're multipart ChangeState(XML_HTTP_REQUEST_COMPLETED, PR_TRUE, !(mState & XML_HTTP_REQUEST_MULTIPART)); if (NS_SUCCEEDED(rv) && domevent) { - NotifyEventListeners(onLoadListener, loadEventListeners, domevent); + NotifyEventListeners(loadEventListeners, domevent); } if (mState & XML_HTTP_REQUEST_MULTIPART) { @@ -1667,6 +1713,14 @@ nsXMLHttpRequest::Send(nsIVariant *aBody) break; } } + } else { + // If we're asynchronous, we need to prevent our event listeners + // from being garbage collected even if this object becomes + // unreachable from script, since they can fire as a result of our + // reachability from the network stack. + rv = nsDOMClassInfo::SetExternallyReferenced(this); + if (NS_SUCCEEDED(rv)) + mState |= XML_HTTP_REQUEST_ROOTED; } if (!mChannel) { @@ -1837,12 +1891,15 @@ nsXMLHttpRequest::Abort(nsIDOMEvent* aEvent) nsresult nsXMLHttpRequest::Error(nsIDOMEvent* aEvent) { + nsCOMArray errorEventListeners; + CopyEventListeners(mOnErrorListener, mErrorEventListeners, + errorEventListeners); + // We need to create the event before nulling out mDocument nsCOMPtr event = aEvent; // There is no NS_PAGE_ERROR event but NS_SCRIPT_ERROR should be ok. nsEvent evt(PR_TRUE, NS_SCRIPT_ERROR); - if (!event && - (mOnErrorListener || mErrorEventListeners.Count())) { + if (!event && errorEventListeners.Count()) { CreateEvent(&evt, EmptyString(), getter_AddRefs(event)); } @@ -1851,13 +1908,10 @@ nsXMLHttpRequest::Error(nsIDOMEvent* aEvent) mState &= ~XML_HTTP_REQUEST_SYNCLOOPING; - nsCOMPtr onErrorListener = mOnErrorListener; - nsCOMArray errorEventListeners(mErrorEventListeners); - ClearEventListeners(); if (event) { - NotifyEventListeners(onErrorListener, errorEventListeners, event); + NotifyEventListeners(errorEventListeners, event); } return NS_OK; @@ -1867,7 +1921,7 @@ nsresult nsXMLHttpRequest::ChangeState(PRUint32 aState, PRBool aBroadcast, PRBool aClearEventListeners) { - // If we are setting one of the mutually exclusing states, + // If we are setting one of the mutually exclusive states, // unset those state bits first. if (aState & XML_HTTP_REQUEST_LOADSTATES) { mState &= ~XML_HTTP_REQUEST_LOADSTATES; @@ -1876,10 +1930,10 @@ nsXMLHttpRequest::ChangeState(PRUint32 aState, PRBool aBroadcast, nsresult rv = NS_OK; // Grab private copies of the listeners we need - nsCOMPtr onReadyStateChangeListener = - mOnReadystatechangeListener; - nsCOMArray readystatechangeEventListeners( - mReadystatechangeEventListeners); + nsCOMArray readystatechangeEventListeners; + CopyEventListeners(mOnReadystatechangeListener, + mReadystatechangeEventListeners, + readystatechangeEventListeners); if (aClearEventListeners) { ClearEventListeners(); @@ -1888,15 +1942,13 @@ nsXMLHttpRequest::ChangeState(PRUint32 aState, PRBool aBroadcast, if ((mState & XML_HTTP_REQUEST_ASYNC) && (aState & XML_HTTP_REQUEST_LOADSTATES) && // Broadcast load states only aBroadcast && - (onReadyStateChangeListener || readystatechangeEventListeners.Count())) { + readystatechangeEventListeners.Count()) { nsCOMPtr event; rv = CreateEvent(nsnull, NS_LITERAL_STRING(READYSTATE_STR), getter_AddRefs(event)); NS_ENSURE_SUCCESS(rv, rv); - NotifyEventListeners(onReadyStateChangeListener, - readystatechangeEventListeners, - event); + NotifyEventListeners(readystatechangeEventListeners, event); } return rv; @@ -1967,12 +2019,16 @@ nsXMLHttpRequest::OnProgress(nsIRequest *aRequest, nsISupports *aContext, PRUint // XML_HTTP_REQUEST_SENT PRBool downloading = !((XML_HTTP_REQUEST_OPENED | XML_HTTP_REQUEST_SENT) & mState); - nsIDOMEventListener* progressListener = - downloading ? mOnProgressListener : mOnUploadProgressListener; - nsCOMArray & progressListenerArray = - downloading ? mProgressEventListeners : mUploadProgressEventListeners; + nsCOMArray progressListeners; + if (downloading) { + CopyEventListeners(mOnProgressListener, + mProgressEventListeners, progressListeners); + } else { + CopyEventListeners(mOnUploadProgressListener, + mUploadProgressEventListeners, progressListeners); + } - if (progressListener || progressListenerArray.Count()) { + if (progressListeners.Count()) { nsCOMPtr event; nsresult rv = CreateEvent(nsnull, NS_LITERAL_STRING(PROGRESS_STR), getter_AddRefs(event)); @@ -1984,9 +2040,7 @@ nsXMLHttpRequest::OnProgress(nsIRequest *aRequest, nsISupports *aContext, PRUint return NS_ERROR_OUT_OF_MEMORY; event = progressEvent; - nsCOMPtr onProgressListener = progressListener; - nsCOMArray progressListeners(progressListenerArray); - NotifyEventListeners(onProgressListener, progressListeners, event); + NotifyEventListeners(progressListeners, event); } if (mProgressEventSink) { @@ -2062,6 +2116,24 @@ nsXMLHttpRequest::GetInterface(const nsIID & aIID, void **aResult) return QueryInterface(aIID, aResult); } +///////////////////////////////////////////////////// +// nsIDOMGCParticipant methods: +// +/* virtual */ nsIDOMGCParticipant* +nsXMLHttpRequest::GetSCCIndex() +{ + return this; +} + +/* virtual */ void +nsXMLHttpRequest::AppendReachableList(nsCOMArray& aArray) +{ + nsCOMPtr gcp = do_QueryInterface(mDocument); + if (gcp) + aArray.AppendObject(gcp); +} + + NS_IMPL_ISUPPORTS1(nsXMLHttpRequest::nsHeaderVisitor, nsIHttpHeaderVisitor) NS_IMETHODIMP nsXMLHttpRequest:: diff --git a/content/base/src/nsXMLHttpRequest.h b/content/base/src/nsXMLHttpRequest.h index 5be5060dc65..03eb9aa5699 100644 --- a/content/base/src/nsXMLHttpRequest.h +++ b/content/base/src/nsXMLHttpRequest.h @@ -57,6 +57,8 @@ #include "nsIHttpHeaderVisitor.h" #include "nsIProgressEventSink.h" #include "nsCOMArray.h" +#include "nsJSUtils.h" +#include "nsTArray.h" #include "nsIDOMLSProgressEvent.h" @@ -68,8 +70,9 @@ class nsXMLHttpRequest : public nsIXMLHttpRequest, public nsIDOMEventTarget, public nsIStreamListener, public nsIChannelEventSink, - public nsIInterfaceRequestor, public nsIProgressEventSink, + public nsIInterfaceRequestor, + public nsIDOMGCParticipant, public nsSupportsWeakReference { public: @@ -111,7 +114,14 @@ public: // nsIInterfaceRequestor NS_DECL_NSIINTERFACEREQUESTOR + + // nsIDOMGCParticipant + virtual nsIDOMGCParticipant* GetSCCIndex(); + virtual void AppendReachableList(nsCOMArray& aArray); + protected: + typedef nsMarkedJSFunctionHolder ListenerHolder; + nsresult GetStreamForWString(const PRUnichar* aStr, PRInt32 aLength, nsIInputStream** aStream); @@ -142,10 +152,15 @@ protected: nsresult CreateEvent(nsEvent* event, const nsAString& aType, nsIDOMEvent** domevent); + // Make a copy of a pair of members to be passed to NotifyEventListeners. + void CopyEventListeners(ListenerHolder& aListener, + const nsTArray& aListenerArray, + nsCOMArray& aCopy); + // aListeners must be a "non-live" list (i.e., addEventListener and - // removeEventListener should not affect it). - void NotifyEventListeners(nsIDOMEventListener* aHandler, - nsCOMArray& aListeners, + // removeEventListener should not affect it). It should be built from + // member variables by calling CopyEventListeners. + void NotifyEventListeners(const nsCOMArray& aListeners, nsIDOMEvent* aEvent); void ClearEventListeners(); already_AddRefed GetCurrentHttpChannel(); @@ -155,19 +170,19 @@ protected: nsCOMPtr mReadRequest; nsCOMPtr mDocument; - nsCOMArray mLoadEventListeners; - nsCOMArray mErrorEventListeners; - nsCOMArray mProgressEventListeners; - nsCOMArray mUploadProgressEventListeners; - nsCOMArray mReadystatechangeEventListeners; + nsTArray mLoadEventListeners; + nsTArray mErrorEventListeners; + nsTArray mProgressEventListeners; + nsTArray mUploadProgressEventListeners; + nsTArray mReadystatechangeEventListeners; nsCOMPtr mScriptContext; - nsCOMPtr mOnLoadListener; - nsCOMPtr mOnErrorListener; - nsCOMPtr mOnProgressListener; - nsCOMPtr mOnUploadProgressListener; - nsCOMPtr mOnReadystatechangeListener; + nsMarkedJSFunctionHolder mOnLoadListener; + nsMarkedJSFunctionHolder mOnErrorListener; + nsMarkedJSFunctionHolder mOnProgressListener; + nsMarkedJSFunctionHolder mOnUploadProgressListener; + nsMarkedJSFunctionHolder mOnReadystatechangeListener; nsCOMPtr mXMLParserStreamListener; diff --git a/dom/src/base/nsDOMClassInfo.cpp b/dom/src/base/nsDOMClassInfo.cpp index f78f992782b..d1bea55053e 100644 --- a/dom/src/base/nsDOMClassInfo.cpp +++ b/dom/src/base/nsDOMClassInfo.cpp @@ -1130,8 +1130,8 @@ static nsDOMClassInfoData sClassInfoData[] = { NS_DEFINE_CLASSINFO_DATA(XMLHttpProgressEvent, nsDOMGenericSH, DOM_DEFAULT_SCRIPTABLE_FLAGS) - NS_DEFINE_CLASSINFO_DATA(XMLHttpRequest, nsDOMGenericSH, - DOM_DEFAULT_SCRIPTABLE_FLAGS) + NS_DEFINE_CLASSINFO_DATA(XMLHttpRequest, nsDOMGCParticipantSH, + GCPARTICIPANT_SCRIPTABLE_FLAGS) // Define MOZ_SVG_FOREIGNOBJECT here so that when it gets switched on, // we preserve binary compatibility. New classes should be added @@ -3588,6 +3588,23 @@ nsDOMClassInfo::InnerObject(nsIXPConnectWrappedNative *wrapper, JSContext * cx, return NS_ERROR_UNEXPECTED; } +// These are declared here so we can assert that they're empty in ShutDown. +/** + * Every XPConnect wrapper that needs to be preserved (a wrapped native + * with JS properties set on it or used by XBL or a wrapped JS event + * handler function) as long as the element it wraps is reachable from + * script (via JS or via DOM APIs accessible from JS) gets an entry in + * this table. + * + * See PreservedWrapperEntry. + */ +static PLDHashTable sPreservedWrapperTable; + +// See ExternallyReferencedEntry. +static PLDHashTable sExternallyReferencedTable; + +// See RootWhenExternallyReferencedEntry +static PLDHashTable sRootWhenExternallyReferencedTable; // static nsIClassInfo * @@ -3648,6 +3665,20 @@ nsDOMClassInfo::GetClassInfoInstance(nsDOMClassInfoData* aData) void nsDOMClassInfo::ShutDown() { + NS_ASSERTION(sPreservedWrapperTable.ops == 0, + "preserved wrapper table not empty at shutdown"); + NS_ASSERTION(sExternallyReferencedTable.ops == 0 || + sExternallyReferencedTable.entryCount == 0, + "rooted participant table not empty at shutdown"); + NS_ASSERTION(sRootWhenExternallyReferencedTable.ops == 0, + "root when externally referenced table not empty at shutdown"); + + if (sExternallyReferencedTable.ops && + sExternallyReferencedTable.entryCount == 0) { + PL_DHashTableFinish(&sExternallyReferencedTable); + sExternallyReferencedTable.ops = nsnull; + } + if (sClassInfoData[0].u.mConstructorFptr) { PRUint32 i; @@ -4919,24 +4950,60 @@ nsDOMConstructor::ToString(nsAString &aResult) #define DEBUG_PRESERVE_WRAPPERS #endif -/** - * Every XPConnect wrapper that needs to be preserved (a wrapped native - * with JS properties set on it or used by XBL or a wrapped JS event - * handler function) as long as the element it wraps is reachable from - * script (via JS or via DOM APIs accessible from JS) gets an entry in - * this table. - */ -static PLDHashTable sPreservedWrapperTable; +struct RootWhenExternallyReferencedEntry : public PLDHashEntryHdr { + // must be first to line up with PLDHashEntryStub + nsIDOMGCParticipant *participant; + PRUint32 refcnt; +}; struct PreservedWrapperEntry : public PLDHashEntryHdr { void *key; // must be first to line up with PLDHashEntryStub nsIXPConnectJSObjectHolder* (*keyToWrapperFunc)(void* aKey); nsIDOMGCParticipant *participant; + PRBool rootWhenExternallyReferenced; // See |WrapperSCCEntry::first|. Valid only during mark phase of GC. PreservedWrapperEntry *next; }; +PR_STATIC_CALLBACK(void) +PreservedWrapperClearEntry(PLDHashTable *table, PLDHashEntryHdr *hdr) +{ + PreservedWrapperEntry *entry = NS_STATIC_CAST(PreservedWrapperEntry*, hdr); + + if (entry->rootWhenExternallyReferenced) { + NS_ASSERTION(sRootWhenExternallyReferencedTable.ops, + "must have been added to rwer table"); + RootWhenExternallyReferencedEntry *rwerEntry = + NS_STATIC_CAST(RootWhenExternallyReferencedEntry*, + PL_DHashTableOperate(&sRootWhenExternallyReferencedTable, + entry->participant, PL_DHASH_LOOKUP)); + NS_ASSERTION(PL_DHASH_ENTRY_IS_BUSY(rwerEntry), + "boolean vs. table mismatch"); + if (PL_DHASH_ENTRY_IS_BUSY(rwerEntry) && --rwerEntry->refcnt == 0) { + PL_DHashTableRawRemove(&sRootWhenExternallyReferencedTable, rwerEntry); + if (sRootWhenExternallyReferencedTable.entryCount == 0) { + PL_DHashTableFinish(&sRootWhenExternallyReferencedTable); + sRootWhenExternallyReferencedTable.ops = nsnull; + } + } + } + + memset(hdr, 0, table->entrySize); +} + +static const PLDHashTableOps sPreservedWrapperTableOps = { + PL_DHashAllocTable, + PL_DHashFreeTable, + PL_DHashGetKeyStub, + PL_DHashVoidPtrKeyStub, + PL_DHashMatchEntryStub, + PL_DHashMoveEntryStub, + PreservedWrapperClearEntry, + PL_DHashFinalizeStub, + nsnull +}; + /** * At the beginning of the mark phase of the GC, we sort all the * wrappers into their strongly connected components. We maintain this @@ -5005,7 +5072,8 @@ static const PLDHashTableOps sWrapperSCCTableOps = { nsresult nsDOMClassInfo::PreserveWrapper(void *aKey, nsIXPConnectJSObjectHolder* (*aKeyToWrapperFunc)(void* aKey), - nsIDOMGCParticipant *aParticipant) + nsIDOMGCParticipant *aParticipant, + PRBool aRootWhenExternallyReferenced) { NS_PRECONDITION(aKey, "unexpected null pointer"); NS_PRECONDITION(aKeyToWrapperFunc, "unexpected null pointer"); @@ -5014,8 +5082,8 @@ nsDOMClassInfo::PreserveWrapper(void *aKey, "cannot change preserved wrapper table during mark phase"); if (!sPreservedWrapperTable.ops && - !PL_DHashTableInit(&sPreservedWrapperTable, PL_DHashGetStubOps(), nsnull, - sizeof(PreservedWrapperEntry), 16)) { + !PL_DHashTableInit(&sPreservedWrapperTable, &sPreservedWrapperTableOps, + nsnull, sizeof(PreservedWrapperEntry), 16)) { sPreservedWrapperTable.ops = nsnull; return NS_ERROR_OUT_OF_MEMORY; } @@ -5024,10 +5092,42 @@ nsDOMClassInfo::PreserveWrapper(void *aKey, PL_DHashTableOperate(&sPreservedWrapperTable, aKey, PL_DHASH_ADD)); if (!entry) return NS_ERROR_OUT_OF_MEMORY; + NS_ASSERTION(!entry->key || + (entry->key == aKey && + entry->keyToWrapperFunc == aKeyToWrapperFunc && + entry->participant == aParticipant && + !entry->rootWhenExternallyReferenced && + !aRootWhenExternallyReferenced), + "preservation key already used"); entry->key = aKey; entry->keyToWrapperFunc = aKeyToWrapperFunc; entry->participant = aParticipant; + entry->rootWhenExternallyReferenced = aRootWhenExternallyReferenced; + + if (aRootWhenExternallyReferenced) { + if (!sRootWhenExternallyReferencedTable.ops && + !PL_DHashTableInit(&sRootWhenExternallyReferencedTable, + PL_DHashGetStubOps(), nsnull, + sizeof(RootWhenExternallyReferencedEntry), 16)) { + PL_DHashTableRawRemove(&sPreservedWrapperTable, entry); + return NS_ERROR_OUT_OF_MEMORY; + } + + RootWhenExternallyReferencedEntry *rwerEntry = + NS_STATIC_CAST(RootWhenExternallyReferencedEntry*, + PL_DHashTableOperate(&sRootWhenExternallyReferencedTable, + aParticipant, PL_DHASH_ADD)); + if (!entry) { + PL_DHashTableRawRemove(&sPreservedWrapperTable, entry); + return NS_ERROR_OUT_OF_MEMORY; + } + NS_ASSERTION(rwerEntry->refcnt == 0 || + rwerEntry->participant == aParticipant, + "entry mismatch"); + rwerEntry->participant = aParticipant; + ++rwerEntry->refcnt; + } return NS_OK; } @@ -5048,7 +5148,7 @@ nsDOMClassInfo::PreserveNodeWrapper(nsIXPConnectWrappedNative *aWrapper) return NS_OK; return nsDOMClassInfo::PreserveWrapper(aWrapper, IdentityKeyToWrapperFunc, - participant); + participant, PR_FALSE); } // static @@ -5103,7 +5203,7 @@ nsDOMClassInfo::MarkReachablePreservedWrappers(nsIDOMGCParticipant *aParticipant // the mark phase. However, we can determine that the mark phase // has begun by detecting the first mark and lazily call // BeginGCMark. - if (!sWrapperSCCTable.ops && !nsDOMClassInfo::BeginGCMark()) { + if (!sWrapperSCCTable.ops && !nsDOMClassInfo::BeginGCMark(cx)) { // We didn't have enough memory to create the temporary data // structures we needed. sWrapperSCCTable.ops = WRAPPER_SCC_OPS_OOM_MARKER; @@ -5159,6 +5259,45 @@ nsDOMClassInfo::MarkReachablePreservedWrappers(nsIDOMGCParticipant *aParticipant } } +struct ExternallyReferencedEntry : public PLDHashEntryHdr { + nsIDOMGCParticipant* key; // must be first to line up with PLDHashEntryStub +}; + +/* static */ nsresult +nsDOMClassInfo::SetExternallyReferenced(nsIDOMGCParticipant *aParticipant) +{ + NS_PRECONDITION(aParticipant, "unexpected null pointer"); + + if (!sExternallyReferencedTable.ops && + !PL_DHashTableInit(&sExternallyReferencedTable, PL_DHashGetStubOps(), + nsnull, sizeof(ExternallyReferencedEntry), 16)) { + sExternallyReferencedTable.ops = nsnull; + return NS_ERROR_OUT_OF_MEMORY; + } + + ExternallyReferencedEntry *entry = NS_STATIC_CAST(ExternallyReferencedEntry*, + PL_DHashTableOperate(&sExternallyReferencedTable, aParticipant, PL_DHASH_ADD)); + if (!entry) + return NS_ERROR_OUT_OF_MEMORY; + + NS_ASSERTION(!entry->key, "participant already rooted"); + entry->key = aParticipant; + + return NS_OK; +} + +/* static */ void +nsDOMClassInfo::UnsetExternallyReferenced(nsIDOMGCParticipant *aParticipant) +{ + NS_PRECONDITION(aParticipant, "unexpected null pointer"); + + PL_DHashTableOperate(&sExternallyReferencedTable, aParticipant, + PL_DHASH_REMOVE); + + // Don't destroy the table when the entryCount hits zero, since that + // is expected to happen often. Instead, destroy it at shutdown. +} + PR_STATIC_CALLBACK(PLDHashOperator) ClassifyWrapper(PLDHashTable *table, PLDHashEntryHdr *hdr, PRUint32 number, void *arg) @@ -5193,9 +5332,34 @@ ClassifyWrapper(PLDHashTable *table, PLDHashEntryHdr *hdr, return PL_DHASH_NEXT; } +PR_STATIC_CALLBACK(PLDHashOperator) +MarkExternallyReferenced(PLDHashTable *table, PLDHashEntryHdr *hdr, + PRUint32 number, void *arg) +{ + ExternallyReferencedEntry *entry = + NS_STATIC_CAST(ExternallyReferencedEntry*, hdr); + JSContext *cx = NS_STATIC_CAST(JSContext*, arg); + + nsIDOMGCParticipant *erParticipant = entry->key; + + if (sRootWhenExternallyReferencedTable.ops) { + RootWhenExternallyReferencedEntry *rwerEntry = + NS_STATIC_CAST(RootWhenExternallyReferencedEntry*, + PL_DHashTableOperate(&sRootWhenExternallyReferencedTable, + erParticipant, PL_DHASH_LOOKUP)); + if (PL_DHASH_ENTRY_IS_BUSY(rwerEntry)) { + NS_ASSERTION(rwerEntry->refcnt > 0, "bad reference count"); + // XXX Construct something to say where the mark is coming from? + nsDOMClassInfo::MarkReachablePreservedWrappers(entry->key, cx, nsnull); + } + } + + return PL_DHASH_NEXT; +} + // static PRBool -nsDOMClassInfo::BeginGCMark() +nsDOMClassInfo::BeginGCMark(JSContext *cx) { NS_PRECONDITION(!sWrapperSCCTable.ops, "table already initialized"); @@ -5209,8 +5373,9 @@ nsDOMClassInfo::BeginGCMark() } PRBool failure = PR_FALSE; - if (sPreservedWrapperTable.ops) + if (sPreservedWrapperTable.ops) { PL_DHashTableEnumerate(&sPreservedWrapperTable, ClassifyWrapper, &failure); + } if (failure) { PL_DHashTableFinish(&sWrapperSCCTable); // caller will reset table ops @@ -5221,6 +5386,11 @@ nsDOMClassInfo::BeginGCMark() printf("Marking:\n"); #endif + if (sExternallyReferencedTable.ops) { + PL_DHashTableEnumerate(&sExternallyReferencedTable, + MarkExternallyReferenced, cx); + } + return PR_TRUE; } diff --git a/dom/src/base/nsDOMClassInfo.h b/dom/src/base/nsDOMClassInfo.h index 7faf0b29e3f..ea55f4aed61 100644 --- a/dom/src/base/nsDOMClassInfo.h +++ b/dom/src/base/nsDOMClassInfo.h @@ -184,10 +184,15 @@ public: * No strong references are held as a result of this function call, so * the caller is responsible for calling |ReleaseWrapper| sometime * before |aParticipant|'s destructor runs. + * + * aRootWhenExternallyReferenced should be true if the preservation is + * for an event handler that could be triggered by the participant + * being externally referenced by a network load. */ static nsresult PreserveWrapper(void* aKey, nsIXPConnectJSObjectHolder* (*aKeyToWrapperFunc)(void* aKey), - nsIDOMGCParticipant *aParticipant); + nsIDOMGCParticipant *aParticipant, + PRBool aRootWhenExternallyReferenced); /** @@ -212,11 +217,36 @@ public: static void MarkReachablePreservedWrappers(nsIDOMGCParticipant *aParticipant, JSContext *cx, void *arg); + /** + * Add/remove |aParticipant| from the table of externally referenced + * participants. Does not maintain a count, so an object should not + * be added when it is already in the table. + * + * The table of externally referenced participants is a list of + * participants that should be marked at GC-time **if they are a + * participant in the preserved wrapper table added with + * aRootWhenExternallyReferenced**, whether or not they are reachable + * from marked participants. This should be used for participants + * that hold onto scripts (typically onload or onerror handlers) that + * can be triggered at the end of a currently-ongoing operation + * (typically a network request) and that could thus expose the + * participant to script again in the future even though it is not + * otherwise reachable. + * + * The caller is responsible for ensuring that the GC participant is + * alive while it is in this table; the table does not own a reference. + * + * UnsetExternallyReferenced must be called exactly once for every + * successful SetExternallyReferenced call, and in no other cases. + */ + static nsresult SetExternallyReferenced(nsIDOMGCParticipant *aParticipant); + static void UnsetExternallyReferenced(nsIDOMGCParticipant *aParticipant); + /** * Classify the wrappers for use by |MarkReachablePreservedWrappers| * during the GC. Returns false to indicate failure (out-of-memory). */ - static PRBool BeginGCMark(); + static PRBool BeginGCMark(JSContext *cx); /** * Clean up data structures (and strong references) created by diff --git a/dom/src/base/nsJSUtils.cpp b/dom/src/base/nsJSUtils.cpp index 649b0df0e17..1db63ce50f2 100644 --- a/dom/src/base/nsJSUtils.cpp +++ b/dom/src/base/nsJSUtils.cpp @@ -268,8 +268,13 @@ nsMarkedJSFunctionHolder_base::TryMarkedSet(nsISupports *aPotentialFunction, if (!wrappedJS) // a non-JS implementation return PR_FALSE; + // XXX We really only need to pass PR_TRUE for + // root-if-externally-referenced if this is an onload, onerror, + // onreadystatechange, etc., so we could pass the responsibility for + // choosing that to the caller. nsresult rv = - nsDOMClassInfo::PreserveWrapper(this, HolderToWrappedJS, aParticipant); + nsDOMClassInfo::PreserveWrapper(this, HolderToWrappedJS, aParticipant, + PR_TRUE); NS_ENSURE_SUCCESS(rv, PR_FALSE); nsIWeakReference* weakRef; // [STRONG] diff --git a/dom/src/base/nsJSUtils.h b/dom/src/base/nsJSUtils.h index b0fa7718c74..f90ac5911be 100644 --- a/dom/src/base/nsJSUtils.h +++ b/dom/src/base/nsJSUtils.h @@ -143,6 +143,10 @@ public: already_AddRefed Get() { return already_AddRefed(NS_STATIC_CAST(T*, nsMarkedJSFunctionHolder_base::Get(NS_GET_TEMPLATE_IID(T)).get())); } + // An overloaded version that's more useful for XPCOM getters + void Get(T** aResult) { + *aResult = Get().get(); + } }; #endif /* nsJSUtils_h__ */