[Shadow DOM] Some distribution invalidation can drop necessary reattachment.
authormorrita@google.com <morrita@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Jul 2012 04:05:24 +0000 (04:05 +0000)
committermorrita@google.com <morrita@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Jul 2012 04:05:24 +0000 (04:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=88843

Reviewed by Dimitri Glazkov.

Source/WebCore:

Following scenario caused this problem:

- Inserting a Text node as a shadow child triggers invalidateDistribution(),
  which doen't reattach the shadow's host element.
- Then inserting a <content> element after that triggers another invalidateDistribution(),
  which should reattach its host because <content> can affect not only distribution of new nodes,
  but also existing distribution.
- Since the first invalidateDistribution() has marked the distribution as invalidated,
  the second invalidateDistribution() call returns early without any reattachment,
  even though it needs one.

This change adds InvalidationType parameter to invalidateDistribution(), which asks ElementShadow to
reattach the host regardless of its validity state. InsertionPoint::insertedInto() uses
this flag to ensure that its insertion always results a reattachment.

Test: fast/dom/shadow/content-after-style.html

* dom/ElementShadow.cpp:
(WebCore::ElementShadow::addShadowRoot): Passes InvalidationType.
(WebCore::ElementShadow::removeAllShadowRoots): Passes InvalidationType.
(WebCore::ElementShadow::invalidateDistribution): Added a InvalidationType parameter.
* dom/ElementShadow.h:
* html/shadow/InsertionPoint.cpp:
(WebCore::InsertionPoint::insertedInto): Passes InvalidationType.

LayoutTests:

* fast/dom/shadow/content-after-style-expected.html: Added.
* fast/dom/shadow/content-after-style.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/shadow/content-after-style-expected.html [new file with mode: 0644]
LayoutTests/fast/dom/shadow/content-after-style.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/ElementShadow.cpp
Source/WebCore/dom/ElementShadow.h
Source/WebCore/html/shadow/InsertionPoint.cpp

index 638ca51..09af57c 100644 (file)
@@ -1,3 +1,13 @@
+2012-07-17  MORITA Hajime <morrita@google.com>
+
+        [Shadow DOM] Some distribution invalidation can drop necessary reattachment.
+        https://bugs.webkit.org/show_bug.cgi?id=88843
+
+        Reviewed by Dimitri Glazkov.
+
+        * fast/dom/shadow/content-after-style-expected.html: Added.
+        * fast/dom/shadow/content-after-style.html: Added.
+
 2012-07-17  Joshua Bell  <jsbell@chromium.org>
 
         IndexedDB: Key generator state not maintained across connections
diff --git a/LayoutTests/fast/dom/shadow/content-after-style-expected.html b/LayoutTests/fast/dom/shadow/content-after-style-expected.html
new file mode 100644 (file)
index 0000000..46f8c31
--- /dev/null
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="host">text<div>host content</div></div>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/shadow/content-after-style.html b/LayoutTests/fast/dom/shadow/content-after-style.html
new file mode 100644 (file)
index 0000000..502cb42
--- /dev/null
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="host"><div>host content</div></div>
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.waitUntilDone();
+var root = new WebKitShadowRoot(document.querySelector('#host'));
+root.innerHTML = '<style>invalid { }</style>' +
+                 'text<content></content>';
+window.setTimeout(function() {
+    if (window.layoutTestController)
+        window.layoutTestController.notifyDone();
+}, 0);
+</script>
+</body>
+</html>
index ce54c60..471ff6c 100644 (file)
@@ -1,3 +1,35 @@
+2012-07-17  MORITA Hajime <morrita@google.com>
+
+        [Shadow DOM] Some distribution invalidation can drop necessary reattachment.
+        https://bugs.webkit.org/show_bug.cgi?id=88843
+
+        Reviewed by Dimitri Glazkov.
+
+        Following scenario caused this problem:
+
+        - Inserting a Text node as a shadow child triggers invalidateDistribution(),
+          which doen't reattach the shadow's host element.
+        - Then inserting a <content> element after that triggers another invalidateDistribution(),
+          which should reattach its host because <content> can affect not only distribution of new nodes,
+          but also existing distribution.
+        - Since the first invalidateDistribution() has marked the distribution as invalidated,
+          the second invalidateDistribution() call returns early without any reattachment,
+          even though it needs one.
+
+        This change adds InvalidationType parameter to invalidateDistribution(), which asks ElementShadow to
+        reattach the host regardless of its validity state. InsertionPoint::insertedInto() uses
+        this flag to ensure that its insertion always results a reattachment.
+
+        Test: fast/dom/shadow/content-after-style.html
+
+        * dom/ElementShadow.cpp:
+        (WebCore::ElementShadow::addShadowRoot): Passes InvalidationType.
+        (WebCore::ElementShadow::removeAllShadowRoots): Passes InvalidationType.
+        (WebCore::ElementShadow::invalidateDistribution): Added a InvalidationType parameter.
+        * dom/ElementShadow.h:
+        * html/shadow/InsertionPoint.cpp:
+        (WebCore::InsertionPoint::insertedInto): Passes InvalidationType.
+
 2012-07-17  Jon Lee  <jonlee@apple.com>
 
         Teach CodeGenerator to support for static, readonly, attributes
index 68a9b01..6f5e332 100644 (file)
@@ -79,7 +79,7 @@ void ElementShadow::addShadowRoot(Element* shadowHost, PassRefPtr<ShadowRoot> sh
     shadowRoot->setHost(shadowHost);
     shadowRoot->setParentTreeScope(shadowHost->treeScope());
     m_shadowRoots.push(shadowRoot.get());
-    invalidateDistribution(shadowHost);
+    invalidateDistribution(shadowHost, InvalidateIfNeeded);
     ChildNodeInsertionNotifier(shadowHost).notify(shadowRoot.get());
 
     if (shadowHost->attached() && !shadowRoot->attached())
@@ -108,7 +108,7 @@ void ElementShadow::removeAllShadowRoots()
         ChildNodeRemovalNotifier(shadowHost).notify(oldRoot.get());
     }
 
-    invalidateDistribution(shadowHost);
+    invalidateDistribution(shadowHost, InvalidateIfNeeded);
 }
 
 void ElementShadow::attach()
