ContentDistribution should be only used for details elements
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Sep 2015 21:31:31 +0000 (21:31 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Sep 2015 21:31:31 +0000 (21:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149148

Reviewed by Antti Koivisto.

Extracted ShadowRootWithInsertionPoints out of ShadowRoot for HTMLDetailsElement and HTMLSummaryElement.

We don't add a separate .h and .cpp files since this is a temporary measure until we replace it with
a slot-based shadow DOM implementation.

No new tests. There should be no observable behavioral change.

* dom/Element.cpp:
(WebCore::Element::addShadowRoot): Removed the call to didShadowBoundaryChange since this function is only
called in ensureUserAgentShadowRoot. Also moved the call to didAddUserAgentShadowRoot for
HTMLDetailsElement's shadow root which uses this function instead of ensureUserAgentShadowRoot.
(WebCore::Element::removeShadowRoot): Removed the call to invalidateDistribution since it's only called by
~Element.
(WebCore::Element::createShadowRoot):
(WebCore::Element::ensureUserAgentShadowRoot): Moved the call didAddUserAgentShadowRoot into addShadowRoot
since HTMLDetailsElement uses a subclass of ShadowRoot.
(WebCore::Element::childrenChanged):
(WebCore::Element::removeAllEventListeners):

* dom/Element.h:
(Element::addShadowRoot): Made this function a protected member as it's now used by HTMLDetailsElement.

* dom/ShadowRoot.cpp:
(WebCore::ShadowRoot::childrenChanged): Deleted.

* dom/ShadowRoot.h:
(WebCore::ShadowRoot::distributor): Made this a virtual function and return nullptr by default.
(WebCore::ShadowRoot::isOrphan):

* html/HTMLDetailsElement.cpp:
(WebCore::HTMLDetailsElement::create): Uses ShadowRootWithInsertionPoints instead of ShadowRoot.

* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::runPostTypeUpdateTasks): Removed the call to invalidateDistribution since it's
only relevant for HTMLDetailsElement's shadow DOM.

* html/HTMLSummaryElement.cpp:
(WebCore::HTMLSummaryElement::create): Uses ShadowRootWithInsertionPoints instead of ShadowRoot.

* html/shadow/ContentDistributor.cpp:
(WebCore::ContentDistributor::distribute):
(WebCore::ContentDistributor::ensureDistribution):
(WebCore::ContentDistributor::invalidateDistribution):

* html/shadow/InsertionPoint.cpp:
(WebCore::InsertionPoint::childrenChanged):
(WebCore::InsertionPoint::insertedInto):
(WebCore::InsertionPoint::removedFrom):
(WebCore::findInsertionPointOf):
(WebCore::ShadowRootWithInsertionPoints::childrenChanged): Moved from ShadowRoot.

* html/shadow/InsertionPoint.h:
(WebCore::ShadowRootWithInsertionPoints::create): Added.
(WebCore::ShadowRootWithInsertionPoints::ShadowRootWithInsertionPoints): Added.

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/ShadowRoot.cpp
Source/WebCore/dom/ShadowRoot.h
Source/WebCore/html/HTMLDetailsElement.cpp
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/HTMLSummaryElement.cpp
Source/WebCore/html/shadow/ContentDistributor.cpp
Source/WebCore/html/shadow/InsertionPoint.cpp
Source/WebCore/html/shadow/InsertionPoint.h

index d678b0e226f7ebda0e1f117fbf7625231d3aec91..9afad0fd0ebc967e6d5b1e3a73b91d9c94fcad9e 100644 (file)
@@ -1,3 +1,65 @@
+2015-09-15  Ryosuke Niwa  <rniwa@webkit.org>
+
+        ContentDistribution should be only used for details elements
+        https://bugs.webkit.org/show_bug.cgi?id=149148
+
+        Reviewed by Antti Koivisto.
+
+        Extracted ShadowRootWithInsertionPoints out of ShadowRoot for HTMLDetailsElement and HTMLSummaryElement.
+
+        We don't add a separate .h and .cpp files since this is a temporary measure until we replace it with
+        a slot-based shadow DOM implementation.
+
+        No new tests. There should be no observable behavioral change.
+
+        * dom/Element.cpp:
+        (WebCore::Element::addShadowRoot): Removed the call to didShadowBoundaryChange since this function is only
+        called in ensureUserAgentShadowRoot. Also moved the call to didAddUserAgentShadowRoot for
+        HTMLDetailsElement's shadow root which uses this function instead of ensureUserAgentShadowRoot.
+        (WebCore::Element::removeShadowRoot): Removed the call to invalidateDistribution since it's only called by
+        ~Element.
+        (WebCore::Element::createShadowRoot):
+        (WebCore::Element::ensureUserAgentShadowRoot): Moved the call didAddUserAgentShadowRoot into addShadowRoot
+        since HTMLDetailsElement uses a subclass of ShadowRoot.
+        (WebCore::Element::childrenChanged):
+        (WebCore::Element::removeAllEventListeners):
+
+        * dom/Element.h:
+        (Element::addShadowRoot): Made this function a protected member as it's now used by HTMLDetailsElement.
+
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::childrenChanged): Deleted.
+
+        * dom/ShadowRoot.h:
+        (WebCore::ShadowRoot::distributor): Made this a virtual function and return nullptr by default.
+        (WebCore::ShadowRoot::isOrphan):
+
+        * html/HTMLDetailsElement.cpp:
+        (WebCore::HTMLDetailsElement::create): Uses ShadowRootWithInsertionPoints instead of ShadowRoot.
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::runPostTypeUpdateTasks): Removed the call to invalidateDistribution since it's
+        only relevant for HTMLDetailsElement's shadow DOM.
+
+        * html/HTMLSummaryElement.cpp:
+        (WebCore::HTMLSummaryElement::create): Uses ShadowRootWithInsertionPoints instead of ShadowRoot.
+
+        * html/shadow/ContentDistributor.cpp:
+        (WebCore::ContentDistributor::distribute):
+        (WebCore::ContentDistributor::ensureDistribution):
+        (WebCore::ContentDistributor::invalidateDistribution):
+
+        * html/shadow/InsertionPoint.cpp:
+        (WebCore::InsertionPoint::childrenChanged):
+        (WebCore::InsertionPoint::insertedInto):
+        (WebCore::InsertionPoint::removedFrom):
+        (WebCore::findInsertionPointOf):
+        (WebCore::ShadowRootWithInsertionPoints::childrenChanged): Moved from ShadowRoot.
+
+        * html/shadow/InsertionPoint.h:
+        (WebCore::ShadowRootWithInsertionPoints::create): Added.
+        (WebCore::ShadowRootWithInsertionPoints::ShadowRootWithInsertionPoints): Added.
+
 2015-09-15  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Tiled drawing is rendering more times than it should
