calling whenDefined with invalid name should throw SyntaxError

calling whenDefined with invalid name should throw SyntaxError

Added test for customElements.whenDefined
Returns a promise rejected with a "SyntaxError" DOMException
if not given a valid custom element name.

BUG=594918
Committed: https://crrev.com/f84387506d8ac7d7993bca26d83385460f38d2e7
Cr-Commit-Position: refs/heads/master@{#404117}

[davaajav, 2016-06-28 00:36:42]

davaajav@google.com changed reviewers:
+ dominicc@chromium.org, dominicc@google.com

[davaajav, 2016-06-28 00:36:42]

Please take a look

[davaajav, 2016-06-28 02:41:54]

davaajav@google.com changed reviewers:
+ kojii@chromium.org
- dominicc@chromium.org, dominicc@google.com

[davaajav, 2016-06-28 02:41:54]

PTL

[kojii, 2016-06-28 04:26:00]

https://codereview.chromium.org/2100403002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/custom-elements/spec/when-defined-invalid-name.html
(right):

https://codereview.chromium.org/2100403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/when-defined-invalid-name.html:35:
return promise_rejects(t, new SyntaxError(),
w.customElements.whenDefined(name));
s/new SyntaxError()/'SYNTAX_ERR'/

"new SyntaxError()" is a JavaScript syntax error, not "SyntaxError"
DOMException.

[davaajav, 2016-06-28 05:08:11]

Replaced new SyntaxError() on line 35 with 'SYNTAX_ERR'

[davaajav, 2016-06-29 03:44:36]

PTL
Wrapped up things a bit. The commented out return statement on line 40 makes
assert_true on line 49 fail, thus making the whole test fail.

[kojii, 2016-06-29 03:57:41]

Isn't the change to custom-elements-helpers.js (to add "return") needed?

https://codereview.chromium.org/2100403002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/custom-elements/spec/when-defined-invalid-name.html
(right):

https://codereview.chromium.org/2100403002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/custom-elements/spec/when-defined-invalid-name.html:4:
<script src="../../resources/testharness-helpers.js"></script>
Please remove this, this file does not exist. I knew some other tests have this,
but they're just errors.

https://codereview.chromium.org/2100403002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/custom-elements/spec/when-defined-invalid-name.html:38:
console.log(name);
Remove this.

https://codereview.chromium.org/2100403002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/custom-elements/spec/when-defined-invalid-name.html:40:
// return promise_rejects_with_dom_exception_syntax_error(w, t,
Promise.reject(new SyntaxError));
Remove this.

[davaajav, 2016-06-29 04:05:34]

PTL

[kojii, 2016-06-29 04:33:25]

Isn't the change to custom-elements-helpers.js (to add "return") needed?

[davaajav, 2016-06-29 04:43:14]

On 2016/06/29 at 04:33:25, kojii wrote:
> Isn't the change to custom-elements-helpers.js (to add "return") needed?

Please note that we are only using create_window_in_test function from
custom-element-helpers.js .
The body of promise_test used in this test is similar to the body of
test_with_window, and instead of just calling
promise_rejects_with_dom_exception... as test_with_window would have done, we
are actually returning promise_rejects_with_dom_exception.

[davaajav, 2016-06-29 04:45:37]

On 2016/06/29 at 04:43:14, davaajav wrote:
> On 2016/06/29 at 04:33:25, kojii wrote:
> > Isn't the change to custom-elements-helpers.js (to add "return") needed?
> 
> Please note that we are only using create_window_in_test function from
custom-element-helpers.js .
> The body of promise_test used in this test is similar to the body of
test_with_window, and instead of just calling
promise_rejects_with_dom_exception... as test_with_window would have done, we
are actually returning promise_rejects_with_dom_exception.

So I guess we dont have to change custom-element-helpers.js

[kojii, 2016-06-29 04:48:23]

