Changing id, className, or attribute should invalidate distribution
authorshinyak@chromium.org <shinyak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Nov 2012 18:14:04 +0000 (18:14 +0000)
committershinyak@chromium.org <shinyak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Nov 2012 18:14:04 +0000 (18:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=100738

Reviewed by Dimitri Glazkov.

PerformanceTests:

Added test code to modify id/class/attribute.

* DOM/ModifyAttribute.html: Added.
* DOM/resources/dom-perf/modify-attribute.js: Added.
(ModifyAttribute.CreateElementToSetUp):
(ModifyAttribute.ModifyId):
(ModifyAttribute.ModifyClass):
(ModifyAttribute.ModifyTitle):

Source/WebCore:

When id, className, or attribute is changed, we might have to invalidate distribution.
However, we don't want to do useless invalidation. So we consult with the RuleFeatureSet of ElementShadow
to invalidate distribution only if necessary.

For the code that className is changed, we can share a lot of code between invalidating distribution and
invalidating style. So we made checkNeedsStyleInvalidationForClassChange a template method, and share it.

Since attributeChanged() is a hot method, we don't want to make it slow. So we made one function to determine
whether we have to invalidate distribution, and make it called only if necessary. Also, we've optimized
shadowOfParentForDistribution() by making isInsertionPoint() de-virtualed. We consuded NodeFlags (IsInsertionPointFlag)
for this purpose.

We've measured how this patch makes changing attribute slow. I've measured each code 3 times.
DOM/ModifyAttribute.html is a micro benchmark which changes attribute a lot of times. The result of this benchmark
will be the most affected by this patch. However, it's only 2% performance regression.

DOM/ModifyAttribute.html
Before this patch:
        median  stdev    min    max    [ms]
  1st    494.0   3.36  490.0  502.0
  2nd    503.5   3.44  497.0  512.0
  3rd    494.0   3.48  488.0  499.0

After this patch:
        median  stdev  min      max    [ms]
  1st    504.0   2.00  501.0  509.0
  2nd    505.5   3.08  500.0  513.0
  3rd    507.0   2.32  502.0  510.0

Tests: fast/dom/shadow/distribution-attribute-modified.html
       fast/dom/shadow/distribution-className-modified.html
       fast/dom/shadow/distribution-id-modified.html
       fast/dom/shadow/reprojection-attribute-modified.html
       fast/dom/shadow/reprojection-className-modified.html
       fast/dom/shadow/reprojection-id-modified.html

* dom/Element.cpp:
(WebCore::Element::attributeChanged):
(WebCore::HasSelectorForClassStyleFunctor::HasSelectorForClassStyleFunctor):
(HasSelectorForClassStyleFunctor):
(WebCore::HasSelectorForClassStyleFunctor::operator()): Returns true if StyleResolver::hasSelectorForClass returns true.
(WebCore):
(WebCore::HasSelectorForClassDistributionFunctor::HasSelectorForClassDistributionFunctor):
(HasSelectorForClassDistributionFunctor):
(WebCore::HasSelectorForClassDistributionFunctor::operator()): Returns true if ElementShadow::hasSelectForClass returns true.
(WebCore::checkFunctorForClassChange):
(WebCore::checkNeedsStyleInvalidationForClassChange):
(WebCore::checkNeedsDistributionInvalidationForClassChange): Extracted the implementation to checkFunctorForClassChange.
(WebCore::Element::shouldInvalidateDistributionWhenAttributeChanged):
* dom/Element.h:
(Element):
* dom/Node.h:
(WebCore::Node::isInsertionPoint):
* html/HTMLElement.h:
(HTMLElement):
* html/shadow/InsertionPoint.cpp:
(WebCore::InsertionPoint::InsertionPoint):
* html/shadow/InsertionPoint.h:
(InsertionPoint):
(WebCore::isInsertionPoint):
(WebCore::shadowOfParentForDistribution):
(WebCore::resolveReprojection):

LayoutTests:

We have test cases that id/class/attribute is changed, and thier reprojection cases.

* fast/dom/shadow/distribution-attribute-modified-expected.html: Added.
* fast/dom/shadow/distribution-attribute-modified.html: Added.
* fast/dom/shadow/distribution-className-modified-expected.html: Added.
* fast/dom/shadow/distribution-className-modified.html: Added.
* fast/dom/shadow/distribution-id-modified-expected.html: Added.
* fast/dom/shadow/distribution-id-modified.html: Added.
* fast/dom/shadow/reprojection-attribute-modified-expected.html: Added.
* fast/dom/shadow/reprojection-attribute-modified.html: Added.
* fast/dom/shadow/reprojection-className-modified-expected.html: Added.
* fast/dom/shadow/reprojection-className-modified.html: Added.
* fast/dom/shadow/reprojection-id-modified-expected.html: Added.
* fast/dom/shadow/reprojection-id-modified.html: Added.

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

23 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/shadow/distribution-attribute-modified-expected.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/distribution-attribute-modified.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/distribution-className-modified-expected.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/distribution-className-modified.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/distribution-id-modified-expected.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/distribution-id-modified.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/reprojection-attribute-modified-expected.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/reprojection-attribute-modified.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/reprojection-className-modified-expected.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/reprojection-className-modified.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/reprojection-id-modified-expected.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/reprojection-id-modified.html [new file with mode: 0644]
PerformanceTests/ChangeLog
PerformanceTests/DOM/ModifyAttribute.html [new file with mode: 0644]
PerformanceTests/DOM/resources/dom-perf/modify-attribute.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/Node.h
Source/WebCore/html/HTMLElement.h
Source/WebCore/html/shadow/InsertionPoint.cpp
Source/WebCore/html/shadow/InsertionPoint.h

index ea1b2ce..705368c 100644 (file)
@@ -1,3 +1,25 @@
+2012-11-19  Shinya Kawanaka  <shinyak@chromium.org>
+
+        Changing id, className, or attribute should invalidate distribution
+        https://bugs.webkit.org/show_bug.cgi?id=100738
+
+        Reviewed by Dimitri Glazkov.
+
+        We have test cases that id/class/attribute is changed, and thier reprojection cases.
+
+        * fast/dom/shadow/distribution-attribute-modified-expected.html: Added.
+        * fast/dom/shadow/distribution-attribute-modified.html: Added.
+        * fast/dom/shadow/distribution-className-modified-expected.html: Added.
+        * fast/dom/shadow/distribution-className-modified.html: Added.
+        * fast/dom/shadow/distribution-id-modified-expected.html: Added.
+        * fast/dom/shadow/distribution-id-modified.html: Added.
+        * fast/dom/shadow/reprojection-attribute-modified-expected.html: Added.
+        * fast/dom/shadow/reprojection-attribute-modified.html: Added.
+        * fast/dom/shadow/reprojection-className-modified-expected.html: Added.
+        * fast/dom/shadow/reprojection-className-modified.html: Added.
+        * fast/dom/shadow/reprojection-id-modified-expected.html: Added.
+        * fast/dom/shadow/reprojection-id-modified.html: Added.
+
 2012-11-19  Nate Chapin  <japhet@chromium.org>
 
         Move empty loading to DocumentLoader, simplify FrameLoader::init()
diff --git a/LayoutTests/fast/dom/shadow/distribution-attribute-modified-expected.html b/LayoutTests/fast/dom/shadow/distribution-attribute-modified-expected.html
new file mode 100644 (file)
index 0000000..4c282c7
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<div id="host">
+    <div>F</div>
+</div>
+
+</html>
diff --git a/LayoutTests/fast/dom/shadow/distribution-attribute-modified.html b/LayoutTests/fast/dom/shadow/distribution-attribute-modified.html
new file mode 100644 (file)
index 0000000..b7c0e62
--- /dev/null
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<script src="resources/polyfill.js"></script>
+
+<div id="host">
+    <div>A</div>
+    <div>B</div>
+    <div>C</div>
+    <div>D</div>
+    <div>E</div>
+    <div id="F">F</div>
+</div>
+
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+var shadowRoot = new WebKitShadowRoot(host);
+shadowRoot.innerHTML = '<content select="div[title]"></content>';
+
+setTimeout(function() {
+    F.title = 'something';
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, 0);
+</script>
+</html>
diff --git a/LayoutTests/fast/dom/shadow/distribution-className-modified-expected.html b/LayoutTests/fast/dom/shadow/distribution-className-modified-expected.html
new file mode 100644 (file)
index 0000000..d9a695c
--- /dev/null
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<div id="host">
+    <div>A</div>
+    <div>D</div>
+    <div>F</div>
+</div>
+
+</html>
diff --git a/LayoutTests/fast/dom/shadow/distribution-className-modified.html b/LayoutTests/fast/dom/shadow/distribution-className-modified.html
new file mode 100644 (file)
index 0000000..d548e0d
--- /dev/null
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<script src="resources/polyfill.js"></script>
+
+<div id="host">
+    <div class="selected">A</div>
+    <div>B</div>
+    <div>C</div>
+    <div class="selected">D</div>
+    <div>E</div>
+    <div id="F">F</div>
+</div>
+
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+var shadowRoot = new WebKitShadowRoot(host);
+shadowRoot.innerHTML = '<content select=".selected"></content>';
+
+setTimeout(function() {
+    F.className = 'selected';
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, 0);
+</script>
+</html>
diff --git a/LayoutTests/fast/dom/shadow/distribution-id-modified-expected.html b/LayoutTests/fast/dom/shadow/distribution-id-modified-expected.html
new file mode 100644 (file)
index 0000000..4c282c7
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<div id="host">
+    <div>F</div>
+</div>
+
+</html>
diff --git a/LayoutTests/fast/dom/shadow/distribution-id-modified.html b/LayoutTests/fast/dom/shadow/distribution-id-modified.html
new file mode 100644 (file)
index 0000000..4e35ce7
--- /dev/null
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<script src="resources/polyfill.js"></script>
+
+<div id="host">
+    <div id="selected">A</div>
+    <div>B</div>
+    <div>C</div>
+    <div>D</div>
+    <div>E</div>
+    <div id="F">F</div>
+</div>
+
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+var shadowRoot = new WebKitShadowRoot(host);
+shadowRoot.innerHTML = '<content select="#selected"></content>';
+
+setTimeout(function() {
+    A = selected;
+    F.id = 'selected';    
+    A.id = '';
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, 0);
+</script>
+</html>
diff --git a/LayoutTests/fast/dom/shadow/reprojection-attribute-modified-expected.html b/LayoutTests/fast/dom/shadow/reprojection-attribute-modified-expected.html
new file mode 100644 (file)
index 0000000..2e5a421
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<div id="host"><div>
+    <div>F</div>
+</div></div>
+
+</html>
diff --git a/LayoutTests/fast/dom/shadow/reprojection-attribute-modified.html b/LayoutTests/fast/dom/shadow/reprojection-attribute-modified.html
new file mode 100644 (file)
index 0000000..335165b
--- /dev/null
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<script src="resources/polyfill.js"></script>
+
+<div id="host1">
+    <div>A</div>
+    <div>B</div>
+    <div>C</div>
+    <div>D</div>
+    <div>E</div>
+    <div id="F">F</div>
+</div>
+
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+var shadowRoot1 = new WebKitShadowRoot(host1);
+shadowRoot1.innerHTML = '<div id="host2"><shadow></shadow></div>';
+
+var host2 = shadowRoot1.getElementById('host2');
+var shadowRoot2 = new WebKitShadowRoot(host2);
+shadowRoot2.innerHTML = '<content select="div[title=selected]"></content>';
+
+setTimeout(function() {
+    F.title = 'selected';
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, 0);
+</script>
+</html>
diff --git a/LayoutTests/fast/dom/shadow/reprojection-className-modified-expected.html b/LayoutTests/fast/dom/shadow/reprojection-className-modified-expected.html
new file mode 100644 (file)
index 0000000..0ee8a60
--- /dev/null
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<div id="host"><div>
+    <div>A</div>
+    <div>D</div>
+    <div>F</div>
+</div></div>
+
+</html>
diff --git a/LayoutTests/fast/dom/shadow/reprojection-className-modified.html b/LayoutTests/fast/dom/shadow/reprojection-className-modified.html
new file mode 100644 (file)
index 0000000..973da66
--- /dev/null
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<script src="resources/polyfill.js"></script>
+
+<div id="host1">
+    <div class="selected">A</div>
+    <div>B</div>
+    <div>C</div>
+    <div class="selected">D</div>
+    <div>E</div>
+    <div id="F">F</div>
+</div>
+
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+var shadowRoot1 = new WebKitShadowRoot(host1);
+shadowRoot1.innerHTML = '<div id="host2"><shadow></shadow></div>';
+
+var host2 = shadowRoot1.getElementById('host2');
+var shadowRoot2 = new WebKitShadowRoot(host2);
+shadowRoot2.innerHTML = '<content select=".selected"></content>';
+
+setTimeout(function() {
+    F.className = 'selected';
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, 0);
+</script>
+</html>
diff --git a/LayoutTests/fast/dom/shadow/reprojection-id-modified-expected.html b/LayoutTests/fast/dom/shadow/reprojection-id-modified-expected.html
new file mode 100644 (file)
index 0000000..2e5a421
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<div id="host"><div>
+    <div>F</div>
+</div></div>
+
+</html>
diff --git a/LayoutTests/fast/dom/shadow/reprojection-id-modified.html b/LayoutTests/fast/dom/shadow/reprojection-id-modified.html
new file mode 100644 (file)
index 0000000..4000b84
--- /dev/null
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+
+<html>
+<body>
+
+<script src="resources/polyfill.js"></script>
+
+<div id="host1">
+    <div id="selected">A</div>
+    <div>B</div>
+    <div>C</div>
+    <div>D</div>
+    <div>E</div>
+    <div id="F">F</div>
+</div>
+
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+var shadowRoot1 = new WebKitShadowRoot(host1);
+shadowRoot1.innerHTML = '<div id="host2"><shadow></shadow></div>';
+
+var host2 = shadowRoot1.getElementById('host2');
+var shadowRoot2 = new WebKitShadowRoot(host2);
+shadowRoot2.innerHTML = '<content select="#selected"></content>';
+
+setTimeout(function() {
+    A = selected;
+    A.id = '';
+    F.id = 'selected';
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, 0);
+</script>
+</html>
index 6eddee6..3bc5a1e 100644 (file)
@@ -1,3 +1,19 @@
+2012-11-19  Shinya Kawanaka  <shinyak@chromium.org>
+
+        Changing id, className, or attribute should invalidate distribution
+        https://bugs.webkit.org/show_bug.cgi?id=100738
+
+        Reviewed by Dimitri Glazkov.
+
+        Added test code to modify id/class/attribute.
+
+        * DOM/ModifyAttribute.html: Added.
+        * DOM/resources/dom-perf/modify-attribute.js: Added.
+        (ModifyAttribute.CreateElementToSetUp):
+        (ModifyAttribute.ModifyId):
+        (ModifyAttribute.ModifyClass):
+        (ModifyAttribute.ModifyTitle):
+
 2012-11-14  Ryosuke Niwa  <rniwa@webkit.org>
 
         runCount in runner.js should be renamed to iterationCount
diff --git a/PerformanceTests/DOM/ModifyAttribute.html b/PerformanceTests/DOM/ModifyAttribute.html
new file mode 100644 (file)
index 0000000..d1e259c
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="container"><span id="benchmark_content"></span></div>
+<script type="text/javascript" src="../resources/runner.js"></script>
+<script type="text/javascript" src="resources/dom-perf.js"></script>
+<script type="text/javascript" src="resources/dom-perf/modify-attribute.js"></script>
+<script>runBenchmarkSuite(ModifyAttributeTest);</script>
+</body>
+</html>
diff --git a/PerformanceTests/DOM/resources/dom-perf/modify-attribute.js b/PerformanceTests/DOM/resources/dom-perf/modify-attribute.js
new file mode 100644 (file)
index 0000000..a3658e9
--- /dev/null
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2012 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+// ModifyAttribute - measure attribute modification performance
+
+var ModifyAttribute = {};
+var nLoops = 500000;
+
+ModifyAttribute.CreateElementToSetUp = function() {
+    return document.createElement('div');
+}
+
+ModifyAttribute.ModifyId = function(element) {
+    var nLoops = window.nLoops;
+    var idValue1 = 'id1';
+    var idValue2 = 'id2';
+
+    for (var i = 0; i < nLoops; ++i) {
+        element.id = idValue1;
+        element.id = idValue2;                        
+    }
+}
+
+ModifyAttribute.ModifyClass = function(element) {
+    var nLoops = window.nLoops;
+    var className1 = 'class1';
+    var className2 = 'class2';
+
+    for (var i = 0; i < nLoops; ++i) {
+        element.className = className1;
+        element.className = className2;
+    }
+}
+
+ModifyAttribute.ModifyTitle = function(element) {
+    var nLoops = window.nLoops;
+    var title1 = 'title1';
+    var title2 = 'title2';
+
+    for (var i = 0; i < nLoops; ++i) {
+        element.title = title1;
+        element.title = title2;
+    }
+}
+
+var ModifyAttributeTest = new BenchmarkSuite('ModifyAttribute', [
+    new Benchmark("Modify id", ModifyAttribute.ModifyId, ModifyAttribute.CreateElementToSetUp),
+    new Benchmark("Modify class", ModifyAttribute.ModifyClass, ModifyAttribute.CreateElementToSetUp),
+    new Benchmark("Modify title", ModifyAttribute.ModifyTitle, ModifyAttribute.CreateElementToSetUp)
+]);
index 9ea14c4..89d745b 100644 (file)
@@ -1,3 +1,73 @@
+2012-11-19  Shinya Kawanaka  <shinyak@chromium.org>
+
+        Changing id, className, or attribute should invalidate distribution
+        https://bugs.webkit.org/show_bug.cgi?id=100738
+
+        Reviewed by Dimitri Glazkov.
+
+        When id, className, or attribute is changed, we might have to invalidate distribution.
+        However, we don't want to do useless invalidation. So we consult with the RuleFeatureSet of ElementShadow
+        to invalidate distribution only if necessary.
+
+        For the code that className is changed, we can share a lot of code between invalidating distribution and
+        invalidating style. So we made checkNeedsStyleInvalidationForClassChange a template method, and share it.
+
+        Since attributeChanged() is a hot method, we don't want to make it slow. So we made one function to determine
+        whether we have to invalidate distribution, and make it called only if necessary. Also, we've optimized
+        shadowOfParentForDistribution() by making isInsertionPoint() de-virtualed. We consuded NodeFlags (IsInsertionPointFlag)
+        for this purpose.
+
+        We've measured how this patch makes changing attribute slow. I've measured each code 3 times.
+        DOM/ModifyAttribute.html is a micro benchmark which changes attribute a lot of times. The result of this benchmark
+        will be the most affected by this patch. However, it's only 2% performance regression.
+
+        DOM/ModifyAttribute.html
+        Before this patch:
+                median  stdev    min    max    [ms]
+          1st    494.0   3.36  490.0  502.0
+          2nd    503.5   3.44  497.0  512.0
+          3rd    494.0   3.48  488.0  499.0
+
+        After this patch:
+                median  stdev  min      max    [ms]
+          1st    504.0   2.00  501.0  509.0
+          2nd    505.5   3.08  500.0  513.0
+          3rd    507.0   2.32  502.0  510.0
+
+        Tests: fast/dom/shadow/distribution-attribute-modified.html
+               fast/dom/shadow/distribution-className-modified.html
+               fast/dom/shadow/distribution-id-modified.html
+               fast/dom/shadow/reprojection-attribute-modified.html
+               fast/dom/shadow/reprojection-className-modified.html
+               fast/dom/shadow/reprojection-id-modified.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::attributeChanged):
+        (WebCore::HasSelectorForClassStyleFunctor::HasSelectorForClassStyleFunctor):
+        (HasSelectorForClassStyleFunctor):
+        (WebCore::HasSelectorForClassStyleFunctor::operator()): Returns true if StyleResolver::hasSelectorForClass returns true.
+        (WebCore):
+        (WebCore::HasSelectorForClassDistributionFunctor::HasSelectorForClassDistributionFunctor):
+        (HasSelectorForClassDistributionFunctor):
+        (WebCore::HasSelectorForClassDistributionFunctor::operator()): Returns true if ElementShadow::hasSelectForClass returns true.
+        (WebCore::checkFunctorForClassChange):
+        (WebCore::checkNeedsStyleInvalidationForClassChange):
+        (WebCore::checkNeedsDistributionInvalidationForClassChange): Extracted the implementation to checkFunctorForClassChange.
+        (WebCore::Element::shouldInvalidateDistributionWhenAttributeChanged):
+        * dom/Element.h:
+        (Element):
+        * dom/Node.h:
+        (WebCore::Node::isInsertionPoint):
+        * html/HTMLElement.h:
+        (HTMLElement):
+        * html/shadow/InsertionPoint.cpp:
+        (WebCore::InsertionPoint::InsertionPoint):
+        * html/shadow/InsertionPoint.h:
+        (InsertionPoint):
+        (WebCore::isInsertionPoint):
+        (WebCore::shadowOfParentForDistribution):
+        (WebCore::resolveReprojection):
+
 2012-11-19  Nate Chapin  <japhet@chromium.org>
 
         Move empty loading to DocumentLoader, simplify FrameLoader::init()
index 9da5cb4..c4c3b8d 100644 (file)
@@ -53,6 +53,7 @@
 #include "HTMLOptionsCollection.h"
 #include "HTMLParserIdioms.h"
 #include "HTMLTableRowsCollection.h"
+#include "InsertionPoint.h"
 #include "InspectorInstrumentation.h"
 #include "MutationObserverInterestGroup.h"
 #include "MutationRecord.h"
@@ -758,6 +759,11 @@ static bool checkNeedsStyleInvalidationForIdChange(const AtomicString& oldId, co
 
 void Element::attributeChanged(const QualifiedName& name, const AtomicString& newValue)
 {
+    if (ElementShadow* parentElementShadow = shadowOfParentForDistribution(this)) {
+        if (shouldInvalidateDistributionWhenAttributeChanged(parentElementShadow, name, newValue))
+            parentElementShadow->invalidateDistribution();
+    }
+
     parseAttribute(name, newValue);
 
     document()->incDOMTreeVersion();
@@ -818,21 +824,49 @@ static inline bool classStringHasClassName(const AtomicString& newClassString)
     return classStringHasClassName(newClassString.characters16(), length);
 }
 
-static bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& changedClasses, StyleResolver* styleResolver)
+struct HasSelectorForClassStyleFunctor {
+    explicit HasSelectorForClassStyleFunctor(StyleResolver* resolver)
+        : styleResolver(resolver)
+    { }
+
+    bool operator()(const AtomicString& className) const
+    {
+        return styleResolver->hasSelectorForClass(className);
+    }
+
+    StyleResolver* styleResolver;
+};
+
+struct HasSelectorForClassDistributionFunctor {
+    explicit HasSelectorForClassDistributionFunctor(ElementShadow* elementShadow)
+        : elementShadow(elementShadow)
+    { }
+
+    bool operator()(const AtomicString& className) const
+    {
+        return elementShadow->selectRuleFeatureSet().hasSelectorForClass(className);
+    }
+
+    ElementShadow* elementShadow;
+};
+
+template<typename Functor>
+static bool checkFunctorForClassChange(const SpaceSplitString& changedClasses, Functor functor)
 {
     unsigned changedSize = changedClasses.size();
     for (unsigned i = 0; i < changedSize; ++i) {
-        if (styleResolver->hasSelectorForClass(changedClasses[i]))
+        if (functor(changedClasses[i]))
             return true;
     }
     return false;
 }
 
-static bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses, StyleResolver* styleResolver)
+template<typename Functor>
+static bool checkFunctorForClassChange(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses, Functor functor)
 {
     unsigned oldSize = oldClasses.size();
     if (!oldSize)
-        return checkNeedsStyleInvalidationForClassChange(newClasses, styleResolver);
+        return checkFunctorForClassChange(newClasses, functor);
     BitVector remainingClassBits;
     remainingClassBits.ensureSize(oldSize);
     // Class vectors tend to be very short. This is faster than using a hash table.
@@ -844,19 +878,39 @@ static bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& ol
                 continue;
             }
         }
-        if (styleResolver->hasSelectorForClass(newClasses[i]))
+        if (functor(newClasses[i]))
             return true;
     }
     for (unsigned i = 0; i < oldSize; ++i) {
         // If the bit is not set the the corresponding class has been removed.
         if (remainingClassBits.quickGet(i))
             continue;
-        if (styleResolver->hasSelectorForClass(oldClasses[i]))
+        if (functor(oldClasses[i]))
             return true;
     }
     return false;
 }
 
+static inline bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& changedClasses, StyleResolver* styleResolver)
+{
+    return checkFunctorForClassChange(changedClasses, HasSelectorForClassStyleFunctor(styleResolver));
+}
+
+static inline bool checkNeedsStyleInvalidationForClassChange(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses, StyleResolver* styleResolver)
+{
+    return checkFunctorForClassChange(oldClasses, newClasses, HasSelectorForClassStyleFunctor(styleResolver));
+}
+
+static inline bool checkNeedsDistributionInvalidationForClassChange(const SpaceSplitString& changedClasses, ElementShadow* elementShadow)
+{
+    return checkFunctorForClassChange(changedClasses, HasSelectorForClassDistributionFunctor(elementShadow));
+}
+
+static inline bool checkNeedsDistributionInvalidationForClassChange(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses, ElementShadow* elementShadow)
+{
+    return checkFunctorForClassChange(oldClasses, newClasses, HasSelectorForClassDistributionFunctor(elementShadow));
+}
+
 void Element::classAttributeChanged(const AtomicString& newClassString)
 {
     StyleResolver* styleResolver = document()->styleResolverIfExists();
@@ -886,6 +940,41 @@ void Element::classAttributeChanged(const AtomicString& newClassString)
         setNeedsStyleRecalc();
 }
 
+bool Element::shouldInvalidateDistributionWhenAttributeChanged(ElementShadow* elementShadow, const QualifiedName& name, const AtomicString& newValue)
+{
+    ASSERT(elementShadow);
+    elementShadow->ensureSelectFeatureSetCollected();
+
+    if (isIdAttributeName(name)) {
+        AtomicString oldId = attributeData()->idForStyleResolution();
+        AtomicString newId = makeIdForStyleResolution(newValue, document()->inQuirksMode());
+        if (newId != oldId) {
+            if (!oldId.isEmpty() && elementShadow->selectRuleFeatureSet().hasSelectorForId(oldId))
+                return true;
+            if (!newId.isEmpty() && elementShadow->selectRuleFeatureSet().hasSelectorForId(newId))
+                return true;
+        }
+    }
+
+    if (name == HTMLNames::classAttr) {
+        const AtomicString& newClassString = newValue;
+        if (classStringHasClassName(newClassString)) {
+            const ElementAttributeData* attributeData = ensureAttributeData();
+            const bool shouldFoldCase = document()->inQuirksMode();
+            const SpaceSplitString& oldClasses = attributeData->classNames();
+            const SpaceSplitString newClasses(newClassString, shouldFoldCase);
+            if (checkNeedsDistributionInvalidationForClassChange(oldClasses, newClasses, elementShadow))
+                return true;
+        } else if (const ElementAttributeData* attributeData = this->attributeData()) {
+            const SpaceSplitString& oldClasses = attributeData->classNames();
+            if (checkNeedsDistributionInvalidationForClassChange(oldClasses, elementShadow))
+                return true;
+        }
+    }
+
+    return elementShadow->selectRuleFeatureSet().hasSelectorForAttribute(name.localName());
+}
+
 // Returns true is the given attribute is an event handler.
 // We consider an event handler any attribute that begins with "on".
 // It is a simple solution that has the advantage of not requiring any
index 7fdb316..2f24bd6 100644 (file)
@@ -523,6 +523,8 @@ private:
 
     void createMutableAttributeData();
 
+    bool shouldInvalidateDistributionWhenAttributeChanged(ElementShadow*, const QualifiedName&, const AtomicString&);
+
 private:
     ElementRareData* elementRareData() const;
     ElementRareData* ensureElementRareData();
index a98bcc8..2d9cacd 100644 (file)
@@ -235,6 +235,7 @@ public:
     virtual bool isPluginElement() const { return false; }
     bool isDocumentNode() const;
     bool isShadowRoot() const { return getFlag(IsShadowRootFlag); }
+    bool isInsertionPoint() const { return getFlag(IsInsertionPointFlag); }
     bool inNamedFlow() const { return getFlag(InNamedFlowFlag); }
     bool hasCustomCallbacks() const { return getFlag(HasCustomCallbacksFlag); }
 
@@ -724,6 +725,7 @@ private:
         HasScopedHTMLStyleChildFlag = 1 << 26,
         HasEventTargetDataFlag = 1 << 27,
         InEdenFlag = 1 << 28,
+        IsInsertionPointFlag = 1 << 29,
 
 #if ENABLE(SVG)
         DefaultNodeFlags = IsParsingChildrenFinishedFlag | AreSVGAttributesValidFlag,
@@ -732,7 +734,7 @@ private:
 #endif
     };
 