index feac984ec4ee27d31877abcb8cf46452aba77a63..73fc474a7d458db1ce8b45c97ff1771524edf46d 100644 (file)
@@ -1613,7 +1613,6 @@ void Element::addShadowRoot(Ref<ShadowRoot>&& newShadowRoot)
 
     shadowRoot.setHost(this);
     shadowRoot.setParentTreeScope(&treeScope());
-    shadowRoot.distributor().didShadowBoundaryChange(this);
 
     NodeVector postInsertionNotificationTargets;
     ChildNodeInsertionNotifier(*this).notify(shadowRoot, postInsertionNotificationTargets);
@@ -1626,6 +1625,9 @@ void Element::addShadowRoot(Ref<ShadowRoot>&& newShadowRoot)
     setNeedsStyleRecalc(ReconstructRenderTree);
 
     InspectorInstrumentation::didPushShadowRoot(*this, shadowRoot);
+
+    if (shadowRoot.type() == ShadowRoot::UserAgentShadowRoot)
+        didAddUserAgentShadowRoot(&shadowRoot);
 }
 
 void Element::removeShadowRoot()
@@ -1644,8 +1646,6 @@ void Element::removeShadowRoot()
     oldRoot->setParentTreeScope(&document());
 
     ChildNodeRemovalNotifier(*this).notify(*oldRoot);
-
-    oldRoot->distributor().invalidateDistribution(this);
 }
 
 RefPtr<ShadowRoot> Element::createShadowRoot(ExceptionCode& ec)
@@ -1672,7 +1672,6 @@ ShadowRoot& Element::ensureUserAgentShadowRoot()
     if (!shadowRoot) {
         addShadowRoot(ShadowRoot::create(document(), ShadowRoot::UserAgentShadowRoot));
         shadowRoot = userAgentShadowRoot();
-        didAddUserAgentShadowRoot(shadowRoot);
     }
     return *shadowRoot;
 }
@@ -1795,8 +1794,10 @@ void Element::childrenChanged(const ChildChange& change)
         checkForSiblingStyleChanges(*this, checkType, change.previousSiblingElement, change.nextSiblingElement);
     }
 
-    if (ShadowRoot* shadowRoot = this->shadowRoot())
-        shadowRoot->invalidateDistribution();
+    if (ShadowRoot* shadowRoot = this->shadowRoot()) {
+        if (auto* distributor = shadowRoot->distributor())
+            distributor->invalidateDistribution(this);
+    }
 }
 
 void Element::removeAllEventListeners()
index ae827ddc2be7925e961c8a6b27f85ccd14739586..c41698f66c4aa7d77a4ecb7aeafad5ca1823d806 100644 (file)
@@ -500,6 +500,8 @@ protected:
     // svgAttributeChanged (called when element.className.baseValue is set)
     void classAttributeChanged(const AtomicString& newClassString);
 
+    void addShadowRoot(Ref<ShadowRoot>&&);
+
     static void mergeWithNextTextNode(Text& node, ExceptionCode&);
 
 private:
@@ -564,7 +566,6 @@ private:
     virtual Ref<Node> cloneNodeInternal(Document&, CloningOperation) override;
     virtual Ref<Element> cloneElementWithoutAttributesAndChildren(Document&);
 
-    void addShadowRoot(Ref<ShadowRoot>&&);
     void removeShadowRoot();
 
     bool rareDataStyleAffectedByEmpty() const;
index 886100db469e241e46b956bc5701da0cc5e6695a..b3e6cb1d15d59713c5b751e5b9c638cec2e13c72 100644 (file)
@@ -38,7 +38,6 @@ namespace WebCore {
 
 struct SameSizeAsShadowRoot : public DocumentFragment, public TreeScope {
     unsigned countersAndFlags[1];
-    ContentDistributor distributor;
     void* host;
 };
 
@@ -122,15 +121,6 @@ void ShadowRoot::setResetStyleInheritance(bool value)
     }
 }
 
