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.
This commit is contained in:
Bobby Holley 2013-03-19 09:04:57 -07:00
parent 8c76da55f8
commit b9b61977e0
8 changed files with 29 additions and 31 deletions

View File

@ -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;

View File

@ -47,7 +47,6 @@ protected:
private:
nsCOMPtr<nsISupports> mStream;
nsCOMPtr<nsIRequestObserver> mObserver;
nsCOMPtr<nsISupports> mCtxt;
nsresult mStatus;
};

View File

@ -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);
};

View File

@ -656,13 +656,14 @@ NS_ImplementChannelOpen(nsIChannel *channel,
inline nsresult
NS_NewRequestObserverProxy(nsIRequestObserver **result,
nsIRequestObserver *observer)
nsIRequestObserver *observer,
nsISupports *context)
{
nsresult rv;
nsCOMPtr<nsIRequestObserverProxy> 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
}

View File

@ -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);
}

View File

@ -39,7 +39,6 @@ private:
nsCOMPtr<nsIOutputStream> mSink;
nsCOMPtr<nsIRequestObserver> mObserver;
nsCOMPtr<nsISupports> mObserverContext;
nsCOMPtr<nsIEventTarget> mTarget;

View File

@ -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<nsRequestObserverProxy> 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<nsRequestObserverProxy> 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;
}

View File

@ -31,6 +31,7 @@ protected:
virtual ~nsRequestObserverProxy();
nsCOMPtr<nsIRequestObserver> mObserver;
nsCOMPtr<nsISupports> 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<nsIRequest> mRequest;
nsCOMPtr<nsISupports> mContext;
};
#endif // nsRequestObserverProxy_h__