Custom Elements: Complete HTMLConstructor Algorithm

Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS:
    HTMLDivElement.idl
    HTMLParagraphElement.idl

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

The changes should allow Layout test imported/wpt/custom-elements/htmlconstructor/newtarget.html to pass, but it's buggy.
Bug fix for test here: https://github.com/w3c/web-platform-tests/commit/ac12d9a15df23cf25cbfb904902dad2bd6dfdf6a

BUG=648828
Committed: https://crrev.com/e8d4a1b8a06a252894820686f1570f5943bb5947
Cr-Commit-Position: refs/heads/master@{#428286}

[yurak, 2016-10-18 02:56:30]

Description was changed from

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002.

Depends on Issue#2419383002.

Fix custom element define algorithm.

Update custom-elements tests and expected results. Tests are currently crashing.

BUG=
==========

to

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Fix custom element define algorithm.

Update custom-elements tests and expected results.

BUG=648828
==========

[yurak, 2016-10-18 02:56:30]

yurak@google.com changed reviewers:
+ dominicc@chromium.org

[yurak, 2016-10-19 06:29:18]

Description was changed from

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Fix custom element define algorithm.

Update custom-elements tests and expected results.

BUG=648828
==========

to

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Fix custom element define algorithm.

Added HTMLConstructor extended attribute to most HTML element IDL files.

Update custom-elements tests and expected results.

BUG=648828
==========

[yurak, 2016-10-20 01:50:53]

Description was changed from

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Fix custom element define algorithm.

Added HTMLConstructor extended attribute to most HTML element IDL files.

Update custom-elements tests and expected results.

BUG=648828
==========

to

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

BUG=648828
==========

[yurak, 2016-10-20 01:50:54]

yurak@google.com changed reviewers:
+ haraken@chromium.org

[yurak, 2016-10-20 02:06:59]

Description was changed from

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

BUG=648828
==========

to

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS
    HTMLEmbedElement, HTMLFormElement, and HTMLObjectElement crash at runtime
because of the [OverrideBuiltins] extended attribute

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

BUG=648828
==========

[dominicc (has gone to gerrit), 2016-10-21 02:30:07]

Excellent work; some feedback inline.

I didn't see any interfaces with Constructor, ConstructorCustom at a glance.

Could you add a test under LayoutTests/custom-elements which exercises this a
bit with one element? Define one, create it with new, access some properties and
call some methods, etc.

https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/imported/wpt/custom-elements/htmlconstructor/newtarget-expected.txt
(right):

https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/imported/wpt/custom-elements/htmlconstructor/newtarget-expected.txt:8:
FAIL If prototype is not object (null), derives the fallback from NewTarget's
realm (customized built-in elements) assert_equals: Must use the
HTMLParagraphElement from the realm of NewTarget expected (function) function
"function HTMLParagraphElement() { [native code] }" but got (object) object
"[object HTMLParagraphElement]"
It might be worth noting in the description that these particular tests are
buggy, maybe pointing to the github pull request to fix them.

https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8HTMLConstructor.cpp (right):

https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8HTMLConstructor.cpp:73:
v8::Local<v8::Function> htmlElement =
I think the spec is not referring to any particular realm when it makes that
test, so you should check whether wrapperTypeInfo !=
&V8HTMLElement::wrapperTypeInfo. This has the added benefit of being faster,
too.

https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8HTMLConstructor.cpp:77:
V8ThrowException::throwTypeError(isolate, "Illegal constructor");
You want to return at this point! The algorithm is done.

By the way, the "Illegal constructor" messages are so Blink keeps throwing
exceptions with the same message it did in the distant past. But by the time you
reach this point, all the uses are new and related to custom elements, and you
could use a descriptive error message. Same for line 83, etc.

It will be helpful to give developers hints about what went wrong in these
cases.

https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElementRegistry.cpp
(right):

https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElementRegistry.cpp:173: //
ElementDefinitionIsRunning destructor(m_elementDefinitionIsRunning);
I assume we still need this code?

https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMenuItemElement.idl (right):

https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMenuItemElement.idl:8:
RuntimeEnabled=ContextMenu,
Ah, this is an interesting case. Your HTML names lookup thing may have to behave
differently depending on whether an interface is enabled or not.

[yurak, 2016-10-21 04:23:48]

Description was changed from

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS
    HTMLEmbedElement, HTMLFormElement, and HTMLObjectElement crash at runtime
because of the [OverrideBuiltins] extended attribute

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

BUG=648828
==========

to

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS
    HTMLEmbedElement, HTMLFormElement, and HTMLObjectElement crash at runtime
because of the [OverrideBuiltins] extended attribute

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

BUG=648828, 649158
==========

[yurak, 2016-10-21 05:57:33]

On 2016/10/21 at 02:30:07, dominicc wrote:
>
https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/dom/custom/CustomElementRegistry.cpp
(right):
> 
>
https://codereview.chromium.org/2424693002/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/dom/custom/CustomElementRegistry.cpp:173: //
ElementDefinitionIsRunning destructor(m_elementDefinitionIsRunning);
> I assume we still need this code?

Yes, it's part of the algorithm. Adding that line of code causes some crashing
in tests, I'll tackle it later.

[yurak, 2016-10-21 06:10:43]

PTL

[dominicc (has gone to gerrit), 2016-10-24 03:03:57]

On 2016/10/21 at 06:10:43, yurak wrote:
> PTL

Looks pretty good.

There are some parts of the patch at https://codereview.chromium.org/2441943002
we'll want to merge, but whomever commits second can just resolve the conflicts.

What's the status of the other layout tests including
imported/wpt/custom-elements and webkit_unit_tests --gtest_filter=CustomElement*
? This looks like it is nearly ready to land.

[yurak, 2016-10-24 03:57:29]

On 2016/10/24 at 03:03:57, dominicc wrote:
> On 2016/10/21 at 06:10:43, yurak wrote:
> > PTL
> 
> Looks pretty good.
> 
> There are some parts of the patch at
https://codereview.chromium.org/2441943002 we'll want to merge, but whomever
commits second can just resolve the conflicts.
> 
> What's the status of the other layout tests including
imported/wpt/custom-elements and webkit_unit_tests --gtest_filter=CustomElement*
? This looks like it is nearly ready to land.

All tests pass.

[dominicc (has gone to gerrit), 2016-10-25 04:30:17]

Some brief comments inline.

https://codereview.chromium.org/2424693002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/custom/CustomElementRegistry.cpp
(right):

https://codereview.chromium.org/2424693002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/custom/CustomElementRegistry.cpp:173: //
ElementDefinitionIsRunning destructor(m_elementDefinitionIsRunning);
Time to tackle this!

https://codereview.chromium.org/2424693002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8HTMLConstructor.cpp (right):

https://codereview.chromium.org/2424693002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8HTMLConstructor.cpp:58:
ScriptCustomElementDefinition* definition =
By the way, it is important that new HTMLButtonElement() continues to fail in
the same way ("Illegal constructor") before your change and after your change
*if the runtime feature is off.*

This bit of code is subtly what does that--if the runtime feature is not
enabled, then definition will always be null.

https://codereview.chromium.org/2424693002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8HTMLConstructor.cpp:108:
V8PerContextData* perContextData = V8PerContextData::from(context);
I think new need a null check here. If the target context has shut down then I
guess you should return undefined, I think there's a v8SetReturnValueUndefined
or something for that, and exit early.

https://codereview.chromium.org/2424693002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8HTMLConstructor.h (right):

https://codereview.chromium.org/2424693002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8HTMLConstructor.h:5: #include
"bindings/core/v8/DOMWrapperWorld.h"
You should be able to slim down this include list a ton to just:

- v8/include/v8.h for FunctionCallbackInfo and Value
- class WrapperTypeInfo; forward decl. for WrapperTypeInfo.
- ??? that helpers thing for HTMLElementType.

You may still need some of these includes in the implementation file, which is
fine, move them there.

Making the set of includes very minimal in the header file makes the build
faster because less needs to be processed for each compilation unit and makes it
more likely that people will include headers they need so it's easier to
refactor code later without dealing with lots of problems with missing includes.
(In this case you get HTMLElementType via something but it is not clear what,
for example, and if that thing stopped using and including HTMLElementType it
would cause compiling this definition to fail.)

[yurak, 2016-10-26 02:47:22]

Description was changed from

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS
    HTMLEmbedElement, HTMLFormElement, and HTMLObjectElement crash at runtime
because of the [OverrideBuiltins] extended attribute

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

BUG=648828, 649158
==========

to

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS:
    HTMLDivElement.idl
    HTMLParagraphElement.idl

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

The changes should allow Layout test
imported/wpt/custom-elements/htmlconstructor/newtarget.html to pass, but it's
buggy.

BUG=648828
==========

[yurak, 2016-10-26 02:47:23]

yurak@google.com changed reviewers:
+ tkent@chromium.org

[yurak, 2016-10-26 02:48:54]

Made suggested changes and resolved merge conflicts.

PTL :)

[haraken, 2016-10-26 08:14:55]

LGTM

[dominicc (has gone to gerrit), 2016-10-27 04:52:28]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2016-10-27 04:52:29]

lgtm

[commit-bot: I haz the power, 2016-10-27 04:52:48]

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-10-27 05:32:25]

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

[commit-bot: I haz the power, 2016-10-27 05:32:26]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[yurak, 2016-10-27 06:12:42]

The CQ bit was checked by yurak@google.com

[yurak, 2016-10-27 06:12:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dominicc@chromium.org
Link to the patchset: https://codereview.chromium.org/2424693002/#ps260001
(title: "Redefinition guard")

[commit-bot: I haz the power, 2016-10-27 06:13:05]

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-10-27 07:57:31]

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

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

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yurak, 2016-10-27 09:08:36]

Description was changed from

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS:
    HTMLDivElement.idl
    HTMLParagraphElement.idl

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

The changes should allow Layout test
imported/wpt/custom-elements/htmlconstructor/newtarget.html to pass, but it's
buggy.

BUG=648828
==========

to

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS:
    HTMLDivElement.idl
    HTMLParagraphElement.idl

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

The changes should allow Layout test
imported/wpt/custom-elements/htmlconstructor/newtarget.html to pass, but it's
buggy.
Bug fix for test here:
https://github.com/w3c/web-platform-tests/commit/ac12d9a15df23cf25cbfb904902d...

BUG=648828
==========

[yurak, 2016-10-28 03:42:08]

The CQ bit was checked by yurak@google.com

[commit-bot: I haz the power, 2016-10-28 03:42:25]

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-10-28 05:00:35]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2016-10-28 05:03:35]

Description was changed from

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS:
    HTMLDivElement.idl
    HTMLParagraphElement.idl

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

The changes should allow Layout test
imported/wpt/custom-elements/htmlconstructor/newtarget.html to pass, but it's
buggy.
Bug fix for test here:
https://github.com/w3c/web-platform-tests/commit/ac12d9a15df23cf25cbfb904902d...

BUG=648828
==========

to

==========
Complete HTMLConstructor algorithm, based off Issue#2386563002 by davaajav.

Depends on Issue#2419383002.

Completed HTMLConstructor algorithm, modified custom element definition
algorithm
    /bindings/core/v8/V8HTMLConstructor.cpp
    /core/dom/custom/CustomElementRegistry.cpp

Added HTMLConstructor extended attribute to HTML element IDLS:
    HTMLDivElement.idl
    HTMLParagraphElement.idl

Created separate class for HTMLConstructor
    /bindings/bindings.gni
    /bindings/core/v8/V8HTMLConstructor.cpp
    /bindings/core/v8/V8HTMLConstructor.h
    /bindings/scripts/v8_interface.py
    /bindings/templates/interface.cpp.tmpl
    /bindings/templates/interface.h.tmpl

Updated custom-elements tests and expected results.

The changes should allow Layout test
imported/wpt/custom-elements/htmlconstructor/newtarget.html to pass, but it's
buggy.
Bug fix for test here:
https://github.com/w3c/web-platform-tests/commit/ac12d9a15df23cf25cbfb904902d...

BUG=648828

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

[commit-bot: I haz the power, 2016-10-28 05:03:36]

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