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.
Created attachment 126437 [details] make ShadowRoot DoublyLinkedList-able
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?
(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.
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.
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.
As discussed with shinyak, ShadowRoot's setHost(element) calls setParent(). So this patch should work as is.
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.
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.
(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.
bug 78069, which merges this change, was landed.