Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add tests for ways in which DOMException is unusual #6361

Merged
merged 5 commits into from
Jul 6, 2017
Merged

Add tests for ways in which DOMException is unusual #6361

merged 5 commits into from
Jul 6, 2017

Conversation

domenic
Copy link
Member

@domenic domenic commented Jun 28, 2017 •
edited by wpt-pr-bot
Loading

See whatwg/webidl#55.

DO NOT MERGE; this is in fact meant to demonstrate that nobody implements the current spec, which as discussed in that Web IDL issue I kind of messed up. So, likely we will fix the spec, then switch these tests to be -historical.html tests that test the opposite of their current values.

@wpt-pr-bot wpt-pr-bot requested review from tobie, jensl and yuki3 June 28, 2017 16:18
@domenic domenic mentioned this pull request Jun 28, 2017
Closed
@domenic
Copy link
Member Author

domenic commented Jun 28, 2017 •
edited
Loading

I've switched these from testing the current-as-of-now spec to testing whatwg/webidl#378, which is much closer to what's implemented. The results are now:

  • Firefox fails the brand-checking test for name/message and the DOMException.prototype.toString() throwing test
  • Chrome fails the stack property test (which is a "should" in the proposed spec) and the toString() inheritance test
  • Safari fails the stack property test, the toString() inheritance tests, and seems to have made message/name/code own properties, like the current-as-of-now spec. But they did not implement any of the other non-interface-like behaviors.
  • Edge fails all the tests because they have no DOMException constructor.

@ghost
Copy link

ghost commented Jun 28, 2017 •
edited by ghost
Loading

View the complete job log.

Firefox (nightly)

Testing web-platform-tests at revision dd553b3
Using browser at version BuildID 20170612100241; SourceStamp 27cad9749cddf68e11fdd4e5d73dad84a8f8cf23
Starting 10 test iterations
All results were stable

All results

