Bug 1613900 - P2 - Report original JS error message to StructuredCloneCallbacksError; r=baku,sfink

The idea is to propagate error messages defined in js.msg to StructuredCloneHolder and throw it with a data clone error later. So that developers can still understand the reason why serialization/deserialization fails and we don't need to implement two similar set of error messages on JS and DOM sides. Note that this patch gave up the original idea (report error message to console), but developers can stil get the error message by catching the exception. Differential Revision: https://phabricator.services.mozilla.com/D62260 --HG-- extra : moz-landing-system : lando
2020-02-14 13:45:47 +00:00 · 2020-02-14 13:45:47 +00:00 · 0e080c84cd
--- a/dom/base/StructuredCloneHolder.cpp
+++ b/dom/base/StructuredCloneHolder.cpp
@ -113,8 +113,13 @@ bool StructuredCloneCallbacksCanTransfer(JSContext* aCx,
                                          aSameProcessScopeRequired);
 }

-void StructuredCloneCallbacksError(JSContext* aCx, uint32_t aErrorId) {
+void StructuredCloneCallbacksError(JSContext* aCx, uint32_t aErrorId,
+                                   void* aClosure, const char* aErrorMessage) {
  NS_WARNING("Failed to clone data.");
+  StructuredCloneHolderBase* holder =
+      static_cast<StructuredCloneHolderBase*>(aClosure);
+  MOZ_ASSERT(holder);
+  return holder->SetErrorMessage(aErrorMessage);
 }

 void AssertTagValues() {
@ -276,7 +281,7 @@ void StructuredCloneHolder::Write(JSContext* aCx, JS::Handle<JS::Value> aValue,
                                  ErrorResult& aRv) {
  if (!StructuredCloneHolderBase::Write(aCx, aValue, aTransfer,
                                        aCloneDataPolicy)) {
-    aRv.Throw(NS_ERROR_DOM_DATA_CLONE_ERR);
+    aRv.ThrowDataCloneError(mErrorMessage);
    return;
  }
 }
@ -294,11 +299,12 @@ void StructuredCloneHolder::Read(nsIGlobalObject* aGlobal, JSContext* aCx,
  MOZ_ASSERT(aGlobal);

  mozilla::AutoRestore<nsIGlobalObject*> guard(mGlobal);
+  auto errorMessageGuard = MakeScopeExit([&] { mErrorMessage.Truncate(); });
  mGlobal = aGlobal;

  if (!StructuredCloneHolderBase::Read(aCx, aValue, aCloneDataPolicy)) {
    JS_ClearPendingException(aCx);
-    aRv.Throw(NS_ERROR_DOM_DATA_CLONE_ERR);
+    aRv.ThrowDataCloneError(mErrorMessage);
    return;
  }

@ -327,12 +333,14 @@ void StructuredCloneHolder::ReadFromBuffer(
  MOZ_ASSERT(!mBuffer, "ReadFromBuffer() must be called without a Write().");

  mozilla::AutoRestore<nsIGlobalObject*> guard(mGlobal);
+  auto errorMessageGuard = MakeScopeExit([&] { mErrorMessage.Truncate(); });
  mGlobal = aGlobal;

  if (!JS_ReadStructuredClone(aCx, aBuffer, aAlgorithmVersion, CloneScope(),
                              aValue, aCloneDataPolicy, &sCallbacks, this)) {
    JS_ClearPendingException(aCx);
-    aRv.Throw(NS_ERROR_DOM_DATA_CLONE_ERR);
+    aRv.ThrowDataCloneError(mErrorMessage);
+    return;
  }
 }

--- a/dom/base/StructuredCloneHolder.h
+++ b/dom/base/StructuredCloneHolder.h
@ -127,11 +127,20 @@ class StructuredCloneHolderBase {
    return size;
  }

+  void SetErrorMessage(const char* aErrorMessage) {
+    mErrorMessage.Assign(aErrorMessage);
+  }
+
 protected:
  UniquePtr<JSAutoStructuredCloneBuffer> mBuffer;

  StructuredCloneScope mStructuredCloneScope;

+  // Error message when a data clone error is about to throw. It's held while
+  // the error callback is fired and it will be throw with a data clone error
+  // later.
+  nsCString mErrorMessage;
+
 #ifdef DEBUG
  bool mClearCalled;
 #endif
--- a/js/public/StructuredClone.h
+++ b/js/public/StructuredClone.h
@ -275,7 +275,8 @@ typedef bool (*WriteStructuredCloneOp)(JSContext* cx,
 * To follow HTML5, the application must throw a DATA_CLONE_ERR DOMException
 * with error set to one of the JS_SCERR_* values.
 */
-typedef void (*StructuredCloneErrorOp)(JSContext* cx, uint32_t errorid);
+typedef void (*StructuredCloneErrorOp)(JSContext* cx, uint32_t errorid,
+                                       void* closure, const char* errorMessage);

 /**
 * This is called when JS_ReadStructuredClone receives a transferable object
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@ -4870,6 +4870,20 @@ JS_PUBLIC_API void JS_ReportAllocationOverflow(JSContext* cx) {
  ReportAllocationOverflow(cx);
 }

+JS_PUBLIC_API bool JS_ExpandErrorArgumentsASCII(JSContext* cx,
+                                                JSErrorCallback errorCallback,
+                                                const unsigned errorNumber,
+                                                JSErrorReport* reportp, ...) {
+  va_list ap;
+  bool ok;
+
+  AssertHeapIsIdle();
+  va_start(ap, reportp);
+  ok = ExpandErrorArgumentsVA(cx, errorCallback, nullptr, errorNumber, nullptr,
+                              ArgumentsAreASCII, reportp, ap);
+  va_end(ap);
+  return ok;
+}
 /************************************************************************/

 JS_PUBLIC_API bool JS_SetDefaultLocale(JSRuntime* rt, const char* locale) {
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@ -2479,6 +2479,10 @@ extern JS_PUBLIC_API bool JS_ReportErrorFlagsAndNumberUC(
 */
 extern MOZ_COLD JS_PUBLIC_API void JS_ReportOutOfMemory(JSContext* cx);

+extern JS_PUBLIC_API bool JS_ExpandErrorArgumentsASCII(
+    JSContext* cx, JSErrorCallback errorCallback, const unsigned errorNumber,
+    JSErrorReport* reportp, ...);
+
 /**
 * Complain when an allocation size overflows the maximum supported limit.
 */
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@ -592,59 +592,67 @@ static_assert(Scalar::Int8 == 0);
 template <typename... Args>
 static void ReportDataCloneError(JSContext* cx,
                                 const JSStructuredCloneCallbacks* callbacks,
-                                 uint32_t errorId, Args&&... aArgs) {
-  if (callbacks && callbacks->reportError) {
-    callbacks->reportError(cx, errorId);
-    return;
-  }
-
+                                 uint32_t errorId, void* closure,
+                                 Args&&... aArgs) {
+  unsigned errorNumber;
  switch (errorId) {
    case JS_SCERR_DUP_TRANSFERABLE:
-      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                JSMSG_SC_DUP_TRANSFERABLE);
+      errorNumber = JSMSG_SC_DUP_TRANSFERABLE;
      break;

    case JS_SCERR_TRANSFERABLE:
-      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                JSMSG_SC_NOT_TRANSFERABLE);
+      errorNumber = JSMSG_SC_NOT_TRANSFERABLE;
      break;

    case JS_SCERR_UNSUPPORTED_TYPE:
-      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                JSMSG_SC_UNSUPPORTED_TYPE);
+      errorNumber = JSMSG_SC_UNSUPPORTED_TYPE;
      break;

    case JS_SCERR_SHMEM_TRANSFERABLE:
-      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                JSMSG_SC_SHMEM_TRANSFERABLE);
+      errorNumber = JSMSG_SC_SHMEM_TRANSFERABLE;
      break;

    case JS_SCERR_TYPED_ARRAY_DETACHED:
-      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                JSMSG_TYPED_ARRAY_DETACHED);
+      errorNumber = JSMSG_TYPED_ARRAY_DETACHED;
      break;

    case JS_SCERR_WASM_NO_TRANSFER:
-      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                JSMSG_WASM_NO_TRANSFER);
+      errorNumber = JSMSG_WASM_NO_TRANSFER;
      break;

    case JS_SCERR_NOT_CLONABLE:
-      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                JSMSG_SC_NOT_CLONABLE,
-                                std::forward<Args>(aArgs)...);
+      errorNumber = JSMSG_SC_NOT_CLONABLE;
      break;

    case JS_SCERR_NOT_CLONABLE_WITH_COOP_COEP:
-      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                                JSMSG_SC_NOT_CLONABLE_WITH_COOP_COEP,
-                                std::forward<Args>(aArgs)...);
+      errorNumber = JSMSG_SC_NOT_CLONABLE_WITH_COOP_COEP;
      break;

    default:
      MOZ_CRASH("Unkown errorId");
      break;
  }
