Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-26 06:11:37 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1072144 part 5. Stop fiddling with compartments on the JSContext before calling PostRun in WorkerRunnable::Run. Add some documentation explaining what's going on. r=khuey

Browse Source
This commit is contained in:
Boris Zbarsky 2016-03-01 16:52:26 -05:00
parent 9679610dbb
commit 7d17080433
2 changed files with 28 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
dom/workers/WorkerRunnable.cpp
View File

@ -312,9 +312,6 @@ WorkerRunnable::Run()
// http://www.whatwg.org/specs/web-apps/current-work/#run-a-worker
// If we don't have a globalObject we have to use an AutoJSAPI instead, but
// this is OK as we won't be running script in these circumstances.
// It's important that aes is declared after jsapi, because if WorkerRun
// creates a global then we construct aes before PostRun and we need them to
// be destroyed in the correct order.
Maybe<mozilla::dom::AutoJSAPI> jsapi;
Maybe<mozilla::dom::AutoEntryScript> aes;
JSContext* cx;
@ -377,15 +374,26 @@ WorkerRunnable::Run()
// We can't even assert that this didn't create our global, since in the case
// of CompileScriptRunnable it _does_.
// In the case of CompileScriptRunnnable, WorkerRun above can cause us to
// lazily create a global, so we construct aes here before calling PostRun.
if (targetIsWorkerThread && !aes && DefaultGlobalObject()) {
aes.emplace(DefaultGlobalObject(), "worker runnable",
false, GetCurrentThreadJSContext());
cx = aes->cx();
}
// It would be nice to avoid passing a JSContext to PostRun, but in the case
// of ScriptExecutorRunnable we need to know the current compartment on the
// JSContext (the one we set up based on the global returned from PreRun) so
// that we can sanely do exception reporting. In particular, we want to make
// sure that we do our JS_SetPendingException while still in that compartment,
// because otherwise we might end up trying to create a cross-compartment
// wrapper when we try to move the JS exception from our runnable's
// ErrorResult to the JSContext, and that's not desirable in this case.
//
// We _could_ skip passing a JSContext here and then in
// ScriptExecutorRunnable::PostRun end up grabbing it from the WorkerPrivate
// and looking at its current compartment. But that seems like slightly weird
// action-at-a-distance...
//
// In any case, we do NOT try to change the compartment on the JSContext at
// this point; in the one case in which we could do that
// (CompileScriptRunnable) it actually doesn't matter which compartment we're
// in for PostRun.
PostRun(cx, mWorkerPrivate, result);
MOZ_ASSERT(!JS_IsExceptionPending(cx));
return result ? NS_OK : NS_ERROR_FAILURE;
}

14
dom/workers/WorkerRunnable.h
View File

@ -142,10 +142,12 @@ protected:
// in the null compartment if there is no parent global. For other mBehavior
// values, we're running on the worker thread and aCx is in whatever
// compartment GetCurrentThreadJSContext() was in when nsIRunnable::Run() got
// called (XXXbz: Why is this a sane thing to be doing now that we have
// multiple globals per worker???). If it wasn't in a compartment, aCx will
// be in either the debugger global's compartment or the worker's global's
// compartment depending on whether IsDebuggerRunnable() is true.
// called. This is actually important for cases when a runnable spins a
// syncloop and wants everything that happens during the syncloop to happen in
// the compartment that runnable set up (which may, for example, be a debugger
// sandbox compartment!). If aCx wasn't in a compartment to start with, aCx
// will be in either the debugger global's compartment or the worker's
// global's compartment depending on whether IsDebuggerRunnable() is true.
//
// Immediately after WorkerRun returns, the caller will assert that either it
// returns false or there is no exception pending on aCx. Then it will report
@ -158,7 +160,9 @@ protected:
// busy count was previously modified in PreDispatch().
//
// The aCx passed here is the same one as was passed to WorkerRun and is
// still in the same compartment.
// still in the same compartment. PostRun implementations must NOT leave an
// exception on the JSContext and must not run script, because the incoming
// JSContext may be in the null compartment.
virtual void
PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult);