Add function to check what HTML Element interface corresponds to a given tag name.

Add function to check what HTML Element interface corresponds to a given tag name.

HTMLElementTypeHelpers.h (generated) now has an enum, HTMLElementType, for each HTML element interface in htmltagnames.in
(Not generated for SVG elements)

New function htmlElementTypeForTag returns HTMLElementType
	Tag names that are undefined return HTMLUnknownElement

New ElementTypeHelpers.cpp.tmpl for HTML namespace with definition of function
Generates HTMLElementTypeHelpers.cpp

This function is needed in custom elements algorithms.

BUG=

Committed: https://crrev.com/631706583e06a2b9209cc5a6c6ce9d776da910dd
Cr-Commit-Position: refs/heads/master@{#427045}

[yurak, 2016-10-17 05:17:24]

Description was changed from

==========
Add function to check what HTML Element interface corresponds to a given tag
name.
Undefined tag names return HTMLUnknownElement.

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

to

==========
Add function to check what HTML Element interface corresponds to a given tag
name.

HTMLElementTypeHelpers.h (generated) now has an enum, HTMLElementType, for each
HTML element interface in htmltagnames.in
(Not generated for SVG elements)

New function htmlElementTypeForTag returns HTMLElementType
	Tag names that are undefined return HTMLUnknownElement

New ElementTypeHelpers.cpp.tmpl for HTML namespace with definition of function
Generates HTMLElementTypeHelpers.cpp

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

[yurak, 2016-10-17 05:17:24]

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

[yurak, 2016-10-19 01:58:18]

Description was changed from

==========
Add function to check what HTML Element interface corresponds to a given tag
name.

HTMLElementTypeHelpers.h (generated) now has an enum, HTMLElementType, for each
HTML element interface in htmltagnames.in
(Not generated for SVG elements)

New function htmlElementTypeForTag returns HTMLElementType
	Tag names that are undefined return HTMLUnknownElement

New ElementTypeHelpers.cpp.tmpl for HTML namespace with definition of function
Generates HTMLElementTypeHelpers.cpp

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

to

==========
Add function to check what HTML Element interface corresponds to a given tag
name.

HTMLElementTypeHelpers.h (generated) now has an enum, HTMLElementType, for each
HTML element interface in htmltagnames.in
(Not generated for SVG elements)

New function htmlElementTypeForTag returns HTMLElementType
	Tag names that are undefined return HTMLUnknownElement

New ElementTypeHelpers.cpp.tmpl for HTML namespace with definition of function
Generates HTMLElementTypeHelpers.cpp

This function is needed in the custom elements algorithm.

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

[yurak, 2016-10-19 01:58:33]

Description was changed from

==========
Add function to check what HTML Element interface corresponds to a given tag
name.

HTMLElementTypeHelpers.h (generated) now has an enum, HTMLElementType, for each
HTML element interface in htmltagnames.in
(Not generated for SVG elements)

New function htmlElementTypeForTag returns HTMLElementType
	Tag names that are undefined return HTMLUnknownElement

New ElementTypeHelpers.cpp.tmpl for HTML namespace with definition of function
Generates HTMLElementTypeHelpers.cpp

This function is needed in the custom elements algorithm.

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

to

==========
Add function to check what HTML Element interface corresponds to a given tag
name.

HTMLElementTypeHelpers.h (generated) now has an enum, HTMLElementType, for each
HTML element interface in htmltagnames.in
(Not generated for SVG elements)

New function htmlElementTypeForTag returns HTMLElementType
	Tag names that are undefined return HTMLUnknownElement

New ElementTypeHelpers.cpp.tmpl for HTML namespace with definition of function
Generates HTMLElementTypeHelpers.cpp

This function is needed in custom elements algorithms.

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

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

This is great!

Could you include whether the interface is RuntimeEnabled and have the generator
check RuntimeEnabledFeatures when it initializes the map? I think
HTMLSlotElement and one other are runtime enabled in this way.

[yurak, 2016-10-21 07:36:50]

On 2016/10/21 at 02:31:40, dominicc wrote:
> This is great!
> 
> Could you include whether the interface is RuntimeEnabled and have the
generator check RuntimeEnabledFeatures when it initializes the map? I think
HTMLSlotElement and one other are runtime enabled in this way.

I added a check in the function. The file is generate by Jinja/Python and I'm
not sure how check if a feature is enabled from there.

PTL

[yurak, 2016-10-24 02:17:25]

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

[dominicc (has gone to gerrit), 2016-10-24 02:54:10]

On 2016/10/21 at 07:36:50, yurak wrote:
> On 2016/10/21 at 02:31:40, dominicc wrote:
> > This is great!
> > 
> > Could you include whether the interface is RuntimeEnabled and have the
generator check RuntimeEnabledFeatures when it initializes the map? I think
HTMLSlotElement and one other are runtime enabled in this way.
> 
> I added a check in the function. The file is generate by Jinja/Python and I'm
not sure how check if a feature is enabled from there.

You need to check in the function somehow, because the features are enabled at
runtime, so you don't know for sure at generation time if the tag will be
enabled.

Putting all of the name checks up front could be slow if we have a lot of
runtime enabled tag names but I think there's only one or two so that's fine.

LGTM, ask tkent to review BUILD, etc. changes.

[tkent, 2016-10-24 03:22:32]

https://codereview.chromium.org/2419383002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl
(right):

https://codereview.chromium.org/2419383002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl:17:
ASSERT(!html_type_map);
Please use two-space indentation, and use DCHECK instead of ASSERT.

[yurak, 2016-10-24 03:41:06]

On 2016/10/24 at 03:22:32, tkent wrote:
>
https://codereview.chromium.org/2419383002/diff/40001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl
(right):
> 
>
https://codereview.chromium.org/2419383002/diff/40001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl:17:
ASSERT(!html_type_map);
> Please use two-space indentation, and use DCHECK instead of ASSERT.

Made changes, PTL.

[tkent, 2016-10-24 04:04:43]

https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl
(right):

https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl:12:
typedef HashMap<AtomicString, HTMLElementType> HTMLTypeMap;
nit: We prefer 'using' to 'typedef'.

using HTMLTypeMap = ashMap<AtomicString, HTMLElementType>;

https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl:37:
}
nit: it should be:

    } else {

https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl
(right):

https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl:42:
{% for element in elements|sort %}        {{element}},
Are the enum members look like HTMLFooElemnet?
If so, let's prepend 'k' like kHTMLFooElement.  We are switching enum naming
scheme, and we prepend 'k' in the new scheme.

https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl:47:
HTMLElementType htmlElementTypeForTag(const AtomicString& tagName);
|tagName| is unclear.  Is it a local name?  Should it be lower-cased or
upper-cased?  Do you think it's possible to call this function with non-HTML tag
names like <svg:a> ?

[yurak, 2016-10-24 05:24:20]

On 2016/10/24 at 04:04:43, tkent wrote:
>
https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl
(right):
> 
>
https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl:12:
typedef HashMap<AtomicString, HTMLElementType> HTMLTypeMap;
> nit: We prefer 'using' to 'typedef'.
> 
> using HTMLTypeMap = ashMap<AtomicString, HTMLElementType>;
> 
>
https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.cpp.tmpl:37:
}
> nit: it should be:
> 
>     } else {
> 
>
https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl
(right):
> 
>
https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl:42:
{% for element in elements|sort %}        {{element}},
> Are the enum members look like HTMLFooElemnet?
> If so, let's prepend 'k' like kHTMLFooElement.  We are switching enum naming
scheme, and we prepend 'k' in the new scheme.
> 
>
https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl:47:
HTMLElementType htmlElementTypeForTag(const AtomicString& tagName);
> |tagName| is unclear.  Is it a local name?  Should it be lower-cased or
upper-cased?  Do you think it's possible to call this function with non-HTML tag
names like <svg:a> ?

Yes, tagName is a localName. The enum and function are only generated in
HTMLElementTypeHelpers.h/.cpp. If the function was called with a non-HTML tag
name it will return ::kHTMLUnknownElement. I don't think there is a need for
this function with SVG elements, am I wrong?

Made changes, PTL.

[tkent, 2016-10-24 05:58:13]

On 2016/10/24 at 05:24:20, yurak wrote:
> >
https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
> >
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl:47:
HTMLElementType htmlElementTypeForTag(const AtomicString& tagName);
> > |tagName| is unclear.  Is it a local name?  Should it be lower-cased or
upper-cased?  Do you think it's possible to call this function with non-HTML tag
names like <svg:a> ?
> 
> Yes, tagName is a localName. The enum and function are only generated in
HTMLElementTypeHelpers.h/.cpp. If the function was called with a non-HTML tag
name it will return ::kHTMLUnknownElement. I don't think there is a need for
this function with SVG elements, am I wrong?

The code is still unclear.  Please add a comment to ElementTypeHelpers.h.tmpl
about the details of |tagName|. Lowercase or uppercase? A caller of the function
must ensure tagName is an HTML tag name, right?

[yurak, 2016-10-24 06:35:18]

On 2016/10/24 at 05:58:13, tkent wrote:
> On 2016/10/24 at 05:24:20, yurak wrote:
> > >
https://codereview.chromium.org/2419383002/diff/60001/third_party/WebKit/Sour...
> > >
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl:47:
HTMLElementType htmlElementTypeForTag(const AtomicString& tagName);
> > > |tagName| is unclear.  Is it a local name?  Should it be lower-cased or
upper-cased?  Do you think it's possible to call this function with non-HTML tag
names like <svg:a> ?
> > 
> > Yes, tagName is a localName. The enum and function are only generated in
HTMLElementTypeHelpers.h/.cpp. If the function was called with a non-HTML tag
name it will return ::kHTMLUnknownElement. I don't think there is a need for
this function with SVG elements, am I wrong?
> 
> The code is still unclear.  Please add a comment to ElementTypeHelpers.h.tmpl
about the details of |tagName|. Lowercase or uppercase? A caller of the function
must ensure tagName is an HTML tag name, right?

Sorry, I didn't realize you were asking for comments in code--I added some. Hope
that helps, PTL.

[tkent, 2016-10-24 06:40:36]

https://codereview.chromium.org/2419383002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl
(right):

https://codereview.chromium.org/2419383002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl:48:
// If tagNAme is undefined html local name it's assumed to be HTMLUnknownElement
It's not enough.

Both of HTML and SVG have 'a' element, Blink developers need to be careful to
avoid to use this function against SVG elements.  Please note it.

[yurak, 2016-10-24 07:16:12]

On 2016/10/24 at 06:40:36, tkent wrote:
>
https://codereview.chromium.org/2419383002/diff/100001/third_party/WebKit/Sou...
> File
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl
(right):
> 
>
https://codereview.chromium.org/2419383002/diff/100001/third_party/WebKit/Sou...
>
third_party/WebKit/Source/build/scripts/templates/ElementTypeHelpers.h.tmpl:48:
// If tagNAme is undefined html local name it's assumed to be HTMLUnknownElement
> It's not enough.
> 
> Both of HTML and SVG have 'a' element, Blink developers need to be careful to
avoid to use this function against SVG elements.  Please note it.

I made a note for developers not to use SVG elements. PTL.

[tkent, 2016-10-24 07:26:55]

lgtm

[tkent, 2016-10-24 07:37:21]

The CQ bit was checked by tkent@chromium.org

[tkent, 2016-10-24 07:37:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominicc@chromium.org
Link to the patchset: https://codereview.chromium.org/2419383002/#ps140001
(title: "Made changes")

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

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-24 09:32:13]

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

[commit-bot: I haz the power, 2016-10-24 09:32:14]

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

[tkent, 2016-10-24 09:42:20]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2016-10-24 09:42:33]

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-24 11:14:26]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-10-24 11:18:36]

