Custom Elements: add prototype check for "failed" elements

Custom Elements: add prototype check for "failed" elements

There 2 possible ways to set an element`s state to "failed":
1. create the element for the token
   https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-token
2. upgrade the element
   https://html.spec.whatwg.org/#concept-upgrade-an-element
According to the spec, "failed" element should implement HTMLUnknownElement
only while it is being created for the token, not while it is being upgraded.

Added prototype sanity check when an element is created inside a template.

BUG=643509
Committed: https://crrev.com/44d4690c35729c1cfa0f6d78df1de3e5effdf871
Cr-Commit-Position: refs/heads/master@{#416886}

[davaajav, 2016-09-05 05:10:07]

Description was changed from

==========
CustomelElements: add prototype check for "failed" elements

There 2 possible ways to set an element`s state to "failed":
1. create the element for the token
  
https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-...
2. upgrade the element
   https://html.spec.whatwg.org/#concept-upgrade-an-element
According to spec, "failed" element should implement HTMLUnknownElement only
while
it is being created for the token, not while it is being upgraded.

Added prototype sanity check when an element is created inside a template.

BUG=643509
==========

to

==========
CustomelElements: add prototype check for "failed" elements

There 2 possible ways to set an element`s state to "failed":
1. create the element for the token
  
https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-...
2. upgrade the element
   https://html.spec.whatwg.org/#concept-upgrade-an-element
According to the spec, "failed" element should implement HTMLUnknownElement 
only while it is being created for the token, not while it is being upgraded.

Added prototype sanity check when an element is created inside a template.

BUG=643509
==========

[davaajav, 2016-09-05 05:10:52]

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

[davaajav, 2016-09-05 05:10:53]

PTL

[kojii, 2016-09-06 04:58:51]

https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html
(right):

https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html:21:
assert_equals(Object.getPrototypeOf(element), w.HTMLElement.prototype);
I'm not sure what is the good way to test "implement
HTMLElement/HTMLUnknownElement", but any reasons to use getProrotypeOf()? I
thought "instanceof" looks more reasonable, i.e.,
  assert_false(element instanceof w.HTMLUnknownElement);
but would like to know if there were any reasons.

[davaajav, 2016-09-06 05:32:03]

On 2016/09/06 at 04:58:51, kojii wrote:
>
https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
> File
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html
(right):
> 
>
https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
>
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html:21:
assert_equals(Object.getPrototypeOf(element), w.HTMLElement.prototype);
> I'm not sure what is the good way to test "implement
HTMLElement/HTMLUnknownElement", but any reasons to use getProrotypeOf()? I
thought "instanceof" looks more reasonable, i.e.,
>   assert_false(element instanceof w.HTMLUnknownElement);
> but would like to know if there were any reasons.

Since "HTMLUnknownElement.prototype instanceof HTMLElement" is true, if an
element e does implement HTMLUnknownElement, "e instanceof HTMLUnknownElement"
and "e instanceof HTMLElement" both give true.
I would like to know if I am missing something else.

[kojii, 2016-09-06 05:45:31]

On 2016/09/06 at 05:32:03, davaajav wrote:
> On 2016/09/06 at 04:58:51, kojii wrote:
> >
https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
> > File
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html
(right):
> > 
> >
https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
> >
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html:21:
assert_equals(Object.getPrototypeOf(element), w.HTMLElement.prototype);
> > I'm not sure what is the good way to test "implement
HTMLElement/HTMLUnknownElement", but any reasons to use getProrotypeOf()? I
thought "instanceof" looks more reasonable, i.e.,
> >   assert_false(element instanceof w.HTMLUnknownElement);
> > but would like to know if there were any reasons.
> 
> Since "HTMLUnknownElement.prototype instanceof HTMLElement" is true, if an
element e does implement HTMLUnknownElement, "e instanceof HTMLUnknownElement"
and "e instanceof HTMLElement" both give true.
> I would like to know if I am missing something else.

But you can test it is NOT an HTMLUnknownElement, right? If no strong reasons, I
prefer using instanceof, since comparing prototype checks "is", not
"implements".

[davaajav, 2016-09-06 05:47:04]

On 2016/09/06 at 05:45:31, kojii wrote:
> On 2016/09/06 at 05:32:03, davaajav wrote:
> > On 2016/09/06 at 04:58:51, kojii wrote:
> > >
https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
> > > File
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html
(right):
> > > 
> > >
https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
> > >
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html:21:
assert_equals(Object.getPrototypeOf(element), w.HTMLElement.prototype);
> > > I'm not sure what is the good way to test "implement
HTMLElement/HTMLUnknownElement", but any reasons to use getProrotypeOf()? I
thought "instanceof" looks more reasonable, i.e.,
> > >   assert_false(element instanceof w.HTMLUnknownElement);
> > > but would like to know if there were any reasons.
> > 
> > Since "HTMLUnknownElement.prototype instanceof HTMLElement" is true, if an
element e does implement HTMLUnknownElement, "e instanceof HTMLUnknownElement"
and "e instanceof HTMLElement" both give true.
> > I would like to know if I am missing something else.
> 
> But you can test it is NOT an HTMLUnknownElement, right? If no strong reasons,
I prefer using instanceof, since comparing prototype checks "is", not
"implements".

I see, I will change it in create-element-inside-template.html

[davaajav, 2016-09-06 08:00:58]

On 2016/09/06 at 05:47:04, davaajav wrote:
> On 2016/09/06 at 05:45:31, kojii wrote:
> > On 2016/09/06 at 05:32:03, davaajav wrote:
> > > On 2016/09/06 at 04:58:51, kojii wrote:
> > > >
https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
> > > > File
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html
(right):
> > > > 
> > > >
https://codereview.chromium.org/2313573002/diff/1/third_party/WebKit/LayoutTe...
> > > >
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html:21:
assert_equals(Object.getPrototypeOf(element), w.HTMLElement.prototype);
> > > > I'm not sure what is the good way to test "implement
HTMLElement/HTMLUnknownElement", but any reasons to use getProrotypeOf()? I
thought "instanceof" looks more reasonable, i.e.,
> > > >   assert_false(element instanceof w.HTMLUnknownElement);
> > > > but would like to know if there were any reasons.
> > > 
> > > Since "HTMLUnknownElement.prototype instanceof HTMLElement" is true, if an
element e does implement HTMLUnknownElement, "e instanceof HTMLUnknownElement"
and "e instanceof HTMLElement" both give true.
> > > I would like to know if I am missing something else.
> > 
> > But you can test it is NOT an HTMLUnknownElement, right? If no strong
reasons, I prefer using instanceof, since comparing prototype checks "is", not
"implements".
> 
> I see, I will change it in create-element-inside-template.html

I have changed it to "instanceof".

After looking at the spec again, I am having second thoughts on whether we need
to check the prototype of an element inside a template. What do you think?

[kojii, 2016-09-06 08:10:55]

> After looking at the spec again, I am having second thoughts on whether we
need to check the prototype of an element inside a template. What do you think?

Isn't that create-element-inside-template.html already does?

https://codereview.chromium.org/2313573002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html
(right):

https://codereview.chromium.org/2313573002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html:19:
console.log(element + '');
Please remove this one too.

https://codereview.chromium.org/2313573002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/custom-elements/spec/state-failed-upgrade.html
(right):

https://codereview.chromium.org/2313573002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/custom-elements/spec/state-failed-upgrade.html:41:
assert_equals(Object.getPrototypeOf(element), w.HTMLElement.prototype);
Replace this one to "instanceof" too?

[dominicc (has gone to gerrit), 2016-09-06 16:54:07]

There's a typo in the description, elElement.

https://codereview.chromium.org/2313573002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/custom-elements/spec/state-failed-upgrade.html
(right):

https://codereview.chromium.org/2313573002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/custom-elements/spec/state-failed-upgrade.html:41:
assert_equals(Object.getPrototypeOf(element), w.HTMLElement.prototype);
On 2016/09/06 at 08:10:55, kojii wrote:
> Replace this one to "instanceof" too?

I think the assert_equals checks are better because they're more precise; it
means we're returning something whose prototype is exactly HTMLUnknownElement?

instanceof HTMLElement in particular won't work because HTMLUnknownElement and
HTMLElement are both instanceof HTMLElement.

[kojii, 2016-09-07 02:33:19]

On 2016/09/06 at 16:54:07, dominicc wrote:
> 
> I think the assert_equals checks are better because they're more precise; it
means we're returning something whose prototype is exactly HTMLUnknownElement?

Ah, ok, sorry for the trouble, please follow dominicc@ and revert the change.

[davaajav, 2016-09-07 05:22:52]

PTL

[kojii, 2016-09-07 06:46:32]

The CQ bit was checked by kojii@chromium.org

[kojii, 2016-09-07 06:46:33]

lgtm

[commit-bot: I haz the power, 2016-09-07 06:46:56]

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-09-07 09:22:20]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-07 09:24:03]

Description was changed from

==========
CustomelElements: add prototype check for "failed" elements

There 2 possible ways to set an element`s state to "failed":
1. create the element for the token
  
