From b732278733edd424bb50aec47107f3503719e76f Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 19 Mar 2013 09:04:57 -0700 Subject: [PATCH] Bug 850247 - Make nsRequestObserverProxy hold onto its context. r=mcmanus Both the consumers just do this manually. The context here is sometimes an XPCWrappedJS, so passing it around with nsCOMPtrs off-main-thread can't happen anymore. We add assertions in OnStartRequest/OnStopRequest to catch any consumers that might try to change contexts midstream. --- dom/file/AsyncHelper.cpp | 8 +++--- dom/file/AsyncHelper.h | 1 - .../base/public/nsIRequestObserverProxy.idl | 8 ++++-- netwerk/base/public/nsNetUtil.h | 5 ++-- netwerk/base/src/nsAsyncStreamCopier.cpp | 7 ++--- netwerk/base/src/nsAsyncStreamCopier.h | 1 - netwerk/base/src/nsRequestObserverProxy.cpp | 26 +++++++++---------- netwerk/base/src/nsRequestObserverProxy.h | 4 +-- 8 files changed, 29 insertions(+), 31 deletions(-) diff --git a/dom/file/AsyncHelper.cpp b/dom/file/AsyncHelper.cpp index 5adbba949ed4..e9714195a59d 100644 --- a/dom/file/AsyncHelper.cpp +++ b/dom/file/AsyncHelper.cpp @@ -23,12 +23,10 @@ AsyncHelper::AsyncWork(nsIRequestObserver* aObserver, nsISupports* aCtxt) if (aObserver) { // build proxy for observer events - rv = NS_NewRequestObserverProxy(getter_AddRefs(mObserver), aObserver); + rv = NS_NewRequestObserverProxy(getter_AddRefs(mObserver), aObserver, aCtxt); NS_ENSURE_SUCCESS(rv, rv); } - mCtxt = aCtxt; - FileService* service = FileService::GetOrCreate(); NS_ENSURE_TRUE(service, NS_ERROR_FAILURE); @@ -46,13 +44,13 @@ AsyncHelper::Run() NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!"); if (mObserver) { - mObserver->OnStartRequest(this, mCtxt); + mObserver->OnStartRequest(this, nullptr); } mStatus = DoStreamWork(mStream); if (mObserver) { - mObserver->OnStopRequest(this, mCtxt, mStatus); + mObserver->OnStopRequest(this, nullptr, mStatus); } return NS_OK; diff --git a/dom/file/AsyncHelper.h b/dom/file/AsyncHelper.h index 3a819d6042dc..257e690d3c54 100644 --- a/dom/file/AsyncHelper.h +++ b/dom/file/AsyncHelper.h @@ -47,7 +47,6 @@ protected: private: nsCOMPtr mStream; nsCOMPtr mObserver; - nsCOMPtr mCtxt; nsresult mStatus; }; diff --git a/netwerk/base/public/nsIRequestObserverProxy.idl b/netwerk/base/public/nsIRequestObserverProxy.idl index 9365d75084d1..7b79f5342741 100644 --- a/netwerk/base/public/nsIRequestObserverProxy.idl +++ b/netwerk/base/public/nsIRequestObserverProxy.idl @@ -15,13 +15,17 @@ interface nsIEventTarget; * This interface only provides the initialization needed after construction. * Otherwise, these objects are used simply as nsIRequestObserver's. */ -[scriptable, uuid(fe743352-34db-4bd1-9694-fe58e4442d7b)] +[scriptable, uuid(c2b06151-1bf8-4eef-aea9-1532f12f5a10)] interface nsIRequestObserverProxy : nsIRequestObserver { /** * Initializes an nsIRequestObserverProxy. * * @param observer - receives observer notifications on the main thread + * @param context - the context argument that will be passed to OnStopRequest + * and OnStartRequest. This has to be stored permanently on + * initialization because it sometimes can't be + * AddRef/Release'd off-main-thread. */ - void init(in nsIRequestObserver observer); + void init(in nsIRequestObserver observer, in nsISupports context); }; diff --git a/netwerk/base/public/nsNetUtil.h b/netwerk/base/public/nsNetUtil.h index 2fa643d56af4..3567a08f4b7a 100644 --- a/netwerk/base/public/nsNetUtil.h +++ b/netwerk/base/public/nsNetUtil.h @@ -656,13 +656,14 @@ NS_ImplementChannelOpen(nsIChannel *channel, inline nsresult NS_NewRequestObserverProxy(nsIRequestObserver **result, - nsIRequestObserver *observer) + nsIRequestObserver *observer, + nsISupports *context) { nsresult rv; nsCOMPtr proxy = do_CreateInstance(NS_REQUESTOBSERVERPROXY_CONTRACTID, &rv); if (NS_SUCCEEDED(rv)) { - rv = proxy->Init(observer); + rv = proxy->Init(observer, context); if (NS_SUCCEEDED(rv)) NS_ADDREF(*result = proxy); // cannot use nsCOMPtr::swap } diff --git a/netwerk/base/src/nsAsyncStreamCopier.cpp b/netwerk/base/src/nsAsyncStreamCopier.cpp index d1e8a4772988..92f507f35922 100644 --- a/netwerk/base/src/nsAsyncStreamCopier.cpp +++ b/netwerk/base/src/nsAsyncStreamCopier.cpp @@ -67,9 +67,7 @@ nsAsyncStreamCopier::Complete(nsresult status) // setup OnStopRequest callback and release references... observer = mObserver; - ctx = mObserverContext; mObserver = nullptr; - mObserverContext = nullptr; } } @@ -226,7 +224,7 @@ nsAsyncStreamCopier::AsyncCopy(nsIRequestObserver *observer, nsISupports *ctx) if (observer) { // build proxy for observer events - rv = NS_NewRequestObserverProxy(getter_AddRefs(mObserver), observer); + rv = NS_NewRequestObserverProxy(getter_AddRefs(mObserver), observer, ctx); if (NS_FAILED(rv)) return rv; } @@ -234,9 +232,8 @@ nsAsyncStreamCopier::AsyncCopy(nsIRequestObserver *observer, nsISupports *ctx) // will be reported via OnStopRequest. mIsPending = true; - mObserverContext = ctx; if (mObserver) { - rv = mObserver->OnStartRequest(this, mObserverContext); + rv = mObserver->OnStartRequest(this, nullptr); if (NS_FAILED(rv)) Cancel(rv); } diff --git a/netwerk/base/src/nsAsyncStreamCopier.h b/netwerk/base/src/nsAsyncStreamCopier.h index 909969c85ef3..6df2618b86d9 100644 --- a/netwerk/base/src/nsAsyncStreamCopier.h +++ b/netwerk/base/src/nsAsyncStreamCopier.h @@ -39,7 +39,6 @@ private: nsCOMPtr mSink; nsCOMPtr mObserver; - nsCOMPtr mObserverContext; nsCOMPtr mTarget; diff --git a/netwerk/base/src/nsRequestObserverProxy.cpp b/netwerk/base/src/nsRequestObserverProxy.cpp index 363266a00184..c0b9fcc0543e 100644 --- a/netwerk/base/src/nsRequestObserverProxy.cpp +++ b/netwerk/base/src/nsRequestObserverProxy.cpp @@ -26,10 +26,8 @@ static PRLogModuleInfo *gRequestObserverProxyLog; // nsARequestObserverEvent internal class... //----------------------------------------------------------------------------- -nsARequestObserverEvent::nsARequestObserverEvent(nsIRequest *request, - nsISupports *context) +nsARequestObserverEvent::nsARequestObserverEvent(nsIRequest *request) : mRequest(request) - , mContext(context) { NS_PRECONDITION(mRequest, "null pointer"); } @@ -43,9 +41,8 @@ class nsOnStartRequestEvent : public nsARequestObserverEvent nsRefPtr mProxy; public: nsOnStartRequestEvent(nsRequestObserverProxy *proxy, - nsIRequest *request, - nsISupports *context) - : nsARequestObserverEvent(request, context) + nsIRequest *request) + : nsARequestObserverEvent(request) , mProxy(proxy) { NS_PRECONDITION(mProxy, "null pointer"); @@ -63,7 +60,7 @@ public: } LOG(("handle startevent=%p\n", this)); - nsresult rv = mProxy->mObserver->OnStartRequest(mRequest, mContext); + nsresult rv = mProxy->mObserver->OnStartRequest(mRequest, mProxy->mContext); if (NS_FAILED(rv)) { LOG(("OnStartRequest failed [rv=%x] canceling request!\n", rv)); rv = mRequest->Cancel(rv); @@ -83,8 +80,8 @@ class nsOnStopRequestEvent : public nsARequestObserverEvent nsRefPtr mProxy; public: nsOnStopRequestEvent(nsRequestObserverProxy *proxy, - nsIRequest *request, nsISupports *context) - : nsARequestObserverEvent(request, context) + nsIRequest *request) + : nsARequestObserverEvent(request) , mProxy(proxy) { NS_PRECONDITION(mProxy, "null pointer"); @@ -109,7 +106,7 @@ public: NS_ASSERTION(NS_SUCCEEDED(rv), "GetStatus failed for request!"); LOG(("handle stopevent=%p\n", this)); - (void) observer->OnStopRequest(mRequest, mContext, status); + (void) observer->OnStopRequest(mRequest, mProxy->mContext, status); return NS_OK; } @@ -148,10 +145,11 @@ NS_IMETHODIMP nsRequestObserverProxy::OnStartRequest(nsIRequest *request, nsISupports *context) { + MOZ_ASSERT(!context || context == mContext); LOG(("nsRequestObserverProxy::OnStartRequest [this=%x req=%x]\n", this, request)); nsOnStartRequestEvent *ev = - new nsOnStartRequestEvent(this, request, context); + new nsOnStartRequestEvent(this, request); if (!ev) return NS_ERROR_OUT_OF_MEMORY; @@ -167,6 +165,7 @@ nsRequestObserverProxy::OnStopRequest(nsIRequest *request, nsISupports *context, nsresult status) { + MOZ_ASSERT(!context || context == mContext); LOG(("nsRequestObserverProxy: OnStopRequest [this=%x req=%x status=%x]\n", this, request, status)); @@ -176,7 +175,7 @@ nsRequestObserverProxy::OnStopRequest(nsIRequest *request, // called when the OnStopRequestEvent is actually processed (see above). nsOnStopRequestEvent *ev = - new nsOnStopRequestEvent(this, request, context); + new nsOnStopRequestEvent(this, request); if (!ev) return NS_ERROR_OUT_OF_MEMORY; @@ -192,7 +191,7 @@ nsRequestObserverProxy::OnStopRequest(nsIRequest *request, //----------------------------------------------------------------------------- NS_IMETHODIMP -nsRequestObserverProxy::Init(nsIRequestObserver *observer) +nsRequestObserverProxy::Init(nsIRequestObserver *observer, nsISupports *context) { NS_ENSURE_ARG_POINTER(observer); @@ -202,6 +201,7 @@ nsRequestObserverProxy::Init(nsIRequestObserver *observer) #endif mObserver = observer; + mContext = context; return NS_OK; } diff --git a/netwerk/base/src/nsRequestObserverProxy.h b/netwerk/base/src/nsRequestObserverProxy.h index 7df6831be905..eb5d0c4d77ad 100644 --- a/netwerk/base/src/nsRequestObserverProxy.h +++ b/netwerk/base/src/nsRequestObserverProxy.h @@ -31,6 +31,7 @@ protected: virtual ~nsRequestObserverProxy(); nsCOMPtr mObserver; + nsCOMPtr mContext; friend class nsOnStartRequestEvent; friend class nsOnStopRequestEvent; @@ -39,13 +40,12 @@ protected: class nsARequestObserverEvent : public nsRunnable { public: - nsARequestObserverEvent(nsIRequest *, nsISupports *); + nsARequestObserverEvent(nsIRequest *); protected: virtual ~nsARequestObserverEvent() {} nsCOMPtr mRequest; - nsCOMPtr mContext; }; #endif // nsRequestObserverProxy_h__