From 0a621fd2462d7296fcf62153035468d162acf538 Mon Sep 17 00:00:00 2001 From: Gabor Krizsanits Date: Thu, 4 Apr 2013 11:30:36 +0200 Subject: [PATCH] Bug 820170 - nsDOMEventTarget holds a global. r=smaug --- content/base/public/nsIXMLHttpRequest.idl | 11 +-- content/base/src/nsXMLHttpRequest.cpp | 24 +++++-- content/base/src/nsXMLHttpRequest.h | 18 +++-- content/events/src/nsDOMEventTargetHelper.cpp | 67 ++++++++++++------- content/events/src/nsDOMEventTargetHelper.h | 34 +++++----- content/xul/templates/src/Makefile.in | 1 + .../src/nsXULTemplateQueryProcessorXML.cpp | 8 ++- dom/workers/XMLHttpRequest.cpp | 3 +- js/xpconnect/src/Makefile.in | 1 + js/xpconnect/src/XPCComponents.cpp | 14 ++-- 10 files changed, 117 insertions(+), 64 deletions(-) diff --git a/content/base/public/nsIXMLHttpRequest.idl b/content/base/public/nsIXMLHttpRequest.idl index 52fb039b496..42271d0360f 100644 --- a/content/base/public/nsIXMLHttpRequest.idl +++ b/content/base/public/nsIXMLHttpRequest.idl @@ -12,7 +12,7 @@ interface nsIPrincipal; interface nsIScriptContext; interface nsIURI; interface nsIVariant; -interface nsPIDOMWindow; +interface nsIGlobalObject; interface nsIInputStream; interface nsIDOMBlob; @@ -79,7 +79,7 @@ interface nsIXMLHttpRequestUpload : nsIXMLHttpRequestEventTarget { * you're aware of all the security implications. And then think twice about * it. */ -[scriptable, uuid(cd31a34e-71b5-4bea-8366-c926de9d3d62)] +[scriptable, uuid(977f1406-416a-40ac-ab89-ccd7ca0622ea)] interface nsIXMLHttpRequest : nsISupports { /** @@ -301,13 +301,16 @@ interface nsIXMLHttpRequest : nsISupports * null. * @param scriptContext The script context to use for the request. May be * null. - * @param ownerWindow The associated window for the request. May be null. + * @param globalObject The associated global for the request. Can be the + * outer window, a sandbox, or a backstage pass. + * May be null, but then the request cannot create a + * document. * @param baseURI The base URI to use when resolving relative URIs. May be * null. */ [noscript] void init(in nsIPrincipal principal, in nsIScriptContext scriptContext, - in nsPIDOMWindow ownerWindow, + in nsIGlobalObject globalObject, in nsIURI baseURI); /** diff --git a/content/base/src/nsXMLHttpRequest.cpp b/content/base/src/nsXMLHttpRequest.cpp index 28a0665feb0..0a33ff48fa7 100644 --- a/content/base/src/nsXMLHttpRequest.cpp +++ b/content/base/src/nsXMLHttpRequest.cpp @@ -342,7 +342,11 @@ nsXMLHttpRequest::Init() secMan->GetSystemPrincipal(getter_AddRefs(subjectPrincipal)); } NS_ENSURE_STATE(subjectPrincipal); - Construct(subjectPrincipal, nullptr); + + // Instead of grabbing some random global from the context stack, + // let's use the default one (junk drawer) for now. + // We should move away from this Init... + Construct(subjectPrincipal, xpc::GetNativeForGlobal(xpc::GetJunkScope())); return NS_OK; } @@ -352,15 +356,21 @@ nsXMLHttpRequest::Init() NS_IMETHODIMP nsXMLHttpRequest::Init(nsIPrincipal* aPrincipal, nsIScriptContext* aScriptContext, - nsPIDOMWindow* aOwnerWindow, + nsIGlobalObject* aGlobalObject, nsIURI* aBaseURI) { - NS_ASSERTION(!aOwnerWindow || aOwnerWindow->IsOuterWindow(), - "Expecting an outer window here!"); NS_ENSURE_ARG_POINTER(aPrincipal); - Construct(aPrincipal, - aOwnerWindow ? aOwnerWindow->GetCurrentInnerWindow() : nullptr, - aBaseURI); + + if (nsCOMPtr win = do_QueryInterface(aGlobalObject)) { + if (win->IsOuterWindow()) { + // Must be bound to inner window, innerize if necessary. + nsCOMPtr inner = do_QueryInterface( + win->GetCurrentInnerWindow()); + aGlobalObject = inner.get(); + } + } + + Construct(aPrincipal, aGlobalObject, aBaseURI); return NS_OK; } diff --git a/content/base/src/nsXMLHttpRequest.h b/content/base/src/nsXMLHttpRequest.h index 60724b245cb..208c0f63209 100644 --- a/content/base/src/nsXMLHttpRequest.h +++ b/content/base/src/nsXMLHttpRequest.h @@ -147,16 +147,16 @@ public: const mozilla::dom::MozXMLHttpRequestParameters& aParams, ErrorResult& aRv) { - nsCOMPtr window = do_QueryInterface(aGlobal.Get()); + nsCOMPtr global = do_QueryInterface(aGlobal.Get()); nsCOMPtr principal = do_QueryInterface(aGlobal.Get()); - if (!window || ! principal) { + if (!global || ! principal) { aRv.Throw(NS_ERROR_FAILURE); return nullptr; } nsRefPtr req = new nsXMLHttpRequest(); - req->Construct(principal->GetPrincipal(), window); + req->Construct(principal->GetPrincipal(), global); req->InitParameters(aParams.mMozAnon, aParams.mMozSystem); return req.forget(); } @@ -178,13 +178,14 @@ public: } void Construct(nsIPrincipal* aPrincipal, - nsPIDOMWindow* aOwnerWindow, + nsIGlobalObject* aGlobalObject, nsIURI* aBaseURI = nullptr) { MOZ_ASSERT(aPrincipal); - MOZ_ASSERT_IF(aOwnerWindow, aOwnerWindow->IsInnerWindow()); + MOZ_ASSERT_IF(nsCOMPtr win = do_QueryInterface( + aGlobalObject), win->IsInnerWindow()); mPrincipal = aPrincipal; - BindToOwner(aOwnerWindow); + BindToOwner(aGlobalObject); mBaseURI = aBaseURI; } @@ -458,6 +459,11 @@ public: // This is called by the factory constructor. nsresult Init(); + nsresult init(nsIPrincipal* principal, + nsIScriptContext* scriptContext, + nsPIDOMWindow* globalObject, + nsIURI* baseURI); + void SetRequestObserver(nsIRequestObserver* aObserver); NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS_INHERITED(nsXMLHttpRequest, diff --git a/content/events/src/nsDOMEventTargetHelper.cpp b/content/events/src/nsDOMEventTargetHelper.cpp index fb2098b0f9d..e7c8e153a32 100644 --- a/content/events/src/nsDOMEventTargetHelper.cpp +++ b/content/events/src/nsDOMEventTargetHelper.cpp @@ -26,8 +26,8 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(nsDOMEventTargetHelper) if (MOZ_UNLIKELY(cb.WantDebugInfo())) { char name[512]; nsAutoString uri; - if (tmp->mOwner && tmp->mOwner->GetExtantDocument()) { - tmp->mOwner->GetExtantDocument()->GetDocumentURI(uri); + if (tmp->mOwnerWindow && tmp->mOwnerWindow->GetExtantDocument()) { + tmp->mOwnerWindow->GetExtantDocument()->GetDocumentURI(uri); } PR_snprintf(name, sizeof(name), "nsDOMEventTargetHelper %s", NS_ConvertUTF16toUTF8(uri).get()); @@ -77,8 +77,8 @@ NS_IMPL_DOMTARGET_DEFAULTS(nsDOMEventTargetHelper) nsDOMEventTargetHelper::~nsDOMEventTargetHelper() { - if (mOwner) { - static_cast(mOwner)->RemoveEventTargetObject(this); + if (nsPIDOMWindow* owner = GetOwner()) { + static_cast(owner)->RemoveEventTargetObject(this); } if (mListenerManager) { mListenerManager->Disconnect(); @@ -89,32 +89,51 @@ nsDOMEventTargetHelper::~nsDOMEventTargetHelper() void nsDOMEventTargetHelper::BindToOwner(nsPIDOMWindow* aOwner) { - if (mOwner) { - static_cast(mOwner)->RemoveEventTargetObject(this); - mOwner = nullptr; - mHasOrHasHadOwner = false; + nsCOMPtr glob = do_QueryInterface(aOwner); + BindToOwner(glob); +} + +void +nsDOMEventTargetHelper::BindToOwner(nsIGlobalObject* aOwner) +{ + if (mParentObject) { + if (mOwnerWindow) { + static_cast(mOwnerWindow)->RemoveEventTargetObject(this); + mOwnerWindow = nullptr; + } + mParentObject = nullptr; + mHasOrHasHadOwnerWindow = false; } if (aOwner) { - mOwner = aOwner; - mHasOrHasHadOwner = true; - static_cast(mOwner)->AddEventTargetObject(this); + mParentObject = aOwner; + // Let's cache the result of this QI for fast access and off main thread usage + mOwnerWindow = nsCOMPtr(do_QueryInterface(aOwner)).get(); + if (mOwnerWindow) { + mHasOrHasHadOwnerWindow = true; + static_cast(mOwnerWindow)->AddEventTargetObject(this); + } } } void nsDOMEventTargetHelper::BindToOwner(nsDOMEventTargetHelper* aOther) { - if (mOwner) { - static_cast(mOwner)->RemoveEventTargetObject(this); - mOwner = nullptr; - mHasOrHasHadOwner = false; + if (mOwnerWindow) { + static_cast(mOwnerWindow)->RemoveEventTargetObject(this); + mOwnerWindow = nullptr; + mParentObject = nullptr; + mHasOrHasHadOwnerWindow = false; } if (aOther) { - mHasOrHasHadOwner = aOther->HasOrHasHadOwner(); - if (aOther->GetOwner()) { - mOwner = aOther->GetOwner(); - mHasOrHasHadOwner = true; - static_cast(mOwner)->AddEventTargetObject(this); + mHasOrHasHadOwnerWindow = aOther->HasOrHasHadOwner(); + if (aOther->GetParentObject()) { + mParentObject = aOther->GetParentObject(); + // Let's cache the result of this QI for fast access and off main thread usage + mOwnerWindow = nsCOMPtr(do_QueryInterface(mParentObject)).get(); + if (mOwnerWindow) { + mHasOrHasHadOwnerWindow = true; + static_cast(mOwnerWindow)->AddEventTargetObject(this); + } } } } @@ -122,7 +141,8 @@ nsDOMEventTargetHelper::BindToOwner(nsDOMEventTargetHelper* aOther) void nsDOMEventTargetHelper::DisconnectFromOwner() { - mOwner = nullptr; + mOwnerWindow = nullptr; + mParentObject = nullptr; // Event listeners can't be handled anymore, so we can release them here. if (mListenerManager) { mListenerManager->Disconnect(); @@ -308,8 +328,9 @@ nsDOMEventTargetHelper::GetContextForEventHandlers(nsresult* aRv) if (NS_FAILED(*aRv)) { return nullptr; } - return mOwner ? static_cast(mOwner)->GetContextInternal() - : nullptr; + nsPIDOMWindow* owner = GetOwner(); + return owner ? static_cast(owner)->GetContextInternal() + : nullptr; } void diff --git a/content/events/src/nsDOMEventTargetHelper.h b/content/events/src/nsDOMEventTargetHelper.h index fdc42f768f6..d851e0d4876 100644 --- a/content/events/src/nsDOMEventTargetHelper.h +++ b/content/events/src/nsDOMEventTargetHelper.h @@ -25,7 +25,7 @@ class nsDOMEvent; class nsDOMEventTargetHelper : public mozilla::dom::EventTarget { public: - nsDOMEventTargetHelper() : mOwner(nullptr), mHasOrHasHadOwner(false) {} + nsDOMEventTargetHelper() : mParentObject(nullptr), mOwnerWindow(nullptr), mHasOrHasHadOwnerWindow(false) {} virtual ~nsDOMEventTargetHelper(); NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS(nsDOMEventTargetHelper) @@ -36,10 +36,9 @@ public: void GetParentObject(nsIScriptGlobalObject **aParentObject) { - if (mOwner) { - CallQueryInterface(mOwner, aParentObject); - } - else { + if (mParentObject) { + CallQueryInterface(mParentObject, aParentObject); + } else { *aParentObject = nullptr; } } @@ -90,22 +89,24 @@ public: nsresult CheckInnerWindowCorrectness() { - NS_ENSURE_STATE(!mHasOrHasHadOwner || mOwner); - if (mOwner) { - NS_ASSERTION(mOwner->IsInnerWindow(), "Should have inner window here!\n"); - nsPIDOMWindow* outer = mOwner->GetOuterWindow(); - if (!outer || outer->GetCurrentInnerWindow() != mOwner) { + NS_ENSURE_STATE(!mHasOrHasHadOwnerWindow || mOwnerWindow); + if (mOwnerWindow) { + NS_ASSERTION(mOwnerWindow->IsInnerWindow(), "Should have inner window here!\n"); + nsPIDOMWindow* outer = mOwnerWindow->GetOuterWindow(); + if (!outer || outer->GetCurrentInnerWindow() != mOwnerWindow) { return NS_ERROR_FAILURE; } } return NS_OK; } + nsPIDOMWindow* GetOwner() const { return mOwnerWindow; } + void BindToOwner(nsIGlobalObject* aOwner); void BindToOwner(nsPIDOMWindow* aOwner); void BindToOwner(nsDOMEventTargetHelper* aOther); virtual void DisconnectFromOwner(); - nsPIDOMWindow* GetOwner() const { return mOwner; } - bool HasOrHasHadOwner() { return mHasOrHasHadOwner; } + nsIGlobalObject* GetParentObject() const { return mParentObject; } + bool HasOrHasHadOwner() { return mHasOrHasHadOwnerWindow; } protected: nsRefPtr mListenerManager; // Dispatch a trusted, non-cancellable and non-bubbling event to |this|. @@ -113,9 +114,12 @@ protected: // Make |event| trusted and dispatch |aEvent| to |this|. nsresult DispatchTrustedEvent(nsIDOMEvent* aEvent); private: - // These may be null (native callers or xpcshell). - nsPIDOMWindow* mOwner; // Inner window. - bool mHasOrHasHadOwner; + // Inner window or sandbox. + nsIGlobalObject* mParentObject; + // mParentObject pre QI-ed and cached + // (it is needed for off main thread access) + nsPIDOMWindow* mOwnerWindow; + bool mHasOrHasHadOwnerWindow; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsDOMEventTargetHelper, diff --git a/content/xul/templates/src/Makefile.in b/content/xul/templates/src/Makefile.in index 34360082f6e..13705a4ee73 100644 --- a/content/xul/templates/src/Makefile.in +++ b/content/xul/templates/src/Makefile.in @@ -50,6 +50,7 @@ include $(topsrcdir)/config/rules.mk LOCAL_INCLUDES = -I$(srcdir)/../../../base/src \ -I$(srcdir)/../../content/src \ + -I$(srcdir)/../../../../dom/base \ -I$(srcdir)/../../../../layout/xul/tree/ \ $(NULL) diff --git a/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp b/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp index ea77713e699..100bb99ef01 100644 --- a/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp +++ b/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp @@ -21,6 +21,7 @@ #include "nsArrayUtils.h" #include "nsPIDOMWindow.h" #include "nsXULContentUtils.h" +#include "nsXMLHttpRequest.h" #include "nsXULTemplateQueryProcessorXML.h" #include "nsXULTemplateResultXML.h" @@ -174,9 +175,10 @@ nsXULTemplateQueryProcessorXML::GetDatasource(nsIArray* aDataSources, do_CreateInstance(NS_XMLHTTPREQUEST_CONTRACTID, &rv); NS_ENSURE_SUCCESS(rv, rv); - nsCOMPtr owner = do_QueryInterface(scriptObject); - req->Init(docPrincipal, context, owner ? owner->GetOuterWindow() : nullptr, - nullptr); + rv = req->Init(docPrincipal, context, + scriptObject ? scriptObject : doc->GetScopeObject(), + nullptr); + NS_ENSURE_SUCCESS(rv, rv); rv = req->Open(NS_LITERAL_CSTRING("GET"), uriStr, true, EmptyString(), EmptyString()); diff --git a/dom/workers/XMLHttpRequest.cpp b/dom/workers/XMLHttpRequest.cpp index 4ed53e7053f..9a85fab197b 100644 --- a/dom/workers/XMLHttpRequest.cpp +++ b/dom/workers/XMLHttpRequest.cpp @@ -172,9 +172,10 @@ public: mXHR = new nsXMLHttpRequest(); + nsCOMPtr global = do_QueryInterface(ownerWindow); if (NS_FAILED(mXHR->Init(mWorkerPrivate->GetPrincipal(), mWorkerPrivate->GetScriptContext(), - ownerWindow, mWorkerPrivate->GetBaseURI()))) { + global, mWorkerPrivate->GetBaseURI()))) { mXHR = nullptr; return false; } diff --git a/js/xpconnect/src/Makefile.in b/js/xpconnect/src/Makefile.in index 8a52ef2a1cc..e4dc3b73ea0 100644 --- a/js/xpconnect/src/Makefile.in +++ b/js/xpconnect/src/Makefile.in @@ -65,6 +65,7 @@ LOCAL_INCLUDES = \ -I$(srcdir)/../loader \ -I$(topsrcdir)/caps/include \ -I$(topsrcdir)/content/base/src \ + -I$(topsrcdir)/content/base/public \ -I$(topsrcdir)/content/events/src \ -I$(topsrcdir)/content/html/content/src \ -I$(topsrcdir)/content/html/document/src \ diff --git a/js/xpconnect/src/XPCComponents.cpp b/js/xpconnect/src/XPCComponents.cpp index 5ba5954462d..4a1b26fef61 100644 --- a/js/xpconnect/src/XPCComponents.cpp +++ b/js/xpconnect/src/XPCComponents.cpp @@ -2952,14 +2952,18 @@ CreateXMLHttpRequest(JSContext *cx, unsigned argc, jsval *vp) if (!subjectPrincipal) return false; - nsCOMPtr xhr = new nsXMLHttpRequest(); - nsresult rv = xhr->Init(subjectPrincipal, NULL, NULL, NULL); - if (NS_FAILED(rv)) - return false; - JSObject *global = JS_GetGlobalForScopeChain(cx); MOZ_ASSERT(global); + nsIScriptObjectPrincipal *sop = + static_cast(xpc_GetJSPrivate(global)); + nsCOMPtr iglobal = do_QueryInterface(sop); + + nsCOMPtr xhr = new nsXMLHttpRequest(); + nsresult rv = xhr->Init(subjectPrincipal, nullptr, iglobal, nullptr); + if (NS_FAILED(rv)) + return false; + rv = nsContentUtils::WrapNative(cx, global, xhr, vp); if (NS_FAILED(rv)) return false;