Introduce toHTMLSelectElement and remove isHTMLSelectElement

Introduce toHTMLSelectElement and remove isHTMLSelectElement

To avoid direct use of static_cast, this patch introduces toHTMLSelectElement for code cleanup.
Additionally, this patch removes isHTMLSelectElement because not all element subclasses can be checked by a combination of tag names.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=154365

[kangil_, 2013-07-17 01:42:11]

review please? :)

[dominicc (has gone to gerrit), 2013-07-17 01:47:05]

On 2013/07/17 01:42:11, kangil_ wrote:
> review please? :)

LGTM

[tkent, 2013-07-17 01:47:55]

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
File Source/core/html/HTMLSelectElement.h (left):

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
Source/core/html/HTMLSelectElement.h:220: inline const HTMLSelectElement*
toHTMLSelectElement(const Node* node)
Why do you remove this function?

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
Source/core/html/HTMLSelectElement.h:226: void toHTMLSelectElement(const
HTMLSelectElement*); // This overload will catch anyone doing an unnecessary
cast.
Ditto.

https://codereview.chromium.org/19488002/diff/1/Source/core/testing/Internals...
File Source/core/testing/Internals.cpp (left):

https://codereview.chromium.org/19488002/diff/1/Source/core/testing/Internals...
Source/core/testing/Internals.cpp:1990: if (!isHTMLSelectElement(node))
This isHTMLSelectElement usage is good because toHTMLSelectElement follows.  You
don't need to revert this.

https://codereview.chromium.org/19488002/diff/1/Source/web/WebSearchableFormD...
File Source/web/WebSearchableFormData.cpp (right):

https://codereview.chromium.org/19488002/diff/1/Source/web/WebSearchableFormD...
Source/web/WebSearchableFormData.cpp:146: } else if
(formElement->hasTagName(HTMLNames::selectTag)) {
We may use isHTMLSelectElement because toHTMLSelectELement follows.

[tkent, 2013-07-17 01:53:52]

I don't object the removal of isHTMLSelectElement. However the removal of
toHTMLSelectElement variants looks unreasonable.

[kangil_, 2013-07-17 02:16:21]

Hi Tamura!

I left some questions for clarity.
Would you please take a look at those comments?

Thx!! :)

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
File Source/core/html/HTMLSelectElement.h (left):

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
Source/core/html/HTMLSelectElement.h:220: inline const HTMLSelectElement*
toHTMLSelectElement(const Node* node)
On 2013/07/17 01:47:56, tkent wrote:
> Why do you remove this function?

Just because we don't use this at the moment.
But, I don't have strong opinion on this if you want to keep this as it is. :)

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
Source/core/html/HTMLSelectElement.h:226: void toHTMLSelectElement(const
HTMLSelectElement*); // This overload will catch anyone doing an unnecessary
cast.
On 2013/07/17 01:47:56, tkent wrote:
> Ditto.

I don't have strong opinion on this either, but should ask for clarity :)
  1. Why there isn't void toHTMLSelectElement(HTMLSelectElement*);
  2. Some HTMLFooElement have this function but others haven't. Do we need to
introduce same to others?

https://codereview.chromium.org/19488002/diff/1/Source/core/testing/Internals...
File Source/core/testing/Internals.cpp (left):

https://codereview.chromium.org/19488002/diff/1/Source/core/testing/Internals...
Source/core/testing/Internals.cpp:1990: if (!isHTMLSelectElement(node))
On 2013/07/17 01:47:56, tkent wrote:
> This isHTMLSelectElement usage is good because toHTMLSelectElement follows. 
You
> don't need to revert this.

I understood that we agree to remove isFooElement because not all elements are
presented by tag names. :)
So, I would like to remove usage of isFooElement here. :)

[kangil_, 2013-07-17 02:21:46]

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
File Source/core/html/HTMLSelectElement.h (left):

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
Source/core/html/HTMLSelectElement.h:226: void toHTMLSelectElement(const
HTMLSelectElement*); // This overload will catch anyone doing an unnecessary
cast.
On 2013/07/17 02:16:22, kangil_ wrote:
> On 2013/07/17 01:47:56, tkent wrote:
> > Ditto.
> 
> I don't have strong opinion on this either, but should ask for clarity :)
>   1. Why there isn't void toHTMLSelectElement(HTMLSelectElement*);

Ah, this is wrong question. Please ignore this. :(

>   2. Some HTMLFooElement have this function but others haven't. Do we need to
> introduce same to others?

[tkent, 2013-07-17 03:47:05]

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
File Source/core/html/HTMLSelectElement.h (left):

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
Source/core/html/HTMLSelectElement.h:220: inline const HTMLSelectElement*
toHTMLSelectElement(const Node* node)
On 2013/07/17 02:16:22, kangil_ wrote:
> On 2013/07/17 01:47:56, tkent wrote:
> > Why do you remove this function?
> 
> Just because we don't use this at the moment.
> But, I don't have strong opinion on this if you want to keep this as it is. :)

Hmm, it seems this function is actually unused.
I think it's ok to remove it because 'const' is rarely used for DOM nodes.

https://codereview.chromium.org/19488002/diff/1/Source/core/html/HTMLSelectEl...
Source/core/html/HTMLSelectElement.h:226: void toHTMLSelectElement(const
HTMLSelectElement*); // This overload will catch anyone doing an unnecessary
cast.
On 2013/07/17 02:16:22, kangil_ wrote:
> On 2013/07/17 01:47:56, tkent wrote:
> > Ditto.
> 
> I don't have strong opinion on this either, but should ask for clarity :)
>   1. Why there isn't void toHTMLSelectElement(HTMLSelectElement*);
>   2. Some HTMLFooElement have this function but others haven't. Do we need to
> introduce same to others?

Removing this function is not a goal of this CL, right? I don't think you should
remove it in this patch.
IMO, this function is not so helpful, but it sometimes helps to improve code
quality.  I have no opinion about 2.

[kangil_, 2013-07-17 04:15:01]

Done for comments, thx! :)
review please?

[tkent, 2013-07-17 04:32:15]

lgtm

[commit-bot: I haz the power, 2013-07-17 04:32:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kangil.han@samsung.com/19488002/9001

[commit-bot: I haz the power, 2013-07-17 05:20:31]

Change committed as 154365