@@ -188,22 +188,26 @@ void ElementShadow::ensureDistribution()
     m_distributor.distribute(host());
 }
 
-void ElementShadow::invalidateDistribution()
+void ElementShadow::invalidateDistribution(InvalidationType type)
 {
-    invalidateDistribution(host());
+    invalidateDistribution(host(), type);
 }
 
-void ElementShadow::invalidateDistribution(Element* host)
+void ElementShadow::invalidateDistribution(Element* host, InvalidationType type)
 {
-    if (!m_distributor.needsInvalidation())
-        return;
-    bool needsReattach = m_distributor.invalidate(host);
+    bool needsReattach = (type == InvalidateAndForceReattach);
+    bool needsInvalidation = m_distributor.needsInvalidation();
+
+    if (needsInvalidation)
+        needsReattach |= m_distributor.invalidate(host);
+
     if (needsReattach && host->attached()) {
         host->detach();
         host->lazyAttach(Node::DoNotSetAttached);
     }
 
-    m_distributor.finishInivalidation();
+    if (needsInvalidation)
+        m_distributor.finishInivalidation();
 }
 
 } // namespace
index 6f65011..3ab5c07 100644 (file)
@@ -62,8 +62,13 @@ public:
     bool needsStyleRecalc();
     void recalcStyle(Node::StyleChange);
 
+    enum InvalidationType {
+        InvalidateIfNeeded,
+        InvalidateAndForceReattach
+    };
+
+    void invalidateDistribution(InvalidationType = InvalidateIfNeeded);
     void ensureDistribution();
-    void invalidateDistribution();
  
     InsertionPoint* insertionPointFor(const Node*) const;
 
@@ -71,7 +76,7 @@ public:
     const ContentDistributor& distributor() const;
 
 private:
-    void invalidateDistribution(Element* host);
+    void invalidateDistribution(Element* host, InvalidationType);
 
     DoublyLinkedList<ShadowRoot> m_shadowRoots;
     ContentDistributor m_distributor;
index c1e0200..fadbb45 100644 (file)
@@ -119,7 +119,7 @@ Node::InsertionNotificationRequest InsertionPoint::insertedInto(ContainerNode* i
     HTMLElement::insertedInto(insertionPoint);
     if (insertionPoint->inDocument()) {
         if (ShadowRoot* root = shadowRoot())
-            root->owner()->invalidateDistribution();
+            root->owner()->invalidateDistribution(ElementShadow::InvalidateAndForceReattach);
     }
 
     return InsertionDone;