Bug 78778 – [Refactoring] Use HTMLInsertionPoint instead of HTMLContentElement

• Component: DOM
• Status: RESOLVED FIXED

[Shinya Kawanaka]

We had assumed shadow insertion point was only HTMLContentElement before.
So a lot of code use HTMLContentElement* type instead of HTMLInsertionPoint type.
But we now have <shadow> element, which is another insertion point.
We should convert HTMLContentElement* type to HTMLInsertionPoint* in NodeRenderingContext and so on.

[Shinya Kawanaka]

Created attachment 127775 [details]
Test

[Hayato Ito]

Created attachment 127796 [details]
use InsertionPoint

[Hajime Morrita]

Comment on attachment 127796 [details]
use InsertionPoint

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

> Source/WebCore/html/shadow/HTMLContentElement.h:NaN
>  public:

It looks we no longer need some #include.

> Source/WebCore/html/shadow/InsertionPoint.cpp:43
> +    , m_selections(adoptPtr(new HTMLContentSelectionList()))

If we instantiate here and have a #include, there would be no reason to allocate this in heap.
I guess the original class lose the point at some point.

[Hayato Ito]

Created attachment 127881 [details]
update

[Hayato Ito]

(In reply to comment #3)
> (From update of attachment 127796 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127796&action=review
> 
> > Source/WebCore/html/shadow/HTMLContentElement.h:NaN
> >  public:
> 
> It looks we no longer need some #include.

Done. I've cleaned up #include.

> 
> > Source/WebCore/html/shadow/InsertionPoint.cpp:43
> > +    , m_selections(adoptPtr(new HTMLContentSelectionList()))
> 
> If we instantiate here and have a #include, there would be no reason to allocate this in heap.
> I guess the original class lose the point at some point.

Done.

[Hajime Morrita]

Comment on attachment 127881 [details]
update

Let's make sure that win bot green. It looks unrelated to this patch though.

[Shinya Kawanaka]

(In reply to comment #6)
> (From update of attachment 127881 [details])
> Let's make sure that win bot green. It looks unrelated to this patch though.

Unfortunately, win bot is failing these days i(In reply to comment #6)
> (From update of attachment 127881 [details])
> Let's make sure that win bot green. It looks unrelated to this patch though.

This is an error message. 

6>c:\cygwin\home\buildbot\WebKit\Source\WebCore\html\shadow\HTMLContentElement.cpp(100) : error C2819: type 'WebCore::HTMLContentSelectionList' does not have an overloaded member 'operator ->'
6>        c:\cygwin\home\buildbot\webkit\source\webcore\html\shadow\HTMLContentSelector.h(77) : see declaration of 'WebCore::HTMLContentSelectionList'
6>        did you intend to use '.' instead?
6>c:\cygwin\home\buildbot\WebKit\Source\WebCore\html\shadow\HTMLContentElement.cpp(100) : error C2232: '->WebCore::HTMLContentSelectionList::isEmpty' : left operand has 'class' type, use '.'

[Hayato Ito]

Created attachment 127907 [details]
Patch for landing

[Hayato Ito]

This patch is related. I've fixed the debug build.

(In reply to comment #6)
> (From update of attachment 127881 [details])
> Let's make sure that win bot green. It looks unrelated to this patch though.

[WebKit Review Bot]

The commit-queue encountered the following flaky tests while processing attachment 127907 [details]:

perf/object-keys.html bug 63769 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Review Bot]

Comment on attachment 127907 [details]
Patch for landing

Clearing flags on attachment: 127907

Committed r108303: <http://trac.webkit.org/changeset/108303>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.