Changeset 110935 in webkit

Timestamp:

Mar 15, 2012 8:20:26 PM (12 years ago)

Author:

shinyak@chromium.org

Message:

[Crash] Adding <content> into a ShadowRoot causes crash.
​https://bugs.webkit.org/show_bug.cgi?id=80020

Reviewed by Hajime Morita.

Source/WebCore:

The problem is <content> tries to select host children though it is not prepared.
Since populating host children for insertion points is performed just before
attaching a shadow tree, we should recalculate whole shadow tree if <content> is
appended as a child.

However, element->appendChild() does not know the element is in a shadow tree or not.
We have to ensure reattaching whole shadow tree here.

So this patch adds some phases to HTMLContentSelector so that we can check node
distribution algorihm is begin processed or not. If not we cannot select anything,
but we have to enable a flag to reattach whole shadow tree.

Tests: fast/dom/shadow/shadow-content-crash-expected.html

fast/dom/shadow/shadow-content-crash.html

• dom/ShadowTree.cpp:

(WebCore::ShadowTree::attach):
(WebCore::ShadowTree::insertionPointFor):

• dom/ShadowTree.h:

(WebCore):
(ShadowTree):
(WebCore::ShadowTree::selector):

• html/shadow/HTMLContentSelector.cpp:

(WebCore::HTMLContentSelector::HTMLContentSelector):
(WebCore::HTMLContentSelector::select):
(WebCore::HTMLContentSelector::willSelect):
(WebCore):
(WebCore::HTMLContentSelector::didSelect):
(WebCore::HTMLContentSelector::populateIfNecessary):

• html/shadow/HTMLContentSelector.h:

(HTMLContentSelector):
(WebCore::HTMLContentSelector::isSelecting):
(WebCore):
(WebCore::HTMLContentSelector::hasPopulated):

• html/shadow/InsertionPoint.cpp:

(WebCore::InsertionPoint::distributeHostChildren):
(WebCore::InsertionPoint::clearDistribution):

LayoutTests:

• fast/dom/shadow/shadow-content-crash-expected.html: Added.
• fast/dom/shadow/shadow-content-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/shadow/shadow-content-crash-expected.html (added)
• LayoutTests/fast/dom/shadow/shadow-content-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/ShadowTree.cpp (modified) (4 diffs)
• Source/WebCore/dom/ShadowTree.h (modified) (5 diffs)
• Source/WebCore/html/shadow/HTMLContentSelector.cpp (modified) (3 diffs)
• Source/WebCore/html/shadow/HTMLContentSelector.h (modified) (2 diffs)
• Source/WebCore/html/shadow/InsertionPoint.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-03-15  Shinya Kawanaka  <shinyak@chromium.org>
+
+        [Crash] Adding <content> into a ShadowRoot causes crash.
+        https://bugs.webkit.org/show_bug.cgi?id=80020
+
+        Reviewed by Hajime Morita.
+
+        * fast/dom/shadow/shadow-content-crash-expected.html: Added.
+        * fast/dom/shadow/shadow-content-crash.html: Added.
+
 2012-03-15  Mike Lawther  <mikelawther@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-03-15  Shinya Kawanaka  <shinyak@chromium.org>
