Bug 78318

Summary: Make ShadowRoot DoublyLinkedList-able.
Product: WebKit Reporter: Hayato Ito <hayato>
Component: DOMAssignee: Hayato Ito <hayato>
Status: RESOLVED WONTFIX    
Severity: Normal CC: dglazkov, dominicc, morrita, rolandsteiner, shinyak
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 76433, 78069    
Attachments:
Description Flags
make ShadowRoot DoublyLinkedList-able none

Hayato Ito
Reported 2012-02-09 19:43:15 PST
To support multiple ShadowRoots, we have to update ShadowRoot so that it can be a node of DoublyLinkedList. We need doubly since: - We have to traverse to youngerShadowRoot when we move back to the insertion point, <shadow>, from node which is direct child of older ShadowRoot. - We have to traverse to olderShadowRoot when we encounter <shadow> element. Upcoming changes require this change.
Attachments
make ShadowRoot DoublyLinkedList-able (2.78 KB, patch)
2012-02-09 19:47 PST, Hayato Ito 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Hayato Ito
Comment 1 2012-02-09 19:47:42 PST
Created attachment 126437 [details] make ShadowRoot DoublyLinkedList-able
Dominic Cooney
Comment 2 2012-02-09 21:09:03 PST
Comment on attachment 126437 [details] make ShadowRoot DoublyLinkedList-able View in context: https://bugs.webkit.org/attachment.cgi?id=126437&action=review > Source/WebCore/dom/ShadowRoot.h:82 > + ShadowRoot* m_prev; Is the intent that these are kept alive by being TreeShared?
Hayato Ito
Comment 3 2012-02-09 21:26:51 PST
(In reply to comment #2) > (From update of attachment 126437 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126437&action=review > > > Source/WebCore/dom/ShadowRoot.h:82 > > + ShadowRoot* m_prev; > > Is the intent that these are kept alive by being TreeShared? No. I intend that future owner of DoublyLinkedList<ShadowRoot> should delete every ShadowRoot in the list.
Hayato Ito
Comment 4 2012-02-09 21:35:02 PST
But there is a case that each ShadowRoot is exposed to JavaScript. Let us rethink lifecycle of each ShadowRoot. (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 126437 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=126437&action=review > > > > > Source/WebCore/dom/ShadowRoot.h:82 > > > + ShadowRoot* m_prev; > > > > Is the intent that these are kept alive by being TreeShared? > > No. I intend that future owner of DoublyLinkedList<ShadowRoot> should delete every ShadowRoot in the list.
Hayato Ito
Comment 5 2012-02-09 21:56:02 PST
I am sure that wtf/DoublyLInkedList is not suitable for this case since we must use RefPtr to hold next and prev ShadowRoot: RefPtr<ShadowRoot> m_prev But wtf/DoubleLinkedList forces us to use raw pointer. Let me rewrite this patch without wtf/DoublyLinkedList.
Hayato Ito
Comment 6 2012-02-09 22:12:53 PST
As discussed with shinyak, ShadowRoot's setHost(element) calls setParent(). So this patch should work as is.
Hajime Morrita
Comment 7 2012-02-09 22:37:58 PST
Comment on attachment 126437 [details] make ShadowRoot DoublyLinkedList-able Could you merge this patch with the real use? I cannot see how the lifetimes of multiple shadows are managed.
Hayato Ito
Comment 8 2012-02-09 22:54:54 PST
The real use will be coming in bug 78069. (In reply to comment #7) > (From update of attachment 126437 [details]) > Could you merge this patch with the real use? I cannot see how the lifetimes of multiple shadows are managed.
Shinya Kawanaka
Comment 9 2012-02-12 20:28:23 PST
(In reply to comment #8) > The real use will be coming in bug 78069. > > (In reply to comment #7) > > (From update of attachment 126437 [details] [details]) > > Could you merge this patch with the real use? I cannot see how the lifetimes of multiple shadows are managed. Let's make this bug INVALID.
Hayato Ito
Comment 10 2012-02-12 20:30:13 PST
bug 78069, which merges this change, was landed.
Note You need to log in before you can comment on or make changes to this bug.