Bug 1250975. Stop passing a JSContext argument to WorkerRunnable::PreDispatch and its overrides. r=khuey

This commit is contained in:
Boris Zbarsky 2016-02-25 16:05:39 -05:00
parent 7c0e305a4c
commit c23223ed6b
8 changed files with 95 additions and 95 deletions

View File

@ -2500,20 +2500,21 @@ public:
{
}
bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
{
aWorkerPrivate->AssertIsOnWorkerThread();
aWorkerPrivate->ModifyBusyCountFromWorker(aCx, true);
return !NS_FAILED(mImpl->CancelInternal());
}
void PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult)
void PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
bool aRunResult) override
{
aWorkerPrivate->ModifyBusyCountFromWorker(aCx, false);
}
bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// We don't call WorkerRunnable::PreDispatch because it would assert the
// wrong thing about which thread we're on.
@ -2523,7 +2524,7 @@ public:
void
PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
bool aDispatchResult)
bool aDispatchResult) override
{
// We don't call WorkerRunnable::PostDispatch because it would assert the
// wrong thing about which thread we're on.
@ -2672,7 +2673,7 @@ public:
{
}
bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
{
aWorkerPrivate->AssertIsOnWorkerThread();
aWorkerPrivate->ModifyBusyCountFromWorker(aCx, true);
@ -2686,13 +2687,14 @@ public:
return !NS_FAILED(mEvent->Run());
}
void PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult)
void PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
bool aRunResult) override
{
aWorkerPrivate->ModifyBusyCountFromWorker(aCx, false);
}
bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// We don't call WorkerRunnable::PreDispatch because it would assert the
// wrong thing about which thread we're on. We're on whichever thread the
@ -2703,7 +2705,7 @@ public:
void
PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
bool aDispatchResult)
bool aDispatchResult) override
{
// We don't call WorkerRunnable::PreDispatch because it would assert the
// wrong thing about which thread we're on. We're on whichever thread the

View File

@ -373,7 +373,7 @@ protected:
}
bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// We don't call WorkerRunnable::PreDispatch because it would assert the
// wrong thing about which thread we're on.

View File

@ -1069,7 +1069,7 @@ public:
private:
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// May be called on any thread!
return true;

View File

@ -733,7 +733,7 @@ public:
private:
bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// WorkerRunnable asserts that the dispatch is from parent thread if
// the busy count modification is WorkerThreadUnchangedBusyCount.

View File

@ -369,7 +369,7 @@ public:
private:
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// Silence bad assertions.
return true;
@ -594,7 +594,7 @@ public:
private:
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
MOZ_CRASH("Don't call Dispatch() on CloseEventRunnable!");
}
@ -857,7 +857,7 @@ public:
private:
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
aWorkerPrivate->AssertIsOnParentThread();
// Modify here, but not in PostRun! This busy count addition will be matched
@ -1176,7 +1176,7 @@ private:
~TimerRunnable() {}
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// Silence bad assertions.
return true;
@ -1223,7 +1223,7 @@ private:
}
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// Silence bad assertions.
return true;
@ -1271,7 +1271,7 @@ class KillCloseEventRunnable final : public WorkerRunnable
private:
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// Silence bad assertions, this is dispatched from the timer thread.
return true;
@ -1337,7 +1337,7 @@ private:
}
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
MOZ_CRASH("Don't call Dispatch() on KillCloseEventRunnable!");
}
@ -1491,7 +1491,7 @@ public:
private:
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
// Silence bad assertions, this can be dispatched from either the main
// thread or the timer thread..
@ -1682,7 +1682,7 @@ private:
}
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
MOZ_ASSERT_UNREACHABLE("Should never call Dispatch on this!");
return true;

View File

