Title: [140299] trunk
Revision
140299
Author
[email protected]
Date
2013-01-20 22:59:35 -0800 (Sun, 20 Jan 2013)

Log Message

Distribution state becomes inconsistent with content/shadow reprojection
https://bugs.webkit.org/show_bug.cgi?id=106634

Reviewed by Hajime Morita.

Source/WebCore:

Distribution should be resolved from shallower ShadowDOM to deeper Shadow DOM. However, in the current implementation,
there is a case that distribution for deeper ShadowDOM happens to be resolved before distribution
for shallower ShadowDOM is resolved.

Here, we have 2 problems about distribution.
1) Invalidation state is not propagated to nested (= deeper) ShadowDOM.
    - This causes deeper ShadowDOM looks having a valid distribution though it should be invalid.
2) We are not resolving shallower ShadowDOM when deeper ShadowDOM's distribution is needed.
    - Because of (1), we have to check all the ancestor ShadowDOM.

For (1), we change invalidate() to invalidate nested ShadowDOM's distribution as well.
For (2), when resolving distribution, we will check the ancestor ShadowDOM's distribution state. If the ancestor's
distribution is not valid, we resolve it first.

For optimization of (1), actually we can skip invalidating distribution of some ShadowDOMs.
If ShadowRoot of deeper ShadowDOM does not have an InsertionPoint as children, we can skip invalidating
its distribution, because only children can be distributed to InsertionPoint.

Tests: fast/dom/shadow/distribution-crash.html
       fast/dom/shadow/nested-reprojection-inconsistent.html

* dom/ElementShadow.cpp:
(WebCore::ElementShadow::attach): Should resolve distribution from ancestor.
* dom/ElementShadow.h:
* html/shadow/ContentDistributor.cpp:
(WebCore::ContentDistributor::distribute): Added ASSERT that the parent ShadowRoot's distribution is resolved.
(WebCore::ContentDistributor::invalidate): For each InsertionPoint, we have to invalidate
its parent element's distribution (if it has ElementShadow).
(WebCore::ContentDistributor::ensureDistribution):
* html/shadow/ContentDistributor.h:
(WebCore::ContentDistributor::isValid):
* html/shadow/HTMLShadowElement.cpp:
(WebCore::HTMLShadowElement::olderShadowRoot): Should resolve distribution from ancestor.
* html/shadow/InsertionPoint.cpp:
(WebCore::InsertionPoint::attach): ditto.
(WebCore::InsertionPoint::detach): ditto.
(WebCore::InsertionPoint::getDistributedNodes): ditto.
(WebCore::resolveReprojection): ditto.

LayoutTests:

* fast/dom/shadow/distribution-crash-expected.txt: Added.
* fast/dom/shadow/distribution-crash.html: Added.
* fast/dom/shadow/nested-reprojection-inconsistent-expected.txt: Added.
* fast/dom/shadow/nested-reprojection-inconsistent.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (140298 => 140299)


--- trunk/LayoutTests/ChangeLog	2013-01-21 06:48:25 UTC (rev 140298)
+++ trunk/LayoutTests/ChangeLog	2013-01-21 06:59:35 UTC (rev 140299)
@@ -1,3 +1,15 @@
+2013-01-20  Shinya Kawanaka  <[email protected]>
+
+        Distribution state becomes inconsistent with content/shadow reprojection
+        https://bugs.webkit.org/show_bug.cgi?id=106634
+
+        Reviewed by Hajime Morita.
+
+        * fast/dom/shadow/distribution-crash-expected.txt: Added.
+        * fast/dom/shadow/distribution-crash.html: Added.
+        * fast/dom/shadow/nested-reprojection-inconsistent-expected.txt: Added.
+        * fast/dom/shadow/nested-reprojection-inconsistent.html: Added.
+
 2013-01-20  Kent Tamura  <[email protected]>
 
         Re-layout child blocks when border/padding of the box-sizing:border-box parent is updated

Added: trunk/LayoutTests/fast/dom/shadow/distribution-crash-expected.txt (0 => 140299)


--- trunk/LayoutTests/fast/dom/shadow/distribution-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/shadow/distribution-crash-expected.txt	2013-01-21 06:59:35 UTC (rev 140299)
@@ -0,0 +1,8 @@
+When modifying InsertionPoint's child, distribution should not cause crash.
+
+PASS unless crash.
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/dom/shadow/distribution-crash.html (0 => 140299)


