Bug 1251518. Fix js::ErrorReportToString to make a bit more sense, and change worker code to not use it anyway, so it matches the mainthread code. r=bholley,terrence

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 worker script/)) {
+        if (msg.match(/^NetworkError/) || msg.match(/Failed to load worker script/)) {
           // this means CSP blocked it
           msg = "blocked";
         }

dom/workers/WorkerPrivate.cpp

@@ -5742,7 +5742,7 @@ WorkerPrivate::ScheduleKillCloseEventRunnable()
 }
 
 void
-WorkerPrivate::ReportError(JSContext* aCx, const char* aMessage,
+WorkerPrivate::ReportError(JSContext* aCx, const char* aFallbackMessage,
                            JSErrorReport* aReport)
 {
   AssertIsOnWorkerThread();
@@ -5763,19 +5763,9 @@ WorkerPrivate::ReportError(JSContext* aCx, const char* aMessage,
   bool mutedError = aReport && aReport->isMuted;
 
   if (aReport) {
-    // ErrorEvent objects don't have a |name| field the way ES |Error| objects
-    // do. Traditionally (and mostly by accident), the |message| field of
-    // ErrorEvent has corresponded to |Name: Message| of the original Error
-    // object. Things have been cleaned up in the JS engine, so now we need to
-    // format this string explicitly.
-    JS::Rooted<JSString*> messageStr(aCx,
-                                     js::ErrorReportToString(aCx, aReport));
-    if (messageStr) {
-      nsAutoJSString autoStr;
-      if (autoStr.init(aCx, messageStr)) {
-        message = autoStr;
-      }
-    }
+    // We want the same behavior here as xpc::ErrorReport::init here.
+    xpc::ErrorReport::ErrorReportToMessageString(aReport, message);
+
     filename = NS_ConvertUTF8toUTF16(aReport->filename);
     line = aReport->uclinebuf;
     lineNumber = aReport->lineno;
@@ -5791,7 +5781,7 @@ WorkerPrivate::ReportError(JSContext* aCx, const char* aMessage,
   }
 
   if (message.IsEmpty()) {
-    message = NS_ConvertUTF8toUTF16(aMessage);
+    message = NS_ConvertUTF8toUTF16(aFallbackMessage);
   }
 
   mErrorHandlerRecursionCount++;

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: Failed to load worker script at "nonexistent_worker.js"');
+    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);
-    ok(event.message.startsWith(": NetworkError: Failed to load worker script"))
+    ok(event.message.startsWith("NetworkError: Failed to load worker script"))
     event.preventDefault();
     runTests();
   };

dom/workers/test/test_loadError.html

@@ -13,7 +13,7 @@
 <script class="testbody" type="text/javascript">
 "use strict";
 
-var loadErrorMessage = ': SecurityError: Failed to load worker script at "about:blank"';
+var loadErrorMessage = 'SecurityError: Failed to load worker script at "about:blank"';
 
 function nextTest() {
   (function(){

dom/workers/test/test_recursion.html

@@ -47,7 +47,7 @@ Tests of DOM Worker Threads
 
   worker.onerror = function(event) {
     event.preventDefault();
-    if (event.message == "InternalError: too much recursion") {
+    if (event.message == "too much recursion") {
       ok(true, "got correct error message");
       ++errorCount;
     }

dom/workers/test/test_simpleThread.html

@@ -47,7 +47,7 @@ Tests of DOM Worker Threads (Bug 437152)
   worker.onerror = function(event) {
     event.preventDefault();
     is(event.target, worker);
-    is(event.message, "InternalError: uncaught exception: Bad message: asdf");
+    is(event.message, "uncaught exception: Bad message: asdf");
 
     worker.onerror = function(otherEvent) {
       otherEvent.preventDefault();

dom/workers/test/test_throwingOnerror.html

@@ -28,7 +28,7 @@ Tests of DOM Worker Threads
     event.preventDefault();
     var found = false;
     for (var index in errors) {
-      if (event.message == "InternalError: uncaught exception: " + errors[index]) {
+      if (event.message == "uncaught exception: " + errors[index]) {
         errors.splice(index, 1);
         found = true;
         break;

js/src/jsexn.cpp

@@ -637,23 +637,40 @@ IsDuckTypedErrorObject(JSContext* cx, HandleObject exnObject, const char** filen
     return true;
 }
 
-JS_FRIEND_API(JSString*)
-js::ErrorReportToString(JSContext* cx, JSErrorReport* reportp)
+static JSString*
+ErrorReportToString(JSContext* cx, JSErrorReport* reportp)
 {
+    /*
+     * We do NOT want to use GetErrorTypeName() here because it will not do the
+     * "right thing" for JSEXN_INTERNALERR.  That is, the caller of this API
+     * expects that "InternalError: " will be prepended but GetErrorTypeName
+     * goes out of its way to avoid this.
+     */
     JSExnType type = static_cast<JSExnType>(reportp->exnType);
-    RootedString str(cx, cx->runtime()->emptyString);
+    RootedString str(cx);
     if (type != JSEXN_NONE)
         str = ClassName(GetExceptionProtoKey(type), cx);
-    RootedString toAppend(cx, JS_NewUCStringCopyN(cx, MOZ_UTF16(": "), 2));
-    if (!str || !toAppend)
+    /*
+     * If "str" is null at this point, that means we just want to use
+     * reportp->ucmessage without prefixing it with anything.
+     */
+    if (str) {
+        RootedString separator(cx, JS_NewUCStringCopyN(cx, MOZ_UTF16(": "), 2));
+        if (!separator)
             return nullptr;
-    str = ConcatStrings<CanGC>(cx, str, toAppend);
+        str = ConcatStrings<CanGC>(cx, str, separator);
         if (!str)
             return nullptr;
-    toAppend = JS_NewUCStringCopyZ(cx, reportp->ucmessage);
-    if (toAppend)
-        str = ConcatStrings<CanGC>(cx, str, toAppend);
-    return str;
+    }
+
+    RootedString message(cx, JS_NewUCStringCopyZ(cx, reportp->ucmessage));
+    if (!message)
+        return nullptr;
+
+    if (!str)
+        return message;
+
+    return ConcatStrings<CanGC>(cx, str, message);
 }
 
 bool

js/src/jsfriendapi.h

@@ -1379,13 +1379,6 @@ class MOZ_STACK_CLASS AutoStableStringChars
     void operator=(const AutoStableStringChars& other) = delete;
 };
 
-/**
- * Creates a string of the form |ErrorType: ErrorMessage| for a JSErrorReport,
- * which generally matches the toString() behavior of an ErrorObject.
- */
-extern JS_FRIEND_API(JSString*)
-ErrorReportToString(JSContext* cx, JSErrorReport* reportp);
-
 struct MOZ_STACK_CLASS JS_FRIEND_API(ErrorReport)
 {
     explicit ErrorReport(JSContext* cx);

js/xpconnect/src/nsXPConnect.cpp

@@ -177,15 +177,7 @@ xpc::ErrorReport::Init(JSErrorReport* aReport, const char* aFallbackMessage,
                           : NS_LITERAL_CSTRING("content javascript");
     mWindowID = aWindowID;
 
-    const char16_t* m = static_cast<const char16_t*>(aReport->ucmessage);
-    if (m) {
-        JSFlatString* name = js::GetErrorTypeName(CycleCollectedJSRuntime::Get()->Runtime(), aReport->exnType);
-        if (name) {
-            AssignJSFlatString(mErrorMsg, name);
-            mErrorMsg.AppendLiteral(": ");
-        }
-        mErrorMsg.Append(m);
-    }
+    ErrorReportToMessageString(aReport, mErrorMsg);
 
     if (mErrorMsg.IsEmpty() && aFallbackMessage) {
         mErrorMsg.AssignWithConversion(aFallbackMessage);
@@ -271,6 +263,23 @@ xpc::ErrorReport::LogToConsoleWithStack(JS::HandleObject aStack)
 
 }
 
+/* static */
+void
+xpc::ErrorReport::ErrorReportToMessageString(JSErrorReport* aReport,
+                                             nsAString& aString)
+{
+    aString.Truncate();
+    const char16_t* m = aReport->ucmessage;
+    if (m) {
+        JSFlatString* name = js::GetErrorTypeName(CycleCollectedJSRuntime::Get()->Runtime(), aReport->exnType);
+        if (name) {
+            AssignJSFlatString(aString, name);
+            aString.AppendLiteral(": ");
+        }
+        aString.Append(m);
+    }
+}
+
 /***************************************************************************/
 
 

js/xpconnect/src/xpcpublic.h

@@ -513,6 +513,12 @@ class ErrorReport {
     void LogToConsole();
     void LogToConsoleWithStack(JS::HandleObject aStack);
 
+    // Produce an error event message string from the given JSErrorReport.  Note
+    // that this may produce an empty string if aReport doesn't have a
+    // message attached.
+    static void ErrorReportToMessageString(JSErrorReport* aReport,
+                                           nsAString& aString);
+
   public:
 
     nsCString mCategory;

toolkit/components/ctypes/tests/chrome/test_ctypes.xul

@@ -80,8 +80,8 @@
         SimpleTest.finish();
       }
       worker.onerror = function(event) {
-        if (event.message == "InternalError: uncaught exception: 7.5 million years for that?" ||
-            event.message == "InternalError: uncaught exception: Just following orders, sir!") {
+        if (event.message == "uncaught exception: 7.5 million years for that?" ||
+            event.message == "uncaught exception: Just following orders, sir!") {
           // We throw those on purpose in the worker, so ignore them.
           return true;
         }