Title: [214808] releases/WebKitGTK/webkit-2.16
Revision
214808
Author
carlo...@webkit.org
Date
2017-04-03 10:09:21 -0700 (Mon, 03 Apr 2017)

Log Message

Merge r214510 - Only attach Attributes to a given element one time
https://bugs.webkit.org/show_bug.cgi?id=170125
<rdar://problem/31279676>

Reviewed by Chris Dumez.

Source/WebCore:

Attach the attribute node to the Element before calling 'setAttributeInternal', since that method may cause
arbitrary _javascript_ events to fire.

Test: fast/dom/Attr/only-attach-attr-once.html

* dom/Element.cpp:
(WebCore::Element::attachAttributeNodeIfNeeded): Added.
(WebCore::Element::setAttributeNode): Use new method. Revise to attach attribute before calling 'setAttributeInternal'.
(WebCore::Element::setAttributeNodeNS): Ditto.
* dom/Element.h:

LayoutTests:

* fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt: Rebaselined.
* fast/dom/Attr/make-unique-element-data-while-replacing-attr.html: Add check before setting new value.
* fast/dom/Attr/only-attach-attr-once-expected.txt: Added.
* fast/dom/Attr/only-attach-attr-once.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog (214807 => 214808)


--- releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog	2017-04-03 17:08:12 UTC (rev 214807)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog	2017-04-03 17:09:21 UTC (rev 214808)
@@ -1,3 +1,16 @@
+2017-03-27  Brent Fulgham  <bfulg...@apple.com>
+
+        Only attach Attributes to a given element one time
+        https://bugs.webkit.org/show_bug.cgi?id=170125
+        <rdar://problem/31279676>
+
+        Reviewed by Chris Dumez.
+
+        * fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt: Rebaselined.
+        * fast/dom/Attr/make-unique-element-data-while-replacing-attr.html: Add check before setting new value.
+        * fast/dom/Attr/only-attach-attr-once-expected.txt: Added.
+        * fast/dom/Attr/only-attach-attr-once.html: Added.
+
 2017-03-28  Antti Koivisto  <an...@apple.com>
 
         Missing render tree position invalidation when tearing down renderers for display:contents subtree

Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt (214807 => 214808)


--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt	2017-04-03 17:08:12 UTC (rev 214807)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr-expected.txt	2017-04-03 17:09:21 UTC (rev 214808)
@@ -3,6 +3,7 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+PASS element.getAttribute("width") is "a"
 PASS element.getAttribute("width") is "b"
 PASS successfullyParsed is true
 

Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr.html (214807 => 214808)


--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr.html	2017-04-03 17:08:12 UTC (rev 214807)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/make-unique-element-data-while-replacing-attr.html	2017-04-03 17:09:21 UTC (rev 214808)
@@ -13,6 +13,8 @@
 oldAttr.value = 'a';
 element.setAttributeNode(oldAttr);
 
+shouldBeEqualToString('element.getAttribute("width")', 'a');
+
 element.addEventListener('DOMSubtreeModified', () => { element.cloneNode(); }, true);
 
 let newAttr = document.createAttributeNS('http://www.w3.org/1999/xhtml','width');

Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/only-attach-attr-once-expected.txt (0 => 214808)


--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/only-attach-attr-once-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/only-attach-attr-once-expected.txt	2017-04-03 17:09:21 UTC (rev 214808)
@@ -0,0 +1,10 @@
+Test that we properly handle attempts to attach an Attribute to the same node multiple times. Test passes if there is no Debug ASSERT.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS div.setAttributeNodeNS(src) threw exception InUseAttributeError (DOM Exception 10): The attribute is in use..
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/only-attach-attr-once.html (0 => 214808)


