Bug 1618011 part 1. Implement support for error context in ThrowErrorMessage. r=peterv

This does not change behavior at the moment, because the only callers of
ThrowErrorMessage that pass an error number that has a context pass an empty
string for the first (context) arg.  Both of those callers are changed to pass
nullptr for the context in this patch.

We want to support nullptr to mean "empty context", because that way at
callsites we can avoid having extra empty strings.

We could avoid putting this machinery in place if we hardcoded the trailing
": " at all the callsites, but that would reduce future flexibility in where the
context is placed in the message string (e.g. if we wanted to move it to the
end instead of the beginning) and increase the amount of string data we have to
cart around in the binary quite noticeably: we have a _lot_ of places in
bindings where we call ThrowErrorMessage.

Differential Revision: https://phabricator.services.mozilla.com/D64882

--HG--
extra : moz-landing-system : lando

dom/bindings/BindingUtils.cpp

@@ -124,11 +124,52 @@ uint16_t GetErrorArgCount(const ErrNum aErrorNumber) {
   return GetErrorMessage(nullptr, aErrorNumber)->argCount;
 }
 
+// aErrorNumber needs to be unsigned, not an ErrNum, because the latter makes
+// va_start have undefined behavior, and we do not want undefined behavior.
 void binding_detail::ThrowErrorMessage(JSContext* aCx,
                                        const unsigned aErrorNumber, ...) {
   va_list ap;
   va_start(ap, aErrorNumber);
-  JS_ReportErrorNumberUTF8VA(aCx, GetErrorMessage, nullptr, aErrorNumber, ap);
+
+  if (!ErrorFormatHasContext[aErrorNumber]) {
+    JS_ReportErrorNumberUTF8VA(aCx, GetErrorMessage, nullptr, aErrorNumber, ap);
+    va_end(ap);
+    return;
+  }
+
+  // Our first arg is the context arg.  We want to replace nullptr with empty
+  // string, leave empty string alone, and for anything else append ": " to the
+  // end.  See also the behavior of
+  // TErrorResult::SetPendingExceptionWithMessage, which this is mirroring for
+  // exceptions that are thrown directly, not via an ErrorResult.
+  //
+  // XXXbz Once bug 1619087 is fixed, we can avoid the conversions to UTF-16
+  // here.
+  const char16_t* args[JS::MaxNumErrorArguments + 1];
+  size_t argCount = GetErrorArgCount(static_cast<ErrNum>(aErrorNumber));
+  MOZ_ASSERT(argCount > 0, "We have a context arg!");
+  // We statically assert that all these arg counts are smaller than
+  // JS::MaxNumErrorArguments already.
+  nsTArray<nsString> argHolders(argCount);
+
+  for (size_t i = 0; i < argCount; ++i) {
+    const char* arg = va_arg(ap, const char*);
+    if (i == 0) {
+      if (!arg || !*arg) {
+        // Append an empty string
+        argHolders.AppendElement();
+      } else {
+        argHolders.AppendElement(NS_ConvertUTF8toUTF16(arg));
+        argHolders[0].AppendLiteral(": ");
+      }
+    } else {
+      argHolders.AppendElement(NS_ConvertUTF8toUTF16(arg));
+    }
+    args[i] = argHolders[i].get();
+  }
+
+  JS_ReportErrorNumberUCArray(aCx, GetErrorMessage, nullptr, aErrorNumber,
+                              args);
   va_end(ap);
 }
 
@@ -1277,7 +1318,7 @@ void GetInterfaceImpl(JSContext* aCx, nsIInterfaceRequestor* aRequestor,
 }
 
 bool ThrowingConstructor(JSContext* cx, unsigned argc, JS::Value* vp) {
-  return ThrowErrorMessage<MSG_ILLEGAL_CONSTRUCTOR>(cx, "");
+  return ThrowErrorMessage<MSG_ILLEGAL_CONSTRUCTOR>(cx, nullptr);
 }
 
 bool ThrowConstructorWithoutNew(JSContext* cx, const char* name) {

dom/bindings/PrimitiveConversions.h

@@ -209,7 +209,7 @@ inline bool PrimitiveConversionTraits_EnforceRange(JSContext* cx,
       rounded > PrimitiveConversionTraits_Limits<T>::max()) {
     // XXXbz a better context (first arg after cx) would sure be nice.
     return ThrowErrorMessage<MSG_ENFORCE_RANGE_OUT_OF_RANGE>(
-        cx, "", TypeName<T>::value());
+        cx, nullptr, TypeName<T>::value());
   }
 
   *retval = static_cast<T>(rounded);