Bug 948647, part 3 - Change js_ErrorToException to return true iff cx->throwing was set, and document the convention. r=Waldo.

--HG--
extra : rebase_source : d7db5309c651b8d8954bb480746efd9dd04db6d9

js/src/jsexn.cpp

@@ -642,14 +642,12 @@ bool
 js_ErrorToException(JSContext *cx, const char *message, JSErrorReport *reportp,
                     JSErrorCallback callback, void *userRef)
 {
-    /*
-     * Tell our caller to report immediately if this report is just a warning.
-     */
+    // Tell our caller to report immediately if this report is just a warning.
     JS_ASSERT(reportp);
     if (JSREPORT_IS_WARNING(reportp->flags))
         return false;
 
-    /* Find the exception index associated with this error. */
+    // Find the exception index associated with this error.
     JSErrNum errorNumber = static_cast<JSErrNum>(reportp->errorNumber);
     const JSErrorFormatString *errorString;
     if (!callback || callback == js_GetErrorMessage)
@@ -659,48 +657,47 @@ js_ErrorToException(JSContext *cx, const char *message, JSErrorReport *reportp,
     JSExnType exnType = errorString ? static_cast<JSExnType>(errorString->exnType) : JSEXN_NONE;
     MOZ_ASSERT(exnType < JSEXN_LIMIT);
 
-    /*
-     * Return false (no exception raised) if no exception is associated
-     * with the given error number.
-     */
+    // Return false (no exception raised) if no exception is associated
+    // with the given error number.
     if (exnType == JSEXN_NONE)
         return false;
 
-    /* Prevent infinite recursion. */
+    // Prevent infinite recursion.
     if (cx->generatingError)
         return false;
     AutoScopedAssign<bool> asa(&cx->generatingError, true);
 
+    // Create an exception object.
     RootedString messageStr(cx, reportp->ucmessage ? JS_NewUCStringCopyZ(cx, reportp->ucmessage)
                                                    : JS_NewStringCopyZ(cx, message));
     if (!messageStr)
-        return false;
+        return cx->isExceptionPending();
 
     RootedString fileName(cx, JS_NewStringCopyZ(cx, reportp->filename));
     if (!fileName)
-        return false;
+        return cx->isExceptionPending();
 
     uint32_t lineNumber = reportp->lineno;
     uint32_t columnNumber = reportp->column;
 
     RootedString stack(cx, ComputeStackString(cx));
     if (!stack)
-        return false;
+        return cx->isExceptionPending();
 
     js::ScopedJSFreePtr<JSErrorReport> report(CopyErrorReport(cx, reportp));
     if (!report)
-        return false;
+        return cx->isExceptionPending();
 
     RootedObject errObject(cx, ErrorObject::create(cx, exnType, stack, fileName,
-                                                   lineNumber, columnNumber, &report,
-                                                   messageStr));
+                                                   lineNumber, columnNumber, &report, messageStr));
     if (!errObject)
-        return false;
+        return cx->isExceptionPending();
 
+    // Throw it.
     RootedValue errValue(cx, ObjectValue(*errObject));
     JS_SetPendingException(cx, errValue);
 
-    /* Flag the error report passed in to indicate an exception was raised. */
+    // Flag the error report passed in to indicate an exception was raised.
     reportp->flags |= JSREPORT_EXCEPTION;
     return true;
 }

js/src/jsexn.h

@@ -23,8 +23,26 @@ class ErrorObject;
  * the error number.  If there is, then create an appropriate exception object,
  * set it as the pending exception, and set the JSREPORT_EXCEPTION flag on the
  * error report.  Exception-aware host error reporters should probably ignore
- * error reports so flagged.  Returns true if an associated exception is
- * found and set, false otherwise.
+ * error reports so flagged.
+ *
+ * Return true if cx->throwing and cx->exception were set.
+ *
+ * This means that:
+ *
+ *   - If the error is successfully converted to an exception and stored in
+ *     cx->exception, the return value is true. This is the "normal", happiest
+ *     case for the caller.
+ *
+ *   - If we try to convert, but fail with OOM or some other error that ends up
+ *     setting cx->throwing to true and setting cx->exception, then we also
+ *     return true (because callers want to treat that case the same way).
+ *     The original error described by *reportp typically won't be reported
+ *     anywhere; instead OOM is reported.
+ *
+ *   - If *reportp is just a warning, or the error code is unrecognized, or if
+ *     we decided to do nothing in order to avoid recursion, then return
+ *     false. In those cases, this error is just being swept under the rug
+ *     unless the caller decides to call CallErrorReporter explicitly.
  */
 extern bool
 js_ErrorToException(JSContext *cx, const char *message, JSErrorReport *reportp,