REGRESSION(r103452): 100% CPU usage and 5s pause after clicking on a link in Yahoo...
authoradamk@chromium.org <adamk@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Mar 2012 01:24:07 +0000 (01:24 +0000)
committeradamk@chromium.org <adamk@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Mar 2012 01:24:07 +0000 (01:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=81141

Reviewed by Ojan Vafai.

Source/WebCore:

Revert the behavior change from r103452: don't fire DOMSubtreeModified
events when an attribute value merely changes. Still fire that event
when an attribute is added or removed from an element.

Though this contradicts the DOM3 spec, it matches legacy WebKit behavior,
and given that Mutation Events are deprecated, it seems unwise to make
any additions to WebKit's implementation of them.

Test: fast/dom/subtree-modified-attributes.html

* dom/Element.cpp:
(WebCore::Element::didAddAttribute): Renamed from didModifyAttribute.
(WebCore::Element::didModifyAttribute): Remove the call to dispatchSubtreeModifiedEvent.
(WebCore):
* dom/Element.h:
(Element):
* dom/ElementAttributeData.cpp:
(WebCore::ElementAttributeData::addAttribute): Call didAddAttribute instead of didModifyAttribute.

LayoutTests:

* fast/dom/subtree-modified-attributes-expected.txt: Added.
* fast/dom/subtree-modified-attributes.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/subtree-modified-attributes-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/subtree-modified-attributes.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/ElementAttributeData.cpp

index 2e5e001..f9b0b40 100644 (file)
@@ -1,3 +1,13 @@
+2012-03-15  Adam Klein  <adamk@chromium.org>
+
+        REGRESSION(r103452): 100% CPU usage and 5s pause after clicking on a link in Yahoo Mail
+        https://bugs.webkit.org/show_bug.cgi?id=81141
+
+        Reviewed by Ojan Vafai.
+
+        * fast/dom/subtree-modified-attributes-expected.txt: Added.
+        * fast/dom/subtree-modified-attributes.html: Added.
+
 2012-03-15  Levi Weintraub  <leviw@chromium.org>
 
         Unreviewed gardening. Fixing Chromium expectations after we began falling back to the failing
diff --git a/LayoutTests/fast/dom/subtree-modified-attributes-expected.txt b/LayoutTests/fast/dom/subtree-modified-attributes-expected.txt
new file mode 100644 (file)
index 0000000..8c17db1
--- /dev/null
@@ -0,0 +1,13 @@
+DOMSubtreeModified should fire when attributes are added or removed, but not modified (see bug 81141)
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS expected is true
+PASS expected is true
+PASS expected is true
+PASS expected is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/subtree-modified-attributes.html b/LayoutTests/fast/dom/subtree-modified-attributes.html
new file mode 100644 (file)
index 0000000..d143473
--- /dev/null
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<body>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+description('DOMSubtreeModified should fire when attributes are added or removed, but not modified (see bug 81141)');
+
+var div = document.createElement('div');
+document.body.appendChild(div);
+var expected = false;
+div.addEventListener('DOMSubtreeModified', function(evt) {
+    shouldBeTrue('expected');
+});
+expected = true;
+div.setAttribute('foo', 'bar');
+expected = false;
+div.setAttribute('foo', 'baz');
+expected = true;
+div.removeAttribute('foo');
+
+var attr = document.createAttribute('bar');
+attr.value = 'foo';
+expected = true;
+div.setAttributeNode(attr);
+expected = false;
+attr.value = 'bar';
+expected = true;
+div.removeAttributeNode(attr);
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
index 1fb844d..b44b0fe 100644 (file)
@@ -1,3 +1,29 @@
+2012-03-15  Adam Klein  <adamk@chromium.org>
+
+        REGRESSION(r103452): 100% CPU usage and 5s pause after clicking on a link in Yahoo Mail
+        https://bugs.webkit.org/show_bug.cgi?id=81141
+
+        Reviewed by Ojan Vafai.
+
+        Revert the behavior change from r103452: don't fire DOMSubtreeModified
+        events when an attribute value merely changes. Still fire that event
+        when an attribute is added or removed from an element.
+
+        Though this contradicts the DOM3 spec, it matches legacy WebKit behavior,
+        and given that Mutation Events are deprecated, it seems unwise to make
+        any additions to WebKit's implementation of them.
+
+        Test: fast/dom/subtree-modified-attributes.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::didAddAttribute): Renamed from didModifyAttribute.
+        (WebCore::Element::didModifyAttribute): Remove the call to dispatchSubtreeModifiedEvent.
+        (WebCore):
+        * dom/Element.h:
+        (Element):
+        * dom/ElementAttributeData.cpp:
+        (WebCore::ElementAttributeData::addAttribute): Call didAddAttribute instead of didModifyAttribute.
+
 2012-03-15  Dana Jansens  <danakj@chromium.org>
 
         [chromium] Move overdraw metrics into a templated class for both paint and draw metrics.
index c697f00..2ba7ebf 100644 (file)
@@ -2005,14 +2005,20 @@ void Element::willModifyAttribute(const QualifiedName& name, const AtomicString&
 #endif
 }
 
-void Element::didModifyAttribute(Attribute* attr)
+void Element::didAddAttribute(Attribute* attr)
 {
     attributeChanged(attr);
-
     InspectorInstrumentation::didModifyDOMAttr(document(), this, attr->name().localName(), attr->value());
     dispatchSubtreeModifiedEvent();
 }
 
+void Element::didModifyAttribute(Attribute* attr)
+{
+    attributeChanged(attr);
+    InspectorInstrumentation::didModifyDOMAttr(document(), this, attr->name().localName(), attr->value());
+    // Do not dispatch a DOMSubtreeModified event here; see bug 81141.
+}
+
 void Element::didRemoveAttribute(Attribute* attr)
 {
     if (attr->isNull())
index 7355098..4e563ff 100644 (file)
@@ -295,6 +295,7 @@ public:
 
     void willModifyAttribute(const QualifiedName&, const AtomicString& oldValue, const AtomicString& newValue);
     void willRemoveAttribute(const QualifiedName&, const AtomicString& value);
+    void didAddAttribute(Attribute*);
     void didModifyAttribute(Attribute*);
     void didRemoveAttribute(Attribute*);
 
index 141ec7e..d32981f 100644 (file)
@@ -105,7 +105,7 @@ void ElementAttributeData::addAttribute(PassRefPtr<Attribute> prpAttribute, Elem
         attr->m_element = element;
 
     if (element && inUpdateStyleAttribute == NotInUpdateStyleAttribute)
-        element->didModifyAttribute(attribute.get());
+        element->didAddAttribute(attribute.get());
 }
 
 void ElementAttributeData::removeAttribute(size_t index, Element* element, EInUpdateStyleAttribute inUpdateStyleAttribute)