+
+        [Crash] Adding <content> into a ShadowRoot causes crash.
+        https://bugs.webkit.org/show_bug.cgi?id=80020
+
+        Reviewed by Hajime Morita.
+
+        The problem is <content> tries to select host children though it is not prepared.
+        Since populating host children for insertion points is performed just before
+        attaching a shadow tree, we should recalculate whole shadow tree if <content> is
+        appended as a child.
+
+        However, element->appendChild() does not know the element is in a shadow tree or not.
+        We have to ensure reattaching whole shadow tree here.
+
+        So this patch adds some phases to HTMLContentSelector so that we can check node
+        distribution algorihm is begin processed or not. If not we cannot select anything,
+        but we have to enable a flag to reattach whole shadow tree.
+
+        Tests: fast/dom/shadow/shadow-content-crash-expected.html
+               fast/dom/shadow/shadow-content-crash.html
+
+        * dom/ShadowTree.cpp:
+        (WebCore::ShadowTree::attach):
+        (WebCore::ShadowTree::insertionPointFor):
+        * dom/ShadowTree.h:
+        (WebCore):
+        (ShadowTree):
+        (WebCore::ShadowTree::selector):
+        * html/shadow/HTMLContentSelector.cpp:
+        (WebCore::HTMLContentSelector::HTMLContentSelector):
+        (WebCore::HTMLContentSelector::select):
+        (WebCore::HTMLContentSelector::willSelect):
+        (WebCore):
+        (WebCore::HTMLContentSelector::didSelect):
+        (WebCore::HTMLContentSelector::populateIfNecessary):
+        * html/shadow/HTMLContentSelector.h:
+        (HTMLContentSelector):
+        (WebCore::HTMLContentSelector::isSelecting):
+        (WebCore):
+        (WebCore::HTMLContentSelector::hasPopulated):
+        * html/shadow/InsertionPoint.cpp:
+        (WebCore::InsertionPoint::distributeHostChildren):
+        (WebCore::InsertionPoint::clearDistribution):
+
 2012-03-15  Mike Lawther  <mikelawther@chromium.org>
 

trunk/Source/WebCore/dom/ShadowTree.cpp

@@ -31 +31 @@
 #include "Document.h"
 #include "Element.h"
-#include "HTMLContentSelector.h"
 #include "HTMLShadowElement.h"
 #include "InspectorInstrumentation.h"
@@ -179 +178 @@
     // Children of m_selector is populated lazily in
     // ensureSelector(), and here we just ensure that it is in clean state.
-    ASSERT(!selector() || !selector()->hasCandidates());
-
+    ASSERT(!selector().hasPopulated());
+
+    selector().willSelect();
     for (ShadowRoot* root = youngestShadowRoot(); root; root = root->olderShadowRoot()) {
         if (!root->attached())
             root->attach();
     }
-
-    if (HTMLContentSelector* contentSelector = selector())
-        contentSelector->didSelect();
+    selector().didSelect();
 }
 
@@ -223 +221 @@
     }
 
-    if (!m_selector)
-        return 0;
-    HTMLContentSelection* found = m_selector->findFor(node);
+    HTMLContentSelection* found = selector().findFor(node);
     if (!found)
         return 0;
@@ -318 +314 @@
 }
 
-HTMLContentSelector* ShadowTree::ensureSelector()
-{
-    if (!m_selector)
-        m_selector = adoptPtr(new HTMLContentSelector());
-    m_selector->willSelectOver(host());
-    return m_selector.get();
-}
-
 } // namespace

trunk/Source/WebCore/dom/ShadowTree.h

@@ -29 +29 @@
 
 #include "ExceptionCode.h"
+#include "HTMLContentSelector.h"
 #include "ShadowRoot.h"
 #include <wtf/DoublyLinkedList.h>
@@ -39 +40 @@
 class Node;
 class Element;
-class HTMLContentSelector;
 class InsertionPoint;
 class TreeScope;
@@ -47 +47 @@
     ShadowTree();
     ~ShadowTree();
+
+    Element* host() const;
 
     bool hasShadowRoot() const;
@@ -80 +82 @@
     InsertionPoint* insertionPointFor(Node*) const;
 
-    HTMLContentSelector* selector() const;
-    HTMLContentSelector* ensureSelector();
+    HTMLContentSelector& selector();
+    const HTMLContentSelector& selector() const;
 
 private:
-    Element* host() const;
 
     DoublyLinkedList<ShadowRoot> m_shadowRoots;
-    OwnPtr<HTMLContentSelector> m_selector;
+    HTMLContentSelector m_selector;
     bool m_needsRecalculateContent : 1;
     WTF_MAKE_NONCOPYABLE(ShadowTree);
