[Shadow] ShadowRoot should be able to know the existence of <content>

[Shinya Kawanaka]

We would like to check whether we have to invalidate distribution when element attribute is changed.
If we know the non-existence of <content>, we can skip traversing the descendants of ShadowRoot to check invalidation necessity.

Larger context is: https://bugs.webkit.org/show_bug.cgi?id=100451

[Shinya Kawanaka]

We will be able to speed up ShadowRoot::hasInsertionPoint() maybe?

[Shinya Kawanaka]

Created attachment 171786 [details]
Patch

[Hajime Morrita]

Comment on attachment 171786 [details]
Patch

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

> Source/WebCore/html/shadow/HTMLContentElement.cpp:104
> +        if (ShadowRoot* root = shadowRoot()) {

Can this be null?

> Source/WebCore/html/shadow/HTMLContentElement.cpp:118
> +            root = insertionPoint->shadowRoot();

I don't have confident but this seems wrong. 
insertionPoint can come from outside the shadow subtree. 
Coud you add tests to cover nested shadow subtree case?

[Shinya Kawanaka]

Comment on attachment 171786 [details]
Patch

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

>> Source/WebCore/html/shadow/HTMLContentElement.cpp:118
>> +            root = insertionPoint->shadowRoot();
> 
> I don't have confident but this seems wrong. 
> insertionPoint can come from outside the shadow subtree. 
> Coud you add tests to cover nested shadow subtree case?

Let me check. In that case, <shadow> might have the same bug.

[Shinya Kawanaka]

I've confirmed that insertionPoint can be from the outside of shadow tree.
But in that case, shadowRoot() returns non-null value.

I think this code works as expected, but it's confusing.

[Shinya Kawanaka]

I mean: when node is removed from ShadowTree, shadowRoot() return null, since we cannot reach ShadowRoot by moving parent-side. Otherwise, we can reach shadowRoot(), so it's OK.

[Shinya Kawanaka]

(In reply to comment #3)
> (From update of attachment 171786 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=171786&action=review
> 
> > Source/WebCore/html/shadow/HTMLContentElement.cpp:104
> > +        if (ShadowRoot* root = shadowRoot()) {
> 
> Can this be null?
> 

Since it's active, this should not be null. Thanks.

[Shinya Kawanaka]

Created attachment 171972 [details]
Patch

[WebKit Review Bot]

Comment on attachment 171972 [details]
Patch

Rejecting attachment 171972 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
webkit-commit-queue/Source/WebKit/chromium/webkit --revision 165669 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
51>At revision 165669.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/14670881

[Shinya Kawanaka]

Created attachment 172209 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 172209 [details]
Patch for landing

Clearing flags on attachment: 172209

Committed r133392: <http://trac.webkit.org/changeset/133392>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.