9 tests ran
/WebIDL/ecmascript-binding/es-exceptions/DOMException-constants.any.html
Subtest Results Messages
OK
Untitled PASS
Constant INDEX_SIZE_ERR on DOMException constructor object PASS
Constant INDEX_SIZE_ERR on DOMException prototype object PASS
Constant DOMSTRING_SIZE_ERR on DOMException constructor object PASS
Constant DOMSTRING_SIZE_ERR on DOMException prototype object PASS
Constant HIERARCHY_REQUEST_ERR on DOMException constructor object PASS
Constant HIERARCHY_REQUEST_ERR on DOMException prototype object PASS
Constant WRONG_DOCUMENT_ERR on DOMException constructor object PASS
Constant WRONG_DOCUMENT_ERR on DOMException prototype object PASS
Constant INVALID_CHARACTER_ERR on DOMException constructor object PASS
Constant INVALID_CHARACTER_ERR on DOMException prototype object PASS
Constant NO_DATA_ALLOWED_ERR on DOMException constructor object PASS
Constant NO_DATA_ALLOWED_ERR on DOMException prototype object PASS
Constant NO_MODIFICATION_ALLOWED_ERR on DOMException constructor object PASS
Constant NO_MODIFICATION_ALLOWED_ERR on DOMException prototype object PASS
Constant NOT_FOUND_ERR on DOMException constructor object PASS
Constant NOT_FOUND_ERR on DOMException prototype object PASS
Constant NOT_SUPPORTED_ERR on DOMException constructor object PASS
Constant NOT_SUPPORTED_ERR on DOMException prototype object PASS
Constant INUSE_ATTRIBUTE_ERR on DOMException constructor object PASS
Constant INUSE_ATTRIBUTE_ERR on DOMException prototype object PASS
Constant INVALID_STATE_ERR on DOMException constructor object PASS
Constant INVALID_STATE_ERR on DOMException prototype object PASS
Constant SYNTAX_ERR on DOMException constructor object PASS
Constant SYNTAX_ERR on DOMException prototype object PASS
Constant INVALID_MODIFICATION_ERR on DOMException constructor object PASS
Constant INVALID_MODIFICATION_ERR on DOMException prototype object PASS
Constant NAMESPACE_ERR on DOMException constructor object PASS
Constant NAMESPACE_ERR on DOMException prototype object PASS
Constant INVALID_ACCESS_ERR on DOMException constructor object PASS
Constant INVALID_ACCESS_ERR on DOMException prototype object PASS
Constant VALIDATION_ERR on DOMException constructor object PASS
Constant VALIDATION_ERR on DOMException prototype object PASS
Constant TYPE_MISMATCH_ERR on DOMException constructor object PASS
Constant TYPE_MISMATCH_ERR on DOMException prototype object PASS
Constant SECURITY_ERR on DOMException constructor object PASS
Constant SECURITY_ERR on DOMException prototype object PASS
Constant NETWORK_ERR on DOMException constructor object PASS
Constant NETWORK_ERR on DOMException prototype object PASS
Constant ABORT_ERR on DOMException constructor object PASS
Constant ABORT_ERR on DOMException prototype object PASS
Constant URL_MISMATCH_ERR on DOMException constructor object PASS
Constant URL_MISMATCH_ERR on DOMException prototype object PASS
Constant QUOTA_EXCEEDED_ERR on DOMException constructor object PASS
Constant QUOTA_EXCEEDED_ERR on DOMException prototype object PASS
Constant TIMEOUT_ERR on DOMException constructor object PASS
Constant TIMEOUT_ERR on DOMException prototype object PASS
Constant INVALID_NODE_TYPE_ERR on DOMException constructor object PASS
Constant INVALID_NODE_TYPE_ERR on DOMException prototype object PASS
Constant DATA_CLONE_ERR on DOMException constructor object PASS
Constant DATA_CLONE_ERR on DOMException prototype object PASS
/WebIDL/ecmascript-binding/es-exceptions/DOMException-constants.any.worker.html
Subtest Results Messages
OK
Untitled PASS
Constant INDEX_SIZE_ERR on DOMException constructor object PASS
Constant INDEX_SIZE_ERR on DOMException prototype object PASS
Constant DOMSTRING_SIZE_ERR on DOMException constructor object PASS
Constant DOMSTRING_SIZE_ERR on DOMException prototype object PASS
Constant HIERARCHY_REQUEST_ERR on DOMException constructor object PASS
Constant HIERARCHY_REQUEST_ERR on DOMException prototype object PASS
Constant WRONG_DOCUMENT_ERR on DOMException constructor object PASS
Constant WRONG_DOCUMENT_ERR on DOMException prototype object PASS
Constant INVALID_CHARACTER_ERR on DOMException constructor object PASS
Constant INVALID_CHARACTER_ERR on DOMException prototype object PASS
Constant NO_DATA_ALLOWED_ERR on DOMException constructor object PASS
Constant NO_DATA_ALLOWED_ERR on DOMException prototype object PASS
Constant NO_MODIFICATION_ALLOWED_ERR on DOMException constructor object PASS
Constant NO_MODIFICATION_ALLOWED_ERR on DOMException prototype object PASS
Constant NOT_FOUND_ERR on DOMException constructor object PASS
Constant NOT_FOUND_ERR on DOMException prototype object PASS
Constant NOT_SUPPORTED_ERR on DOMException constructor object PASS
Constant NOT_SUPPORTED_ERR on DOMException prototype object PASS
Constant INUSE_ATTRIBUTE_ERR on DOMException constructor object PASS
Constant INUSE_ATTRIBUTE_ERR on DOMException prototype object PASS
Constant INVALID_STATE_ERR on DOMException constructor object PASS
Constant INVALID_STATE_ERR on DOMException prototype object PASS
Constant SYNTAX_ERR on DOMException constructor object PASS
Constant SYNTAX_ERR on DOMException prototype object PASS
Constant INVALID_MODIFICATION_ERR on DOMException constructor object PASS
Constant INVALID_MODIFICATION_ERR on DOMException prototype object PASS
Constant NAMESPACE_ERR on DOMException constructor object PASS
Constant NAMESPACE_ERR on DOMException prototype object PASS
Constant INVALID_ACCESS_ERR on DOMException constructor object PASS
Constant INVALID_ACCESS_ERR on DOMException prototype object PASS
Constant VALIDATION_ERR on DOMException constructor object PASS
Constant VALIDATION_ERR on DOMException prototype object PASS
Constant TYPE_MISMATCH_ERR on DOMException constructor object PASS
Constant TYPE_MISMATCH_ERR on DOMException prototype object PASS
Constant SECURITY_ERR on DOMException constructor object PASS
Constant SECURITY_ERR on DOMException prototype object PASS
Constant NETWORK_ERR on DOMException constructor object PASS
Constant NETWORK_ERR on DOMException prototype object PASS
Constant ABORT_ERR on DOMException constructor object PASS
Constant ABORT_ERR on DOMException prototype object PASS
Constant URL_MISMATCH_ERR on DOMException constructor object PASS
Constant URL_MISMATCH_ERR on DOMException prototype object PASS
Constant QUOTA_EXCEEDED_ERR on DOMException constructor object PASS
Constant QUOTA_EXCEEDED_ERR on DOMException prototype object PASS
Constant TIMEOUT_ERR on DOMException constructor object PASS
Constant TIMEOUT_ERR on DOMException prototype object PASS
Constant INVALID_NODE_TYPE_ERR on DOMException constructor object PASS
Constant INVALID_NODE_TYPE_ERR on DOMException prototype object PASS
Constant DATA_CLONE_ERR on DOMException constructor object PASS
Constant DATA_CLONE_ERR on DOMException prototype object PASS
/WebIDL/ecmascript-binding/es-exceptions/DOMException-constructor-and-prototype.any.worker.html
Subtest Results Messages
OK
existence and property descriptor of DOMException PASS
existence and property descriptor of DOMException.prototype PASS
existence and property descriptor of DOMException.prototype.constructor PASS
/WebIDL/ecmascript-binding/es-exceptions/DOMException-constructor-and-prototype.any.html
Subtest Results Messages
OK
existence and property descriptor of DOMException PASS
existence and property descriptor of DOMException.prototype PASS
existence and property descriptor of DOMException.prototype.constructor PASS
/WebIDL/ecmascript-binding/es-exceptions/DOMException-constructor-behavior.any.worker.html
Subtest Results Messages
OK
new DOMException() PASS
new DOMException(): inherited-ness PASS
new DOMException(null) PASS
new DOMException(undefined) PASS
new DOMException(undefined): inherited-ness PASS
new DOMException("foo") PASS
new DOMException("foo"): inherited-ness PASS
new DOMException("bar", undefined) PASS
new DOMException("bar", "NotSupportedError") PASS
new DOMException("bar", "NotSupportedError"): inherited-ness PASS
new DOMException("bar", "foo") PASS
new DOMexception("msg", "IndexSizeError") PASS
new DOMexception("msg", "HierarchyRequestError") PASS
new DOMexception("msg", "WrongDocumentError") PASS
new DOMexception("msg", "InvalidCharacterError") PASS
new DOMexception("msg", "NoModificationAllowedError") PASS
new DOMexception("msg", "NotFoundError") PASS
new DOMexception("msg", "NotSupportedError") PASS
new DOMexception("msg", "InUseAttributeError") PASS
new DOMexception("msg", "InvalidStateError") PASS
new DOMexception("msg", "SyntaxError") PASS
new DOMexception("msg", "InvalidModificationError") PASS
new DOMexception("msg", "NamespaceError") PASS
new DOMexception("msg", "InvalidAccessError") PASS
new DOMexception("msg", "SecurityError") PASS
new DOMexception("msg", "NetworkError") PASS
new DOMexception("msg", "AbortError") PASS
new DOMexception("msg", "URLMismatchError") PASS
new DOMexception("msg", "QuotaExceededError") PASS
new DOMexception("msg", "TimeoutError") PASS
new DOMexception("msg", "InvalidNodeTypeError") PASS
new DOMexception("msg", "DataCloneError") PASS
new DOMException("bar", "UnknownError") PASS
/WebIDL/ecmascript-binding/es-exceptions/DOMException-constructor-behavior.any.html
Subtest Results Messages
OK
new DOMException() PASS
new DOMException(): inherited-ness PASS
new DOMException(null) PASS
new DOMException(undefined) PASS
new DOMException(undefined): inherited-ness PASS
new DOMException("foo") PASS
new DOMException("foo"): inherited-ness PASS
new DOMException("bar", undefined) PASS
new DOMException("bar", "NotSupportedError") PASS
new DOMException("bar", "NotSupportedError"): inherited-ness PASS
new DOMException("bar", "foo") PASS
new DOMexception("msg", "IndexSizeError") PASS
new DOMexception("msg", "HierarchyRequestError") PASS
new DOMexception("msg", "WrongDocumentError") PASS
new DOMexception("msg", "InvalidCharacterError") PASS
new DOMexception("msg", "NoModificationAllowedError") PASS
new DOMexception("msg", "NotFoundError") PASS
new DOMexception("msg", "NotSupportedError") PASS
new DOMexception("msg", "InUseAttributeError") PASS
new DOMexception("msg", "InvalidStateError") PASS
new DOMexception("msg", "SyntaxError") PASS
new DOMexception("msg", "InvalidModificationError") PASS
new DOMexception("msg", "NamespaceError") PASS
new DOMexception("msg", "InvalidAccessError") PASS
new DOMexception("msg", "SecurityError") PASS
new DOMexception("msg", "NetworkError") PASS
new DOMexception("msg", "AbortError") PASS
new DOMexception("msg", "URLMismatchError") PASS
new DOMexception("msg", "QuotaExceededError") PASS
new DOMexception("msg", "TimeoutError") PASS
new DOMexception("msg", "InvalidNodeTypeError") PASS
new DOMexception("msg", "DataCloneError") PASS
new DOMException("bar", "UnknownError") PASS
/WebIDL/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.html
Subtest Results Messages
OK
Cannot construct without new PASS
inherits from Error: prototype-side PASS
does not inherit from Error: class-side PASS
message property descriptor PASS
message getter performs brand checks (i.e. is not [LenientThis] FAIL assert_throws: function "() => getter.apply({})" did not throw
name property descriptor PASS
name getter performs brand checks (i.e. is not [LenientThis] FAIL assert_throws: function "() => getter.apply({})" did not throw
code property descriptor PASS
code getter performs brand checks (i.e. is not [LenientThis] PASS
code property is not affected by shadowing the name property PASS
Object.prototype.toString behavior is like other interfaces PASS
Inherits its toString() from Error.prototype PASS
toString() behavior from Error.prototype applies as expected PASS
DOMException.prototype.toString() applied to DOMException.prototype throws because of name/message brand checks FAIL assert_throws: function "() => DOMException.prototype.toString()" did not throw
If the implementation has a stack property on normal errors, it also does on DOMExceptions PASS
/WebIDL/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.worker.html
Subtest Results Messages
OK
Cannot construct without new PASS
inherits from Error: prototype-side PASS
does not inherit from Error: class-side PASS
message property descriptor PASS
message getter performs brand checks (i.e. is not [LenientThis] FAIL assert_throws: function "() => getter.apply({})" did not throw
name property descriptor PASS
name getter performs brand checks (i.e. is not [LenientThis] FAIL assert_throws: function "() => getter.apply({})" did not throw
code property descriptor PASS
code getter performs brand checks (i.e. is not [LenientThis] PASS
code property is not affected by shadowing the name property PASS
Object.prototype.toString behavior is like other interfaces PASS
Inherits its toString() from Error.prototype PASS
toString() behavior from Error.prototype applies as expected PASS
DOMException.prototype.toString() applied to DOMException.prototype throws because of name/message brand checks FAIL assert_throws: function "() => DOMException.prototype.toString()" did not throw
If the implementation has a stack property on normal errors, it also does on DOMExceptions PASS
/WebIDL/ecmascript-binding/es-exceptions/exceptions.html
Subtest Results Messages
OK
Object.getPrototypeOf(exception) === DOMException.prototype PASS
exception.hasOwnProperty("name") PASS
exception.hasOwnProperty("message") PASS
exception.name === "HierarchyRequestError" PASS
exception.code === DOMException.HIERARCHY_REQUEST_ERR PASS
Object.prototype.toString.call(exception) === "[object DOMException]" PASS
In iframe: Object.getPrototypeOf(exception) === DOMException.prototype PASS
In iframe: exception.hasOwnProperty("name") PASS
In iframe: exception.hasOwnProperty("message") PASS
In iframe: exception.name === "HierarchyRequestError" PASS
In iframe: exception.code === DOMException.HIERARCHY_REQUEST_ERR PASS
In iframe: Object.prototype.toString.call(exception) === "[object DOMException]" PASS

@ghost
Copy link

ghost commented Jun 28, 2017 •
edited by ghost
Loading

View the complete job log.

Sauce (safari)

Testing web-platform-tests at revision dd553b3
Using browser at version 10.0
Starting 10 test iterations
No tests run.

@ghost
Copy link

ghost commented Jun 28, 2017 •
edited by ghost
Loading

View the complete job log.

Chrome (unstable)

Testing web-platform-tests at revision 8031e53
Using browser at version 61.0.3141.7 dev
Starting 10 test iterations
No tests run.

@ghost
Copy link

ghost commented Jun 28, 2017 •
edited by ghost
Loading

View the complete job log.

Sauce (MicrosoftEdge)

Testing web-platform-tests at revision 8031e53
Using browser at version 14.14393
Starting 10 test iterations
No tests run.

@domenic domenic mentioned this pull request Jun 28, 2017
Merged
@sideshowbarker
Copy link
Contributor

w3c-test:mirror

@rakuco
Copy link
Member

rakuco commented Jun 29, 2017

Chrome fails the stack property test (which is a "should" in the proposed spec) and the toString() inheritance test

The toString() part is easy to fix, but the stack property test needs some work in V8, which currently registers it in %Error% instead of %ErrorPrototype% (I'm assuming we do want to move towards the latter given https://tc39.github.io/proposal-error-stacks/)

@rakuco
Copy link
Member

rakuco commented Jun 29, 2017

Don't you also need to update some of the other tests?

  • constructor-object.js is checking if DOMException inherits from %Error% and that name and code are not set in DOMException.prototype.
  • exceptions.html is checking for the wrong property descriptors and in the wrong location.

@domenic
Copy link
Member Author

domenic commented Jun 29, 2017

Great catches, thanks. Let me update. Also I should probably make these tests .any.js tests.

@foolip
Copy link
Member

foolip commented Jul 4, 2017

@bobholt, is the "No tests run" for 3/4 browsers in this PR a known issue? If not, can you file one with the infra label?

@foolip foolip mentioned this pull request Jul 4, 2017
Merged
@domenic
Copy link
Member Author

domenic commented Jul 4, 2017

@rakuco since the spec PR is merged, this is ready for review, when you have time :)

@rakuco
Copy link
Member

rakuco commented Jul 5, 2017

Sorry, I didn't know you were waiting for me. The change looks fine on my side, and Chromium passes all the new tests (modulo those you'd mentioned before). One of the 3 reviewers need to lgtm the PR though.

@bobholt
Copy link
Contributor

bobholt commented Jul 5, 2017

@foolip This PR addressed an issue that caused only Firefox to return results. That's my guess about what went on here. I've restarted the build to see if that fixed this particular issue.

@rakuco
Copy link
Member

rakuco commented Jul 6, 2017

@tobie @yuki3 @jensl can any of you take a look and approve the PR?

yuki3
yuki3 reviewed Jul 6, 2017
View reviewed changes
WebIDL/ecmascript-binding/es-exceptions/DOMException-constructor-behavior.any.js Outdated
@@ -135,5 +127,3 @@
assert_equals(ex.code, 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test case ("bar", "UnknownError") a dupe of line 81 ("bar", "foo")?

yuki3
yuki3 reviewed Jul 6, 2017
View reviewed changes
WebIDL/ecmascript-binding/es-exceptions/DOMException-custom-bindings.any.js Outdated

const propDesc = Object.getOwnPropertyDescriptor(DOMException.prototype, "message");
assert_equals(typeof propDesc.get, "function", "property descriptor is a getter");
assert_equals(propDesc.set, undefined, "property descriptor is not a getter");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean "not a setter"?
Ditto for all belows.

rakuco
rakuco reviewed Jul 6, 2017
View reviewed changes
Copy link
Member

@rakuco rakuco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er, it turns out I can actually approve the change, sorry for the brain fart :-) Yuki's question makes sense though, so not doing it just yet.

yuki3
yuki3 approved these changes Jul 6, 2017
View reviewed changes
Copy link
Member

@yuki3 yuki3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits.

@domenic domenic merged commit c080c71 into master Jul 6, 2017
@domenic domenic deleted the domexception branch July 6, 2017 20:58
scheib pushed a commit to scheib/chromium that referenced this pull request Jul 12, 2017
Remove toString() override from DOMException.
be24cfe 
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}
@rakuco rakuco mentioned this pull request Jul 14, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuki3 yuki3 yuki3 approved these changes

@jgraham jgraham jgraham approved these changes

@rakuco rakuco rakuco approved these changes

@tobie tobie Awaiting requested review from tobie

@jensl jensl Awaiting requested review from jensl

Assignees
No one assigned
Labels
status:needs-spec-decision webidl wg-webplatform
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@domenic @sideshowbarker @rakuco @foolip @bobholt @jgraham @yuki3 @wpt-pr-bot