+
+  if (callbacks && callbacks->reportError) {
+    MOZ_RELEASE_ASSERT(!cx->isExceptionPending());
+
+    JSErrorReport report;
+    // Get js error message if it's possible and propagate it through callback.
+    if (JS_ExpandErrorArgumentsASCII(cx, GetErrorMessage, errorNumber, &report,
+                                     std::forward<Args>(aArgs)...) &&
+        report.message()) {
+      callbacks->reportError(cx, errorId, closure, report.message().c_str());
+    } else {
+      ReportOutOfMemory(cx);
+
+      callbacks->reportError(cx, errorId, closure, "");
+    }
+
+    return;
+  }
+
+  JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, errorNumber,
+                            std::forward<Args>(aArgs)...);
 }

 bool WriteStructuredClone(JSContext* cx, HandleValue v,
@ -1154,7 +1162,7 @@ bool JSStructuredCloneWriter::parseTransferable() {
 template <typename... Args>
 bool JSStructuredCloneWriter::reportDataCloneError(uint32_t errorId,
                                                   Args&&... aArgs) {
-  ReportDataCloneError(context(), out.buf.callbacks_, errorId,
+  ReportDataCloneError(context(), out.buf.callbacks_, errorId, out.buf.closure_,
                       std::forward<Args>(aArgs)...);
  return false;
 }
@ -2260,7 +2268,8 @@ bool JSStructuredCloneReader::readSharedArrayBuffer(MutableHandleValue vp) {
    auto error = context()->realm()->creationOptions().getCoopAndCoepEnabled()
                     ? JS_SCERR_NOT_CLONABLE_WITH_COOP_COEP
                     : JS_SCERR_NOT_CLONABLE;
-    ReportDataCloneError(context(), callbacks, error, "SharedArrayBuffer");
+    ReportDataCloneError(context(), callbacks, error, closure,
+                         "SharedArrayBuffer");
    return false;
  }

@ -2323,7 +2332,7 @@ bool JSStructuredCloneReader::readSharedWasmMemory(uint32_t nbytes,
    auto error = context()->realm()->creationOptions().getCoopAndCoepEnabled()
                     ? JS_SCERR_NOT_CLONABLE_WITH_COOP_COEP
                     : JS_SCERR_NOT_CLONABLE;
-    ReportDataCloneError(cx, callbacks, error, "WebAssembly.Memory");
+    ReportDataCloneError(cx, callbacks, error, closure, "WebAssembly.Memory");
    return false;
  }

@ -2754,7 +2763,7 @@ bool JSStructuredCloneReader::readTransferMap() {
    }

    if (tag == SCTAG_TRANSFER_MAP_PENDING_ENTRY) {
-      ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE);
+      ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE, closure);
      return false;
    }

@ -2777,7 +2786,7 @@ bool JSStructuredCloneReader::readTransferMap() {
        // Transferred ArrayBuffers in a DifferentProcess clone buffer
        // are treated as if they weren't Transferred at all. We should
        // only see SCTAG_TRANSFER_MAP_STORED_ARRAY_BUFFER.
-        ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE);
+        ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE, closure);
        return false;
      }

@ -2800,7 +2809,7 @@ bool JSStructuredCloneReader::readTransferMap() {
        return false;
      }
      if (tag != SCTAG_ARRAY_BUFFER_OBJECT) {
-        ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE);
+        ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE, closure);
        return false;
      }
      RootedValue val(cx);
@ -2810,7 +2819,7 @@ bool JSStructuredCloneReader::readTransferMap() {
      obj = &val.toObject();
    } else {
      if (!callbacks || !callbacks->readTransfer) {
-        ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE);
+        ReportDataCloneError(cx, callbacks, JS_SCERR_TRANSFERABLE, closure);
        return false;
      }
      if (!callbacks->readTransfer(cx, this, tag, content, extraData, closure,