ComposedTreeIterator may traverse slotted nodes multiple times
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Mar 2016 09:29:12 +0000 (09:29 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Mar 2016 09:29:12 +0000 (09:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154983

Reviewed by Ryosuke Niwa.

Source/WebCore:

Traversal of slotted nodes can escape to real siblings. Those siblings are then traversed again as slotted nodes.

Test: fast/shadow-dom/composed-tree-slots.html

* dom/ComposedTreeIterator.cpp:
(WebCore::ComposedTreeIterator::initializeContextStack):
(WebCore::ComposedTreeIterator::traverseNextInShadowTree):
(WebCore::ComposedTreeIterator::traverseNextLeavingContext):
(WebCore::ComposedTreeIterator::advanceInSlot):
* dom/ComposedTreeIterator.h:
(WebCore::ComposedTreeIterator::Context::Context):

    Include end iterator to the context.
    For slotted nodes set it up to point to the next sibling of the node.

(WebCore::ComposedTreeIterator::context):
(WebCore::ComposedTreeIterator::traverseNextSkippingChildren):

LayoutTests:

* fast/shadow-dom/composed-tree-slots-expected.txt: Added.
* fast/shadow-dom/composed-tree-slots.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@197553 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/composed-tree-slots-expected.txt [new file with mode: 0644]
LayoutTests/fast/shadow-dom/composed-tree-slots.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/ComposedTreeIterator.cpp
Source/WebCore/dom/ComposedTreeIterator.h

index ce83de6..4b921c9 100644 (file)
@@ -1,3 +1,13 @@
+2016-03-03  Antti Koivisto  <antti@apple.com>
+
+        ComposedTreeIterator may traverse slotted nodes multiple times
+        https://bugs.webkit.org/show_bug.cgi?id=154983
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/shadow-dom/composed-tree-slots-expected.txt: Added.
+        * fast/shadow-dom/composed-tree-slots.html: Added.
+
 2016-03-03  Filip Pizlo  <fpizlo@apple.com>
 
         DFG/FTL should inline accesses to RegExpObject::m_lastIndex
diff --git a/LayoutTests/fast/shadow-dom/composed-tree-slots-expected.txt b/LayoutTests/fast/shadow-dom/composed-tree-slots-expected.txt
new file mode 100644 (file)
index 0000000..cef617e
--- /dev/null
@@ -0,0 +1,123 @@
+
+Test 1
+  div (shadow root)
+    slot
+
+Test 2
+  div (shadow root)
+    slot
+      #text
+
+Test 3
+  div (shadow root)
+    slot
+      #text
+      div
+
+Test 4
+  div (shadow root)
+    slot
+      div
+        #text
+      div
+        #text
+
+Test 5
+  div (shadow root)
+    div
+      slot
+    #text
+
+Test 6
+  div (shadow root)
+    div
+      slot
+        #text
+    #text
+
+Test 7
+  div (shadow root)
+    div
+      slot
+        #text
+        div
+    #text
+
+Test 8
+  div (shadow root)
+    div
+      slot
+        div
+          #text
+        div
+          #text
+    #text
+
+Test 9
+  div (shadow root)
+    slot
+      slot-default
+        #text
+
+Test 10
+  div (shadow root)
+    slot
+      #text
+
+Test 11
+  div (shadow root)
+    slot
+      #text
+      div
+
+Test 12
+  div (shadow root)
+    slot
+      div
+        #text
+      div
+        #text
+
+Test 13
+  div (shadow root)
+    div (shadow root)
+      div
+        slot
+          #text
+          slot
+          #text
+
+Test 14
+  div (shadow root)
+    div (shadow root)
+      div
+        slot
+          #text
+          slot
+            #text
+          #text
+
+Test 15
+  div (shadow root)
+    div (shadow root)
+      div
+        slot
+          #text
+          slot
+            #text
+            div
+          #text
+
+Test 16
+  div (shadow root)
+    div (shadow root)
+      div
+        slot
+          #text
+          slot
+            div
+              #text
+            div
+              #text
+          #text
+
diff --git a/LayoutTests/fast/shadow-dom/composed-tree-slots.html b/LayoutTests/fast/shadow-dom/composed-tree-slots.html
new file mode 100644 (file)
index 0000000..7a5fd3f
--- /dev/null
@@ -0,0 +1,60 @@
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+
+<template id=shadow1><slot></slot></template>
+<template id=shadow2><div><slot></slot></div>text</template>
+<template id=shadow3><slot><slot-default>text</slot-default></slot></div></template>
+<template id=shadow4><div shadow=shadow41>text<slot></slot>text</div></template>
+<template id=shadow41><div><slot></slot></div></template>
+
+<template test=1><div shadow=shadow1></div></template>
+<template test=2><div shadow=shadow1>text</div></template>
+<template test=3><div shadow=shadow1>text<div></div></div></template>
+<template test=4><div shadow=shadow1><div>text</div><div>text</div></div></template>
+
+<template test=5><div shadow=shadow2></div></template>
+<template test=6><div shadow=shadow2>text</div></template>
+<template test=7><div shadow=shadow2>text<div></div></div></template>
+<template test=8><div shadow=shadow2><div>text</div><div>text</div></div></template>
+
+<template test=9><div shadow=shadow3></div></template>
+<template test=10><div shadow=shadow3>text</div></template>
+<template test=11><div shadow=shadow3>text<div></div></div></template>
+<template test=12><div shadow=shadow3><div>text</div><div>text</div></div></template>
+
+<template test=13><div shadow=shadow4></div></template>
+<template test=14><div shadow=shadow4>text</div></template>
+<template test=15><div shadow=shadow4>text<div></div></div></template>
+<template test=16><div shadow=shadow4><div>text</div><div>text</div></div></template>
+
+<body>
+<pre id=console></pre>
+<script>
+function installShadows(tree)
+{
+    var shadowHosts = tree.querySelectorAll("[shadow]");
+    for (var i = 0; i < shadowHosts.length; ++i) {
+        var shadowId = shadowHosts[i].getAttribute("shadow");
+        var shadowContents = document.querySelector("#"+shadowId).content.cloneNode(true);
+
+        installShadows(shadowContents);
+
+        var shadowRoot = shadowHosts[i].attachShadow({ mode: "open" });
+        shadowRoot.appendChild(shadowContents);
+    }
+}
+
+var console = document.querySelector("#console");
+
+var tests = document.querySelectorAll("[test]");
+for (var i = 0; i < tests.length; ++i) {
+    var test = tests[i].content.cloneNode(true);
+    installShadows(test);
+    console.innerText += "\nTest " + tests[i].getAttribute("test") + "\n";
+    console.innerText += internals.composedTreeAsText(test);
+}
+
+</script>
+</body>
index 1811b9d..55c13d6 100644 (file)
@@ -1,3 +1,28 @@
+2016-03-03  Antti Koivisto  <antti@apple.com>
+
+        ComposedTreeIterator may traverse slotted nodes multiple times
+        https://bugs.webkit.org/show_bug.cgi?id=154983
+
+        Reviewed by Ryosuke Niwa.
+
+        Traversal of slotted nodes can escape to real siblings. Those siblings are then traversed again as slotted nodes.
+
+        Test: fast/shadow-dom/composed-tree-slots.html
+
+        * dom/ComposedTreeIterator.cpp:
+        (WebCore::ComposedTreeIterator::initializeContextStack):
+        (WebCore::ComposedTreeIterator::traverseNextInShadowTree):
+        (WebCore::ComposedTreeIterator::traverseNextLeavingContext):
+        (WebCore::ComposedTreeIterator::advanceInSlot):
+        * dom/ComposedTreeIterator.h:
+        (WebCore::ComposedTreeIterator::Context::Context):
+
+            Include end iterator to the context.
+            For slotted nodes set it up to point to the next sibling of the node.
+
+        (WebCore::ComposedTreeIterator::context):
+        (WebCore::ComposedTreeIterator::traverseNextSkippingChildren):
+
 2016-03-04  Andreas Kling  <akling@apple.com>
 
         Drop DocumentSharedObjectPool immediately when going into PageCache.
index feccd35..7f5d75d 100644 (file)
 
 namespace WebCore {
 
+ComposedTreeIterator::Context::Context()
+{
+}
+
+ComposedTreeIterator::Context::Context(ContainerNode& root)
+    : iterator(root)
+{
+}
+
+ComposedTreeIterator::Context::Context(ContainerNode& root, Node& node)
+    : iterator(root, &node)
+{
+}
+
+#if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
+ComposedTreeIterator::Context::Context(ContainerNode& root, Node& node, SlottedTag)
+    : iterator(root, &node)
+    , end(iterator)
+{
+    end.traverseNextSibling();
+}
+#endif
+
 ComposedTreeIterator::ComposedTreeIterator(ContainerNode& root)
 {
     ASSERT(!is<ShadowRoot>(root));
@@ -75,7 +98,9 @@ void ComposedTreeIterator::initializeContextStack(ContainerNode& root, Node& cur
         }
         if (is<ShadowRoot>(*parent)) {
             auto& shadowRoot = downcast<ShadowRoot>(*parent);
-            m_contextStack.append(Context(shadowRoot, *contextCurrent, currentSlotNodeIndex));
+            m_contextStack.append(Context(shadowRoot, *contextCurrent));
+            m_contextStack.last().slotNodeIndex = currentSlotNodeIndex;
+
             node = shadowRoot.host();
             contextCurrent = node;
             currentSlotNodeIndex = notFound;
@@ -83,7 +108,9 @@ void ComposedTreeIterator::initializeContextStack(ContainerNode& root, Node& cur
         }
         if (auto* shadowRoot = parent->shadowRoot()) {
 #if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
-            m_contextStack.append(Context(*parent, *contextCurrent, currentSlotNodeIndex));
+            m_contextStack.append(Context(*parent, *contextCurrent, Context::Slotted));
+            m_contextStack.last().slotNodeIndex = currentSlotNodeIndex;
+
             auto* assignedSlot = shadowRoot->findAssignedSlot(*node);
             if (assignedSlot) {
                 currentSlotNodeIndex = assignedSlot->assignedNodes()->find(node);
@@ -101,7 +128,8 @@ void ComposedTreeIterator::initializeContextStack(ContainerNode& root, Node& cur
         }
         node = parent;
     }
-    m_contextStack.append(Context(root, *contextCurrent, currentSlotNodeIndex));
+    m_contextStack.append(Context(root, *contextCurrent));
+    m_contextStack.last().slotNodeIndex = currentSlotNodeIndex;
 
     m_contextStack.reverse();
 }
@@ -138,7 +166,7 @@ void ComposedTreeIterator::traverseNextInShadowTree()
         if (auto* assignedNodes = slot.assignedNodes()) {
             context().slotNodeIndex = 0;
             auto* assignedNode = assignedNodes->at(0);
-            m_contextStack.append(Context(*assignedNode->parentElement(), *assignedNode));
+            m_contextStack.append(Context(*assignedNode->parentElement(), *assignedNode, Context::Slotted));
             return;
         }
     }
@@ -146,7 +174,7 @@ void ComposedTreeIterator::traverseNextInShadowTree()
 
     context().iterator.traverseNext();
 
-    if (!context().iterator)
+    if (context().iterator == context().end)
         traverseNextLeavingContext();
 }
 
@@ -154,9 +182,9 @@ void ComposedTreeIterator::traverseNextLeavingContext()
 {
     ASSERT(m_contextStack.size() > 1);
 
-    while (!context().iterator && m_contextStack.size() > 1) {
+    while (context().iterator == context().end && m_contextStack.size() > 1) {
         m_contextStack.removeLast();
-        if (!context().iterator)
+        if (context().iterator == context().end)
             return;
 #if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
         if (is<HTMLSlotElement>(current()) && advanceInSlot(1))
@@ -178,7 +206,7 @@ bool ComposedTreeIterator::advanceInSlot(int direction)
         return false;
 
     auto* slotNode = assignedNodes.at(context().slotNodeIndex);
-    m_contextStack.append(Context(*slotNode->parentElement(), *slotNode));
+    m_contextStack.append(Context(*slotNode->parentElement(), *slotNode, Context::Slotted));
     return true;
 }
 
index f3d4650..566cf55 100644 (file)
@@ -67,16 +67,16 @@ private:
 #endif
 
     struct Context {
-        Context() { }
-        explicit Context(ContainerNode& root)
-            : iterator(root)
-        { }
-        Context(ContainerNode& root, Node& node, size_t slotNodeIndex = notFound)
-            : iterator(root, &node)
-            , slotNodeIndex(slotNodeIndex)
-        { }
+        Context();
+        explicit Context(ContainerNode& root);
+        Context(ContainerNode& root, Node& node);
 
+#if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
+        enum SlottedTag { Slotted };
+        Context(ContainerNode& root, Node& node, SlottedTag);
+#endif
         ElementAndTextDescendantIterator iterator;
+        ElementAndTextDescendantIterator end;
         size_t slotNodeIndex { notFound };
     };
     Context& context() { return m_contextStack.last(); }
@@ -84,7 +84,7 @@ private:
     Node& current() { return *context().iterator; }
 
     bool m_didDropAssertions { false };
-    Vector<Context, 4> m_contextStack;
+    Vector<Context, 8> m_contextStack;
 };
 
 inline ComposedTreeIterator::ComposedTreeIterator()
@@ -112,7 +112,7 @@ inline ComposedTreeIterator& ComposedTreeIterator::traverseNextSkippingChildren(
 {
     context().iterator.traverseNextSkippingChildren();
 
-    if (!context().iterator && m_contextStack.size() > 1)
+    if (context().iterator == context().end && m_contextStack.size() > 1)
         traverseNextLeavingContext();
     
     return *this;