Revert "Reland: Serialize native errors"

This reverts commit 8f8ae4f8.

Reason for revert: Still failing layout tests: https://ci.chromium.org/p/v8/builders/ci/V8-Blink%20Linux%2064/33036

Original change's description:
> Reland: Serialize native errors
> 
> This is a reland of https://crrev.com/c/v8/v8/+/1649257. The original
> change was reverted because it conflicted with a blink-side serialization
> tag.
> 
> Make native errors serializable.
> 
> The implementation is mostly straightforward, but there is one
> exception: the stack property. Although the property is not specified,
> the spec for error cloning asks us to preserve the property if
> possible. This implementation serializes the property only when it is
> a string, and otherwise ignores it.
> 
> Spec: https://github.com/whatwg/html/pull/4665
> Intent-to-Ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/f8JngIi8qYs
> 
> Bug: chromium:970079
> Change-Id: Ic1ff07be2c5be415bfb564fa3975bc1a55a06a72
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1692366
> Reviewed-by: Simon Zünd <szuend@chromium.org>
> Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#62607}

TBR=jbroman@chromium.org,yhirano@chromium.org,szuend@chromium.org

Change-Id: Ia52b3e3997663fc293e9d217e5a56544b28d050d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:970079
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1695462Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62614}

src/objects/value-serializer.cc

@@ -22,7 +22,6 @@
 #include "src/objects/objects-inl.h"
 #include "src/objects/oddball-inl.h"
 #include "src/objects/ordered-hash-table-inl.h"
-#include "src/objects/property-descriptor.h"
 #include "src/objects/smi.h"
 #include "src/objects/transitions-inl.h"
 #include "src/snapshot/code-serializer.h"