-    // 3 bits remaining
+    // 2 bits remaining
 
     bool getFlag(NodeFlags mask) const { return m_nodeFlags & mask; }
     void setFlag(bool f, NodeFlags mask) const { m_nodeFlags = (m_nodeFlags & ~mask) | (-(int32_t)f & mask); } 
@@ -750,7 +752,8 @@ protected:
         CreateHTMLElement = CreateStyledElement | IsHTMLFlag, 
         CreateFrameOwnerElement = CreateHTMLElement | HasCustomCallbacksFlag,
         CreateSVGElement = CreateStyledElement | IsSVGFlag,
-        CreateDocument = CreateContainer | InDocumentFlag
+        CreateDocument = CreateContainer | InDocumentFlag,
+        CreateInsertionPoint = CreateHTMLElement | IsInsertionPointFlag
     };
     Node(Document*, ConstructionType);
 
index e837df4..28521a8 100644 (file)
@@ -106,7 +106,6 @@ public:
     virtual bool isHTMLUnknownElement() const { return false; }
 #endif
 
-    virtual bool isInsertionPoint() const { return false; }
     virtual bool isLabelable() const { return false; }
 
 protected:
index d34a079..0443683 100644 (file)
@@ -38,7 +38,7 @@
 namespace WebCore {
 
 InsertionPoint::InsertionPoint(const QualifiedName& tagName, Document* document)
-    : HTMLElement(tagName, document)
+    : HTMLElement(tagName, document, CreateInsertionPoint)
     , m_shouldResetStyleInheritance(false)
 {
 }
index 23dad8d..f1c0ac7 100644 (file)
@@ -62,7 +62,6 @@ public:
 
     virtual void attach();
     virtual void detach();
-    virtual bool isInsertionPoint() const OVERRIDE { return true; }
 
     bool shouldUseFallbackElements() const;
 
@@ -89,10 +88,8 @@ private:
 
 inline bool isInsertionPoint(const Node* node)
 {
-    if (node->isHTMLElement() && toHTMLElement(node)->isInsertionPoint())
-        return true;
-
-    return false;
+    ASSERT(node);
+    return node->isInsertionPoint();
 }
 
 inline InsertionPoint* toInsertionPoint(Node* node)
@@ -144,9 +141,7 @@ inline Element* parentElementForDistribution(const Node* node)
 
 inline ElementShadow* shadowOfParentForDistribution(const Node* node)
 {
-    if (!node)
-        return 0;
-
+    ASSERT(node);
     if (Element* parent = parentElementForDistribution(node))
         return parent->shadow();
 
@@ -158,7 +153,7 @@ inline InsertionPoint* resolveReprojection(const Node* projectedNode)
     InsertionPoint* insertionPoint = 0;
     const Node* current = projectedNode;
 
-    while (true) {
+    while (current) {
         if (ElementShadow* shadow = shadowOfParentForDistribution(current)) {
             shadow->ensureDistribution();
             if (InsertionPoint* insertedTo = shadow->distributor().findInsertionPointFor(projectedNode)) {