Bug 1251369. Use an AutoJSAPI that reports its own exceptions around the main runloop in workers. r=khuey

The silly leading ": " on the error messages is due to bug 1251518.

dom/security/test/csp/file_child-src_worker-redirect.html

@@ -24,7 +24,7 @@
 
       worker.onerror = function(error) {
         var msg = error.message;
-        if (msg.match(/^: NetworkError/) || msg.match(/Failed to load script/)) {
+        if (msg.match(/^: NetworkError/) || msg.match(/Failed to load worker script/)) {
           // this means CSP blocked it
           msg = "blocked";
         }

dom/workers/RuntimeService.cpp

@@ -2636,8 +2636,10 @@ WorkerThreadPrimaryRunnable::Run()
         JSAutoRequest ar(cx);
 
         mWorkerPrivate->DoRunLoop(cx);
-
+        // The AutoJSAPI in DoRunLoop should have reported any exceptions left
-        JS_ReportPendingException(cx);
+        // on cx.  Note that we still need the JSAutoRequest above because
+        // AutoJSAPI on workers does NOT enter a request!
+        MOZ_ASSERT(!JS_IsExceptionPending(cx));
       }
 
       BackgroundChild::CloseForCurrentThread();

dom/workers/ScriptLoader.cpp

@@ -1777,7 +1777,7 @@ ScriptExecutorRunnable::WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
     mScriptLoader.mRv.MightThrowJSException();
     if (NS_FAILED(loadInfo.mLoadResult)) {
       scriptloader::ReportLoadError(aCx, mScriptLoader.mRv,
-                                    loadInfo.mLoadResult);
+                                    loadInfo.mLoadResult, loadInfo.mURL);
       // Top level scripts only!
       if (mIsWorkerScript) {
         aWorkerPrivate->MaybeDispatchLoadFailedRunnable();
@@ -2019,37 +2019,56 @@ ChannelFromScriptURLWorkerThread(JSContext* aCx,
   return getter->GetResult();
 }
 
-void ReportLoadError(JSContext* aCx, ErrorResult& aRv, nsresult aLoadResult)
+void ReportLoadError(JSContext* aCx, ErrorResult& aRv, nsresult aLoadResult,
+                     const nsAString& aScriptURL)
 {
   MOZ_ASSERT(!aRv.Failed());
 
   switch (aLoadResult) {
     case NS_ERROR_FILE_NOT_FOUND:
     case NS_ERROR_NOT_AVAILABLE:
-      aRv.Throw(NS_ERROR_DOM_NETWORK_ERR);
+      aLoadResult = NS_ERROR_DOM_NETWORK_ERR;
       break;
 
     case NS_ERROR_MALFORMED_URI:
       aLoadResult = NS_ERROR_DOM_SYNTAX_ERR;
-      MOZ_FALLTHROUGH;
+      break;
+
     case NS_BINDING_ABORTED:
       // Note: we used to pretend like we didn't set an exception for
       // NS_BINDING_ABORTED, but then ShutdownScriptLoader did it anyway.  The
       // other callsite, in WorkerPrivate::Constructor, never passed in
       // NS_BINDING_ABORTED.  So just throw it directly here.  Consumers will
-      // deal as needed.
+      // deal as needed.  But note that we do NOT want to ThrowDOMException()
-      MOZ_FALLTHROUGH;
+      // for this case, because that will make it impossible for consumers to
+      // realize that our error was NS_BINDING_ABORTED.
+      aRv.Throw(aLoadResult);
+      return;
+
     case NS_ERROR_DOM_SECURITY_ERR:
     case NS_ERROR_DOM_SYNTAX_ERR:
-      aRv.Throw(aLoadResult);
+      break;
+
+    case NS_ERROR_DOM_BAD_URI:
+      // This is actually a security error.
+      aLoadResult = NS_ERROR_DOM_SECURITY_ERR;
       break;
 
     default:
-      // We _could_ probably just throw aLoadResult on aRv, but let's preserve
+      // For lack of anything better, go ahead and throw a NetworkError here.
-      // the old behavior for now and throw this string as an Error instead.
+      // We don't want to throw a JS exception, because for toplevel script
-      JS_ReportError(aCx, "Failed to load script (nsresult = 0x%x)", aLoadResult);
+      // loads that would get squelched.
-      aRv.StealExceptionFromJSContext(aCx);
+      aRv.ThrowDOMException(NS_ERROR_DOM_NETWORK_ERR,
+        nsPrintfCString("Failed to load worker script at %s (nsresult = 0x%x)",
+                        NS_ConvertUTF16toUTF8(aScriptURL).get(),
+                        aLoadResult));
+      return;
   }
+
+  aRv.ThrowDOMException(aLoadResult,
+                        NS_LITERAL_CSTRING("Failed to load worker script at \"") +
+                        NS_ConvertUTF16toUTF8(aScriptURL) +
+                        NS_LITERAL_CSTRING("\""));
 }
 
 void

dom/workers/ScriptLoader.h

@@ -47,7 +47,8 @@ ChannelFromScriptURLWorkerThread(JSContext* aCx,
                                  const nsAString& aScriptURL,
                                  nsIChannel** aChannel);
 
-void ReportLoadError(JSContext* aCx, ErrorResult& aRv, nsresult aLoadResult);
+void ReportLoadError(JSContext* aCx, ErrorResult& aRv, nsresult aLoadResult,
+                     const nsAString& aScriptURL);
 
 void LoadMainScript(JSContext* aCx, const nsAString& aScriptURL,
                     WorkerScriptType aWorkerScriptType,

dom/workers/WorkerPrivate.cpp

@@ -4032,7 +4032,7 @@ WorkerPrivate::Constructor(JSContext* aCx,
                               aWorkerType, stackLoadInfo.ptr());
     aRv.MightThrowJSException();
     if (NS_FAILED(rv)) {
-      scriptloader::ReportLoadError(aCx, aRv, rv);
+      scriptloader::ReportLoadError(aCx, aRv, rv, aScriptURL);
       return nullptr;
     }
 
@@ -4378,6 +4378,14 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
     mStatus = Running;
   }
 
+  // Now that we've done that, we can go ahead and set up our AutoJSAPI.  We
+  // can't before this point, because it can't find the right JSContext before
+  // then, since it gets it from our mJSContext.
+  AutoJSAPI jsapi;
+  jsapi.Init();
+  MOZ_ASSERT(jsapi.cx() == aCx);
+  jsapi.TakeOwnershipOfErrorReporting();
+
   EnableMemoryReporter();
 
   InitializeGCTimers();

dom/workers/test/test_404.html

@@ -25,7 +25,7 @@ Tests of DOM Worker Threads
 
   worker.onerror = function(event) {
     is(event.target, worker);
-    is(event.message, ": NetworkError: A network error occurred.");
+    is(event.message, ': NetworkError: Failed to load worker script at "nonexistent_worker.js"');
     event.preventDefault();
     SimpleTest.finish();
   };

dom/workers/test/test_bug1036484.html

@@ -25,7 +25,7 @@ function test(script) {
 
   worker.onerror = function(event) {
     is(event.target, worker);
-    is(event.message, ": NetworkError: A network error occurred.");
+    ok(event.message.startsWith(": NetworkError: Failed to load worker script"))
     event.preventDefault();
     runTests();
   };

dom/workers/test/test_loadError.html

@@ -13,6 +13,8 @@
 <script class="testbody" type="text/javascript">
 "use strict";
 
+var loadErrorMessage = ': SecurityError: Failed to load worker script at "about:blank"';
+
 function nextTest() {
   (function(){
     function workerfunc() {
@@ -35,10 +37,14 @@ function nextTest() {
         return;
       }
       w.onmessage = function(e) {
-        is(e.data.indexOf('Error: Failed to load script'), 0, "Error: Failed to load script");
+        is(e.data, loadErrorMessage,
+           "Should catch the error when loading inner script");
         if (++i < 2) callworker(i);
         else SimpleTest.finish();
       };
+      w.onerrror = function(e) {
+        ok(false, "Should not get any errors from this worker");
+      }
     }
     callworker(0);
   })();
@@ -48,7 +54,8 @@ try {
   var worker = new Worker("about:blank");
   worker.onerror = function(e) {
     e.preventDefault();
-    ok(true, "Shouldn't success!");
+    is(e.message, loadErrorMessage,
+       "Should get the right error from the toplevel script");
     nextTest();
   }