Use new is*Element() helper functions more in HTML code

Use new is*Element() helper functions more in HTML code

Use new is*Element() helpers more in HTML code. This makes the code clearer and
simpler. Also use the new Traversal<*Element> API when suitable.

R=eseidel, adamk
BUG=346095
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169192

[philipj_slow, 2014-03-13 02:49:56]

Non-owner LGTM with enthusiasm, bike shed and request for education :)

https://codereview.chromium.org/198453003/diff/1/Source/core/html/MediaDocume...
File Source/core/html/MediaDocument.cpp (right):

https://codereview.chromium.org/198453003/diff/1/Source/core/html/MediaDocume...
Source/core/html/MediaDocument.cpp:144: HTMLVideoElement* video =
descendentVideoElement(*targetNode);
Just inline descendentVideoElement? Seems like it would be very readable with
the new Traversal<> style.

https://codereview.chromium.org/198453003/diff/1/Source/core/html/forms/Radio...
File Source/core/html/forms/RadioInputType.cpp (right):

https://codereview.chromium.org/198453003/diff/1/Source/core/html/forms/Radio...
Source/core/html/forms/RadioInputType.cpp:131: if
(isHTMLInputElement(currentFocusedElement)) {
isHTMLInputElement(0) returns false, I guess? I tried to find the answer in
ElementTypeHelpers.h.tmpl but was not immediately successful.

[philipj_slow, 2014-03-13 03:19:16]

Oh, I found the answer:

inline bool is{{tag.interface}}(const Node* node) { return node &&
node->isElementNode() ? is{{tag.interface}}(*toElement(node)) : false; }

[Inactive, 2014-03-13 04:35:54]

https://codereview.chromium.org/198453003/diff/1/Source/core/html/MediaDocume...
File Source/core/html/MediaDocument.cpp (right):

https://codereview.chromium.org/198453003/diff/1/Source/core/html/MediaDocume...
Source/core/html/MediaDocument.cpp:144: HTMLVideoElement* video =
descendentVideoElement(*targetNode);
On 2014/03/13 02:49:56, philipj wrote:
> Just inline descendentVideoElement? Seems like it would be very readable with
> the new Traversal<> style.

I hesitated :) I will do this then.

[Inactive, 2014-03-13 04:53:55]

https://codereview.chromium.org/198453003/diff/1/Source/core/html/MediaDocume...
File Source/core/html/MediaDocument.cpp (right):

https://codereview.chromium.org/198453003/diff/1/Source/core/html/MediaDocume...
Source/core/html/MediaDocument.cpp:144: HTMLVideoElement* video =
descendentVideoElement(*targetNode);
On 2014/03/13 02:49:56, philipj wrote:
> Just inline descendentVideoElement? Seems like it would be very readable with
> the new Traversal<> style.

Done.

[philipj_slow, 2014-03-13 09:25:33]

Yep, looks nice :)

[Inactive, 2014-03-13 22:38:16]

Rebased

[adamk, 2014-03-13 23:33:31]

lgtm

[Inactive, 2014-03-14 00:29:07]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-03-14 00:29:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/198453003/50001

[commit-bot: I haz the power, 2014-03-14 01:21:24]

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

[commit-bot: I haz the power, 2014-03-14 01:21:26]

Try jobs failed on following builders:
  tryserver.chromium on win_blink_rel

[Inactive, 2014-03-14 01:24:00]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-03-14 01:24:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/198453003/50001

[commit-bot: I haz the power, 2014-03-14 02:06:05]

Change committed as 169192

[tkent, 2014-03-14 02:19:11]

I feel using isHTMLFoo in HTML parser is semantically incorrect.
The purpose of isHTMLFoo is to check if an object is an instance of C++ HTMLFoo
class or an IDL interface HTMLFoo.  HTML parser shouldn't take care of C++
classes or IDL interface, and should handle just tag names.

[Inactive, 2014-03-14 02:48:33]

On 2014/03/14 02:19:11, tkent wrote:
> I feel using isHTMLFoo in HTML parser is semantically incorrect.
> The purpose of isHTMLFoo is to check if an object is an instance of C++
HTMLFoo
> class or an IDL interface HTMLFoo.  HTML parser shouldn't take care of C++
> classes or IDL interface, and should handle just tag names.

Your argument sounds reasonable to me. However, if you look at the specific
changes I made in the parser, most of them already call toHTML*Element(). I felt
it made sense to use isHTML*Element() before calling toHTML*Element() instead of
hasTagName(), doesn't it?
Note that the parser code still has a lot of hasTagName() calls because the code
is not dealing with HTMLElements but HTMLStackItems. Maybe I converted a bit too
much to using isHTML*Element() in the parser code? or do you really feel there
should be no isHTML*Element() call at all in parser code?

[Inactive, 2014-03-14 02:50:44]

https://codereview.chromium.org/198453003/diff/50001/Source/core/html/parser/...
File Source/core/html/parser/HTMLConstructionSite.cpp (right):

https://codereview.chromium.org/198453003/diff/50001/Source/core/html/parser/...
Source/core/html/parser/HTMLConstructionSite.cpp:98: if
(isHTMLTemplateElement(*task.parent))
for example here, I feel it makes sense to use isHTMLTemplateElement() before
calling to HTMLTemplateElement().

However, I have really no issue reverting my changes in parser/ in cases where
toHTML*Element() is not called afterwards.