From 6ae293b233460e552b6279a22f5a5739a2114b16 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Fri, 24 Apr 2009 12:18:37 +0300 Subject: [PATCH] Bug 489581 - Reduce Addref/Release when calling event handlers, r+sr=jst --- content/base/public/nsContentUtils.h | 4 +- content/base/src/nsContentUtils.cpp | 47 ++++++++++++++-------- content/base/src/nsDOMAttribute.h | 4 +- content/base/src/nsDocument.h | 4 +- content/base/src/nsGenericDOMDataNode.h | 4 +- content/base/src/nsGenericElement.h | 4 +- content/base/src/nsXMLHttpRequest.cpp | 25 +++++++----- content/base/src/nsXMLHttpRequest.h | 2 +- content/events/public/nsPIDOMEventTarget.h | 7 ++-- dom/base/nsGlobalWindow.cpp | 11 +++-- dom/base/nsGlobalWindow.h | 2 +- dom/base/nsWindowRoot.h | 6 +-- 12 files changed, 69 insertions(+), 51 deletions(-) diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h index fab1859f1cbf..1dcec00feb11 100644 --- a/content/base/public/nsContentUtils.h +++ b/content/base/public/nsContentUtils.h @@ -1362,8 +1362,8 @@ public: static nsresult ProcessViewportInfo(nsIDocument *aDocument, const nsAString &viewportInfo); - static nsresult GetContextForEventHandlers(nsINode* aNode, - nsIScriptContext** aContext); + static nsIScriptContext* GetContextForEventHandlers(nsINode* aNode, + nsresult* aRv); static JSContext *GetCurrentJSContext(); diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp index bea701c69099..4b916ef92b8f 100644 --- a/content/base/src/nsContentUtils.cpp +++ b/content/base/src/nsContentUtils.cpp @@ -2710,8 +2710,9 @@ nsCxPusher::Push(nsPIDOMEventTarget *aCurrentTarget) } NS_ENSURE_TRUE(aCurrentTarget, PR_FALSE); - nsCOMPtr scx; - nsresult rv = aCurrentTarget->GetContextForEventHandlers(getter_AddRefs(scx)); + nsresult rv; + nsIScriptContext* scx = + aCurrentTarget->GetContextForEventHandlers(&rv); NS_ENSURE_SUCCESS(rv, PR_FALSE); if (!scx) { @@ -4503,26 +4504,38 @@ nsContentUtils::URIIsLocalFile(nsIURI *aURI) } /* static */ -nsresult +nsIScriptContext* nsContentUtils::GetContextForEventHandlers(nsINode* aNode, - nsIScriptContext** aContext) + nsresult* aRv) { - *aContext = nsnull; + *aRv = NS_OK; nsIDocument* ownerDoc = aNode->GetOwnerDoc(); - NS_ENSURE_STATE(ownerDoc); - nsCOMPtr sgo; - PRBool hasHadScriptObject = PR_TRUE; - sgo = ownerDoc->GetScriptHandlingObject(hasHadScriptObject); - // It is bad if the document doesn't have event handling context, - // but it used to have one. - NS_ENSURE_STATE(sgo || !hasHadScriptObject); - if (sgo) { - NS_IF_ADDREF(*aContext = sgo->GetContext()); - // Bad, no context from script global object! - NS_ENSURE_STATE(*aContext); + if (!ownerDoc) { + *aRv = NS_ERROR_UNEXPECTED; + return nsnull; } - return NS_OK; + PRBool hasHadScriptObject = PR_TRUE; + nsIScriptGlobalObject* sgo = + ownerDoc->GetScriptHandlingObject(hasHadScriptObject); + // It is bad if the document doesn't have event handling context, + // but it used to have one. + if (!sgo && hasHadScriptObject) { + *aRv = NS_ERROR_UNEXPECTED; + return nsnull; + } + + if (sgo) { + nsIScriptContext* scx = sgo->GetContext(); + // Bad, no context from script global object! + if (!scx) { + *aRv = NS_ERROR_UNEXPECTED; + return nsnull; + } + return scx; + } + + return nsnull; } /* static */ diff --git a/content/base/src/nsDOMAttribute.h b/content/base/src/nsDOMAttribute.h index 4d81d122fbcc..cac135e26cc9 100644 --- a/content/base/src/nsDOMAttribute.h +++ b/content/base/src/nsDOMAttribute.h @@ -109,9 +109,9 @@ public: virtual nsresult RemoveEventListenerByIID(nsIDOMEventListener *aListener, const nsIID& aIID); virtual nsresult GetSystemEventGroup(nsIDOMEventGroup** aGroup); - virtual nsresult GetContextForEventHandlers(nsIScriptContext** aContext) + virtual nsIScriptContext* GetContextForEventHandlers(nsresult* aRv) { - return nsContentUtils::GetContextForEventHandlers(this, aContext); + return nsContentUtils::GetContextForEventHandlers(this, aRv); } virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const; diff --git a/content/base/src/nsDocument.h b/content/base/src/nsDocument.h index d3669c6252af..c0d21c34a166 100644 --- a/content/base/src/nsDocument.h +++ b/content/base/src/nsDocument.h @@ -821,9 +821,9 @@ public: virtual nsresult RemoveEventListenerByIID(nsIDOMEventListener *aListener, const nsIID& aIID); virtual nsresult GetSystemEventGroup(nsIDOMEventGroup** aGroup); - virtual nsresult GetContextForEventHandlers(nsIScriptContext** aContext) + virtual nsIScriptContext* GetContextForEventHandlers(nsresult* aRv) { - return nsContentUtils::GetContextForEventHandlers(this, aContext); + return nsContentUtils::GetContextForEventHandlers(this, aRv); } virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const { diff --git a/content/base/src/nsGenericDOMDataNode.h b/content/base/src/nsGenericDOMDataNode.h index 4daa24cf48ab..e689b2caf9ad 100644 --- a/content/base/src/nsGenericDOMDataNode.h +++ b/content/base/src/nsGenericDOMDataNode.h @@ -179,9 +179,9 @@ public: virtual nsresult RemoveEventListenerByIID(nsIDOMEventListener *aListener, const nsIID& aIID); virtual nsresult GetSystemEventGroup(nsIDOMEventGroup** aGroup); - virtual nsresult GetContextForEventHandlers(nsIScriptContext** aContext) + virtual nsIScriptContext* GetContextForEventHandlers(nsresult* aRv) { - return nsContentUtils::GetContextForEventHandlers(this, aContext); + return nsContentUtils::GetContextForEventHandlers(this, aRv); } // Implementation for nsIContent diff --git a/content/base/src/nsGenericElement.h b/content/base/src/nsGenericElement.h index 172ac5736210..e9564cbebaef 100644 --- a/content/base/src/nsGenericElement.h +++ b/content/base/src/nsGenericElement.h @@ -366,9 +366,9 @@ public: virtual nsresult RemoveEventListenerByIID(nsIDOMEventListener *aListener, const nsIID& aIID); virtual nsresult GetSystemEventGroup(nsIDOMEventGroup** aGroup); - virtual nsresult GetContextForEventHandlers(nsIScriptContext** aContext) + virtual nsIScriptContext* GetContextForEventHandlers(nsresult* aRv) { - return nsContentUtils::GetContextForEventHandlers(this, aContext); + return nsContentUtils::GetContextForEventHandlers(this, aRv); } // nsIContent interface methods diff --git a/content/base/src/nsXMLHttpRequest.cpp b/content/base/src/nsXMLHttpRequest.cpp index 2fecb30f792a..b85e9b237af9 100644 --- a/content/base/src/nsXMLHttpRequest.cpp +++ b/content/base/src/nsXMLHttpRequest.cpp @@ -589,8 +589,10 @@ nsXHREventTarget::AddEventListener(const nsAString& aType, nsIDOMEventListener* aListener, PRBool aUseCapture) { - nsCOMPtr context; - GetContextForEventHandlers(getter_AddRefs(context)); + nsresult rv; + nsIScriptContext* context = + GetContextForEventHandlers(&rv); + NS_ENSURE_SUCCESS(rv, rv); nsCOMPtr doc = GetDocumentFromScriptContext(context); PRBool wantsUntrusted = doc && !nsContentUtils::IsChromeDoc(doc); return AddEventListener(aType, aListener, aUseCapture, wantsUntrusted); @@ -826,13 +828,14 @@ nsXHREventTarget::GetSystemEventGroup(nsIDOMEventGroup** aGroup) return rv; } -nsresult -nsXHREventTarget::GetContextForEventHandlers(nsIScriptContext** aContext) +nsIScriptContext* +nsXHREventTarget::GetContextForEventHandlers(nsresult* aRv) { - nsresult rv = CheckInnerWindowCorrectness(); - NS_ENSURE_SUCCESS(rv, rv); - NS_IF_ADDREF(*aContext = mScriptContext); - return NS_OK; + *aRv = CheckInnerWindowCorrectness(); + if (NS_FAILED(*aRv)) { + return nsnull; + } + return mScriptContext; } ///////////////////////////////////////////// @@ -3357,8 +3360,10 @@ NS_IMETHODIMP nsXMLHttpRequest::GetUpload(nsIXMLHttpRequestUpload** aUpload) { *aUpload = nsnull; - nsCOMPtr scriptContext; - nsresult rv = GetContextForEventHandlers(getter_AddRefs(scriptContext)); + + nsresult rv; + nsIScriptContext* scriptContext = + GetContextForEventHandlers(&rv); NS_ENSURE_SUCCESS(rv, rv); if (!mUpload) { mUpload = new nsXMLHttpRequestUpload(mOwner, scriptContext); diff --git a/content/base/src/nsXMLHttpRequest.h b/content/base/src/nsXMLHttpRequest.h index 51871891a13c..692890c2c5b5 100644 --- a/content/base/src/nsXMLHttpRequest.h +++ b/content/base/src/nsXMLHttpRequest.h @@ -186,7 +186,7 @@ public: virtual nsresult RemoveEventListenerByIID(nsIDOMEventListener *aListener, const nsIID& aIID); virtual nsresult GetSystemEventGroup(nsIDOMEventGroup** aGroup); - virtual nsresult GetContextForEventHandlers(nsIScriptContext** aContext); + virtual nsIScriptContext* GetContextForEventHandlers(nsresult* aRv); PRBool HasListenersFor(const nsAString& aType) { diff --git a/content/events/public/nsPIDOMEventTarget.h b/content/events/public/nsPIDOMEventTarget.h index 14f4f4bbb9d2..a3e28a99a766 100644 --- a/content/events/public/nsPIDOMEventTarget.h +++ b/content/events/public/nsPIDOMEventTarget.h @@ -52,8 +52,8 @@ class nsIScriptContext; // 25982813-af2e-4ab6-b512-e6c6ada6d0ec #define NS_PIDOMEVENTTARGET_IID \ - { 0x25982813, 0xaf2e, 0x4ab6, \ - { 0xb5, 0x12, 0xe6, 0xc6, 0xad, 0xa6, 0xd0, 0xec } } + { 0x358f2990, 0x5107, 0x49ba, \ + { 0x88, 0x94, 0x14, 0x34, 0x86, 0xd5, 0x99, 0x85 } } class nsPIDOMEventTarget : public nsISupports { @@ -163,8 +163,9 @@ public: /** * Get the script context in which the event handlers should be run. * May return null. + * @note Caller *must* check the value of aRv. */ - virtual nsresult GetContextForEventHandlers(nsIScriptContext** aContext) = 0; + virtual nsIScriptContext* GetContextForEventHandlers(nsresult* aRv) = 0; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsPIDOMEventTarget, NS_PIDOMEVENTTARGET_IID) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index e5b65cab2a8b..7055b1042e69 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -6456,13 +6456,12 @@ nsGlobalWindow::GetSystemEventGroup(nsIDOMEventGroup **aGroup) return NS_ERROR_FAILURE; } -nsresult -nsGlobalWindow::GetContextForEventHandlers(nsIScriptContext** aContext) +nsIScriptContext* +nsGlobalWindow::GetContextForEventHandlers(nsresult* aRv) { - NS_IF_ADDREF(*aContext = GetContext()); - // Bad, no context from script global object! - NS_ENSURE_STATE(*aContext); - return NS_OK; + nsIScriptContext* scx = GetContext(); + *aRv = scx ? NS_OK : NS_ERROR_UNEXPECTED; + return scx; } //***************************************************************************** diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index e4a7efff5b1e..38cc61cff4bc 100644 --- a/dom/base/nsGlobalWindow.h +++ b/dom/base/nsGlobalWindow.h @@ -335,7 +335,7 @@ public: virtual NS_HIDDEN_(nsresult) RemoveEventListenerByIID(nsIDOMEventListener *aListener, const nsIID& aIID); virtual NS_HIDDEN_(nsresult) GetSystemEventGroup(nsIDOMEventGroup** aGroup); - virtual NS_HIDDEN_(nsresult) GetContextForEventHandlers(nsIScriptContext** aContext); + virtual NS_HIDDEN_(nsIScriptContext*) GetContextForEventHandlers(nsresult* aRv); virtual NS_HIDDEN_(void) SetDocShell(nsIDocShell* aDocShell); virtual NS_HIDDEN_(nsresult) SetNewDocument(nsIDocument *aDocument, diff --git a/dom/base/nsWindowRoot.h b/dom/base/nsWindowRoot.h index cfc270822781..78e1ed61a579 100644 --- a/dom/base/nsWindowRoot.h +++ b/dom/base/nsWindowRoot.h @@ -83,10 +83,10 @@ public: virtual nsresult RemoveEventListenerByIID(nsIDOMEventListener *aListener, const nsIID& aIID); virtual nsresult GetSystemEventGroup(nsIDOMEventGroup** aGroup); - virtual nsresult GetContextForEventHandlers(nsIScriptContext** aContext) + virtual nsIScriptContext* GetContextForEventHandlers(nsresult* aRv) { - *aContext = nsnull; - return NS_OK; + *aRv = NS_OK; + return nsnull; } // nsPIWindowRoot