Commit be24cfe8 authored by Raphael Kubo da Costa's avatar Raphael Kubo da Costa Committed by Commit Bot

Remove toString() override from DOMException.

Adapt to https://github.com/heycam/webidl/pull/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
https://github.com/w3c/web-platform-tests/pull/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) <raphael.kubo.da.costa@intel.com>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Reviewed-by: default avatarYuki Shiino <yukishiino@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485960}
parent 8e3b5044
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -179,7 +179,6 @@ interface DOMException
getter message
getter name
method constructor
method toString
interface DOMMatrix : DOMMatrixReadOnly
attribute @@toStringTag
getter a
......
......@@ -112,7 +112,6 @@ interface DOMException
getter message
getter name
method constructor
method toString
interface DOMMatrix : DOMMatrixReadOnly
attribute @@toStringTag
getter a
......
......@@ -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
......
......@@ -711,7 +711,6 @@ interface DOMException
getter message
getter name
method constructor
method toString
interface DOMImplementation
attribute @@toStringTag
method constructor
......
......@@ -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
......
......@@ -640,7 +640,6 @@ interface DOMException
getter message
getter name
method constructor
method toString
interface DOMImplementation
attribute @@toStringTag
method constructor
......
......@@ -179,7 +179,6 @@ interface DOMException
getter message
getter name
method constructor
method toString
interface DOMMatrix : DOMMatrixReadOnly
attribute @@toStringTag
getter a
......
......@@ -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
......
......@@ -1130,7 +1130,6 @@ interface DOMException
getter message
getter name
method constructor
method toString
interface DOMImplementation
attribute @@toStringTag
method constructor
......
......@@ -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
......
......@@ -152,7 +152,6 @@ interface DOMException
getter message
getter name
method constructor
method toString
interface DOMMatrix : DOMMatrixReadOnly
getter a
getter b
......
......@@ -112,7 +112,6 @@ interface DOMException
getter message
getter name
method constructor
method toString
interface DOMMatrix : DOMMatrixReadOnly
attribute @@toStringTag
getter a
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -1130,7 +1130,6 @@ interface DOMException
getter message
getter name
method constructor
method toString
interface DOMImplementation
attribute @@toStringTag
method constructor
......
......@@ -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
......
......@@ -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) {
......
......@@ -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
......
......@@ -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();
}
......
......@@ -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.
......
......@@ -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;
......
......@@ -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
......
......@@ -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_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment