Bug 78318 – Make ShadowRoot DoublyLinkedList-able.

• Status: RESOLVED
• Resolution: WONTFIX
• Component: DOM

[Hayato Ito]

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.

[Hayato Ito]

Created attachment 126437 [details]
make ShadowRoot DoublyLinkedList-able

[Dominic Cooney]

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]

(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]

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]

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]

As discussed with shinyak, ShadowRoot's setHost(element) calls setParent(). So this patch should work as is.

[Hajime Morrita]

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]

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]

(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]

bug 78069, which merges this change, was landed.