https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-...
2. upgrade the element
   https://html.spec.whatwg.org/#concept-upgrade-an-element
According to the spec, "failed" element should implement HTMLUnknownElement 
only while it is being created for the token, not while it is being upgraded.

Added prototype sanity check when an element is created inside a template.

BUG=643509
==========

to

==========
CustomelElements: add prototype check for "failed" elements

There 2 possible ways to set an element`s state to "failed":
1. create the element for the token
  
https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-...
2. upgrade the element
   https://html.spec.whatwg.org/#concept-upgrade-an-element
According to the spec, "failed" element should implement HTMLUnknownElement
only while it is being created for the token, not while it is being upgraded.

Added prototype sanity check when an element is created inside a template.

BUG=643509

Committed: https://crrev.com/44d4690c35729c1cfa0f6d78df1de3e5effdf871
Cr-Commit-Position: refs/heads/master@{#416886}
==========

[commit-bot: I haz the power, 2016-09-07 09:24:04]

Patchset 6 (id:??) landed as
https://crrev.com/44d4690c35729c1cfa0f6d78df1de3e5effdf871
Cr-Commit-Position: refs/heads/master@{#416886}

[davaajav, 2016-09-08 01:59:55]

Description was changed from

==========
CustomelElements: add prototype check for "failed" elements

There 2 possible ways to set an element`s state to "failed":
1. create the element for the token
  
https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-...
2. upgrade the element
   https://html.spec.whatwg.org/#concept-upgrade-an-element
According to the spec, "failed" element should implement HTMLUnknownElement
only while it is being created for the token, not while it is being upgraded.

Added prototype sanity check when an element is created inside a template.

BUG=643509

Committed: https://crrev.com/44d4690c35729c1cfa0f6d78df1de3e5effdf871
Cr-Commit-Position: refs/heads/master@{#416886}
==========

to

==========
Custom Elements: add prototype check for "failed" elements

There 2 possible ways to set an element`s state to "failed":
1. create the element for the token
  
https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-...
2. upgrade the element
   https://html.spec.whatwg.org/#concept-upgrade-an-element
According to the spec, "failed" element should implement HTMLUnknownElement
only while it is being created for the token, not while it is being upgraded.

Added prototype sanity check when an element is created inside a template.

BUG=643509

Committed: https://crrev.com/44d4690c35729c1cfa0f6d78df1de3e5effdf871
Cr-Commit-Position: refs/heads/master@{#416886}
==========