Bug 812249 - When "javascript.options.strict" is enabled related warnings/errors trigger worker.onerror handler. r=bent

--HG--
extra : rebase_source : a0bf6f37f2e83cbcc3586240d3312b84b1ca28a5

dom/workers/WorkerPrivate.cpp

@@ -999,65 +999,69 @@ public:
       return false;
     }
 
-    // First fire an ErrorEvent at the worker.
-    if (aTarget) {
-      JSObject* event = 
-        CreateErrorEvent(aCx, message, filename, aLineNumber, !aWorkerPrivate);
-      if (!event) {
-        return false;
-      }
-
-      bool preventDefaultCalled;
-      if (!DispatchEventToTarget(aCx, aTarget, event, &preventDefaultCalled)) {
-        return false;
-      }
-
-      if (preventDefaultCalled) {
-        return true;
-      }
-    }
-
-    // Now fire an event at the global object, but don't do that if the error
-    // code is too much recursion and this is the same script threw the error.
-    if (aFireAtScope && (aTarget || aErrorNumber != JSMSG_OVER_RECURSED)) {
-      aTarget = JS_GetGlobalForScopeChain(aCx);
-      NS_ASSERTION(aTarget, "This should never be null!");
-
-      bool preventDefaultCalled;
-      nsIScriptGlobalObject* sgo;
-
-      if (aWorkerPrivate ||
-          !(sgo = nsJSUtils::GetStaticScriptGlobal(aTarget))) {
-        // Fire a normal ErrorEvent if we're running on a worker thread.
+    // We should not fire error events for warnings but instead make sure that
+    // they show up in the error console.
+    if (!JSREPORT_IS_WARNING(aFlags)) {
+      // First fire an ErrorEvent at the worker.
+      if (aTarget) {
         JSObject* event =
-          CreateErrorEvent(aCx, message, filename, aLineNumber, false);
+          CreateErrorEvent(aCx, message, filename, aLineNumber, !aWorkerPrivate);
         if (!event) {
           return false;
         }
 
-        if (!DispatchEventToTarget(aCx, aTarget, event,
-                                   &preventDefaultCalled)) {
+        bool preventDefaultCalled;
+        if (!DispatchEventToTarget(aCx, aTarget, event, &preventDefaultCalled)) {
           return false;
         }
-      }
-      else {
-        // Icky, we have to fire an nsScriptErrorEvent...
-        nsScriptErrorEvent event(true, NS_LOAD_ERROR);
-        event.lineNr = aLineNumber;
-        event.errorMsg = aMessage.get();
-        event.fileName = aFilename.get();
 
-        nsEventStatus status = nsEventStatus_eIgnore;
-        if (NS_FAILED(sgo->HandleScriptError(&event, &status))) {
-          NS_WARNING("Failed to dispatch main thread error event!");
-          status = nsEventStatus_eIgnore;
+        if (preventDefaultCalled) {
+          return true;
+        }
+      }
+
+      // Now fire an event at the global object, but don't do that if the error
+      // code is too much recursion and this is the same script threw the error.
+      if (aFireAtScope && (aTarget || aErrorNumber != JSMSG_OVER_RECURSED)) {
+        aTarget = JS_GetGlobalForScopeChain(aCx);
+        NS_ASSERTION(aTarget, "This should never be null!");
+
+        bool preventDefaultCalled;
+        nsIScriptGlobalObject* sgo;
+
+        if (aWorkerPrivate ||
+            !(sgo = nsJSUtils::GetStaticScriptGlobal(aTarget))) {
+          // Fire a normal ErrorEvent if we're running on a worker thread.
+          JSObject* event =
+            CreateErrorEvent(aCx, message, filename, aLineNumber, false);
+          if (!event) {
+            return false;
+          }
+
+          if (!DispatchEventToTarget(aCx, aTarget, event,
+                                     &preventDefaultCalled)) {
+            return false;
+          }
+        }
+        else {
+          // Icky, we have to fire an nsScriptErrorEvent...
+          nsScriptErrorEvent event(true, NS_LOAD_ERROR);
+          event.lineNr = aLineNumber;
+          event.errorMsg = aMessage.get();
+          event.fileName = aFilename.get();
+
+          nsEventStatus status = nsEventStatus_eIgnore;
+          if (NS_FAILED(sgo->HandleScriptError(&event, &status))) {
+            NS_WARNING("Failed to dispatch main thread error event!");
+            status = nsEventStatus_eIgnore;
+          }
+
+          preventDefaultCalled = status == nsEventStatus_eConsumeNoDefault;
         }
 
-        preventDefaultCalled = status == nsEventStatus_eConsumeNoDefault;
-      }
-
-      if (preventDefaultCalled) {
-        return true;
+        if (preventDefaultCalled) {
+          return true;
+        }
       }
     }
 
@@ -1485,7 +1489,7 @@ struct WorkerJSRuntimeStats : public JS::RuntimeStats
   {
     MOZ_ASSERT(!cstats->extra1);
     MOZ_ASSERT(!cstats->extra2);
-    
+
     // ReportJSRuntimeExplicitTreeStats expects that cstats->{extra1,extra2}
     // are char pointers.
 
@@ -1505,7 +1509,7 @@ struct WorkerJSRuntimeStats : public JS::RuntimeStats
 private:
   nsCString mRtPath;
 };
-  
+
 BEGIN_WORKERS_NAMESPACE
 
 class WorkerMemoryReporter MOZ_FINAL : public nsIMemoryMultiReporter

dom/workers/test/Makefile.in

@@ -102,6 +102,8 @@ MOCHITEST_FILES = \
   csp_worker.js \
   test_transferable.html \
   transferable_worker.js \
+  test_errorwarning.html \
+  errorwarning_worker.js \
   $(NULL)
 
 _SUBDIRMOCHITEST_FILES = \

dom/workers/test/errorwarning_worker.js

@@ -0,0 +1,38 @@
+/**
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+function errorHandler() {
+  postMessage({ type: 'error' });
+}
+
+onmessage = function(event) {
+  if (event.data.errors) {
+    try {
+      // This is an error:
+      postMessage({ type: 'ignore', value: b.aaa });
+    } catch(e) {
+      errorHandler();
+    }
+  } else {
+    var a = {};
+    // This is a warning:
+    postMessage({ type: 'ignore', value: a.foo });
+  }
+
+  if (event.data.loop != 0) {
+    var worker = new Worker('errorwarning_worker.js');
+    worker.onerror = errorHandler;
+    worker.postMessage({ loop: event.data.loop - 1, errors: event.data.errors });
+
+    worker.onmessage = function(e) {
+      postMessage(e.data);
+    }
+
+  } else {
+    postMessage({ type: 'finish' });
+  }
+}
+
+onerror = errorHandler;

dom/workers/test/test_errorwarning.html

@@ -0,0 +1,95 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<!DOCTYPE HTML>
+<html>
+<!--
+Test javascript.options.strict in Workers
+-->
+<head>
+  <title>Test javascript.options.strict in Workers</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+<script class="testbody" language="javascript">
+
+  var errors = 0;
+  function errorHandler(e) {
+    ok(true, "An error has been received!");
+    errors++;
+  }
+
+  function test_noErrors() {
+    errors = 0;
+
+    var worker = new Worker('errorwarning_worker.js');
+    worker.onerror = errorHandler;
+    worker.onmessage = function(e) {
+      if (e.data.type == 'ignore')
+        return;
+
+      if (e.data.type == 'error') {
+        errorHandler();
+        return;
+      }
+
+      if (e.data.type == 'finish') {
+        ok(errors == 0, "Here we are with 0 errors!");
+        runTests();
+        return;
+      }
+    }
+
+    onerror = errorHandler;
+    worker.postMessage({ loop: 5, errors: false });
+  }
+
+  function test_errors() {
+    errors = 0;
+
+    var worker = new Worker('errorwarning_worker.js');
+    worker.onerror = errorHandler;
+    worker.onmessage = function(e) {
+      if (e.data.type == 'ignore')
+        return;
+
+      if (e.data.type == 'error') {
+        errorHandler();
+        return;
+      }
+
+      if (e.data.type == 'finish') {
+        ok(errors != 0, "Here we are with errors!");
+        runTests();
+        return;
+      }
+    }
+
+    onerror = errorHandler;
+    worker.postMessage({ loop: 5, errors: true });
+  }
+
+  var tests = [ test_noErrors, test_errors ];
+  function runTests() {
+    var test = tests.shift();
+    if (test) {
+      test();
+    } else {
+      SimpleTest.finish();
+    }
+  }
+
+  runTests();
+  SimpleTest.waitForExplicitFinish();
+
+</script>
+</pre>
+</body>
+</html>