-void ShadowRoot::childrenChanged(const ChildChange& change)
-{
-    if (isOrphan())
-        return;
-
-    ContainerNode::childrenChanged(change);
-    invalidateDistribution();
-}
-
 Ref<Node> ShadowRoot::cloneNodeInternal(Document&, CloningOperation)
 {
     RELEASE_ASSERT_NOT_REACHED();
index 631608af1ad4436a25ba61eb39ded8ff6a22bbd9..4554b369705fdb1ffa0766a8ddaab8fb1cc55414 100644 (file)
@@ -28,7 +28,6 @@
 #define ShadowRoot_h
 
 #include "ContainerNode.h"
-#include "ContentDistributor.h"
 #include "Document.h"
 #include "DocumentFragment.h"
 #include "Element.h"
@@ -37,7 +36,9 @@
 
 namespace WebCore {
 
-class ShadowRoot final : public DocumentFragment, public TreeScope {
+class ContentDistributor;
+
+class ShadowRoot : public DocumentFragment, public TreeScope {
 public:
     enum ShadowRootType {
         UserAgentShadowRoot = 0,
@@ -65,28 +66,25 @@ public:
 
     PassRefPtr<Node> cloneNode(bool, ExceptionCode&);
 
-    ContentDistributor& distributor() { return m_distributor; }
-    void invalidateDistribution() { m_distributor.invalidateDistribution(m_host); }
-
     virtual void removeAllEventListeners() override;
 
-private:
+    virtual ContentDistributor* distributor() { return nullptr; }
+
+protected:
     ShadowRoot(Document&, ShadowRootType);
 
+    // FIXME: This shouldn't happen. https://bugs.webkit.org/show_bug.cgi?id=88834
+    bool isOrphan() const { return !m_host; }
+
+private:
     virtual bool childTypeAllowed(NodeType) const override;
-    virtual void childrenChanged(const ChildChange&) override;
 
     virtual Ref<Node> cloneNodeInternal(Document&, CloningOperation) override;
 
-    // FIXME: This shouldn't happen. https://bugs.webkit.org/show_bug.cgi?id=88834
-    bool isOrphan() const { return !m_host; }
-
     unsigned m_resetStyleInheritance : 1;
     unsigned m_type : 1;
 
     Element* m_host;
-
-    ContentDistributor m_distributor;
 };
 
 inline Element* ShadowRoot::activeElement() const
index 48cc9b848e5417ac79fa65019e48705e17127a09..24a1e9dca070222e38d59a19658646c2ff846814 100644 (file)
@@ -23,6 +23,7 @@
 
 #if ENABLE(DETAILS_ELEMENT)
 #include "AXObjectCache.h"
+#include "ContentDistributor.h"
 #include "ElementIterator.h"
 #include "HTMLSummaryElement.h"
 #include "InsertionPoint.h"
@@ -101,7 +102,7 @@ Ref<DetailsSummaryElement> DetailsSummaryElement::create(Document& document)
 Ref<HTMLDetailsElement> HTMLDetailsElement::create(const QualifiedName& tagName, Document& document)
 {
     Ref<HTMLDetailsElement> details = adoptRef(*new HTMLDetailsElement(tagName, document));
-    details->ensureUserAgentShadowRoot();
+    details->addShadowRoot(ShadowRootWithInsertionPoints::create(document));
     return details;
 }
 
index 3642a9ab6600f211f291d17f27a214246af70343..be09868e865b9df66758ab689d93a801d2936432 100644 (file)
@@ -531,9 +531,6 @@ inline void HTMLInputElement::runPostTypeUpdateTasks()
     if (document().focusedElement() == this)
         updateFocusAppearance(true);
 
-    if (ShadowRoot* shadowRoot = shadowRootOfParentForDistribution(this))
-        shadowRoot->invalidateDistribution();
-
     setChangedSinceLastFormControlChangeEvent(false);
 
     addToRadioButtonGroup();
index 8b5c18710317b871445031b7819d020944c43c88..d14ba81098645147548c6e88ee19a59392401e27 100644 (file)
@@ -55,7 +55,7 @@ Ref<SummaryContentElement> SummaryContentElement::create(Document& document)
 Ref<HTMLSummaryElement> HTMLSummaryElement::create(const QualifiedName& tagName, Document& document)
 {
     Ref<HTMLSummaryElement> summary = adoptRef(*new HTMLSummaryElement(tagName, document));
-    summary->ensureUserAgentShadowRoot();
+    summary->addShadowRoot(ShadowRootWithInsertionPoints::create(document));
     return summary;
 }
 
index c518ef3d1784a09e5db64d2e151e0524b4b65bf7..dcf5851285be0fc3aa60156ef1088f46df657382 100644 (file)
@@ -71,7 +71,9 @@ void ContentDistributor::distribute(Element* host)
 {
     ASSERT(needsDistribution());
     ASSERT(m_nodeToInsertionPoint.isEmpty());
-    ASSERT(!host->containingShadowRoot() || host->containingShadowRoot()->distributor().isValid());
+    ASSERT(!host->containingShadowRoot()
+        || !host->containingShadowRoot()->distributor()
+        || host->containingShadowRoot()->distributor()->isValid());
 
     m_validity = Valid;
 
@@ -128,13 +130,13 @@ void ContentDistributor::ensureDistribution(ShadowRoot* shadowRoot)
     Vector<ShadowRoot*, 8> shadowRoots;
     for (Element* current = shadowRoot->host(); current; current = current->shadowHost()) {
         ShadowRoot* currentRoot = current->shadowRoot();
-        if (!currentRoot->distributor().needsDistribution())
+        if (!currentRoot->distributor() || !currentRoot->distributor()->needsDistribution())
             break;
         shadowRoots.append(currentRoot);
     }
 
     for (size_t i = shadowRoots.size(); i > 0; --i)
-        shadowRoots[i - 1]->distributor().distribute(shadowRoots[i - 1]->host());
+        shadowRoots[i - 1]->distributor()->distribute(shadowRoots[i - 1]->host());
 }
 
 void ContentDistributor::invalidateDistribution(Element* host)
index 9cce2145cf7a30a6dd49e9b985e4958960f0718c..5b1dd2cf79e7e47cc525d9dfc7059f3958bbaefe 100644 (file)
@@ -76,8 +76,10 @@ bool InsertionPoint::rendererIsNeeded(const RenderStyle& style)
 void InsertionPoint::childrenChanged(const ChildChange& change)
 {
     HTMLElement::childrenChanged(change);
-    if (ShadowRoot* root = containingShadowRoot())
-        root->invalidateDistribution();
+    if (ShadowRoot* root = containingShadowRoot()) {
+        RELEASE_ASSERT(root->distributor());
+        root->distributor()->invalidateDistribution(root->host());
+    }
 }
 
 Node::InsertionNotificationRequest InsertionPoint::insertedInto(ContainerNode& insertionPoint)
@@ -85,8 +87,9 @@ Node::InsertionNotificationRequest InsertionPoint::insertedInto(ContainerNode& i
     HTMLElement::insertedInto(insertionPoint);
 
     if (ShadowRoot* root = containingShadowRoot()) {
-        root->distributor().didShadowBoundaryChange(root->host());
-        root->distributor().invalidateInsertionPointList();
+        RELEASE_ASSERT(root->distributor());
+        root->distributor()->didShadowBoundaryChange(root->host());
+        root->distributor()->invalidateInsertionPointList();
     }
 
     return InsertionDone;
@@ -99,8 +102,9 @@ void InsertionPoint::removedFrom(ContainerNode& insertionPoint)
         root = insertionPoint.containingShadowRoot();
 
     if (root && root->host()) {
-        root->invalidateDistribution();
-        root->distributor().invalidateInsertionPointList();
+        RELEASE_ASSERT(root->distributor());
+        root->distributor()->invalidateDistribution(root->host());
+        root->distributor()->invalidateInsertionPointList();
     }
 
     // Since this insertion point is no longer visible from the shadow subtree, it need to clean itself up.
@@ -154,9 +158,18 @@ InsertionPoint* findInsertionPointOf(const Node* projectedNode)
     if (ShadowRoot* shadowRoot = shadowRootOfParentForDistribution(projectedNode)) {
         if (ShadowRoot* root = projectedNode->containingShadowRoot())
             ContentDistributor::ensureDistribution(root);
-        return shadowRoot->distributor().findInsertionPointFor(projectedNode);
+        if (auto* distributor = shadowRoot->distributor())
+            return distributor->findInsertionPointFor(projectedNode);
     }
     return 0;
 }
 
+void ShadowRootWithInsertionPoints::childrenChanged(const ChildChange& change)
+{
+    ContainerNode::childrenChanged(change);
+    
+    if (!isOrphan())
+        m_distributor.invalidateDistribution(host());
+}
+
 } // namespace WebCore
index c63b728873e0a8e775a0b11ab2228ee7fb215cf7..c2037e199716e5d5d7b7a6a8cb6a995e65e1d440 100644 (file)
@@ -75,6 +75,26 @@ private:
     bool m_hasDistribution;
 };
 
+class ShadowRootWithInsertionPoints : public ShadowRoot {
+public:
+    
+    static Ref<ShadowRootWithInsertionPoints> create(Document& document)
+    {
+        return adoptRef(*new ShadowRootWithInsertionPoints(document));
+    }
+    
+    virtual ContentDistributor* distributor() override { return &m_distributor; }
+    
+private:
+    ShadowRootWithInsertionPoints(Document& document)
+        : ShadowRoot(document, UserAgentShadowRoot)
+    { }
+
+    virtual void childrenChanged(const ChildChange&) override;
+
+    ContentDistributor m_distributor;
+};
+
 inline bool isActiveInsertionPoint(const Node* node)
 {
     return is<InsertionPoint>(node) && downcast<InsertionPoint>(*node).isActive();