Remove toString() override from DOMException.

Adapt to whatwg/webidl#378 ("Re-align DOMException objects with what is implemented"). We were reimplementing toString() in DOMException because of WebKit r29058 ("Acid3 expects ExeceptionCode constants to be defined on DOMException objects") from almost 10 years ago. A lot has happened since, and we can (and should) just use the toString() implementation from ECMAScript's %ErrorProtoype% (which is explicitly mandated to be in DOMException's inheritance chain by the WebIDL spec). Contrary to what's originally described in bug 556950, we do throw an exception when DOMException.prototype.toString() is called directly: the WebIDL spec now expects it to, and web-platform-tests/wpt#6361 tests this behavior. Additionally, we've changed the way DOMException inherits from %ErrorPrototype%: instead of doing it in V8PerContextData, we now do it in V8DOMException::installV8DOMExceptionTemplate(), as the former had problems with (i)frames detached from the DOM and toString() would just call Object.prototype.toString() instead. The only user-visible part of the change is that "toString" is no longer part of DOMException.prototype's own properties; the error message format is exactly the same in most cases (the exact steps are described in https://tc39.github.io/ecma262/#sec-error.prototype.tostring). Finally, part of http/tests/plugins/cross-frame-object-access.html's output will change from: "Error: Uncaught [object DOMException]" to "Error: Uncaught" This is tricky because it involves PPAPI and its separate process, but basically the plugin in an iframe is trying to access top.location.href, Blink is throwing a SecurityError, but the error message is sent truncated to PPAPI. The message is truncated because V8 is calling its NoSideEffectsErrorToString() when creating the message, and this one does not use the message/name accessors we install, leading to an empty message (it looked slightly better before because we the presence of our own toString() caused Object::NoSideEffectsToString() to choose a different albeit still wrong code path). Blink's handling of this is fine, as the code in V8Initializer takes care of extracting the name and error message from the DOMException V8 object that threw the exception. Bug: 556950, 737497 Change-Id: I9d81edca1de903364bb1aca5950c313885c5964a Reviewed-on: https://chromium-review.googlesource.com/558904 Commit-Queue: Raphael Kubo da Costa (rakuco) <[email protected]> Reviewed-by: Mike West <[email protected]> Reviewed-by: Yuki Shiino <[email protected]> Reviewed-by: Kentaro Hara <[email protected]> Cr-Commit-Position: refs/heads/master@{#485960}
scheib · Jul 12, 2017 · be24cfe · be24cfe
1 parent 8e3b504
commit be24cfe
Show file tree

Hide file tree

Showing 28 changed files with 36 additions and 61 deletions.
diff --git a/third_party/WebKit/LayoutTests/crypto/subtle/aes-cbc/decrypt-failures-expected.txt b/third_party/WebKit/LayoutTests/crypto/subtle/aes-cbc/decrypt-failures-expected.txt
@@ -4,19 +4,19 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 PASS: Decryption succeeded
-error is: OperationError: 
+error is: OperationError
 PASS: decrypting failed. ciphertext length: 0
-error is: OperationError: 
+error is: OperationError
 PASS: decrypting failed. ciphertext length: 79
-error is: OperationError: 
+error is: OperationError
 PASS: decrypting failed. ciphertext length: 64
-error is: OperationError: 
+error is: OperationError
 PASS: decrypting failed. ciphertext length: 1
-error is: OperationError: 
+error is: OperationError
 PASS: decrypting failed. ciphertext length: 15
-error is: OperationError: 
+error is: OperationError
 PASS: decrypting failed. ciphertext length: 16
-error is: OperationError: 
+error is: OperationError
 PASS: decrypting failed. ciphertext length: 17
 PASS successfullyParsed is true
 

