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

@@ -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<JSObject*> 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<JSObject*> 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);
 

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
   {

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<nsIGlobalObject> 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<mozilla::dom::AutoJSAPI> jsapi;
   Maybe<mozilla::dom::AutoEntryScript> 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()) {

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