Bug 78318 - Make ShadowRoot DoublyLinkedList-able.
Summary: Make ShadowRoot DoublyLinkedList-able.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks: 76433 78069
  Show dependency treegraph
 
Reported: 2012-02-09 19:43 PST by Hayato Ito
Modified: 2012-02-14 09:47 PST (History)
5 users (show)

See Also:


Attachments
make ShadowRoot DoublyLinkedList-able (2.78 KB, patch)
2012-02-09 19:47 PST, Hayato Ito
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 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.
Comment 1 Hayato Ito 2012-02-09 19:47:42 PST
Created attachment 126437 [details]
make ShadowRoot DoublyLinkedList-able
Comment 2 Dominic Cooney 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?
Comment 3 Hayato Ito 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.
Comment 4 Hayato Ito 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.
Comment 5 Hayato Ito 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.
Comment 6 Hayato Ito 2012-02-09 22:12:53 PST
As discussed with shinyak, ShadowRoot's setHost(element) calls setParent(). So this patch should work as is.
Comment 7 Hajime Morrita 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.
Comment 8 Hayato Ito 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.
Comment 9 Shinya Kawanaka 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.
Comment 10 Hayato Ito 2012-02-12 20:30:13 PST
bug 78069, which merges this change, was landed.