Bug 1270783. If we end up erroring out of a worker main-script load before we get to creating a global for the worker, don't try to enter that global's compartment to throw an exception. Just squelch it instead. r=khuey a=ritu
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 13 May 2016 20:09:50 -0400
changeset 332888 39d73775d903e37dfcd2f779e136033807789dc3
parent 332887 df9b55a0fed6e85bfde84a9f6debe2c772fdc7f5
child 332889 5d6b894490f27d7e8ee098fd8279f2954e89a712
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey, ritu
bugs1270783
milestone48.0a2
Bug 1270783. If we end up erroring out of a worker main-script load before we get to creating a global for the worker, don't try to enter that global's compartment to throw an exception. Just squelch it instead. r=khuey a=ritu
dom/workers/ScriptLoader.cpp
dom/workers/WorkerPrivate.cpp
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -1769,18 +1769,20 @@ ScriptExecutorRunnable::PreRun(WorkerPri
   jsapi.Init();
 
   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.
+    // the exception.  We'll report some sort of error to our caller in
+    // ShutdownScriptLoader, but it will get squelched for the same reason we're
+    // squelching here: all the error reporting machinery relies on being able
+    // to enter a compartment to report the error.
     jsapi.ClearException();
     return false;
   }
 
   return true;
 }
 
 bool
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -515,28 +515,36 @@ private:
     // Explicitly ignore NS_BINDING_ABORTED on rv.  Or more precisely, still
     // return false and don't SetWorkerScriptExecutedSuccessfully() in that
     // case, but don't throw anything on aCx.  The idea is to not dispatch error
     // events if our load is canceled with that error code.
     if (rv.ErrorCodeIs(NS_BINDING_ABORTED)) {
       rv.SuppressException();
       return false;
     }
+
+    WorkerGlobalScope* globalScope = aWorkerPrivate->GlobalScope();
+    if (NS_WARN_IF(!globalScope)) {
+      // We never got as far as calling GetOrCreateGlobalScope, or it failed.
+      // We have no way to enter a compartment, hence no sane way to report this
+      // error.  :(
+      rv.SuppressException();
+      return false;
+    }
+
     // Make sure to propagate exceptions from rv onto aCx, so that they will get
     // reported after we return.  We do this for all failures on rv, because now
     // we're using rv to track all the state we care about.
     //
     // This is a little dumb, but aCx is in the null compartment here because we
     // set it up that way in our Run(), since we had not created the global at
     // that point yet.  So we need to enter the compartment of our global,
     // because setting a pending exception on aCx involves wrapping into its
-    // current compartment.  Luckily we have a global now (else how would we
-    // have a JS exception?) so we can just enter its compartment.
-    JSAutoCompartment ac(aCx,
-                         aWorkerPrivate->GlobalScope()->GetGlobalJSObject());
+    // current compartment.  Luckily we have a global now.
+    JSAutoCompartment ac(aCx, globalScope->GetGlobalJSObject());
     if (rv.MaybeSetPendingException(aCx)) {
       return false;
     }
 
     aWorkerPrivate->SetWorkerScriptExecutedSuccessfully();
     return true;
   }
 };