ComposedTreeIterator may crash when first child of shadow root is a comment node
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Apr 2016 09:27:22 +0000 (09:27 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Apr 2016 09:27:22 +0000 (09:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156281

Reviewed by Andreas Kling.

Source/WebCore:

It should not use plain firstChild() and assume it is Element or Text.

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

    Add FirstChildTag to various iterator constructors to make clear that they search for the first child.

(WebCore::ComposedTreeIterator::ComposedTreeIterator):
(WebCore::ComposedTreeIterator::traverseShadowRoot):

    Fix by using ElementAndTextDescendantIterator to find the first child.

* dom/ComposedTreeIterator.h:
(WebCore::ComposedTreeIterator::operator*):
(WebCore::ComposedTreeDescendantAdapter::ComposedTreeDescendantAdapter):
(WebCore::ComposedTreeDescendantAdapter::begin):
(WebCore::ComposedTreeDescendantAdapter::end):
(WebCore::ComposedTreeDescendantAdapter::at):
(WebCore::ComposedTreeChildAdapter::Iterator::Iterator):
* dom/ElementAndTextDescendantIterator.h:
(WebCore::ElementAndTextDescendantIterator::operator++):
(WebCore::ElementAndTextDescendantIterator::ElementAndTextDescendantIterator):
(WebCore::ElementAndTextDescendantIteratorAdapter::begin):
(WebCore::ElementAndTextDescendantIteratorAdapter::end):

LayoutTests:

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

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

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/composed-tree-shadow-subtree-expected.txt
LayoutTests/fast/shadow-dom/composed-tree-shadow-subtree.html
Source/WebCore/ChangeLog
Source/WebCore/dom/ComposedTreeIterator.cpp
Source/WebCore/dom/ComposedTreeIterator.h
Source/WebCore/dom/ElementAndTextDescendantIterator.h

index 02328ad..d2b4e21 100644 (file)
@@ -1,3 +1,13 @@
+2016-04-06  Antti Koivisto  <antti@apple.com>
+
+        ComposedTreeIterator may crash when first child of shadow root is a comment node
+        https://bugs.webkit.org/show_bug.cgi?id=156281
+
+        Reviewed by Andreas Kling.
+
+        * fast/shadow-dom/composed-tree-shadow-subtree-expected.txt:
+        * fast/shadow-dom/composed-tree-shadow-subtree.html:
+
 2016-04-05  Chris Dumez  <cdumez@apple.com>
 
         MessageEvent.source window is incorrect once window has been reified
index e3994d0..0f7d8f3 100644 (file)
@@ -1,15 +1,56 @@
 
-Test 1
+Test 1.1
   div (shadow root)
 
 Shadow host subtree
 
-Test 2
+Test 1.2
   div (shadow root)
 
 Shadow host subtree
 
-Test 3
+Test 1.3
+  div (shadow root)
+
+Shadow host subtree
+
+Test 2.1
+  div (shadow root)
+    slot
+      div
+
+Shadow host subtree
+  slot
+    div
+
+Slot subtree
+  div
+
+Test 2.2
+  div (shadow root)
+    slot
+      #text
+
+Shadow host subtree
+  slot
+    #text
+
+Slot subtree
+  #text
+
+Test 2.3
+  div (shadow root)
+    slot
+      #text
+
+Shadow host subtree
+  slot
+    #text
+
+Slot subtree
+  #text
+
+Test 3.1
   div (shadow root)
     slot
       div
@@ -21,7 +62,19 @@ Shadow host subtree
 Slot subtree
   div
 
-Test 4
+Test 3.2
+  div (shadow root)
+    slot
+      #text
+
+Shadow host subtree
+  slot
+    #text
+
+Slot subtree
+  #text
+
+Test 3.3
   div (shadow root)
     slot
       #text
index 751a380..fe2f2f1 100644 (file)
@@ -6,11 +6,19 @@ if (window.testRunner)
 
 <template id=shadow1></template>
 <template id=shadow2><slot><div></div></slot></template>
+<template id=shadow3><!--comment--><slot><div></div></slot></template>
 
-<template test=1><div shadow=shadow1></div></template>
-<template test=2><div shadow=shadow1>text</div></template>
-<template test=3><div shadow=shadow2></div></template>
-<template test=4><div shadow=shadow2>text</div></template>
+<template test=1.1><div shadow=shadow1></div></template>
+<template test=1.2><div shadow=shadow1>text</div></template>
+<template test=1.3><div shadow=shadow1><!--comment-->text</div></template>
+
+<template test=2.1><div shadow=shadow2></div></template>
+<template test=2.2><div shadow=shadow2>text</div></template>
+<template test=2.3><div shadow=shadow2><!--comment-->text</div></template>
+
+<template test=3.1><div shadow=shadow3></div></template>
+<template test=3.2><div shadow=shadow3>text</div></template>
+<template test=3.3><div shadow=shadow3><!--comment-->text</div></template>
 
 <body>
 <pre id=console></pre>
index dcbe17f..893c58a 100644 (file)
@@ -1,3 +1,35 @@
+2016-04-06  Antti Koivisto  <antti@apple.com>
+
+        ComposedTreeIterator may crash when first child of shadow root is a comment node
+        https://bugs.webkit.org/show_bug.cgi?id=156281
+
+        Reviewed by Andreas Kling.
+
+        It should not use plain firstChild() and assume it is Element or Text.
+
+        * dom/ComposedTreeIterator.cpp:
+        (WebCore::ComposedTreeIterator::Context::Context):
+
+            Add FirstChildTag to various iterator constructors to make clear that they search for the first child.
+
+        (WebCore::ComposedTreeIterator::ComposedTreeIterator):
+        (WebCore::ComposedTreeIterator::traverseShadowRoot):
+
+            Fix by using ElementAndTextDescendantIterator to find the first child.
+
+        * dom/ComposedTreeIterator.h:
+        (WebCore::ComposedTreeIterator::operator*):
+        (WebCore::ComposedTreeDescendantAdapter::ComposedTreeDescendantAdapter):
+        (WebCore::ComposedTreeDescendantAdapter::begin):
+        (WebCore::ComposedTreeDescendantAdapter::end):
+        (WebCore::ComposedTreeDescendantAdapter::at):
+        (WebCore::ComposedTreeChildAdapter::Iterator::Iterator):
+        * dom/ElementAndTextDescendantIterator.h:
+        (WebCore::ElementAndTextDescendantIterator::operator++):
+        (WebCore::ElementAndTextDescendantIterator::ElementAndTextDescendantIterator):
+        (WebCore::ElementAndTextDescendantIteratorAdapter::begin):
+        (WebCore::ElementAndTextDescendantIteratorAdapter::end):
+
 2016-04-05  Chris Dumez  <cdumez@apple.com>
 
         Add support for [EnabledAtRuntime] operations on DOMWindow
index f05c91a..17bec5b 100644 (file)
@@ -35,8 +35,8 @@ ComposedTreeIterator::Context::Context()
 {
 }
 
-ComposedTreeIterator::Context::Context(ContainerNode& root)
-    : iterator(root)
+ComposedTreeIterator::Context::Context(ContainerNode& root, FirstChildTag)
+    : iterator(root, ElementAndTextDescendantIterator::FirstChild)
 {
 }
 
@@ -54,7 +54,7 @@ ComposedTreeIterator::Context::Context(ContainerNode& root, Node& node, SlottedT
 }
 #endif
 
-ComposedTreeIterator::ComposedTreeIterator(ContainerNode& root)
+ComposedTreeIterator::ComposedTreeIterator(ContainerNode& root, FirstChildTag)
 {
     ASSERT(!is<ShadowRoot>(root));
 
@@ -68,12 +68,12 @@ ComposedTreeIterator::ComposedTreeIterator(ContainerNode& root)
     }
 #endif
     if (auto* shadowRoot = root.shadowRoot()) {
-        auto* firstChild = shadowRoot->firstChild();
+        ElementAndTextDescendantIterator firstChild(*shadowRoot, ElementAndTextDescendantIterator::FirstChild);
         initializeContextStack(root, firstChild ? *firstChild : root);
         return;
     }
 
-    m_contextStack.uncheckedAppend(Context(root));
+    m_contextStack.uncheckedAppend(Context(root, FirstChild));
 }
 
 ComposedTreeIterator::ComposedTreeIterator(ContainerNode& root, Node& current)
@@ -148,7 +148,7 @@ void ComposedTreeIterator::dropAssertions()
 
 void ComposedTreeIterator::traverseShadowRoot(ShadowRoot& shadowRoot)
 {
-    Context shadowContext(shadowRoot);
+    Context shadowContext(shadowRoot, FirstChild);
     if (!shadowContext.iterator) {
         // Empty shadow root.
         traverseNextSkippingChildren();
index 36ffb59..2d03e33 100644 (file)
@@ -36,7 +36,8 @@ class HTMLSlotElement;
 class ComposedTreeIterator {
 public:
     ComposedTreeIterator();
-    ComposedTreeIterator(ContainerNode& root);
+    enum FirstChildTag { FirstChild };
+    ComposedTreeIterator(ContainerNode& root, FirstChildTag);
     ComposedTreeIterator(ContainerNode& root, Node& current);
 
     Node& operator*() { return current(); }
@@ -68,7 +69,7 @@ private:
 
     struct Context {
         Context();
-        explicit Context(ContainerNode& root);
+        Context(ContainerNode& root, FirstChildTag);
         Context(ContainerNode& root, Node& node);
 
 #if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
@@ -156,7 +157,7 @@ public:
         : m_parent(parent)
     { }
 
-    ComposedTreeIterator begin() { return ComposedTreeIterator(m_parent); }
+    ComposedTreeIterator begin() { return ComposedTreeIterator(m_parent, ComposedTreeIterator::FirstChild); }
     ComposedTreeIterator end() { return { }; }
     ComposedTreeIterator at(const Node& child) { return ComposedTreeIterator(m_parent, const_cast<Node&>(child)); }
     
@@ -170,7 +171,7 @@ public:
     public:
         Iterator() = default;
         explicit Iterator(ContainerNode& root)
-            : ComposedTreeIterator(root)
+            : ComposedTreeIterator(root, ComposedTreeIterator::FirstChild)
         { }
         Iterator(ContainerNode& root, Node& current)
             : ComposedTreeIterator(root, current)
index cb2df22..3bfcd03 100644 (file)
@@ -36,7 +36,8 @@ namespace WebCore {
 class ElementAndTextDescendantIterator {
 public:
     ElementAndTextDescendantIterator();
-    explicit ElementAndTextDescendantIterator(ContainerNode& root);
+    enum FirstChildTag { FirstChild };
+    ElementAndTextDescendantIterator(ContainerNode& root, FirstChildTag);
     ElementAndTextDescendantIterator(ContainerNode& root, Node* current);
 
     ElementAndTextDescendantIterator& operator++() { return traverseNext(); }
@@ -101,7 +102,7 @@ inline ElementAndTextDescendantIterator::ElementAndTextDescendantIterator()
 {
 }
 
-inline ElementAndTextDescendantIterator::ElementAndTextDescendantIterator(ContainerNode& root)
+inline ElementAndTextDescendantIterator::ElementAndTextDescendantIterator(ContainerNode& root, FirstChildTag)
     : m_current(firstChild(root))
 #if !ASSERT_DISABLED
     , m_assertions(m_current)
@@ -301,7 +302,7 @@ inline ElementAndTextDescendantIteratorAdapter::ElementAndTextDescendantIterator
 
 inline ElementAndTextDescendantIterator ElementAndTextDescendantIteratorAdapter::begin()
 {
-    return ElementAndTextDescendantIterator(m_root);
+    return ElementAndTextDescendantIterator(m_root, ElementAndTextDescendantIterator::FirstChild);
 }
 
 inline ElementAndTextDescendantIterator ElementAndTextDescendantIteratorAdapter::end()