@ -59,7 +59,7 @@ WorkerRunnable::DefaultGlobalObject() const
}
bool
WorkerRunnable::PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
WorkerRunnable::PreDispatch(WorkerPrivate* aWorkerPrivate)
{
#ifdef DEBUG
MOZ_ASSERT(aWorkerPrivate);
@ -71,7 +71,6 @@ WorkerRunnable::PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
case WorkerThreadModifyBusyCount:
aWorkerPrivate->AssertIsOnParentThread();
MOZ_ASSERT(aCx);
break;
case WorkerThreadUnchangedBusyCount:
@ -96,7 +95,7 @@ WorkerRunnable::Dispatch(JSContext* aCx)
bool ok;
if (!aCx) {
ok = PreDispatch(nullptr, mWorkerPrivate);
ok = PreDispatch(mWorkerPrivate);
if (ok) {
ok = DispatchInternal();
}
@ -113,8 +112,7 @@ WorkerRunnable::Dispatch(JSContext* aCx)
ac.emplace(aCx, global);
}
ok = PreDispatch(aCx, mWorkerPrivate);
ok = PreDispatch(mWorkerPrivate);
if (ok && !DispatchInternal()) {
ok = false;
}
@ -414,11 +412,11 @@ WorkerDebuggerRunnable::PostDispatch(JSContext* aCx,
// The only way aDispatchResult can be false here is if either PreDispatch or
// DispatchInternal returned false.
//
// Our PreDispatch always returns true and is final. We inherit
// DispatchInternal from WorkerRunnable and don't allow overriding it in our
// subclasses. WorkerRunnable::DispatchInternal only fails if one of its
// runnable dispatching functions fails, and none of those cases can throw a
// JS exception. So we can never have a JS exception here.
// PreDispatch can never throw on a JSContext. We inherit DispatchInternal
// from WorkerRunnable and don't allow overriding it in our subclasses.
// WorkerRunnable::DispatchInternal only fails if one of its runnable
// dispatching functions fails, and none of those cases can throw a JS
// exception. So we can never have a JS exception here.
MOZ_ASSERT_IF(aCx, !JS_IsExceptionPending(aCx));
}
@ -470,10 +468,10 @@ MainThreadWorkerSyncRunnable::PostDispatch(JSContext* aCx,
// The only way aDispatchResult can be false here is if either PreDispatch or
// DispatchInternal returned false.
//
// Our PreDispatch always returns true and is final. We inherit
// DispatchInternal from WorkerSyncRunnable and don't allow overriding it in
// our subclasses. WorkerSyncRunnable::DispatchInternal only returns false if
// if dispatch to the syncloop target fails or if calling up to
// PreDispatch can never throw on a JSContext. We inherit DispatchInternal
// from WorkerSyncRunnable and don't allow overriding it in our subclasses.
// WorkerSyncRunnable::DispatchInternal only returns false if if dispatch to
// the syncloop target fails or if calling up to
// WorkerRunnable::DispatchInternal fails. WorkerRunnable::DispatchInternal
// only fails if one of its runnable dispatching functions fails, and none of
// those cases can throw a JS exception. So we can never have a JS exception
@ -539,10 +537,10 @@ MainThreadStopSyncLoopRunnable::PostDispatch(JSContext* aCx,
// The only way aDispatchResult can be false here is if either PreDispatch or
// DispatchInternal returned false.
//
// Our PreDispatch always returns true and is final. We inherit
// DispatchInternal from StopSyncLoopRunnable, and that itself is final and
// only returns false if dispatch to the syncloop target fails. So we can
// never have a JS exception here.
// PreDispatch can never throw on a JSContext. We inherit DispatchInternal
// from StopSyncLoopRunnable, and that itself is final and only returns false
// if dispatch to the syncloop target fails. So we can never have a JS
// exception here.
MOZ_ASSERT_IF(aCx, !JS_IsExceptionPending(aCx));
}
@ -655,8 +653,7 @@ WorkerCheckAPIExposureOnMainThreadRunnable::Dispatch()
}
bool
WorkerSameThreadRunnable::PreDispatch(JSContext* aCx,
WorkerPrivate* aWorkerPrivate)
WorkerSameThreadRunnable::PreDispatch(WorkerPrivate* aWorkerPrivate)
{
// We don't call WorkerRunnable::PreDispatch, because we're using
// WorkerThreadModifyBusyCount for mBehavior, and WorkerRunnable will assert

View File

@ -115,10 +115,9 @@ protected:
// By default asserts that Dispatch() is being called on the right thread
// (ParentThread if |mTarget| is WorkerThread, or WorkerThread otherwise).
// Also increments the busy count of |mWorkerPrivate| if targeting the
// WorkerThread. The JSContext passed in here is the one that was passed to
// Dispatch().
// WorkerThread.
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate);
PreDispatch(WorkerPrivate* aWorkerPrivate);
// By default asserts that Dispatch() is being called on the right thread
// (ParentThread if |mTarget| is WorkerThread, or WorkerThread otherwise).
@ -185,7 +184,7 @@ private:
}
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override final
PreDispatch(WorkerPrivate* aWorkerPrivate) override final
{
AssertIsOnMainThread();
@ -251,22 +250,11 @@ protected:
virtual ~MainThreadWorkerSyncRunnable()
{ }
// Hook for subclasses that want to override our PreDispatch. This override
// must be infallible and must not leave an exception hanging out on the
// JSContext. We pass the JSContext through so callees that expect to be able
// to do something with script have a way to do it. We'd pass an AutoJSAPI,
// but some of our subclasses are dispatched with a null JSContext*, so we
// can't do that sort of thing ourselves.
virtual void InfalliblePreDispatch(JSContext* aCx)
{}
private:
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override final
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
AssertIsOnMainThread();
InfalliblePreDispatch(aCx);
MOZ_ASSERT_IF(aCx, !JS_IsExceptionPending(aCx));
return true;
}
@ -341,10 +329,8 @@ protected:
{ }
private:
// If this function stops being final, reevaluate the assumptions PostDispatch
// makes.
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override final
PreDispatch(WorkerPrivate* aWorkerPrivate) override final
{
AssertIsOnMainThread();
return true;
@ -404,7 +390,7 @@ protected:
{ }
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
PreDispatch(WorkerPrivate* aWorkerPrivate) override
{
AssertIsOnMainThread();
return true;
@ -432,7 +418,7 @@ protected:
{ }
virtual bool
PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override;
PreDispatch(WorkerPrivate* aWorkerPrivate) override;
virtual void
PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,

View File

@ -541,6 +541,10 @@ class EventRunnable final : public MainThreadProxyRunnable
nsresult mResponseTextResult;
nsresult mStatusResult;
nsresult mResponseResult;
// mScopeObj is used in PreDispatch only. We init it in our constructor, and
// reset() in PreDispatch, to ensure that it's not still linked into the
// runtime once we go off-thread.
JS::PersistentRooted<JSObject*> mScopeObj;
public:
class MOZ_RAII StateDataAutoRooter : private JS::CustomAutoRooter
@ -565,7 +569,8 @@ public:
};
EventRunnable(Proxy* aProxy, bool aUploadEvent, const nsString& aType,
bool aLengthComputable, uint64_t aLoaded, uint64_t aTotal)
bool aLengthComputable, uint64_t aLoaded, uint64_t aTotal,
JS::Handle<JSObject*> aScopeObj)
: MainThreadProxyRunnable(aProxy->mWorkerPrivate, aProxy),
StructuredCloneHolder(CloningSupported, TransferringNotSupported,
SameProcessDifferentThread),
@ -573,10 +578,12 @@ public:
mTotal(aTotal), mEventStreamId(aProxy->mInnerEventStreamId), mStatus(0),
mReadyState(0), mUploadEvent(aUploadEvent), mProgressEvent(true),
mLengthComputable(aLengthComputable), mUseCachedArrayBufferResponse(false),
mResponseTextResult(NS_OK), mStatusResult(NS_OK), mResponseResult(NS_OK)
mResponseTextResult(NS_OK), mStatusResult(NS_OK), mResponseResult(NS_OK),
mScopeObj(nsContentUtils::RootingCxForThread(), aScopeObj)
{ }
EventRunnable(Proxy* aProxy, bool aUploadEvent, const nsString& aType)
EventRunnable(Proxy* aProxy, bool aUploadEvent, const nsString& aType,
JS::Handle<JSObject*> aScopeObj)
: MainThreadProxyRunnable(aProxy->mWorkerPrivate, aProxy),
StructuredCloneHolder(CloningSupported, TransferringNotSupported,
SameProcessDifferentThread),
@ -584,15 +591,20 @@ public:
mEventStreamId(aProxy->mInnerEventStreamId), mStatus(0), mReadyState(0),
mUploadEvent(aUploadEvent), mProgressEvent(false), mLengthComputable(0),
mUseCachedArrayBufferResponse(false), mResponseTextResult(NS_OK),
mStatusResult(NS_OK), mResponseResult(NS_OK)
mStatusResult(NS_OK), mResponseResult(NS_OK),
mScopeObj(nsContentUtils::RootingCxForThread(), aScopeObj)
{ }
void Dispatch()
{
MainThreadProxyRunnable::Dispatch(nullptr);
}
private:
~EventRunnable()
{ }
virtual void
InfalliblePreDispatch(JSContext* aCx) override final;
virtual bool
PreDispatch(WorkerPrivate* /* unused */) override final;
virtual bool
WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override;
@ -1055,8 +1067,6 @@ Proxy::HandleEvent(nsIDOMEvent* aEvent)
nsCOMPtr<nsIXMLHttpRequestUpload> uploadTarget = do_QueryInterface(target);
ProgressEvent* progressEvent = aEvent->InternalDOMEvent()->AsProgressEvent();
RefPtr<EventRunnable> runnable;
if (mInOpen && type.EqualsASCII(sEventStrings[STRING_readystatechange])) {
uint16_t readyState = 0;
if (NS_SUCCEEDED(mXHR->GetReadyState(&readyState)) &&
@ -1065,16 +1075,6 @@ Proxy::HandleEvent(nsIDOMEvent* aEvent)
}
}
if (progressEvent) {
runnable = new EventRunnable(this, !!uploadTarget, type,
progressEvent->LengthComputable(),
progressEvent->Loaded(),
progressEvent->Total());
}
else {
runnable = new EventRunnable(this, !!uploadTarget, type);
}
{
AutoSafeJSContext cx;
JSAutoRequest ar(cx);
@ -1085,9 +1085,20 @@ Proxy::HandleEvent(nsIDOMEvent* aEvent)
}
JS::Rooted<JSObject*> scope(cx, &value.toObject());
JSAutoCompartment ac(cx, scope);
runnable->Dispatch(cx);
RefPtr<EventRunnable> runnable;
if (progressEvent) {
runnable = new EventRunnable(this, !!uploadTarget, type,
progressEvent->LengthComputable(),
progressEvent->Loaded(),
progressEvent->Total(),
scope);
}
else {
runnable = new EventRunnable(this, !!uploadTarget, type, scope);
}
runnable->Dispatch();
}
if (!uploadTarget) {
@ -1170,19 +1181,21 @@ LoadStartDetectionRunnable::HandleEvent(nsIDOMEvent* aEvent)
return NS_OK;
}
void
EventRunnable::InfalliblePreDispatch(JSContext* aCx)
bool
EventRunnable::PreDispatch(WorkerPrivate* /* unused */)
{
AssertIsOnMainThread();
MOZ_ASSERT(aCx);
MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
AutoJSAPI jsapi;
DebugOnly<bool> ok =
jsapi.Init(xpc::NativeGlobal(JS::CurrentGlobalOrNull(aCx)), aCx);
DebugOnly<bool> ok = jsapi.Init(xpc::NativeGlobal(mScopeObj));
MOZ_ASSERT(ok);
MOZ_ASSERT(jsapi.cx() == aCx);
jsapi.TakeOwnershipOfErrorReporting();
JSContext* cx = jsapi.cx();
// Now keep the mScopeObj alive for the duration
JS::Rooted<JSObject*> scopeObj(cx, mScopeObj);
// And reset mScopeObj now, before we have a chance to run its destructor on
// some background thread.
mScopeObj.reset();
RefPtr<nsXMLHttpRequest>& xhr = mProxy->mXHR;
MOZ_ASSERT(xhr);
@ -1199,15 +1212,15 @@ EventRunnable::InfalliblePreDispatch(JSContext* aCx)
}
}
else {
JS::Rooted<JS::Value> response(aCx);
mResponseResult = xhr->GetResponse(aCx, &response);
JS::Rooted<JS::Value> response(cx);
mResponseResult = xhr->GetResponse(cx, &response);
if (NS_SUCCEEDED(mResponseResult)) {
if (!response.isGCThing()) {
mResponse = response;
} else {
bool doClone = true;
JS::Rooted<JS::Value> transferable(aCx);
JS::Rooted<JSObject*> obj(aCx, response.isObjectOrNull() ?
JS::Rooted<JS::Value> transferable(cx);
JS::Rooted<JSObject*> obj(cx, response.isObjectOrNull() ?
response.toObjectOrNull() : nullptr);
if (obj && JS_IsArrayBufferObject(obj)) {
// Use cached response if the arraybuffer has been transfered.
@ -1217,9 +1230,9 @@ EventRunnable::InfalliblePreDispatch(JSContext* aCx)
doClone = false;
} else {
MOZ_ASSERT(!JS_IsDetachedArrayBufferObject(obj));
JS::AutoValueArray<1> argv(aCx);
JS::AutoValueArray<1> argv(cx);
argv[0].set(response);
obj = JS_NewArrayObject(aCx, argv);
obj = JS_NewArrayObject(cx, argv);
if (obj) {
transferable.setObject(*obj);
// Only cache the response when the readyState is DONE.
@ -1235,7 +1248,7 @@ EventRunnable::InfalliblePreDispatch(JSContext* aCx)
if (doClone) {
ErrorResult rv;
Write(aCx, response, transferable, rv);
Write(cx, response, transferable, rv);
if (NS_WARN_IF(rv.Failed())) {
NS_WARNING("Failed to clone response!");
mResponseResult = rv.StealNSResult();
@ -1253,6 +1266,8 @@ EventRunnable::InfalliblePreDispatch(JSContext* aCx)
mReadyState = xhr->ReadyState();
xhr->GetResponseURL(mResponseURL);
return true;
}
bool