ComposedTreeIterator does not traverse all slotted children if the traversal root...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 May 2017 17:20:54 +0000 (17:20 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 May 2017 17:20:54 +0000 (17:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171375
<rdar://problem/31863184>

Reviewed by Zalan Bujtas.

Source/WebCore:

We were hitting an assert when using details element with a flow thread. The root cause for this turned
out to be that we only traversed the first slotted child if the traversal root was a slot element.

Test: fast/html/details-flow-thread.html

* dom/ComposedTreeIterator.cpp:
(WebCore::ComposedTreeIterator::traverseNextLeavingContext):

    Try to traverse to the next slotted child before testing if we at the end of the current context.

LayoutTests:

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

    Expand the test so it also prints out slot subtrees using slots as traversal roots.

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

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

index 052963b..4b2ad79 100644 (file)
@@ -1,3 +1,18 @@
+2017-05-08  Antti Koivisto  <antti@apple.com>
+
+        ComposedTreeIterator does not traverse all slotted children if the traversal root is a slot element.
+        https://bugs.webkit.org/show_bug.cgi?id=171375
+        <rdar://problem/31863184>
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/html/details-flow-thread-expected.txt: Added.
+        * fast/html/details-flow-thread.html: Added.
+        * fast/shadow-dom/composed-tree-slots-expected.txt:
+        * fast/shadow-dom/composed-tree-slots.html:
+
+            Expand the test so it also prints out slot subtrees using slots as traversal roots.
+
 2017-05-08  Chris Dumez  <cdumez@apple.com>
 
         Move 'style' from Element to HTMLElement / SVGElement and make it settable
diff --git a/LayoutTests/fast/html/details-flow-thread-expected.txt b/LayoutTests/fast/html/details-flow-thread-expected.txt
new file mode 100644 (file)
index 0000000..a27ea43
--- /dev/null
@@ -0,0 +1 @@
+This test passes if it doesn't assert
diff --git a/LayoutTests/fast/html/details-flow-thread.html b/LayoutTests/fast/html/details-flow-thread.html
new file mode 100644 (file)
index 0000000..fe34836
--- /dev/null
@@ -0,0 +1,26 @@
+<style>
+details div { -webkit-flow-into: bar; }
+</style>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function gc() {
+    if (window.GCController)
+        return GCController.collect();
+    var a;
+    for (var i=0; i<100; i++)
+        a = new Uint8Array(1024*1024);
+}
+
+function go() {
+    details.offsetLeft;
+    details.open = false;
+    div.innerHTML = "This test passes if it doesn't assert";
+    gc();
+}
+</script>
+<body onload=go()>
+<div id="div">
+<details open id="details">
+<div>foo</div>
index cef617e..79c16ed 100644 (file)
@@ -2,17 +2,26 @@
 Test 1
   div (shadow root)
     slot
+Slot trees:
+
 
 Test 2
   div (shadow root)
     slot
       #text
+Slot trees:
+  #text
+
 
 Test 3
   div (shadow root)
     slot
       #text
       div
+Slot trees:
+  #text
+  div
+
 
 Test 4
   div (shadow root)
@@ -21,12 +30,20 @@ Test 4
         #text
       div
         #text
+Slot trees:
+  div
+    #text
+  div
+    #text
+
 
 Test 5
   div (shadow root)
     div
       slot
     #text
+Slot trees:
+
 
 Test 6
   div (shadow root)
@@ -34,6 +51,9 @@ Test 6
       slot
         #text
     #text
+Slot trees:
+  #text
+
 
 Test 7
   div (shadow root)
@@ -42,6 +62,10 @@ Test 7
         #text
         div
     #text
+Slot trees:
+  #text
+  div
+
 
 Test 8
   div (shadow root)
@@ -52,23 +76,40 @@ Test 8
         div
           #text
     #text
+Slot trees:
+  div
+    #text
+  div
+    #text
+
 
 Test 9
   div (shadow root)
     slot
       slot-default
         #text
+Slot trees:
+  slot-default
+    #text
+
 
 Test 10
   div (shadow root)
     slot
       #text
+Slot trees:
+  #text
+
 
 Test 11
   div (shadow root)
     slot
       #text
       div
+Slot trees:
+  #text
+  div
+
 
 Test 12
   div (shadow root)
@@ -77,6 +118,12 @@ Test 12
         #text
       div
         #text
+Slot trees:
+  div
+    #text
+  div
+    #text
+
 
 Test 13
   div (shadow root)
@@ -86,6 +133,12 @@ Test 13
           #text
           slot
           #text
+Slot trees:
+
+  #text
+  slot
+  #text
+
 
 Test 14
   div (shadow root)
@@ -96,6 +149,14 @@ Test 14
           slot
             #text
           #text
+Slot trees:
+  #text
+
+  #text
+  slot
+    #text
+  #text
+
 
 Test 15
   div (shadow root)
@@ -107,6 +168,16 @@ Test 15
             #text
             div
           #text
+Slot trees:
+  #text
+  div
+
+  #text
+  slot
+    #text
+    div
+  #text
+
 
 Test 16
   div (shadow root)
@@ -120,4 +191,18 @@ Test 16
             div
               #text
           #text
+Slot trees:
+  div
+    #text
+  div
+    #text
+
+  #text
+  slot
+    div
+      #text
+    div
+      #text
+  #text
+
 
index 7a5fd3f..be54063 100644 (file)
@@ -34,26 +34,38 @@ if (window.testRunner)
 <script>
 function installShadows(tree)
 {
-    var shadowHosts = tree.querySelectorAll("[shadow]");
-    for (var i = 0; i < shadowHosts.length; ++i) {
-        var shadowId = shadowHosts[i].getAttribute("shadow");
+    for (const host of tree.querySelectorAll("[shadow]")) {
+        var shadowId = host.getAttribute("shadow");
         var shadowContents = document.querySelector("#"+shadowId).content.cloneNode(true);
 
         installShadows(shadowContents);
 
-        var shadowRoot = shadowHosts[i].attachShadow({ mode: "open" });
+        var shadowRoot = host.attachShadow({ mode: "open" });
         shadowRoot.appendChild(shadowContents);
     }
 }
 
+function findSlots(root)
+{
+    let slots = [];
+    for (const host of root.querySelectorAll("[shadow]")) {
+        for (const slot of host.shadowRoot.querySelectorAll("slot"))
+            slots.push(slot);
+        Array.prototype.push.apply(slots, findSlots(host.shadowRoot));
+    }
+    return slots;
+}
+
 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);
+for (const testTemplate of document.querySelectorAll("[test]")) {
+    var test = testTemplate.content.cloneNode(true);
     installShadows(test);
-    console.innerText += "\nTest " + tests[i].getAttribute("test") + "\n";
+    console.innerText += "\nTest " + testTemplate.getAttribute("test") + "\n";
     console.innerText += internals.composedTreeAsText(test);
+    console.innerText += "Slot trees:\n";
+    for (const slot of findSlots(test))
+        console.innerText += internals.composedTreeAsText(slot) + "\n";
 }
 
 </script>
index b8457b2..e7791ba 100644 (file)
@@ -1,3 +1,21 @@
+2017-05-08  Antti Koivisto  <antti@apple.com>
+
+        ComposedTreeIterator does not traverse all slotted children if the traversal root is a slot element.
+        https://bugs.webkit.org/show_bug.cgi?id=171375
+        <rdar://problem/31863184>
+
+        Reviewed by Zalan Bujtas.
+
+        We were hitting an assert when using details element with a flow thread. The root cause for this turned
+        out to be that we only traversed the first slotted child if the traversal root was a slot element.
+
+        Test: fast/html/details-flow-thread.html
+
+        * dom/ComposedTreeIterator.cpp:
+        (WebCore::ComposedTreeIterator::traverseNextLeavingContext):
+
+            Try to traverse to the next slotted child before testing if we at the end of the current context.
+
 2017-05-08  Mark Lam  <mark.lam@apple.com>
 
         Introduce ExceptionScope::assertNoException() and releaseAssertNoException().
index 7bb7612..af554ac 100644 (file)
@@ -179,10 +179,10 @@ void ComposedTreeIterator::traverseNextLeavingContext()
 {
     while (context().iterator == context().end && m_contextStack.size() > 1) {
         m_contextStack.removeLast();
-        if (context().iterator == context().end)
-            return;
         if (is<HTMLSlotElement>(current()) && advanceInSlot(1))
             return;
+        if (context().iterator == context().end)
+            return;
         context().iterator.traverseNextSkippingChildren();
     }
 }