I'll upload a test case. This should not happen on release build.This happens only on debug build. Let me explain the detail later. We should revise current traversal APIs so that they can handle the case when ShadowRoot is passed directly as a parameter.
Created attachment 134542 [details] crash case
Let me summarize what I am thinking now: - Most of public APIs defined in ReifiedTreeTraversal is assuming that it does not take ShadowRoot as a parameter. I'd like to keep this rule since I don't want to break the following basic assumption: node.firstChild.parent == node (Please consider all traversing operation are done by ReifiedTreeTraversal APIs. I am using concise notation for laziness) If we allow some APIs can take ShadowRoot as parameter, this basic rule will be broken: shadowRoot.firstChild.parent == shadowHost of shadowRoot != shadowRoot But there are some use cases that we'd like to traverse a node, using ShadowRoot as a start node, like a FocusController. So I think we should change the behaviors as follows - If crossing upper boundary is allowed: - APIs must not take ShadowRoot as a parameter. - APIs won't return ShadowRoot as a return value. - If crossing upper boundary is disallowed: - APIs can take ShadowRoot as a parameter. -Some APIs (parentNode) may return youngest ShadowRoot as a return value. I think this can solve the issue. I'd like to implement this on Walker APIs, which will be a successor of current ReifiedTreeTraversal APIs.
(In reply to comment #2) > Let me summarize what I am thinking now: > > - Most of public APIs defined in ReifiedTreeTraversal is assuming that it does not take ShadowRoot as a parameter. I'd like to keep this rule since I don't want to break the following basic assumption: > > node.firstChild.parent == node > > (Please consider all traversing operation are done by ReifiedTreeTraversal APIs. I am using concise notation for laziness) > > If we allow some APIs can take ShadowRoot as parameter, this basic rule will be broken: > > shadowRoot.firstChild.parent == shadowHost of shadowRoot != shadowRoot > > But there are some use cases that we'd like to traverse a node, using ShadowRoot as a start node, like a FocusController. > So I think we should change the behaviors as follows > > - If crossing upper boundary is allowed: > - APIs must not take ShadowRoot as a parameter. > - APIs won't return ShadowRoot as a return value. > > - If crossing upper boundary is disallowed: > - APIs can take ShadowRoot as a parameter. > -Some APIs (parentNode) may return youngest ShadowRoot as a return value. I like it. This made me think some more about the walker API. What if the walker constructor took parameter(s) that define how traversal must happen? For example, ComposedShadowTreeWalker(node, AllowCrossingUpperBoundary, ...). Then, the walker functions could mirror Node traversal API. Will this work? WDYT? > I think this can solve the issue. > I'd like to implement this on Walker APIs, which will be a successor of current ReifiedTreeTraversal APIs.
(In reply to comment #3) > I like it. This made me think some more about the walker API. What if the walker constructor took parameter(s) that define how traversal must happen? For example, ComposedShadowTreeWalker(node, AllowCrossingUpperBoundary, ...). Then, the walker functions could mirror Node traversal API. Will this work? WDYT? That's the exactly same approach I had been taken on Walker. I am glad to hear that you have the same opinion by coincidence :). https://bugs.webkit.org/show_bug.cgi?id=82009 will be ready for a review in a few hours.
Created attachment 135518 [details] update a test
Comment on attachment 135518 [details] update a test Clearing flags on attachment: 135518 Committed r113275: <http://trac.webkit.org/changeset/113275>
All reviewed patches have been landed. Closing bug.