On 2016/06/29 at 04:45:37, davaajav wrote:
> On 2016/06/29 at 04:43:14, davaajav wrote:
> > On 2016/06/29 at 04:33:25, kojii wrote:
> > > Isn't the change to custom-elements-helpers.js (to add "return") needed?
> > 
> > Please note that we are only using create_window_in_test function from
custom-element-helpers.js .
> > The body of promise_test used in this test is similar to the body of
test_with_window, and instead of just calling
promise_rejects_with_dom_exception... as test_with_window would have done, we
are actually returning promise_rejects_with_dom_exception.
> 
> So I guess we dont have to change custom-element-helpers.js

Ah I see, you fixed it locally.

Let's fix custom-element-helpers.js, it's just ours (dominicc wrote original)
and other tests might need the same fix.

Also for promise_rejects_with_dom_exception_syntax_error, can you file a github
issue and put a link in the comment?

[davaajav, 2016-07-06 07:44:57]

Description was changed from

==========
calling whenDefined with invalid name should throw SyntaxError

Supplementary test for customElements.whenDefined
Returns a promise rejected with a "SyntaxError" DOMException
if not given a valid custom element name.

BUG=594918
==========

to

==========
calling whenDefined with invalid name should throw SyntaxError

Added test for customElements.whenDefined
Returns a promise rejected with a "SyntaxError" DOMException
if not given a valid custom element name.

BUG=594918
==========

[davaajav, 2016-07-06 07:46:57]

PTL

[kojii, 2016-07-07 07:48:03]

The CQ bit was checked by kojii@chromium.org

[kojii, 2016-07-07 07:48:03]

lgtm

[commit-bot: I haz the power, 2016-07-07 07:48:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 08:15:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-07 08:15:34]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[kojii, 2016-07-07 08:23:31]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2016-07-07 08:23:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 10:25:57]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-07-07 10:26:08]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-07 10:27:38]

Description was changed from

==========
calling whenDefined with invalid name should throw SyntaxError

Added test for customElements.whenDefined
Returns a promise rejected with a "SyntaxError" DOMException
if not given a valid custom element name.

BUG=594918
==========

to

==========
calling whenDefined with invalid name should throw SyntaxError

Added test for customElements.whenDefined
Returns a promise rejected with a "SyntaxError" DOMException
if not given a valid custom element name.

BUG=594918

Committed: https://crrev.com/f84387506d8ac7d7993bca26d83385460f38d2e7
Cr-Commit-Position: refs/heads/master@{#404117}
==========

[commit-bot: I haz the power, 2016-07-07 10:27:39]

Patchset 8 (id:??) landed as
https://crrev.com/f84387506d8ac7d7993bca26d83385460f38d2e7
Cr-Commit-Position: refs/heads/master@{#404117}

[yosin_UTC9, 2016-09-21 09:42:30]

yosin@chromium.org changed reviewers:
+ yosin@chromium.org

[yosin_UTC9, 2016-09-21 09:42:31]

Tools for simple code:

String template: `literal ${expression}`
Lambda short cut: (x) => { return foo(); } == x => foo()
No-ambiguos unicode literal: \u{...}

https://codereview.chromium.org/2100403002/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html
(right):

https://codereview.chromium.org/2100403002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html:65:
let invalid_names = [
nit: s/let/const/

https://codereview.chromium.org/2100403002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html:82:
'bad-\ufffe', 'bad-\uffff', 'bad-' + String.fromCodePoint(0xf0000)
You can write as 'bad-\u{F0000}'.

https://codereview.chromium.org/2100403002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html:89:
return promise_rejects_with_dom_exception_syntax_error(w, t,
w.customElements.whenDefined(name));
You can write as below:
.then(w => promise_rejects_with_dom_exception_syntax_error(w, t,
w.customElements.whenDefined(name))

https://codereview.chromium.org/2100403002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html:91:
}, 'whenDefined() called with invalid name ' + name + ' should throw
"SyntaxError"DOMException');
You can write as below:
`whenDefined() called with invalid name '${name} should throw
"SyntaxError"DOMException'

String template, using backquote instead of single/double quote, is easier to
read.