Bug 137056 – Add initial is<>() / downcast<>() support for any type of Nodes

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

Add initial is<>() / downcast<>() support for any type of Nodes, not just Elements by:
- Moving the is<>() / downcast<>() declaration from Element.h to Node.h
- Introducing a DEFINE_TYPE_HELPERS_*() macro that generates the needed template specializations for is<>() / downcast<>() to work. This macro will replace NODE_TYPE_CASTS() entirely once the code base is fully ported.

[Chris Dumez]

Created attachment 238584 [details]
WIP Patch

[WebKit Commit Bot]

Attachment 238584 [details] did not pass style-queue:


ERROR: Source/WebCore/html/HTMLFormControlElement.h:195:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 238585 [details]
Patch

[WebKit Commit Bot]

Attachment 238585 [details] did not pass style-queue:


ERROR: Source/WebCore/html/HTMLFormControlElement.h:195:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 238585 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238585&action=review

> Source/WebCore/dom/DocumentFragment.h:55
> +DEFINE_TYPE_HELPERS_BEGIN(DocumentFragment)

The reason the macro has a BEGIN / END is so that isDocumentFragment() can be private. We don't want to have both isDocumentFragment() and is<DocumentFragment>().

> Source/WebCore/dom/Node.h:742
> +template <typename ExpectedType, typename ArgType>

This is all moved from Element.h

> Source/WebCore/html/HTMLMediaElement.h:918
> +struct NodeTypeCastTraits<const HTMLMediaElement, ArgType> {

These explicit specializations will go away once I use DEFINE_TYPE_HELPERS_*() everywhere. I only ported 2 classes in this patch to minimize the size of the change.

[Benjamin Poulain]

Comment on attachment 238585 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238585&action=review

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:-742
>      if (isHTMLTextAreaElement(node))
> -        return toHTMLFormControlElement(node)->isReadOnly();

Uh, that's interesting...

>> Source/WebCore/dom/DocumentFragment.h:55
>> +DEFINE_TYPE_HELPERS_BEGIN(DocumentFragment)
> 
> The reason the macro has a BEGIN / END is so that isDocumentFragment() can be private. We don't want to have both isDocumentFragment() and is<DocumentFragment>().

Interesting take to simplify the definition.

I am not sure with the name. "Helpers" is never really helpful for a name.

Why not simply SPECIALIZE_TYPE_TRAITS or something like that?

> Source/WebCore/dom/Node.h:784
> +// Add support for type checking / casting using is<>() / downcast<>() helpers
> +// for a specific class.

One line.

> Source/WebCore/html/HTMLFormControlElement.cpp:511
>          if (node->isElementNode() && toElement(node)->isFormControlElement())

Why not is<HTMLFormControlElement>(node) ?

> Source/WebCore/html/HTMLFormControlElement.h:195
> +    static bool isHTMLFormControlElement(const Node& node) { return node.isElementNode() && toElement(node).isFormControlElement(); }

return node.isElementNode() && isHTMLFormControlElement(toElement(node));

> Source/WebCore/html/HTMLFormElement.cpp:212
>      return 0;

nullptr

> Source/WebCore/html/HTMLFormElement.cpp:706
>      return 0;

nullptr

[Chris Dumez]

Created attachment 238636 [details]
Patch

[WebKit Commit Bot]

Attachment 238636 [details] did not pass style-queue:


ERROR: Source/WebCore/html/HTMLFormControlElement.h:195:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 238636 [details]
Patch

Clearing flags on attachment: 238636

Committed r173944: <http://trac.webkit.org/changeset/173944>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Daniel Bates]

(In reply to comment #9)
> (From update of attachment 238636 [details])
> Clearing flags on attachment: 238636
> 
> Committed r173944: <http://trac.webkit.org/changeset/173944>

This broke the iOS build. See <rdar://problem/18454708> for more details.

[Chris Dumez]

(In reply to comment #11)
> (In reply to comment #9)
> > (From update of attachment 238636 [details] [details])
> > Clearing flags on attachment: 238636
> > 
> > Committed r173944: <http://trac.webkit.org/changeset/173944>
> 
> This broke the iOS build. See <rdar://problem/18454708> for more details.

This should be fixed in http://trac.webkit.org/changeset/173971. Thanks for notifying me.