--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/only-attach-attr-once.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/dom/Attr/only-attach-attr-once.html	2017-04-03 17:09:21 UTC (rev 214808)
@@ -0,0 +1,25 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description("Test that we properly handle attempts to attach an Attribute to the same node multiple times. Test passes if there is no Debug ASSERT.");
+
+window.callback = () => {
+    window.callback = null;
+
+    shouldThrow("div.setAttributeNodeNS(src)", "'InUseAttributeError (DOM Exception 10): The attribute is in use.'");
+    frame.setAttributeNodeNS(document.createAttribute('src'));
+};
+
+let src = ""
+src.value = '_javascript_:parent.callback()';
+
+let div = document.createElement('div');
+let frame = document.body.appendChild(document.createElement('iframe'));
+frame.setAttributeNodeNS(src);
+</script>
+</body>
+</html>

Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (214807 => 214808)


--- releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog	2017-04-03 17:08:12 UTC (rev 214807)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog	2017-04-03 17:09:21 UTC (rev 214808)
@@ -1,3 +1,22 @@
+2017-03-27  Brent Fulgham  <bfulg...@apple.com>
+
+        Only attach Attributes to a given element one time
+        https://bugs.webkit.org/show_bug.cgi?id=170125
+        <rdar://problem/31279676>
+
+        Reviewed by Chris Dumez.
+
+        Attach the attribute node to the Element before calling 'setAttributeInternal', since that method may cause
+        arbitrary _javascript_ events to fire. 
+
+        Test: fast/dom/Attr/only-attach-attr-once.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::attachAttributeNodeIfNeeded): Added.
+        (WebCore::Element::setAttributeNode): Use new method. Revise to attach attribute before calling 'setAttributeInternal'. 
+        (WebCore::Element::setAttributeNodeNS): Ditto.
+        * dom/Element.h:
+
 2017-03-28  Antti Koivisto  <an...@apple.com>
 
         Missing render tree position invalidation when tearing down renderers for display:contents subtree

Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Element.cpp (214807 => 214808)


--- releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Element.cpp	2017-04-03 17:08:12 UTC (rev 214807)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Element.cpp	2017-04-03 17:09:21 UTC (rev 214808)
@@ -2121,6 +2121,19 @@
     return *attrNodeListForElement(*this);
 }
 