diff --git a/third_party/WebKit/LayoutTests/crypto/subtle/ecdh/import-export-raw-expected.txt b/third_party/WebKit/LayoutTests/crypto/subtle/ecdh/import-export-raw-expected.txt
@@ -20,7 +20,7 @@ PASS publicKey.algorithm.namedCurve is 'P-256'
 Exporting to raw...
 PASS: Exported to raw should be [044ea34391aa73885454bc45df3fdcc4a70262fa4621ffe25b5790590c340a4bd9265ef2b3f9a86e2959a960d90323465d60cd4a90d314c5de3f869ad0d4bf6c10] and was
 Importing invalid raw public key...
-error is: DataError: 
+error is: DataError
 PASS successfullyParsed is true
 
 TEST COMPLETE

diff --git a/third_party/WebKit/LayoutTests/crypto/subtle/rsa-oaep/plaintext-length-expected.txt b/third_party/WebKit/LayoutTests/crypto/subtle/rsa-oaep/plaintext-length-expected.txt
@@ -7,7 +7,7 @@ Importing RSA keys...
 Encrypting a 214 byte buffer with RSA-OAEP SHA-1, 2048 bit key...
 PASS Succeeded
 Encrypting a 215 byte buffer...
-error is: OperationError: 
+error is: OperationError
 PASS Rejected
 PASS successfullyParsed is true
 

diff --git a/third_party/WebKit/LayoutTests/http/tests/plugins/cross-frame-object-access-expected.txt b/third_party/WebKit/LayoutTests/http/tests/plugins/cross-frame-object-access-expected.txt
@@ -8,7 +8,7 @@ Frame: 'childFrame'
 
 This tests that plugins can access objects in other frames as allowed by the security model enforced in WebCore.
 Error: Error: Failed conversion between PP_Var and V8 value
-Error: Uncaught [object DOMException]
+Error: Uncaught
 Error: Error: Failed conversion between PP_Var and V8 value
-Error: Uncaught [object DOMException]
+Error: Uncaught
 SUCCESS
diff --git a/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt b/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt
@@ -179,7 +179,6 @@ interface DOMException
     getter message
     getter name
     method constructor
-    method toString
 interface DOMMatrix : DOMMatrixReadOnly
     attribute @@toStringTag
     getter a

diff --git a/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt b/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt
@@ -112,7 +112,6 @@ interface DOMException
     getter message
     getter name
     method constructor
-    method toString
 interface DOMMatrix : DOMMatrixReadOnly
     attribute @@toStringTag
     getter a

diff --git a/...form/mac/virtual/stable/webexposed/global-interface-listing-dedicated-worker-expected.txt b/...form/mac/virtual/stable/webexposed/global-interface-listing-dedicated-worker-expected.txt
@@ -103,7 +103,6 @@ Starting worker: resources/global-interface-listing-worker.js
 [Worker]     getter message
 [Worker]     getter name
 [Worker]     method constructor
-[Worker]     method toString
 [Worker] interface DOMMatrix : DOMMatrixReadOnly
 [Worker]     attribute @@toStringTag
 [Worker]     getter a

diff --git a/.../LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt b/.../LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt
@@ -711,7 +711,6 @@ interface DOMException
     getter message
     getter name
     method constructor
-    method toString
 interface DOMImplementation
     attribute @@toStringTag
     method constructor

diff --git a/...latform/mac/virtual/stable/webexposed/global-interface-listing-shared-worker-expected.txt b/...latform/mac/virtual/stable/webexposed/global-interface-listing-shared-worker-expected.txt
@@ -103,7 +103,6 @@ Starting worker: resources/global-interface-listing-worker.js
 [Worker]     getter message
 [Worker]     getter name
 [Worker]     method constructor
-[Worker]     method toString
 [Worker] interface DOMMatrix : DOMMatrixReadOnly
 [Worker]     attribute @@toStringTag
 [Worker]     getter a

diff --git a/.../LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt b/.../LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
@@ -640,7 +640,6 @@ interface DOMException
     getter message
     getter name
     method constructor
-    method toString
 interface DOMImplementation
     attribute @@toStringTag
     method constructor

diff --git a/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt b/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt
@@ -179,7 +179,6 @@ interface DOMException
     getter message
     getter name
     method constructor