--- trunk/LayoutTests/fast/dom/shadow/distribution-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/shadow/distribution-crash.html	2013-01-21 06:59:35 UTC (rev 140299)
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<html><body>
+<script src=""
+
+<p>When modifying InsertionPoint's child, distribution should not cause crash.</p>
+<p>PASS unless crash.</p>
+
+<div id="host"></div>
+<pre id="console"></pre>
+
+<script>
+jsTestIsAsync = true;
+
+function createElementLikeDetails()
+{
+    var details = document.createElement('div');
+    var shadowRoot = details.webkitCreateShadowRoot();
+
+    shadowRoot.innerHTML = '<content select="div.summary"></content><content></content>';
+
+    var defaultSummary = document.createElement('div');
+    defaultSummary.webkitCreateShadowRoot().innerHTML = 'Default Summary';
+    shadowRoot.firstChild.appendChild(defaultSummary);
+
+    return details;    
+}
+
+var shadowRoot1 = host.webkitCreateShadowRoot();
+shadowRoot1.innerHTML = 'something 1';
+
+var shadowRoot2 = host.webkitCreateShadowRoot();
+var details = createElementLikeDetails();
+details.innerHTML = '<shadow id="shadow">something 2</shadow>';
+shadowRoot2.appendChild(details);
+
+var shadow = details.firstChild;
+setTimeout(function() {
+    shadow.firstChild.remove();
+    finishJSTest();
+}, 0);
+</script>
+
+<script src=""
+</body></html>

Added: trunk/LayoutTests/fast/dom/shadow/nested-reprojection-inconsistent-expected.txt (0 => 140299)


--- trunk/LayoutTests/fast/dom/shadow/nested-reprojection-inconsistent-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/shadow/nested-reprojection-inconsistent-expected.txt	2013-01-21 06:59:35 UTC (rev 140299)
@@ -0,0 +1,26 @@
+When we modify host children and get distributed nodes in nested ShadowDOM, distribution should occur from the host.
+
+Adds a div to ShadowRoot.
+PASS content.getDistributedNodes().length is 2
+PASS content.getDistributedNodes().item(0) is div1
+PASS content.getDistributedNodes().item(1) is addedDiv
+
+Adds a div as fallback content
+PASS content.getDistributedNodes().length is 3
+PASS content.getDistributedNodes().item(0) is div1
+PASS content.getDistributedNodes().item(1) is anotherAddedDiv
+PASS content.getDistributedNodes().item(2) is addedDiv
+
+Removes the first added div
+PASS content.getDistributedNodes().length is 2
+PASS content.getDistributedNodes().item(0) is div1
+PASS content.getDistributedNodes().item(1) is anotherAddedDiv
+
+Removes the second added div
+PASS content.getDistributedNodes().length is 1
+PASS content.getDistributedNodes().item(0) is div1
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/dom/shadow/nested-reprojection-inconsistent.html (0 => 140299)


--- trunk/LayoutTests/fast/dom/shadow/nested-reprojection-inconsistent.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/shadow/nested-reprojection-inconsistent.html	2013-01-21 06:59:35 UTC (rev 140299)
@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<html><body>
+<script src=""
+
+<p>When we modify host children and get distributed nodes in nested ShadowDOM, distribution should occur from the host.</p>
+
+<div id="container">
+    <div id="host1"></div>
+</div>
+<pre id="console"></pre>
+
+<script>
+jsTestIsAsync = true;
+
+var shadowRoot11 = host1.webkitCreateShadowRoot();
+shadowRoot11.innerHTML = '<div></div><shadow></shadow>';
+var div1 = shadowRoot11.firstChild;
+var shadow1 = shadowRoot11.lastChild;
+
+var shadowRoot12 = host1.webkitCreateShadowRoot();
+shadowRoot12.innerHTML = '<div><shadow></shadow></div>';
+var host2 = shadowRoot12.firstChild;
+
+var shadowRoot21 = host2.webkitCreateShadowRoot();
+shadowRoot21.innerHTML = '<content></content>';
+
+var shadowRoot22 = host2.webkitCreateShadowRoot();
+shadowRoot22.innerHTML = '<div><shadow></shadow></div>';
+var host3 = shadowRoot22.firstChild;
+
+var shadowRoot31 = host3.webkitCreateShadowRoot();
+shadowRoot31.innerHTML = '<content></content>';
+
+var content = shadowRoot31.firstChild;
+
+setTimeout(function() {
+    debug('Adds a div to ShadowRoot.');
+    addedDiv = document.createElement('div');
+    shadowRoot11.appendChild(addedDiv);
+    shouldBe('content.getDistributedNodes().length', '2');
+    shouldBe('content.getDistributedNodes().item(0)', 'div1');
+    shouldBe('content.getDistributedNodes().item(1)', 'addedDiv');
+    debug('');
+
+
+    debug('Adds a div as fallback content');
+    anotherAddedDiv = document.createElement('div');
+    shadow1.appendChild(anotherAddedDiv);
+    shouldBe('content.getDistributedNodes().length', '3');
+    shouldBe('content.getDistributedNodes().item(0)', 'div1');
+    shouldBe('content.getDistributedNodes().item(1)', 'anotherAddedDiv');
+    shouldBe('content.getDistributedNodes().item(2)', 'addedDiv');
+    debug('');
+
+    debug('Removes the first added div');
+    addedDiv.remove();
+    shouldBe('content.getDistributedNodes().length', '2');
+    shouldBe('content.getDistributedNodes().item(0)', 'div1');
+    shouldBe('content.getDistributedNodes().item(1)', 'anotherAddedDiv');
+    debug('');
+
+    debug('Removes the second added div');
+    anotherAddedDiv.remove();
+    shouldBe('content.getDistributedNodes().length', '1');
+    shouldBe('content.getDistributedNodes().item(0)', 'div1');
+    debug('');
+
+    container.innerHTML = "";
+    finishJSTest();
+}, 0);
+</script>
+<script src=""
+</body></html>