+void Element::attachAttributeNodeIfNeeded(Attr& attrNode)
+{
+    ASSERT(!attrNode.ownerElement() || attrNode.ownerElement() == this);
+    if (attrNode.ownerElement() == this)
+        return;
+
+    NoEventDispatchAssertion assertNoEventDispatch;
+
+    attrNode.attachToElement(*this);
+    treeScope().adoptIfNeeded(attrNode);
+    ensureAttrNodeListForElement(*this).append(&attrNode);
+}
+
 ExceptionOr<RefPtr<Attr>> Element::setAttributeNode(Attr& attrNode)
 {
     RefPtr<Attr> oldAttrNode = attrIfExists(attrNode.localName(), shouldIgnoreAttributeCase(*this));
@@ -2132,13 +2145,23 @@
     if (attrNode.ownerElement() && attrNode.ownerElement() != this)
         return Exception { INUSE_ATTRIBUTE_ERR };
 
+    {
+    NoEventDispatchAssertion assertNoEventDispatch;
     synchronizeAllAttributes();
+    }
+
     auto& elementData = ensureUniqueElementData();
 
     auto existingAttributeIndex = elementData.findAttributeIndexByName(attrNode.localName(), shouldIgnoreAttributeCase(*this));
-    if (existingAttributeIndex == ElementData::attributeNotFound)
-        setAttributeInternal(elementData.findAttributeIndexByName(attrNode.qualifiedName()), attrNode.qualifiedName(), attrNode.value(), NotInSynchronizationOfLazyAttribute);
-    else {
+
+    // Attr::value() will return its 'm_standaloneValue' member any time its Element is set to nullptr. We need to cache this value
+    // before making changes to attrNode's Element connections.
+    auto attrNodeValue = attrNode.value();
+
+    if (existingAttributeIndex == ElementData::attributeNotFound) {
+        attachAttributeNodeIfNeeded(attrNode);
+        setAttributeInternal(elementData.findAttributeIndexByName(attrNode.qualifiedName()), attrNode.qualifiedName(), attrNodeValue, NotInSynchronizationOfLazyAttribute);
+    } else {
         const Attribute& attribute = attributeAt(existingAttributeIndex);
         if (oldAttrNode)
             detachAttrNodeFromElementWithValue(oldAttrNode.get(), attribute.value());
@@ -2145,18 +2168,16 @@
         else
             oldAttrNode = Attr::create(document(), attrNode.qualifiedName(), attribute.value());
 
+        attachAttributeNodeIfNeeded(attrNode);
+
         if (attribute.name().matches(attrNode.qualifiedName()))
-            setAttributeInternal(existingAttributeIndex, attrNode.qualifiedName(), attrNode.value(), NotInSynchronizationOfLazyAttribute);
+            setAttributeInternal(existingAttributeIndex, attrNode.qualifiedName(), attrNodeValue, NotInSynchronizationOfLazyAttribute);
         else {
             removeAttributeInternal(existingAttributeIndex, NotInSynchronizationOfLazyAttribute);
-            setAttributeInternal(ensureUniqueElementData().findAttributeIndexByName(attrNode.qualifiedName()), attrNode.qualifiedName(), attrNode.value(), NotInSynchronizationOfLazyAttribute);
+            setAttributeInternal(ensureUniqueElementData().findAttributeIndexByName(attrNode.qualifiedName()), attrNode.qualifiedName(), attrNodeValue, NotInSynchronizationOfLazyAttribute);
         }
     }
-    if (attrNode.ownerElement() != this) {
-        attrNode.attachToElement(*this);
-        treeScope().adoptIfNeeded(attrNode);
-        ensureAttrNodeListForElement(*this).append(&attrNode);
-    }
+
     return WTFMove(oldAttrNode);
 }
 
@@ -2171,10 +2192,19 @@
     if (attrNode.ownerElement() && attrNode.ownerElement() != this)
         return Exception { INUSE_ATTRIBUTE_ERR };
 
+    unsigned index = 0;
+    
+    // Attr::value() will return its 'm_standaloneValue' member any time its Element is set to nullptr. We need to cache this value
+    // before making changes to attrNode's Element connections.
+    auto attrNodeValue = attrNode.value();
+
+    {
+    NoEventDispatchAssertion assertNoEventDispatch;
     synchronizeAllAttributes();
     auto& elementData = ensureUniqueElementData();
 
-    auto index = elementData.findAttributeIndexByName(attrNode.qualifiedName());
+    index = elementData.findAttributeIndexByName(attrNode.qualifiedName());
+
     if (index != ElementData::attributeNotFound) {
         if (oldAttrNode)
             detachAttrNodeFromElementWithValue(oldAttrNode.get(), elementData.attributeAt(index).value());
@@ -2181,13 +2211,11 @@
         else
             oldAttrNode = Attr::create(document(), attrNode.qualifiedName(), elementData.attributeAt(index).value());
     }
+    }
 
-    setAttributeInternal(index, attrNode.qualifiedName(), attrNode.value(), NotInSynchronizationOfLazyAttribute);
+    attachAttributeNodeIfNeeded(attrNode);
+    setAttributeInternal(index, attrNode.qualifiedName(), attrNodeValue, NotInSynchronizationOfLazyAttribute);
 
-    attrNode.attachToElement(*this);
-    treeScope().adoptIfNeeded(attrNode);
-    ensureAttrNodeListForElement(*this).append(&attrNode);
-
     return WTFMove(oldAttrNode);
 }
 

Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Element.h (214807 => 214808)


--- releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Element.h	2017-04-03 17:08:12 UTC (rev 214807)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Element.h	2017-04-03 17:09:21 UTC (rev 214808)
@@ -3,7 +3,7 @@
  *           (C) 1999 Antti Koivisto (koivi...@kde.org)
  *           (C) 2001 Peter Kelly (p...@post.com)
  *           (C) 2001 Dirk Mueller (muel...@kde.org)
- * Copyright (C) 2003-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2003-2017 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -679,6 +679,8 @@
 
     // Anyone thinking of using this should call document instead of ownerDocument.
     void ownerDocument() const = delete;
+    
+    void attachAttributeNodeIfNeeded(Attr&);
 
     QualifiedName m_tagName;
     RefPtr<ElementData> m_elementData;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to