Bug 100921

Summary: [Shadow] ShadowRoot should be able to know the existence of <content>
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal CC: webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 100451    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Shinya Kawanaka
Reported 2012-10-31 23:57:38 PDT
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
Attachments
Patch (13.04 KB, patch)
2012-11-01 00:16 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch (14.06 KB, patch)
2012-11-01 19:15 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Patch for landing (13.89 KB, patch)
2012-11-02 22:57 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1 2012-11-01 00:02:08 PDT
We will be able to speed up ShadowRoot::hasInsertionPoint() maybe?
Shinya Kawanaka
Comment 2 2012-11-01 00:16:37 PDT
Created attachment 171786 [details] Patch
Hajime Morrita
Comment 3 2012-11-01 03:22:12 PDT
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 4 2012-11-01 18:32:12 PDT
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
Comment 5 2012-11-01 19:11:16 PDT
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
Comment 6 2012-11-01 19:13:00 PDT
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
Comment 7 2012-11-01 19:14:22 PDT
(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
Comment 8 2012-11-01 19:15:33 PDT
Created attachment 171972 [details] Patch
WebKit Review Bot
Comment 9 2012-11-02 10:21:24 PDT
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
Comment 10 2012-11-02 22:57:26 PDT
Created attachment 172209 [details] Patch for landing
WebKit Review Bot
Comment 11 2012-11-02 23:40:40 PDT
Comment on attachment 172209 [details] Patch for landing Clearing flags on attachment: 172209 Committed r133392: <http://trac.webkit.org/changeset/133392>
WebKit Review Bot
Comment 12 2012-11-02 23:40:44 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.