Changeset 140299 in webkit

Timestamp:

Jan 20, 2013 10:59:35 PM (11 years ago)

Author:

shinyak@chromium.org

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/shadow/distribution-crash-expected.txt (added)
• LayoutTests/fast/dom/shadow/distribution-crash.html (added)
• LayoutTests/fast/dom/shadow/nested-reprojection-inconsistent-expected.txt (added)
• LayoutTests/fast/dom/shadow/nested-reprojection-inconsistent.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/ElementShadow.cpp (modified) (1 diff)
• Source/WebCore/dom/ElementShadow.h (modified) (1 diff)
• Source/WebCore/html/shadow/ContentDistributor.cpp (modified) (3 diffs)
• Source/WebCore/html/shadow/ContentDistributor.h (modified) (3 diffs)
• Source/WebCore/html/shadow/HTMLShadowElement.cpp (modified) (1 diff)
• Source/WebCore/html/shadow/InsertionPoint.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-01-20  Shinya Kawanaka  <shinyak@chromium.org>
+
+        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  <tkent@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-20  Shinya Kawanaka  <shinyak@chromium.org>
+
+        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  <dmazzoni@google.com>
 

trunk/Source/WebCore/dom/ElementShadow.cpp

@@ -123 +123 @@
 void ElementShadow::attach()
 {
-    ensureDistribution();
+    ContentDistributor::ensureDistribution(youngestShadowRoot());
+
     for (ShadowRoot* root = youngestShadowRoot(); root; root = root->olderShadowRoot()) {
         if (!root->attached())

trunk/Source/WebCore/dom/ElementShadow.h

@@ -65 +65 @@
 
     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()); }

trunk/Source/WebCore/html/shadow/ContentDistributor.cpp

@@ -214 +214 @@
     ASSERT(needsDistribution());
     ASSERT(m_nodeToInsertionPoint.isEmpty());
+    ASSERT(!host->containingShadowRoot() || host->containingShadowRoot()->owner()->distributor().isValid());
 
     m_validity = Valid;
@@ -281 +282 @@
                 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();
+                }
             }
         }
@@ -339 +349 @@
 }
 
-void ContentDistributor::ensureDistribution(Element* host)
-{
-    if (!needsDistribution())
-        return;
-    distribute(host);
-}
-
-void ContentDistributor::ensureDistributionFromDocument(Element* source)
-{
-    ContainerNode* mayShadow = source->treeScope()->rootNode();
-    if (!mayShadow->isShadowRoot())
-        return;
-
-    Vector<Element*, 8> hosts;
-    for (Element* current = toShadowRoot(mayShadow)->host(); current; current = current->shadowHost())
-        hosts.append(current);
-
-    for (size_t i = hosts.size(); i > 0; --i)
-        hosts[i - 1]->shadow()->ensureDistribution();
+void ContentDistributor::ensureDistribution(ShadowRoot* shadowRoot)
+{
+    ASSERT(shadowRoot);
+
+    Vector<ElementShadow*, 8> elementShadows;
+    for (Element* current = shadowRoot->host(); current; current = current->shadowHost()) {
+        ElementShadow* elementShadow = current->shadow();
+        if (!elementShadow->distributor().needsDistribution())
+            break;
+
+        elementShadows.append(elementShadow);
+    }
+
+    for (size_t i = elementShadows.size(); i > 0; --i)
+        elementShadows[i - 1]->distributor().distribute(elementShadows[i - 1]->host());
 }
 

trunk/Source/WebCore/html/shadow/ContentDistributor.h

@@ -129 +129 @@
     void distributeNodeChildrenTo(InsertionPoint*, ContainerNode*);
 
-    void ensureDistribution(Element* host);
     void invalidateDistribution(Element* host);
     void didShadowBoundaryChange(Element* host);
@@ -135 +134 @@
     void willAffectSelector(Element* host);
 
-    static void ensureDistributionFromDocument(Element* source);
+    static void ensureDistribution(ShadowRoot*);
 
 private:
@@ -147 +146 @@
 
     void setValidity(Validity validity) { m_validity = validity; }
+    bool isValid() const { return m_validity == Valid; }
     bool needsDistribution() const;
     bool needsInvalidation() const { return m_validity != Invalidated; }

trunk/Source/WebCore/html/shadow/HTMLShadowElement.cpp

@@ -59 +59 @@
 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;

trunk/Source/WebCore/html/shadow/InsertionPoint.cpp

@@ -54 +54 @@
 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())
@@ -66 +66 @@
 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();
@@ -100 +101 @@
 PassRefPtr<NodeList> InsertionPoint::getDistributedNodes() const
 {
-    ContentDistributor::ensureDistributionFromDocument(const_cast<InsertionPoint*>(this));
+    if (ShadowRoot* shadowRoot = containingShadowRoot())
+        ContentDistributor::ensureDistribution(shadowRoot);
 
     Vector<RefPtr<Node> > nodes;
@@ -205 +207 @@
     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;