Modified: trunk/Source/WebCore/ChangeLog (140298 => 140299)


--- trunk/Source/WebCore/ChangeLog	2013-01-21 06:48:25 UTC (rev 140298)
+++ trunk/Source/WebCore/ChangeLog	2013-01-21 06:59:35 UTC (rev 140299)
@@ -1,3 +1,49 @@
+2013-01-20  Shinya Kawanaka  <[email protected]>
+
+        Distribution state becomes inconsistent with content/shadow reprojection
+        https://bugs.webkit.org/show_bug.cgi?id=106634
+
+        Reviewed by Hajime Morita.
+
+        Distribution should be resolved from shallower ShadowDOM to deeper Shadow DOM. However, in the current implementation,
+        there is a case that distribution for deeper ShadowDOM happens to be resolved before distribution
+        for shallower ShadowDOM is resolved.
+
+        Here, we have 2 problems about distribution.
+        1) Invalidation state is not propagated to nested (= deeper) ShadowDOM.
+            - This causes deeper ShadowDOM looks having a valid distribution though it should be invalid.
+        2) We are not resolving shallower ShadowDOM when deeper ShadowDOM's distribution is needed.
+            - Because of (1), we have to check all the ancestor ShadowDOM.
+
+        For (1), we change invalidate() to invalidate nested ShadowDOM's distribution as well.
+        For (2), when resolving distribution, we will check the ancestor ShadowDOM's distribution state. If the ancestor's
+        distribution is not valid, we resolve it first.
+
+        For optimization of (1), actually we can skip invalidating distribution of some ShadowDOMs.
+        If ShadowRoot of deeper ShadowDOM does not have an InsertionPoint as children, we can skip invalidating
+        its distribution, because only children can be distributed to InsertionPoint.
+
+        Tests: fast/dom/shadow/distribution-crash.html
+               fast/dom/shadow/nested-reprojection-inconsistent.html
+
+        * dom/ElementShadow.cpp:
+        (WebCore::ElementShadow::attach): Should resolve distribution from ancestor.
+        * dom/ElementShadow.h:
+        * html/shadow/ContentDistributor.cpp:
+        (WebCore::ContentDistributor::distribute): Added ASSERT that the parent ShadowRoot's distribution is resolved.
+        (WebCore::ContentDistributor::invalidate): For each InsertionPoint, we have to invalidate
+        its parent element's distribution (if it has ElementShadow).
+        (WebCore::ContentDistributor::ensureDistribution):
+        * html/shadow/ContentDistributor.h:
+        (WebCore::ContentDistributor::isValid):
+        * html/shadow/HTMLShadowElement.cpp:
+        (WebCore::HTMLShadowElement::olderShadowRoot): Should resolve distribution from ancestor.
+        * html/shadow/InsertionPoint.cpp:
+        (WebCore::InsertionPoint::attach): ditto.
+        (WebCore::InsertionPoint::detach): ditto.
+        (WebCore::InsertionPoint::getDistributedNodes): ditto.
+        (WebCore::resolveReprojection): ditto.
+
 2013-01-20  Dominic Mazzoni  <[email protected]>
 
         Make SpeechSynthesis compile in the Chromium port

Modified: trunk/Source/WebCore/dom/ElementShadow.cpp (140298 => 140299)


--- trunk/Source/WebCore/dom/ElementShadow.cpp	2013-01-21 06:48:25 UTC (rev 140298)
+++ trunk/Source/WebCore/dom/ElementShadow.cpp	2013-01-21 06:59:35 UTC (rev 140299)
@@ -122,7 +122,8 @@
 
 void ElementShadow::attach()
 {
-    ensureDistribution();
+    ContentDistributor::ensureDistribution(youngestShadowRoot());
+
     for (ShadowRoot* root = youngestShadowRoot(); root; root = root->olderShadowRoot()) {
         if (!root->attached())
             root->attach();

Modified: trunk/Source/WebCore/dom/ElementShadow.h (140298 => 140299)


--- trunk/Source/WebCore/dom/ElementShadow.h	2013-01-21 06:48:25 UTC (rev 140298)
+++ trunk/Source/WebCore/dom/ElementShadow.h	2013-01-21 06:59:35 UTC (rev 140299)
@@ -64,7 +64,6 @@
     void recalcStyle(Node::StyleChange);
 
     void invalidateDistribution() { m_distributor.invalidateDistribution(host()); }
-    void ensureDistribution() { m_distributor.ensureDistribution(host()); }
     void didAffectSelector(AffectedSelectorMask mask) { m_distributor.didAffectSelector(host(), mask); }
     void willAffectSelector() { m_distributor.willAffectSelector(host()); }
 

Modified: trunk/Source/WebCore/html/shadow/ContentDistributor.cpp (140298 => 140299)


--- trunk/Source/WebCore/html/shadow/ContentDistributor.cpp	2013-01-21 06:48:25 UTC (rev 140298)
+++ trunk/Source/WebCore/html/shadow/ContentDistributor.cpp	2013-01-21 06:59:35 UTC (rev 140299)
@@ -213,6 +213,7 @@
 {
     ASSERT(needsDistribution());
     ASSERT(m_nodeToInsertionPoint.isEmpty());
+    ASSERT(!host->containingShadowRoot() || host->containingShadowRoot()->owner()->distributor().isValid());
 
     m_validity = Valid;
 
@@ -280,6 +281,15 @@
             for (size_t i = 0; i < insertionPoints.size(); ++i) {
                 needsReattach = needsReattach || true;
                 insertionPoints[i]->clearDistribution();
+
+                // After insertionPoint's distribution is invalidated, its reprojection should also be invalidated.
+                if (!insertionPoints[i]->isActive())
+                    continue;
+
+                if (Element* parent = insertionPoints[i]->parentElement()) {
+                    if (ElementShadow* shadow = parent->shadow())
+                        shadow->invalidateDistribution();
+                }
             }
         }
     }
@@ -338,25 +348,21 @@
     insertionPoint->setDistribution(distribution);
 }
 
-void ContentDistributor::ensureDistribution(Element* host)
+void ContentDistributor::ensureDistribution(ShadowRoot* shadowRoot)
 {
-    if (!needsDistribution())
-        return;
-    distribute(host);
-}
+    ASSERT(shadowRoot);
 
-void ContentDistributor::ensureDistributionFromDocument(Element* source)
-{
-    ContainerNode* mayShadow = source->treeScope()->rootNode();
-    if (!mayShadow->isShadowRoot())
-        return;
+    Vector<ElementShadow*, 8> elementShadows;
+    for (Element* current = shadowRoot->host(); current; current = current->shadowHost()) {
+        ElementShadow* elementShadow = current->shadow();
+        if (!elementShadow->distributor().needsDistribution())
+            break;
 
-    Vector<Element*, 8> hosts;
-    for (Element* current = toShadowRoot(mayShadow)->host(); current; current = current->shadowHost())
-        hosts.append(current);
+        elementShadows.append(elementShadow);
+    }
 
-    for (size_t i = hosts.size(); i > 0; --i)
-        hosts[i - 1]->shadow()->ensureDistribution();
+    for (size_t i = elementShadows.size(); i > 0; --i)
+        elementShadows[i - 1]->distributor().distribute(elementShadows[i - 1]->host());
 }
 
 

Modified: trunk/Source/WebCore/html/shadow/ContentDistributor.h (140298 => 140299)