@@ -107 +108 @@
 }
 
-inline HTMLContentSelector* ShadowTree::selector() const
+inline HTMLContentSelector& ShadowTree::selector()
 {
-    return m_selector.get();
+    return m_selector;
+}
+
+inline const HTMLContentSelector& ShadowTree::selector() const
+{
+    return m_selector;
 }
 

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

@@ -101 +101 @@
 
 HTMLContentSelector::HTMLContentSelector()
+    : m_phase(SelectionPrevented)
 {
 }
@@ -111 +112 @@
 void HTMLContentSelector::select(InsertionPoint* insertionPoint, HTMLContentSelectionList* selections)
 {
+    ASSERT(m_phase == HostChildrenPopulated);
     ASSERT(selections->isEmpty());
 
@@ -142 +144 @@
 }
 
+void HTMLContentSelector::willSelect()
+{
+    m_phase = SelectionStarted;
+}
+
 void HTMLContentSelector::didSelect()
 {
+    ASSERT(m_phase != SelectionPrevented);
+    m_phase = SelectionPrevented;
     m_candidates.clear();
 }
 
-void HTMLContentSelector::willSelectOver(Element* shadowHost)
+void HTMLContentSelector::populateIfNecessary(Element* shadowHost)
 {
-    if (!m_candidates.isEmpty())
+    if (hasPopulated())
         return;
+
+    ASSERT(m_candidates.isEmpty());
     ASSERT(shadowHost);
+    ASSERT(m_phase == SelectionStarted);
+
+    m_phase = HostChildrenPopulated;
     for (Node* node = shadowHost->firstChild(); node; node = node->nextSibling())
         m_candidates.append(node);

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

@@ -135 +135 @@
     HTMLContentSelection* findFor(Node* key) const;
 
-    void willSelectOver(Element* shadowHost);
+    void willSelect();
+    bool isSelecting() const;
     void didSelect();
-    bool hasCandidates() const { return !m_candidates.isEmpty(); }
+
+    void populateIfNecessary(Element* shadowHost);
+    bool hasPopulated() const;
 
 private:
+    enum SelectingPhase {
+        SelectionPrevented,
+        SelectionStarted,
+        HostChildrenPopulated,
+    };
+
     void removeFromSet(HTMLContentSelectionList*);
     void addToSet(HTMLContentSelectionList*);
@@ -145 +154 @@
     Vector<RefPtr<Node> > m_candidates;
     HTMLContentSelectionSet m_selectionSet;
+    SelectingPhase m_phase;
 };
+
+inline bool HTMLContentSelector::isSelecting() const
+{
+    return m_phase != SelectionPrevented;
+}
+
+inline bool HTMLContentSelector::hasPopulated() const
+{
+    return m_phase == HostChildrenPopulated;
+}
 
 }

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

@@ -110 +110 @@
 inline void InsertionPoint::distributeHostChildren(ShadowTree* tree)
 {
-    HTMLContentSelector* selector = tree->ensureSelector();
-    selector->unselect(&m_selections);
-    selector->select(this, &m_selections);
+    if (!tree->selector().isSelecting()) {
+        // If HTMLContentSelector is not int selecting phase, it means InsertionPoint is attached from
+        // non-ShadowTree node. To run distribute algorithm, we have to reattach ShadowTree.
+        tree->setNeedsReattachHostChildrenAndShadow();
+        return;
+    }
+
+    tree->selector().populateIfNecessary(tree->host());
+    tree->selector().unselect(&m_selections);
+    tree->selector().select(this, &m_selections);
 }
 
 inline void InsertionPoint::clearDistribution(ShadowTree* tree)
 {
-    if (HTMLContentSelector* selector = tree->selector())
-        selector->unselect(&m_selections);
+    tree->selector().unselect(&m_selections);
 }