Assertion hit on redfin.com: ASSERTION FAILED: collection->length() > 1
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jan 2017 21:26:05 +0000 (21:26 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jan 2017 21:26:05 +0000 (21:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=166687
<rdar://problem/29865854>

Reviewed by Darin Adler.

Source/WebCore:

We were mistakenly calling Document::addWindowNamedItem() / Document::removeWindowNamedItem()
for elements in Shadow DOMs. As a result, the windowNamedItem DocumentOrderedMap would
contain elements in shadow DOMs. This would cause the assertion to be hit in window's
named property getter because of the length mismatch between the windowNamedItem
DocumentOrderedMap and the WindowNameCollection.

Tests: fast/shadow-dom/document-named-property.html
       fast/shadow-dom/window-named-property.html

* dom/Element.cpp:
(WebCore::Element::updateNameForDocument):
(WebCore::Element::updateIdForDocument):
* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::parseAttribute):
* html/HTMLObjectElement.cpp:
(WebCore::HTMLObjectElement::updateDocNamedItem):

LayoutTests:

Add layout test coverage.

* fast/shadow-dom/document-named-property-expected.txt: Added.
* fast/shadow-dom/document-named-property.html: Added.
* fast/shadow-dom/window-named-property-expected.txt: Added.
* fast/shadow-dom/window-named-property.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/document-named-property-expected.txt [new file with mode: 0644]
LayoutTests/fast/shadow-dom/document-named-property.html [new file with mode: 0644]
LayoutTests/fast/shadow-dom/window-named-property-expected.txt [new file with mode: 0644]
LayoutTests/fast/shadow-dom/window-named-property.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/html/HTMLImageElement.cpp
Source/WebCore/html/HTMLObjectElement.cpp

index 6b4e4f3..5989df9 100644 (file)
@@ -1,3 +1,18 @@
+2017-01-04  Chris Dumez  <cdumez@apple.com>
+
+        Assertion hit on redfin.com: ASSERTION FAILED: collection->length() > 1
+        https://bugs.webkit.org/show_bug.cgi?id=166687
+        <rdar://problem/29865854>
+
+        Reviewed by Darin Adler.
+
+        Add layout test coverage.
+
+        * fast/shadow-dom/document-named-property-expected.txt: Added.
+        * fast/shadow-dom/document-named-property.html: Added.
+        * fast/shadow-dom/window-named-property-expected.txt: Added.
+        * fast/shadow-dom/window-named-property.html: Added.
+
 2017-01-04  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [GTK] Two editing tests are passing but marked as failure
diff --git a/LayoutTests/fast/shadow-dom/document-named-property-expected.txt b/LayoutTests/fast/shadow-dom/document-named-property-expected.txt
new file mode 100644 (file)
index 0000000..cf02596
--- /dev/null
@@ -0,0 +1,18 @@
+
+
+
+
+
+PASS Document's named property getter should not return elements in shadow DOMs by id 
+PASS Document's named property getter should not return elements in shadow DOMs by id (duplicate id) 
+PASS Document's named property getter should not return elements in shadow DOMs by id (id attribute update) 
+PASS Document's named property getter should not return elements in shadow DOMs by id (duplicate id attribute update) 
+PASS Document's named property getter should not return elements in shadow DOMs by id (image name change) 
+PASS Document's named property getter should not return elements in shadow DOMs by id (image name change with duplicate id) 
+PASS Document's named property getter should not return elements in shadow DOMs by id (object children change) 
+PASS Document's named property getter should not return elements in shadow DOMs by id (object children change with duplicate id) 
+PASS Document's named property getter should not return elements in shadow DOMs by name 
+PASS Document's named property getter should not return elements in shadow DOMs by name (duplicate name) 
+PASS Document's named property getter should not return elements in shadow DOMs by name (name attribute update) 
+PASS Document's named property getter should not return elements in shadow DOMs by name (duplicate name attribute update) 
+
diff --git a/LayoutTests/fast/shadow-dom/document-named-property.html b/LayoutTests/fast/shadow-dom/document-named-property.html
new file mode 100644 (file)
index 0000000..b55b5d2
--- /dev/null
@@ -0,0 +1,187 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src='../../resources/testharness.js'></script>
+<script src='../../resources/testharnessreport.js'></script>
+<div id="container"></div>
+<script>
+var container = document.getElementById('container');
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img id="foo" name="test2"></img>';
+    container.appendChild(header);
+
+    assert_equals(document.foo, undefined);
+}, "Document's named property getter should not return elements in shadow DOMs by id");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img id="foo1" name="test2"></img>';
+    container.appendChild(header);
+
+    var img = document.createElement("img");
+    img.id = 'foo1';
+    img.name = 'test1'
+    container.appendChild(img);
+
+    assert_equals(document.foo1, img);
+    assert_equals(shadowRoot.firstChild.id, 'foo1');
+    shadowRoot.removeChild(shadowRoot.firstChild);
+    assert_equals(document.foo1, img);
+}, "Document's named property getter should not return elements in shadow DOMs by id (duplicate id)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img name="test2"></img>';
+    container.appendChild(header);
+
+    assert_equals(document.foo2, undefined);
+    shadowRoot.firstChild.id = 'foo2';
+    assert_equals(document.foo2, undefined);
+
+}, "Document's named property getter should not return elements in shadow DOMs by id (id attribute update)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img name="test2"></img>';
+    container.appendChild(header);
+
+    var img = document.createElement("img");
+    img.id = 'foo3';
+    img.setAttribute('name', 'test1');
+    container.appendChild(img);
+
+    assert_equals(document.foo3, img);
+    shadowRoot.firstChild.id = 'foo3';
+    assert_equals(document.foo3, img);
+
+    shadowRoot.firstChild.id = 'other';
+    assert_equals(document.foo3, img);
+}, "Document's named property getter should not return elements in shadow DOMs by id (duplicate id attribute update)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img></img>';
+    container.appendChild(header);
+
+    assert_equals(document.foo4, undefined);
+    shadowRoot.firstChild.id = 'foo4';
+    assert_equals(document.foo4, undefined);
+    shadowRoot.firstChild.setAttribute('name', 'test3');
+    assert_equals(document.foo4, undefined);
+
+}, "Document's named property getter should not return elements in shadow DOMs by id (image name change)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img></img>';
+    container.appendChild(header);
+
+    var img = document.createElement("img");
+    img.id = 'foo5';
+    img.setAttribute('name', 'test1');
+    container.appendChild(img);
+
+    assert_equals(document.foo5, img);
+    shadowRoot.firstChild.id = 'foo5';
+    assert_equals(document.foo5, img);
+    shadowRoot.firstChild.setAttribute('name', 'test3');
+    assert_equals(document.foo5, img);
+
+    shadowRoot.firstChild.removeAttribute('name');
+    assert_equals(document.foo5, img);
+}, "Document's named property getter should not return elements in shadow DOMs by id (image name change with duplicate id)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<object id="foo6">text</object>';
+    container.appendChild(header);
+
+    assert_equals(document.foo6, undefined);
+    assert_equals(shadowRoot.firstChild.firstChild.data, "text");
+    shadowRoot.firstChild.removeChild(shadowRoot.firstChild.firstChild);
+    assert_equals(document.foo6, undefined);
+}, "Document's named property getter should not return elements in shadow DOMs by id (object children change)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<object id="foo7">text</object>';
+    container.appendChild(header);
+
+    var object = document.createElement("object");
+    object.id = 'foo7';
+    container.appendChild(object);
+
+    assert_equals(document.foo7, object);
+    assert_equals(shadowRoot.firstChild.firstChild.data, "text");
+    shadowRoot.firstChild.removeChild(shadowRoot.firstChild.firstChild);
+    assert_equals(document.foo7, object);
+    shadowRoot.firstChild.appendChild(document.createElement("a"));
+    assert_equals(document.foo7, object);
+}, "Document's named property getter should not return elements in shadow DOMs by id (object children change with duplicate id)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img name="bar"></img>';
+    document.body.appendChild(header);
+
+    assert_equals(document.bar, undefined);
+}, "Document's named property getter should not return elements in shadow DOMs by name");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img name="bar1"></img>';
+    document.body.appendChild(header);
+
+    var img = document.createElement("img");
+    img.setAttribute('name', 'bar1');
+    container.appendChild(img);
+
+    assert_equals(document.bar1, img);
+    assert_equals(shadowRoot.firstChild.getAttribute('name'), 'bar1');
+    shadowRoot.removeChild(shadowRoot.firstChild);
+    assert_equals(document.bar1, img);
+}, "Document's named property getter should not return elements in shadow DOMs by name (duplicate name)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img></img>';
+    document.body.appendChild(header);
+
+    assert_equals(document.bar2, undefined);
+    shadowRoot.firstChild.setAttribute('name', 'bar2');
+    assert_equals(document.bar2, undefined);
+}, "Document's named property getter should not return elements in shadow DOMs by name (name attribute update)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<img></img>';
+    document.body.appendChild(header);
+
+    var img = document.createElement("img");
+    img.setAttribute('name', 'bar3');
+    container.appendChild(img);
+
+    assert_equals(document.bar3, img);
+    shadowRoot.firstChild.setAttribute('name', 'bar3');
+    assert_equals(document.bar3, img);
+
+    shadowRoot.firstChild.setAttribute('name', 'other');
+    assert_equals(document.bar3, img);
+}, "Document's named property getter should not return elements in shadow DOMs by name (duplicate name attribute update)");
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/shadow-dom/window-named-property-expected.txt b/LayoutTests/fast/shadow-dom/window-named-property-expected.txt
new file mode 100644 (file)
index 0000000..e548adc
--- /dev/null
@@ -0,0 +1,10 @@
+
+PASS Window's named property getter should not return elements in shadow DOMs by id 
+PASS Window's named property getter should not return elements in shadow DOMs by id (duplicate id) 
+PASS Window's named property getter should not return elements in shadow DOMs by id (id attribute update) 
+PASS Window's named property getter should not return elements in shadow DOMs by id (duplicate id attribute update) 
+PASS Window's named property getter should not return elements in shadow DOMs by name 
+PASS Window's named property getter should not return elements in shadow DOMs by name (duplicate name) 
+PASS Window's named property getter should not return elements in shadow DOMs by name (name attribute update) 
+PASS Window's named property getter should not return elements in shadow DOMs by name (duplicate name attribute update) 
+
diff --git a/LayoutTests/fast/shadow-dom/window-named-property.html b/LayoutTests/fast/shadow-dom/window-named-property.html
new file mode 100644 (file)
index 0000000..daef93d
--- /dev/null
@@ -0,0 +1,119 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src='../../resources/testharness.js'></script>
+<script src='../../resources/testharnessreport.js'></script>
+<div id="container"></div>
+<script>
+var container = document.getElementById('container');
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<span id="foo"></span>';
+    container.appendChild(header);
+
+    assert_equals(window.foo, undefined);
+}, "Window's named property getter should not return elements in shadow DOMs by id");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<span id="foo1"></span>';
+    container.appendChild(header);
+
+    var span = document.createElement("span");
+    span.id = 'foo1';
+    container.appendChild(span);
+
+    assert_equals(window.foo1, span);
+    assert_equals(shadowRoot.firstChild.id, 'foo1');
+    shadowRoot.removeChild(shadowRoot.firstChild);
+    assert_equals(window.foo1, span);
+}, "Window's named property getter should not return elements in shadow DOMs by id (duplicate id)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<span></span>';
+    container.appendChild(header);
+
+    assert_equals(window.foo2, undefined);
+    shadowRoot.firstChild.id = 'foo2';
+    assert_equals(window.foo2, undefined);
+}, "Window's named property getter should not return elements in shadow DOMs by id (id attribute update)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<span></span>';
+    container.appendChild(header);
+
+    var span = document.createElement("span");
+    span.id = 'foo3';
+    container.appendChild(span);
+
+    assert_equals(window.foo3, span);
+    shadowRoot.firstChild.id = 'foo3';
+    assert_equals(window.foo3, span);
+
+    shadowRoot.firstChild.id = 'other';
+    assert_equals(window.foo3, span);
+}, "Window's named property getter should not return elements in shadow DOMs by id (duplicate id attribute update)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<form name="bar"></form>';
+    document.body.appendChild(header);
+
+    assert_equals(window.bar, undefined);
+}, "Window's named property getter should not return elements in shadow DOMs by name");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<form name="bar1"></form>';
+    document.body.appendChild(header);
+
+    var form = document.createElement("form");
+    form.setAttribute('name', 'bar1');
+    container.appendChild(form);
+
+    assert_equals(window.bar1, form);
+    assert_equals(shadowRoot.firstChild.getAttribute('name'), 'bar1');
+    shadowRoot.removeChild(shadowRoot.firstChild);
+    assert_equals(window.bar1, form);
+}, "Window's named property getter should not return elements in shadow DOMs by name (duplicate name)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<form></form>';
+    document.body.appendChild(header);
+
+    assert_equals(window.bar2, undefined);
+    shadowRoot.firstChild.setAttribute('name', 'bar2');
+    assert_equals(window.bar2, undefined);
+}, "Window's named property getter should not return elements in shadow DOMs by name (name attribute update)");
+
+test(function () {
+    var header = document.createElement('header');
+    var shadowRoot = header.attachShadow({mode: 'open'});
+    shadowRoot.innerHTML = '<form></form>';
+    document.body.appendChild(header);
+
+    var form = document.createElement("form");
+    form.setAttribute('name', 'bar3');
+    container.appendChild(form);
+
+    assert_equals(window.bar3, form);
+    shadowRoot.firstChild.setAttribute('name', 'bar3');
+    assert_equals(window.bar3, form);
+
+    shadowRoot.firstChild.setAttribute('name', 'other');
+    assert_equals(window.bar3, form);
+}, "Window's named property getter should not return elements in shadow DOMs by name (duplicate name attribute update)");
+</script>
+</body>
+</html>
index 363faad..cb3cfef 100644 (file)
@@ -1,3 +1,28 @@
+2017-01-04  Chris Dumez  <cdumez@apple.com>
+
+        Assertion hit on redfin.com: ASSERTION FAILED: collection->length() > 1
+        https://bugs.webkit.org/show_bug.cgi?id=166687
+        <rdar://problem/29865854>
+
+        Reviewed by Darin Adler.
+
+        We were mistakenly calling Document::addWindowNamedItem() / Document::removeWindowNamedItem()
+        for elements in Shadow DOMs. As a result, the windowNamedItem DocumentOrderedMap would
+        contain elements in shadow DOMs. This would cause the assertion to be hit in window's
+        named property getter because of the length mismatch between the windowNamedItem
+        DocumentOrderedMap and the WindowNameCollection.
+
+        Tests: fast/shadow-dom/document-named-property.html
+               fast/shadow-dom/window-named-property.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::updateNameForDocument):
+        (WebCore::Element::updateIdForDocument):
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::parseAttribute):
+        * html/HTMLObjectElement.cpp:
+        (WebCore::HTMLObjectElement::updateDocNamedItem):
+
 2017-01-04  John Wilander  <wilander@apple.com>
 
         Validate the BCP47-ness of the language string passed to TrackBase::setLanguage()
index 9cf50d3..4711553 100644 (file)
@@ -3222,6 +3222,9 @@ void Element::updateNameForDocument(HTMLDocument& document, const AtomicString&
 {
     ASSERT(oldName != newName);
 
+    if (isInShadowTree())
+        return;
+
     if (WindowNameCollection::elementMatchesIfNameAttributeMatch(*this)) {
         const AtomicString& id = WindowNameCollection::elementMatchesIfIdAttributeMatch(*this) ? getIdAttribute() : nullAtom;
         if (!oldName.isEmpty() && oldName != id)
@@ -3272,6 +3275,9 @@ void Element::updateIdForDocument(HTMLDocument& document, const AtomicString& ol
     ASSERT(inDocument());
     ASSERT(oldId != newId);
 
+    if (isInShadowTree())
+        return;
+
     if (WindowNameCollection::elementMatchesIfIdAttributeMatch(*this)) {
         const AtomicString& name = condition == UpdateHTMLDocumentNamedItemMapsOnlyIfDiffersFromNameAttribute && WindowNameCollection::elementMatchesIfNameAttributeMatch(*this) ? getNameAttribute() : nullAtom;
         if (!oldId.isEmpty() && oldId != name)
index 96ee3b2..4f49ef1 100644 (file)
@@ -226,7 +226,7 @@ void HTMLImageElement::parseAttribute(const QualifiedName& name, const AtomicStr
     } else {
         if (name == nameAttr) {
             bool willHaveName = !value.isNull();
-            if (m_hadNameBeforeAttributeChanged != willHaveName && inDocument() && is<HTMLDocument>(document())) {
+            if (m_hadNameBeforeAttributeChanged != willHaveName && inDocument() && !isInShadowTree() && is<HTMLDocument>(document())) {
                 HTMLDocument& document = downcast<HTMLDocument>(this->document());
                 const AtomicString& id = getIdAttribute();
                 if (!id.isEmpty() && id != getNameAttribute()) {
index bde6ae2..890aa6a 100644 (file)
@@ -445,7 +445,7 @@ void HTMLObjectElement::updateDocNamedItem()
             isNamedItem = false;
         child = child->nextSibling();
     }
-    if (isNamedItem != wasNamedItem && inDocument() && is<HTMLDocument>(document())) {
+    if (isNamedItem != wasNamedItem && inDocument() && !isInShadowTree() && is<HTMLDocument>(document())) {
         HTMLDocument& document = downcast<HTMLDocument>(this->document());
 
         const AtomicString& id = getIdAttribute();