From de208d989a75a7e82591ba430d68b276432462f6 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 1 Mar 2016 16:52:26 -0500 Subject: [PATCH] Bug 1072144 part 4. Add a WorkerRunnable::PreRun so that we can move worker global creation to it and always have an AutoEntryScript by the time we're evaluating the main worker script. r=khuey --- dom/workers/ScriptLoader.cpp | 65 +++++++++++++++++++++------------- dom/workers/WorkerPrivate.cpp | 4 +++ dom/workers/WorkerRunnable.cpp | 39 +++++++++++++++++--- dom/workers/WorkerRunnable.h | 10 ++++++ 4 files changed, 89 insertions(+), 29 deletions(-) diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index db64a49640f2..6ac4a234c717 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -306,6 +306,9 @@ private: virtual bool IsDebuggerRunnable() const override; + virtual bool + PreRun(WorkerPrivate* aWorkerPrivate) override; + virtual bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override; @@ -1723,6 +1726,41 @@ ScriptExecutorRunnable::IsDebuggerRunnable() const return mScriptLoader.mWorkerScriptType == DebuggerScript; } +bool +ScriptExecutorRunnable::PreRun(WorkerPrivate* aWorkerPrivate) +{ + aWorkerPrivate->AssertIsOnWorkerThread(); + + if (!mIsWorkerScript) { + return true; + } + + if (!aWorkerPrivate->GetJSContext()) { + return false; + } + + MOZ_ASSERT(mFirstIndex == 0); + MOZ_ASSERT(!mScriptLoader.mRv.Failed()); + + AutoJSAPI jsapi; + jsapi.Init(); + jsapi.TakeOwnershipOfErrorReporting(); + + WorkerGlobalScope* globalScope = + aWorkerPrivate->GetOrCreateGlobalScope(jsapi.cx()); + if (NS_WARN_IF(!globalScope)) { + NS_WARNING("Failed to make global!"); + // There's no way to report the exception on jsapi right now, because there + // is no way to even enter a compartment on this thread anymore. Just clear + // the exception. We'll report some sort of error to our caller thread in + // ShutdownScriptLoader. + jsapi.ClearException(); + return false; + } + + return true; +} + bool ScriptExecutorRunnable::WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) { @@ -1745,32 +1783,11 @@ ScriptExecutorRunnable::WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) // If nothing else has failed, our ErrorResult better not be a failure either. MOZ_ASSERT(!mScriptLoader.mRv.Failed(), "Who failed it and why?"); - JS::Rooted global(aCx); - - if (mIsWorkerScript) { - WorkerGlobalScope* globalScope = - aWorkerPrivate->GetOrCreateGlobalScope(aCx); - if (NS_WARN_IF(!globalScope)) { - NS_WARNING("Failed to make global!"); - // There's no way to report the exception on aCx right now, because there - // is no way to even enter a compartment on this thread anymore. Just - // clear the exception. We'll report some sort of error to our caller - // thread in ShutdownScriptLoader. - JS_ClearPendingException(aCx); - return false; - } - - global.set(globalScope->GetWrapper()); - } else { - // XXXbz Icky action at a distance... Would be better to capture this state - // in mScriptLoader! - global.set(JS::CurrentGlobalOrNull(aCx)); - } - + // Slightly icky action at a distance, but there's no better place to stash + // this value, really. + JS::Rooted global(aCx, JS::CurrentGlobalOrNull(aCx)); MOZ_ASSERT(global); - JSAutoCompartment ac(aCx, global); - for (uint32_t index = mFirstIndex; index <= mLastIndex; index++) { ScriptLoadInfo& loadInfo = loadInfos.ElementAt(index); diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index ce6cfe94c4af..f1f7011d9ce8 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -498,6 +498,10 @@ public: { } private: + // We can't implement PreRun effectively, because at the point when that would + // run we have not yet done our load so don't know things like our final + // principal and whatnot. + virtual bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override { diff --git a/dom/workers/WorkerRunnable.cpp b/dom/workers/WorkerRunnable.cpp index 7f6ff50d277a..b77c30fa420f 100644 --- a/dom/workers/WorkerRunnable.cpp +++ b/dom/workers/WorkerRunnable.cpp @@ -155,6 +155,12 @@ WorkerRunnable::PostDispatch(WorkerPrivate* aWorkerPrivate, } } +bool +WorkerRunnable::PreRun(WorkerPrivate* aWorkerPrivate) +{ + return true; +} + void WorkerRunnable::PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult) @@ -256,7 +262,20 @@ WorkerRunnable::Run() return NS_OK; } - // Track down the appropriate global to use for the AutoJSAPI/AutoEntryScript. + bool result = PreRun(mWorkerPrivate); + if (!result) { + MOZ_ASSERT(targetIsWorkerThread, + "The only PreRun implementation that can fail is " + "ScriptExecutorRunnable"); + mWorkerPrivate->AssertIsOnWorkerThread(); + MOZ_ASSERT(!JS_IsExceptionPending(mWorkerPrivate->GetJSContext())); + // We can't enter a useful compartment on the JSContext here; just pass it + // in as-is. + PostRun(mWorkerPrivate->GetJSContext(), mWorkerPrivate, false); + return NS_ERROR_FAILURE; + } + + // Track down the appropriate global, if any, to use for the AutoEntryScript. nsCOMPtr globalObject; bool isMainThread = !targetIsWorkerThread && !mWorkerPrivate->GetParent(); MOZ_ASSERT(isMainThread == NS_IsMainThread()); @@ -273,6 +292,12 @@ WorkerRunnable::Run() } else { globalObject = DefaultGlobalObject(); } + + // We may still not have a globalObject here: in the case of + // CompileScriptRunnable, we don't actually create the global object until + // we have the script data, which happens in a syncloop under + // CompileScriptRunnable::WorkerRun, so we can't assert that it got created + // in the PreRun call above. } else { kungFuDeathGrip = mWorkerPrivate; if (isMainThread) { @@ -290,7 +315,7 @@ WorkerRunnable::Run() // 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. - mozilla::dom::AutoJSAPI jsapi; + Maybe jsapi; Maybe aes; JSContext* cx; if (globalObject) { @@ -299,8 +324,9 @@ WorkerRunnable::Run() isMainThread ? nullptr : GetCurrentThreadJSContext()); cx = aes->cx(); } else { - jsapi.Init(); - cx = jsapi.cx(); + jsapi.emplace(); + jsapi->Init(); + cx = jsapi->cx(); } // Note that we can't assert anything about mWorkerPrivate->GetWrapper() @@ -344,10 +370,13 @@ WorkerRunnable::Run() ac.emplace(cx, mWorkerPrivate->GetWrapper()); } - bool result = WorkerRun(cx, mWorkerPrivate); + result = WorkerRun(cx, mWorkerPrivate); MOZ_ASSERT_IF(result, !JS_IsExceptionPending(cx)); JS_ReportPendingException(cx); + // 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()) { diff --git a/dom/workers/WorkerRunnable.h b/dom/workers/WorkerRunnable.h index bf3cae6965b6..4582b9187700 100644 --- a/dom/workers/WorkerRunnable.h +++ b/dom/workers/WorkerRunnable.h @@ -122,6 +122,16 @@ protected: virtual void PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult); + // May be implemented by subclasses if desired if they need to do some sort of + // setup before we try to set up our JSContext and compartment for real. + // Typically the only thing that should go in here is creation of the worker's + // global. + // + // If false is returned, WorkerRun will not be called at all. PostRun will + // still be called, with false passed for aRunResult. + virtual bool + PreRun(WorkerPrivate* aWorkerPrivate); + // Must be implemented by subclasses. Called on the target thread. The return // value will be passed to PostRun(). The JSContext passed in here comes from // an AutoJSAPI (or AutoEntryScript) that we set up on the stack. If