Description was changed from

==========
Add function to check what HTML Element interface corresponds to a given tag
name.

HTMLElementTypeHelpers.h (generated) now has an enum, HTMLElementType, for each
HTML element interface in htmltagnames.in
(Not generated for SVG elements)

New function htmlElementTypeForTag returns HTMLElementType
	Tag names that are undefined return HTMLUnknownElement

New ElementTypeHelpers.cpp.tmpl for HTML namespace with definition of function
Generates HTMLElementTypeHelpers.cpp

This function is needed in custom elements algorithms.

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

to

==========
Add function to check what HTML Element interface corresponds to a given tag
name.

HTMLElementTypeHelpers.h (generated) now has an enum, HTMLElementType, for each
HTML element interface in htmltagnames.in
(Not generated for SVG elements)

New function htmlElementTypeForTag returns HTMLElementType
	Tag names that are undefined return HTMLUnknownElement

New ElementTypeHelpers.cpp.tmpl for HTML namespace with definition of function
Generates HTMLElementTypeHelpers.cpp

This function is needed in custom elements algorithms.

BUG=

Committed: https://crrev.com/631706583e06a2b9209cc5a6c6ce9d776da910dd
Cr-Commit-Position: refs/heads/master@{#427045}
==========

[commit-bot: I haz the power, 2016-10-24 11:18:37]

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