@@ -162,9 +161,6 @@ enum class SerializationTag : uint8_t {
   // A transferred WebAssembly.Memory object. maximumPages:int32_t, then by
   // SharedArrayBuffer tag and its data.
   kWasmMemoryTransfer = 'm',
-  // A list of (subtag: ErrorTag, [subtag dependent data]). See ErrorTag for
-  // details.
-  kError = 'r',
 };
 
 namespace {
@@ -188,28 +184,6 @@ enum class WasmEncodingTag : uint8_t {
   kRawBytes = 'y',
 };
 
-// Sub-tags only meaningful for error serialization.
-enum class ErrorTag : uint8_t {
-  // The error is a EvalError. No accompanying data.
-  kEvalErrorPrototype = 'E',
-  // The error is a RangeError. No accompanying data.
-  kRangeErrorPrototype = 'R',
-  // The error is a ReferenceError. No accompanying data.
-  kReferenceErrorPrototype = 'F',
-  // The error is a SyntaxError. No accompanying data.
-  kSyntaxErrorPrototype = 'S',
-  // The error is a TypeError. No accompanying data.
-  kTypeErrorPrototype = 'T',
-  // The error is a URIError. No accompanying data.
-  kUriErrorPrototype = 'U',
-  // Followed by message: string.
-  kMessage = 'm',
-  // Followed by stack: string.
-  kStack = 's',
-  // The end of this error information.
-  kEnd = '.',
-};
-
 }  // namespace
 
 ValueSerializer::ValueSerializer(Isolate* isolate,
@@ -546,8 +520,6 @@ Maybe<bool> ValueSerializer::WriteJSReceiver(Handle<JSReceiver> receiver) {
     case JS_TYPED_ARRAY_TYPE:
     case JS_DATA_VIEW_TYPE:
       return WriteJSArrayBufferView(JSArrayBufferView::cast(*receiver));
-    case JS_ERROR_TYPE:
-      return WriteJSError(Handle<JSObject>::cast(receiver));
     case WASM_MODULE_TYPE: {
       auto enabled_features = wasm::WasmFeaturesFromIsolate(isolate_);
       if (!FLAG_wasm_disable_structured_cloning || enabled_features.threads) {
@@ -904,60 +876,6 @@ Maybe<bool> ValueSerializer::WriteJSArrayBufferView(JSArrayBufferView view) {
   return ThrowIfOutOfMemory();
 }
 
-Maybe<bool> ValueSerializer::WriteJSError(Handle<JSObject> error) {
-  Handle<Object> stack;
-  PropertyDescriptor message_desc;
-  Maybe<bool> message_found = JSReceiver::GetOwnPropertyDescriptor(
-      isolate_, error, isolate_->factory()->message_string(), &message_desc);
-  MAYBE_RETURN(message_found, Nothing<bool>());
-
-  WriteTag(SerializationTag::kError);
-
-  Handle<HeapObject> prototype;
-  if (!JSObject::GetPrototype(isolate_, error).ToHandle(&prototype)) {
-    return Nothing<bool>();
-  }
-
-  if (*prototype == isolate_->eval_error_function()->prototype()) {
-    WriteVarint(static_cast<uint8_t>(ErrorTag::kEvalErrorPrototype));
-  } else if (*prototype == isolate_->range_error_function()->prototype()) {
-    WriteVarint(static_cast<uint8_t>(ErrorTag::kRangeErrorPrototype));
-  } else if (*prototype == isolate_->reference_error_function()->prototype()) {
-    WriteVarint(static_cast<uint8_t>(ErrorTag::kReferenceErrorPrototype));
-  } else if (*prototype == isolate_->syntax_error_function()->prototype()) {
-    WriteVarint(static_cast<uint8_t>(ErrorTag::kSyntaxErrorPrototype));
-  } else if (*prototype == isolate_->type_error_function()->prototype()) {
-    WriteVarint(static_cast<uint8_t>(ErrorTag::kTypeErrorPrototype));
-  } else if (*prototype == isolate_->uri_error_function()->prototype()) {
-    WriteVarint(static_cast<uint8_t>(ErrorTag::kUriErrorPrototype));
-  } else {
-    // The default prototype in the deserialization side is Error.prototype, so
-    // we don't have to do anything here.
-  }
-
-  if (message_found.FromJust() &&
-      PropertyDescriptor::IsDataDescriptor(&message_desc)) {
-    Handle<String> message;
-    if (!Object::ToString(isolate_, message_desc.value()).ToHandle(&message)) {
-      return Nothing<bool>();
-    }
-    WriteVarint(static_cast<uint8_t>(ErrorTag::kMessage));
-    WriteString(message);
-  }
-
-  if (!Object::GetProperty(isolate_, error, isolate_->factory()->stack_string())
-           .ToHandle(&stack)) {
-    return Nothing<bool>();
-  }
-  if (stack->IsString()) {
-    WriteVarint(static_cast<uint8_t>(ErrorTag::kStack));
-    WriteString(Handle<String>::cast(stack));
-  }
-
-  WriteVarint(static_cast<uint8_t>(ErrorTag::kEnd));
-  return ThrowIfOutOfMemory();
-}
-
 Maybe<bool> ValueSerializer::WriteWasmModule(Handle<WasmModuleObject> object) {
   if (delegate_ != nullptr) {
     // TODO(titzer): introduce a Utils::ToLocal for WasmModuleObject.
@@ -1340,8 +1258,6 @@ MaybeHandle<Object> ValueDeserializer::ReadObjectInternal() {
       const bool is_shared = true;
       return ReadJSArrayBuffer(is_shared);
     }
-    case SerializationTag::kError:
-      return ReadJSError();
     case SerializationTag::kWasmModule:
       return ReadWasmModule();
     case SerializationTag::kWasmModuleTransfer:
@@ -1857,78 +1773,6 @@ MaybeHandle<JSArrayBufferView> ValueDeserializer::ReadJSArrayBufferView(
   return typed_array;
 }
 
-MaybeHandle<Object> ValueDeserializer::ReadJSError() {
-  Handle<Object> message = isolate_->factory()->undefined_value();
-  Handle<Object> stack = isolate_->factory()->undefined_value();
-  Handle<Object> no_caller;
-  auto constructor = isolate_->error_function();
-  bool done = false;
-
-  while (!done) {
-    uint8_t tag;
-    if (!ReadVarint<uint8_t>().To(&tag)) {
-      return MaybeHandle<JSObject>();
-    }
-    switch (static_cast<ErrorTag>(tag)) {
-      case ErrorTag::kEvalErrorPrototype:
-        constructor = isolate_->eval_error_function();
-        break;
-      case ErrorTag::kRangeErrorPrototype:
-        constructor = isolate_->range_error_function();
-        break;
-      case ErrorTag::kReferenceErrorPrototype:
-        constructor = isolate_->reference_error_function();
-        break;
-      case ErrorTag::kSyntaxErrorPrototype:
-        constructor = isolate_->syntax_error_function();
-        break;
-      case ErrorTag::kTypeErrorPrototype:
-        constructor = isolate_->type_error_function();
-        break;
-      case ErrorTag::kUriErrorPrototype:
-        constructor = isolate_->uri_error_function();
-        break;
-      case ErrorTag::kMessage: {
-        Handle<String> message_string;
-        if (!ReadString().ToHandle(&message_string)) {
-          return MaybeHandle<JSObject>();
-        }
-        message = message_string;
-        break;
-      }
-      case ErrorTag::kStack: {
-        Handle<String> stack_string;
-        if (!ReadString().ToHandle(&stack_string)) {
-          return MaybeHandle<JSObject>();
-        }
-        stack = stack_string;
-        break;
-      }
-      case ErrorTag::kEnd:
-        done = true;
-        break;
-      default:
-        return MaybeHandle<JSObject>();
-    }
-  }
-
-  Handle<Object> error;
-  if (!ErrorUtils::Construct(isolate_, constructor, constructor, message,
-                             SKIP_NONE, no_caller,
-                             ErrorUtils::StackTraceCollection::kNone)
-           .ToHandle(&error)) {
-    return MaybeHandle<Object>();
-  }
-
-  if (Object::SetProperty(
-          isolate_, error, isolate_->factory()->stack_trace_symbol(), stack,
-          StoreOrigin::kMaybeKeyed, Just(ShouldThrow::kThrowOnError))
-          .is_null()) {
-    return MaybeHandle<Object>();
-  }
-  return error;
-}
-
 MaybeHandle<JSObject> ValueDeserializer::ReadWasmModuleTransfer() {
   auto enabled_features = wasm::WasmFeaturesFromIsolate(isolate_);
   if ((FLAG_wasm_disable_structured_cloning && !enabled_features.threads) ||

src/objects/value-serializer.h

@@ -128,7 +128,6 @@ class ValueSerializer {
   Maybe<bool> WriteJSArrayBuffer(Handle<JSArrayBuffer> array_buffer)
       V8_WARN_UNUSED_RESULT;
   Maybe<bool> WriteJSArrayBufferView(JSArrayBufferView array_buffer);
-  Maybe<bool> WriteJSError(Handle<JSObject> error) V8_WARN_UNUSED_RESULT;
   Maybe<bool> WriteWasmModule(Handle<WasmModuleObject> object)
       V8_WARN_UNUSED_RESULT;
   Maybe<bool> WriteWasmMemory(Handle<WasmMemoryObject> object)
@@ -277,7 +276,6 @@ class ValueDeserializer {
       V8_WARN_UNUSED_RESULT;
   MaybeHandle<JSArrayBufferView> ReadJSArrayBufferView(
       Handle<JSArrayBuffer> buffer) V8_WARN_UNUSED_RESULT;
-  MaybeHandle<Object> ReadJSError() V8_WARN_UNUSED_RESULT;
   MaybeHandle<JSObject> ReadWasmModule() V8_WARN_UNUSED_RESULT;
   MaybeHandle<JSObject> ReadWasmModuleTransfer() V8_WARN_UNUSED_RESULT;
   MaybeHandle<WasmMemoryObject> ReadWasmMemory() V8_WARN_UNUSED_RESULT;

test/unittests/objects/value-serializer-unittest.cc

@@ -2885,68 +2885,5 @@ TEST_F(ValueSerializerTestWithLimitedMemory, FailIfNoMemoryInWriteHostObject) {
   EXPECT_TRUE(EvaluateScriptForInput("gotA")->IsFalse());
 }
 
-// We only have basic tests and tests for .stack here, because we have more
-// comprehensive tests as web platform tests.
-TEST_F(ValueSerializerTest, RoundTripError) {
-  Local<Value> value = RoundTripTest("Error('hello')");
-  ASSERT_TRUE(value->IsObject());
-  Local<Object> error = value.As<Object>();
-
-  Local<Value> name;
-  Local<Value> message;
-
-  {
-    Context::Scope scope(deserialization_context());
-    EXPECT_EQ(error->GetPrototype(), Exception::Error(String::Empty(isolate()))
-                                         .As<Object>()
-                                         ->GetPrototype());
-  }
-  ASSERT_TRUE(error->Get(deserialization_context(), StringFromUtf8("name"))
-                  .ToLocal(&name));
-  ASSERT_TRUE(name->IsString());
-  EXPECT_EQ(Utf8Value(name), "Error");
-
-  ASSERT_TRUE(error->Get(deserialization_context(), StringFromUtf8("message"))
-                  .ToLocal(&message));
-  ASSERT_TRUE(message->IsString());
-  EXPECT_EQ(Utf8Value(message), "hello");
-}
-
-TEST_F(ValueSerializerTest, DefaultErrorStack) {
-  Local<Value> value =
-      RoundTripTest("function hkalkcow() { return Error(); } hkalkcow();");
-  ASSERT_TRUE(value->IsObject());
-  Local<Object> error = value.As<Object>();
-
-  Local<Value> stack;
-  ASSERT_TRUE(error->Get(deserialization_context(), StringFromUtf8("stack"))
-                  .ToLocal(&stack));
-  ASSERT_TRUE(stack->IsString());
-  EXPECT_NE(Utf8Value(stack).find("hkalkcow"), std::string::npos);
-}
-
-TEST_F(ValueSerializerTest, ModifiedErrorStack) {
-  Local<Value> value = RoundTripTest("let e = Error(); e.stack = 'hello'; e");
-  ASSERT_TRUE(value->IsObject());
-  Local<Object> error = value.As<Object>();
-
-  Local<Value> stack;
-  ASSERT_TRUE(error->Get(deserialization_context(), StringFromUtf8("stack"))
-                  .ToLocal(&stack));
-  ASSERT_TRUE(stack->IsString());
-  EXPECT_EQ(Utf8Value(stack), "hello");
-}
-
-TEST_F(ValueSerializerTest, NonStringErrorStack) {
-  Local<Value> value = RoundTripTest("let e = Error(); e.stack = 17; e");
-  ASSERT_TRUE(value->IsObject());
-  Local<Object> error = value.As<Object>();
-
-  Local<Value> stack;
-  ASSERT_TRUE(error->Get(deserialization_context(), StringFromUtf8("stack"))
-                  .ToLocal(&stack));
-  EXPECT_TRUE(stack->IsUndefined());
-}
-
 }  // namespace
 }  // namespace v8