--- trunk/Source/WebCore/html/shadow/ContentDistributor.h	2013-01-21 06:48:25 UTC (rev 140298)
+++ trunk/Source/WebCore/html/shadow/ContentDistributor.h	2013-01-21 06:59:35 UTC (rev 140299)
@@ -128,13 +128,12 @@
     void distributeSelectionsTo(InsertionPoint*, const ContentDistribution& pool, Vector<bool>& distributed);
     void distributeNodeChildrenTo(InsertionPoint*, ContainerNode*);
 
-    void ensureDistribution(Element* host);
     void invalidateDistribution(Element* host);
     void didShadowBoundaryChange(Element* host);
     void didAffectSelector(Element* host, AffectedSelectorMask);
     void willAffectSelector(Element* host);
 
-    static void ensureDistributionFromDocument(Element* source);
+    static void ensureDistribution(ShadowRoot*);
 
 private:
     void distribute(Element* host);
@@ -146,6 +145,7 @@
     void setNeedsSelectFeatureSet() { m_needsSelectFeatureSet = true; }
 
     void setValidity(Validity validity) { m_validity = validity; }
+    bool isValid() const { return m_validity == Valid; }
     bool needsDistribution() const;
     bool needsInvalidation() const { return m_validity != Invalidated; }
 

Modified: trunk/Source/WebCore/html/shadow/HTMLShadowElement.cpp (140298 => 140299)


--- trunk/Source/WebCore/html/shadow/HTMLShadowElement.cpp	2013-01-21 06:48:25 UTC (rev 140298)
+++ trunk/Source/WebCore/html/shadow/HTMLShadowElement.cpp	2013-01-21 06:59:35 UTC (rev 140299)
@@ -58,12 +58,13 @@
 
 ShadowRoot* HTMLShadowElement::olderShadowRoot()
 {
-    if (!treeScope()->rootNode()->isShadowRoot())
+    ShadowRoot* containingRoot = containingShadowRoot();
+    if (!containingRoot)
         return 0;
 
-    ContentDistributor::ensureDistributionFromDocument(this);
+    ContentDistributor::ensureDistribution(containingRoot);
 
-    ShadowRoot* older = toShadowRoot(treeScope()->rootNode())->olderShadowRoot();
+    ShadowRoot* older = containingRoot->olderShadowRoot();
     if (!older || older->type() != ShadowRoot::AuthorShadowRoot || ScopeContentDistribution::assignedTo(older) != this)
         return 0;
 

Modified: trunk/Source/WebCore/html/shadow/InsertionPoint.cpp (140298 => 140299)


--- trunk/Source/WebCore/html/shadow/InsertionPoint.cpp	2013-01-21 06:48:25 UTC (rev 140298)
+++ trunk/Source/WebCore/html/shadow/InsertionPoint.cpp	2013-01-21 06:59:35 UTC (rev 140299)
@@ -53,8 +53,8 @@
 
 void InsertionPoint::attach()
 {
-    if (ShadowRoot* root = containingShadowRoot())
-        root->owner()->ensureDistribution();
+    if (ShadowRoot* shadowRoot = containingShadowRoot())
+        ContentDistributor::ensureDistribution(shadowRoot);
     for (size_t i = 0; i < m_distribution.size(); ++i) {
         if (!m_distribution.at(i)->attached())
             m_distribution.at(i)->attach();
@@ -65,8 +65,9 @@
 
 void InsertionPoint::detach()
 {
-    if (ShadowRoot* root = containingShadowRoot())
-        root->owner()->ensureDistribution();
+    if (ShadowRoot* shadowRoot = containingShadowRoot())
+        ContentDistributor::ensureDistribution(shadowRoot);
+
     for (size_t i = 0; i < m_distribution.size(); ++i)
         m_distribution.at(i)->detach();
 
@@ -99,7 +100,8 @@
 
 PassRefPtr<NodeList> InsertionPoint::getDistributedNodes() const
 {
-    ContentDistributor::ensureDistributionFromDocument(const_cast<InsertionPoint*>(this));
+    if (ShadowRoot* shadowRoot = containingShadowRoot())
+        ContentDistributor::ensureDistribution(shadowRoot);
 
     Vector<RefPtr<Node> > nodes;
 
@@ -204,7 +206,8 @@
 
     while (current) {
         if (ElementShadow* shadow = shadowOfParentForDistribution(current)) {
-            shadow->ensureDistribution();
+            if (ShadowRoot* root = current->containingShadowRoot())
+                ContentDistributor::ensureDistribution(root);
             if (InsertionPoint* insertedTo = shadow->distributor().findInsertionPointFor(projectedNode)) {
                 current = insertedTo;
                 insertionPoint = insertedTo;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to