-    method toString
 interface DOMMatrix : DOMMatrixReadOnly
     attribute @@toStringTag
     getter a

diff --git a/...gation-preload-disabled/webexposed/global-interface-listing-dedicated-worker-expected.txt b/...gation-preload-disabled/webexposed/global-interface-listing-dedicated-worker-expected.txt
@@ -141,7 +141,6 @@ Starting worker: resources/global-interface-listing-worker.js
 [Worker]     getter message
 [Worker]     getter name
 [Worker]     method constructor
-[Worker]     method toString
 [Worker] interface DOMMatrix : DOMMatrixReadOnly
 [Worker]     attribute @@toStringTag
 [Worker]     getter a

diff --git a/...rvice-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt b/...rvice-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt
@@ -1130,7 +1130,6 @@ interface DOMException
     getter message
     getter name
     method constructor
-    method toString
 interface DOMImplementation
     attribute @@toStringTag
     method constructor

diff --git a/...avigation-preload-disabled/webexposed/global-interface-listing-shared-worker-expected.txt b/...avigation-preload-disabled/webexposed/global-interface-listing-shared-worker-expected.txt
@@ -141,7 +141,6 @@ Starting worker: resources/global-interface-listing-worker.js
 [Worker]     getter message
 [Worker]     getter name
 [Worker]     method constructor
-[Worker]     method toString
 [Worker] interface DOMMatrix : DOMMatrixReadOnly
 [Worker]     attribute @@toStringTag
 [Worker]     getter a

diff --git a/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt b/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt
@@ -152,7 +152,6 @@ interface DOMException
     getter message
     getter name
     method constructor
-    method toString
 interface DOMMatrix : DOMMatrixReadOnly
     getter a
     getter b

diff --git a/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt b/.../http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt
@@ -112,7 +112,6 @@ interface DOMException
     getter message
     getter name
     method constructor
-    method toString
 interface DOMMatrix : DOMMatrixReadOnly
     attribute @@toStringTag
     getter a

diff --git a/...outTests/virtual/stable/webexposed/global-interface-listing-dedicated-worker-expected.txt b/...outTests/virtual/stable/webexposed/global-interface-listing-dedicated-worker-expected.txt
@@ -103,7 +103,6 @@ Starting worker: resources/global-interface-listing-worker.js
 [Worker]     getter message
 [Worker]     getter name
 [Worker]     method constructor
-[Worker]     method toString
 [Worker] interface DOMMatrix : DOMMatrixReadOnly
 [Worker]     attribute @@toStringTag
 [Worker]     getter a

diff --git a/...LayoutTests/virtual/stable/webexposed/global-interface-listing-shared-worker-expected.txt b/...LayoutTests/virtual/stable/webexposed/global-interface-listing-shared-worker-expected.txt
@@ -103,7 +103,6 @@ Starting worker: resources/global-interface-listing-worker.js
 [Worker]     getter message
 [Worker]     getter name
 [Worker]     method constructor
-[Worker]     method toString
 [Worker] interface DOMMatrix : DOMMatrixReadOnly
 [Worker]     attribute @@toStringTag
 [Worker]     getter a

diff --git a/...arty/WebKit/LayoutTests/webexposed/global-interface-listing-dedicated-worker-expected.txt b/...arty/WebKit/LayoutTests/webexposed/global-interface-listing-dedicated-worker-expected.txt
@@ -141,7 +141,6 @@ Starting worker: resources/global-interface-listing-worker.js
 [Worker]     getter message
 [Worker]     getter name
 [Worker]     method constructor
-[Worker]     method toString
 [Worker] interface DOMMatrix : DOMMatrixReadOnly
 [Worker]     attribute @@toStringTag
 [Worker]     getter a

diff --git a/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
@@ -1130,7 +1130,6 @@ interface DOMException
     getter message
     getter name
     method constructor
-    method toString
 interface DOMImplementation
     attribute @@toStringTag
     method constructor

diff --git a/...d_party/WebKit/LayoutTests/webexposed/global-interface-listing-shared-worker-expected.txt b/...d_party/WebKit/LayoutTests/webexposed/global-interface-listing-shared-worker-expected.txt
@@ -141,7 +141,6 @@ Starting worker: resources/global-interface-listing-worker.js
 [Worker]     getter message
 [Worker]     getter name
 [Worker]     method constructor
-[Worker]     method toString
 [Worker] interface DOMMatrix : DOMMatrixReadOnly
 [Worker]     attribute @@toStringTag
 [Worker]     getter a

diff --git a/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializerTest.cpp b/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializerTest.cpp
@@ -168,7 +168,7 @@ TEST(V8ScriptValueSerializerTest, ThrowsDataCloneError) {
   ASSERT_TRUE(HadDOMException("DataCloneError", script_state, exception_state));
   DOMException* dom_exception =
       V8DOMException::toImpl(exception_state.GetException().As<v8::Object>());
-  EXPECT_TRUE(dom_exception->toString().Contains("postMessage"));
+  EXPECT_TRUE(dom_exception->message().Contains("postMessage"));
 }
 
 TEST(V8ScriptValueSerializerTest, RethrowsScriptError) {

diff --git a/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl b/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
@@ -602,6 +602,30 @@ static void install{{v8_class}}Template(
   interfaceTemplate->Inherit(intrinsicIteratorPrototypeInterfaceTemplate);
   {% endif %}
 
+  {% if interface_name == 'DOMException' %}
+  // The WebIDL spec states that DOMException objects have a few peculiarities.
+  // One of them is similar to what it mandates for Iterator objects when it
+  // comes to the inheritance chain. Instead of
+  //     DOMException -> prototype -> %ObjectPrototype%
+  // we have
+  //     DOMException -> prototype -> %ErrorPrototype% -> %ObjectPrototype%
+  // so that DOMException objects "inherit" toString() and a few properties
+  // from %ErrorPrototype%.
+  // See https://heycam.github.io/webidl/#es-DOMException-specialness.
+  //
+  // We achieve this with the same hack we use for Iterators: create a new
+  // function template with no prototype, set its "prototype" property to
+  // %ErrorPrototype% and make |interfaceTemplate| inherit from it. When
+  // |interfaceTemplate| is instantiated, its prototype.__proto__ will point to
+  // |intrinsicErrorPrototypeInterfaceTemplate|'s "prototype" property.
+  v8::Local<v8::FunctionTemplate> intrinsicErrorPrototypeInterfaceTemplate =
+      v8::FunctionTemplate::New(isolate);
+  intrinsicErrorPrototypeInterfaceTemplate->RemovePrototype();
+  intrinsicErrorPrototypeInterfaceTemplate->SetIntrinsicDataProperty(
+      V8AtomicString(isolate, "prototype"), v8::kErrorPrototype);
+  interfaceTemplate->Inherit(intrinsicErrorPrototypeInterfaceTemplate);
+  {% endif %}
+
   {% if interface_name == 'Location' %}
   // Symbol.toPrimitive
   // Prevent author scripts to inject Symbol.toPrimitive property into location

diff --git a/third_party/WebKit/Source/core/dom/DOMException.cpp b/third_party/WebKit/Source/core/dom/DOMException.cpp
@@ -195,10 +195,6 @@ DOMException* DOMException::Create(const String& message, const String& name) {
   return new DOMException(GetErrorCode(name), name, message, message);
 }
 
-String DOMException::toString() const {
-  return name() + ": " + message();
-}
-
 String DOMException::ToStringForConsole() const {
   return name() + ": " + MessageForConsole();
 }

diff --git a/third_party/WebKit/Source/core/dom/DOMException.h b/third_party/WebKit/Source/core/dom/DOMException.h
@@ -57,7 +57,6 @@ class CORE_EXPORT DOMException final
   // This is the message that's exposed to JavaScript: never return unsanitized
   // data.
   String message() const { return sanitized_message_; }
-  String toString() const;
 
   // This is the message that's exposed to the console: if an unsanitized
   // message is present, we prefer it.

diff --git a/third_party/WebKit/Source/core/dom/DOMException.idl b/third_party/WebKit/Source/core/dom/DOMException.idl
@@ -40,9 +40,6 @@
     readonly attribute DOMString        name;
     readonly attribute DOMString        message;
 
-    // Override in a Mozilla compatible format
-    [NotEnumerable] DOMString toString();
-
     // ExceptionCode
     const unsigned short      INDEX_SIZE_ERR                 = 1;
     const unsigned short      DOMSTRING_SIZE_ERR             = 2;

diff --git a/third_party/WebKit/Source/platform/bindings/V8PerContextData.cpp b/third_party/WebKit/Source/platform/bindings/V8PerContextData.cpp
@@ -51,18 +51,6 @@ V8PerContextData::V8PerContextData(v8::Local<v8::Context> context)
       activity_logger_(nullptr) {
   context_holder_->SetContext(context);
 
-  v8::Context::Scope context_scope(context);
-  DCHECK(error_prototype_.IsEmpty());
-  v8::Local<v8::Value> object_value =
-      context->Global()
-          ->Get(context, V8AtomicString(isolate_, "Error"))
-          .ToLocalChecked();
-  v8::Local<v8::Value> prototype_value =
-      object_value.As<v8::Object>()
-          ->Get(context, V8AtomicString(isolate_, "prototype"))
-          .ToLocalChecked();
-  error_prototype_.Set(isolate_, prototype_value);
-
   if (IsMainThread()) {
     InstanceCounters::IncrementCounter(
         InstanceCounters::kV8PerContextDataCounter);
@@ -87,8 +75,6 @@ V8PerContextData* V8PerContextData::From(v8::Local<v8::Context> context) {
 
 v8::Local<v8::Object> V8PerContextData::CreateWrapperFromCacheSlowCase(
     const WrapperTypeInfo* type) {
-  DCHECK(!error_prototype_.IsEmpty());
-
   v8::Context::Scope scope(GetContext());
   v8::Local<v8::Function> interface_object = ConstructorForType(type);
   CHECK(!interface_object.IsEmpty());
@@ -101,8 +87,6 @@ v8::Local<v8::Object> V8PerContextData::CreateWrapperFromCacheSlowCase(
 
 v8::Local<v8::Function> V8PerContextData::ConstructorForTypeSlowCase(
     const WrapperTypeInfo* type) {
-  DCHECK(!error_prototype_.IsEmpty());
-
   v8::Local<v8::Context> current_context = GetContext();
   v8::Context::Scope scope(current_context);
   const DOMWrapperWorld& world = DOMWrapperWorld::World(current_context);
@@ -147,13 +131,6 @@ v8::Local<v8::Function> V8PerContextData::ConstructorForTypeSlowCase(
     type->PreparePrototypeAndInterfaceObject(current_context, world,
                                              prototype_object, interface_object,
                                              interface_template);
-    if (type->wrapper_type_prototype ==
-        WrapperTypeInfo::kWrapperTypeExceptionPrototype) {
-      if (!V8CallBoolean(prototype_object->SetPrototype(
-              current_context, error_prototype_.NewLocal(isolate_)))) {
-        return v8::Local<v8::Function>();
-      }
-    }
   }
 
   // Origin Trials

diff --git a/third_party/WebKit/Source/platform/bindings/V8PerContextData.h b/third_party/WebKit/Source/platform/bindings/V8PerContextData.h
@@ -150,7 +150,6 @@ class PLATFORM_EXPORT V8PerContextData final {
   std::unique_ptr<gin::ContextHolder> context_holder_;
 
   ScopedPersistent<v8::Context> context_;
-  ScopedPersistent<v8::Value> error_prototype_;
 
   ScopedPersistent<v8::Private> private_custom_element_definition_id_;