2011-04-19 Sheriff Bot <webkit.review.bot@gmail.com>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Apr 2011 13:41:54 +0000 (13:41 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Apr 2011 13:41:54 +0000 (13:41 +0000)
        Unreviewed, rolling out r84050.
        http://trac.webkit.org/changeset/84050
        https://bugs.webkit.org/show_bug.cgi?id=58892

        fast/dom/HTMLKeygenElement/keygen.html is crashing on Chromium
        (Requested by jknotten on #webkit).

        * dom/ContainerNode.cpp:
        (WebCore::ContainerNode::takeAllChildrenFrom):
        (WebCore::ContainerNode::removeBetween):
        (WebCore::ContainerNode::removeChildren):
        (WebCore::ContainerNode::parserAddChild):
        * dom/Document.cpp:
        (WebCore::Document::Document):
        (WebCore::Document::~Document):
        (WebCore::Document::setDocType):
        * dom/Element.h:
        * dom/Node.cpp:
        (WebCore::Node::treeScope):
        (WebCore::Node::setTreeScope):
        (WebCore::Node::setTreeScopeRecursively):
        * dom/Node.h:
        (WebCore::Node::document):
        * dom/ShadowRoot.cpp:
        (WebCore::ShadowRoot::ShadowRoot):
        * dom/ShadowRoot.h:
        * dom/TreeScope.cpp:
        (WebCore::TreeScope::TreeScope):
        (WebCore::TreeScope::setParentTreeScope):
        * dom/TreeScope.h:
        * rendering/RenderSlider.cpp:

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

Source/WebCore/ChangeLog
Source/WebCore/dom/ContainerNode.cpp
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/Node.cpp
Source/WebCore/dom/Node.h
Source/WebCore/dom/ShadowRoot.cpp
Source/WebCore/dom/ShadowRoot.h
Source/WebCore/dom/TreeScope.cpp
Source/WebCore/dom/TreeScope.h
Source/WebCore/rendering/RenderSlider.cpp

index 04fbe1d..cbc1dd4 100644 (file)
@@ -1,3 +1,37 @@
+2011-04-19  Sheriff Bot  <webkit.review.bot@gmail.com>
+
+        Unreviewed, rolling out r84050.
+        http://trac.webkit.org/changeset/84050
+        https://bugs.webkit.org/show_bug.cgi?id=58892
+
+        fast/dom/HTMLKeygenElement/keygen.html is crashing on Chromium
+        (Requested by jknotten on #webkit).
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::takeAllChildrenFrom):
+        (WebCore::ContainerNode::removeBetween):
+        (WebCore::ContainerNode::removeChildren):
+        (WebCore::ContainerNode::parserAddChild):
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        (WebCore::Document::~Document):
+        (WebCore::Document::setDocType):
+        * dom/Element.h:
+        * dom/Node.cpp:
+        (WebCore::Node::treeScope):
+        (WebCore::Node::setTreeScope):
+        (WebCore::Node::setTreeScopeRecursively):
+        * dom/Node.h:
+        (WebCore::Node::document):
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::ShadowRoot):
+        * dom/ShadowRoot.h:
+        * dom/TreeScope.cpp:
+        (WebCore::TreeScope::TreeScope):
+        (WebCore::TreeScope::setParentTreeScope):
+        * dom/TreeScope.h:
+        * rendering/RenderSlider.cpp:
+
 2011-04-19  Kinuko Yasuda  <kinuko@chromium.org>
 
         Not reviewed; windows build fix attempt.
index ce38039..276df56 100644 (file)
@@ -90,10 +90,6 @@ void ContainerNode::takeAllChildrenFrom(ContainerNode* oldParent)
         RefPtr<Node> child = document()->adoptNode(children[i].release(), ec);
         ASSERT(!ec);
         parserAddChild(child.get());
-        // FIXME: Together with adoptNode above, the tree scope might get updated recursively twice
-        // (if the document changed or oldParent was in a shadow tree, AND *this is in a shadow tree).
-        // Can we do better?
-        child->setTreeScopeRecursively(treeScope());
         if (attached() && !child->attached())
             child->attach();
     }
@@ -485,8 +481,6 @@ void ContainerNode::removeBetween(Node* previousChild, Node* nextChild, Node* ol
     oldChild->setNextSibling(0);
     oldChild->setParent(0);
 
-    oldChild->setTreeScopeRecursively(document());
-
     allowEventDispatch();
 }
 
@@ -536,7 +530,6 @@ void ContainerNode::removeChildren()
         n->setPreviousSibling(0);
         n->setNextSibling(0);
         n->setParent(0);
-        n->setTreeScopeRecursively(document());
 
         m_firstChild = next;
         if (n == m_lastChild)
@@ -655,8 +648,6 @@ void ContainerNode::parserAddChild(PassRefPtr<Node> newChild)
     Node* last = m_lastChild;
     // FIXME: This method should take a PassRefPtr.
     appendChildToContainer<Node, ContainerNode>(newChild.get(), this);
-    newChild->setTreeScopeRecursively(treeScope());
-    
     allowEventDispatch();
 
     // FIXME: Why doesn't this use notifyChildInserted(newChild) instead?
index 2f611b2..784c613 100644 (file)
@@ -377,7 +377,7 @@ private:
 uint64_t Document::s_globalTreeVersion = 0;
 
 Document::Document(Frame* frame, const KURL& url, bool isXHTML, bool isHTML)
-    : TreeScope(0)
+    : TreeScope(this)
     , m_guardRefCount(0)
     , m_compatibilityMode(NoQuirksMode)
     , m_compatibilityModeLocked(false)
@@ -433,7 +433,6 @@ Document::Document(Frame* frame, const KURL& url, bool isXHTML, bool isHTML)
     , m_writeRecursionIsTooDeep(false)
     , m_writeRecursionDepth(0)
 {
-
     m_document = this;
 
     m_pageGroupUserSheetCacheValid = false;
@@ -514,7 +513,6 @@ Document::~Document()
     ASSERT(m_ranges.isEmpty());
     ASSERT(!m_styleRecalcTimer.isActive());
     ASSERT(!m_parentTreeScope);
-    ASSERT(!m_guardRefCount);
 
     m_scriptRunner.clear();
 
@@ -672,7 +670,7 @@ void Document::setDocType(PassRefPtr<DocumentType> docType)
     ASSERT(!m_docType || !docType);
     m_docType = docType;
     if (m_docType)
-        m_docType->setTreeScopeRecursively(this);
+        m_docType->setTreeScope(this);
 }
 
 DOMImplementation* Document::implementation()
index 0af6d97..d269dbe 100644 (file)
@@ -39,7 +39,6 @@ class DOMStringMap;
 class DOMTokenList;
 class ElementRareData;
 class IntSize;
-class ShadowRoot;
 class WebKitAnimationList;
 
 enum SpellcheckAttributeState {
@@ -230,7 +229,6 @@ public:
     virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
     virtual void recalcStyle(StyleChange = NoChange);
 
-    // FIXME: Make these return a proper ShadowRoot* (bug 58703).
     ContainerNode* shadowRoot() const;
     ContainerNode* ensureShadowRoot();
     void removeShadowRoot();
index ea8471d..1a8a7b0 100644 (file)
@@ -77,7 +77,6 @@
 #include "ScopedEventQueue.h"
 #include "ScriptController.h"
 #include "SelectorNodeList.h"
-#include "ShadowRoot.h"
 #include "StaticNodeList.h"
 #include "TagNodeList.h"
 #include "Text.h"
@@ -469,22 +468,39 @@ void Node::setDocument(Document* document)
 
 TreeScope* Node::treeScope() const
 {
-    // FIXME: Using m_document directly is not good -> see comment with document() in the header file.
     if (!hasRareData())
-        return m_document;
+        return document();
     TreeScope* scope = rareData()->treeScope();
-    return scope ? scope : m_document;
+    // FIXME: Until we land shadow scopes, there should be no non-document scopes.
+    ASSERT(!scope);
+    return scope ? scope : document();
 }
 
-void Node::setTreeScopeRecursively(TreeScope* newTreeScope, bool includeRoot)
+void Node::setTreeScope(TreeScope* newTreeScope)
 {
-    ASSERT(this);
-    ASSERT(!includeRoot || !isDocumentNode());
+    ASSERT(!isDocumentNode());
     ASSERT(newTreeScope);
-    ASSERT(!m_deletionHasBegun);
+    ASSERT(!inDocument() || treeScope() == newTreeScope);
 
-    TreeScope* currentTreeScope = treeScope();
-    if (currentTreeScope == newTreeScope)
+    if (newTreeScope->isDocumentNode()) {
+        if (hasRareData())
+            rareData()->setTreeScope(0);
+        // Setting the new document scope will be handled implicitly
+        // by setDocument() below.
+    } else {
+        // FIXME: Until we land shadow scopes, this branch should be inert.
+        ASSERT_NOT_REACHED();
+        ensureRareData()->setTreeScope(newTreeScope);
+    }
+
+    setDocument(newTreeScope->document());
+}
+
+void Node::setTreeScopeRecursively(TreeScope* newTreeScope)
+{
+    ASSERT(!isDocumentNode());
+    ASSERT(newTreeScope);
+    if (treeScope() == newTreeScope)
         return;
 
     Document* currentDocument = document();
@@ -496,25 +512,9 @@ void Node::setTreeScopeRecursively(TreeScope* newTreeScope, bool includeRoot)
     if (currentDocument && currentDocument != newDocument)
         currentDocument->incDOMTreeVersion();
 
-    for (Node* node = includeRoot ? this : traverseNextNode(this); node; node = node->traverseNextNode(this)) {
-        if (newTreeScope == newDocument) {
-            if (node->hasRareData())
-                node->rareData()->setTreeScope(0);
-            // Setting the new document tree scope will be handled implicitly
-            // by setDocument() below.
-        } else
-            node->ensureRareData()->setTreeScope(newTreeScope);
-
-        node->setDocument(newDocument);
-
-        if (!node->isElementNode())
-            continue;
-        // FIXME: Remove toShadowRoot() once shadowRoot() returns a proper ShadowRoot* (bug 58703).
-        if (ShadowRoot* shadowRoot = toShadowRoot(toElement(node)->shadowRoot())) {
-            shadowRoot->setParentTreeScope(newTreeScope);
-            if (currentDocument != newDocument)
-                shadowRoot->setDocumentRecursively(newDocument);
-        }
+    for (Node* node = this; node; node = node->traverseNextNode(this)) {
+        node->setTreeScope(newTreeScope);
+        // FIXME: Once shadow scopes are landed, update parent scope, etc.
     }
 }
 
index 2e26885..844f7a0 100644 (file)
@@ -359,16 +359,18 @@ public:
     Document* document() const
     {
         ASSERT(this);
-        // FIXME: below ASSERT is useful, but prevents the use of document() in the constructor or destructor
-        // due to the virtual function call to nodeType().
         ASSERT(m_document || (nodeType() == DOCUMENT_TYPE_NODE && !inDocument()));
         return m_document;
     }
 
     TreeScope* treeScope() const;
 
+    // Do not use this method to change the scope of a node until after the node has been
+    // removed from its previous scope. Do not use to change documents.
+    void setTreeScope(TreeScope*);
+
     // Used by the basic DOM methods (e.g., appendChild()).
-    void setTreeScopeRecursively(TreeScope*, bool includeRoot = true);
+    void setTreeScopeRecursively(TreeScope*);
 
     // Returns true if this node is associated with a document and is in its associated document's
     // node tree, false otherwise.
index e54767e..8fe56b5 100644 (file)
 #include "config.h"
 #include "ShadowRoot.h"
 
-#include "Document.h"
-#include "NodeRareData.h"
-
 namespace WebCore {
 
 ShadowRoot::ShadowRoot(Document* document)
-    : TreeScope(document)
+    : DocumentFragment(document)
 {
     ASSERT(document);
-    
-    // Assume document as parent scope.
-    setParentTreeScope(document);
-    // Shadow tree scopes have the scope pointer point to themselves.
-    // This way, direct children will receive the correct scope pointer.
-    ensureRareData()->setTreeScope(this);
-}
-
-ShadowRoot::~ShadowRoot()
-{
 }
 
 String ShadowRoot::nodeName() const
@@ -53,33 +40,6 @@ String ShadowRoot::nodeName() const
     return "#shadow-root";
 }
 
-Node::NodeType ShadowRoot::nodeType() const
-{
-    // FIXME: Decide correct node type (bug 58704).
-    return DOCUMENT_FRAGMENT_NODE;
-}
-
-PassRefPtr<Node> ShadowRoot::cloneNode(bool)
-{
-    // ShadowRoot should not be arbitrarily cloned.
-    return 0;
-}
-
-bool ShadowRoot::childTypeAllowed(NodeType type) const
-{
-    switch (type) {
-    case ELEMENT_NODE:
-    case PROCESSING_INSTRUCTION_NODE:
-    case COMMENT_NODE:
-    case TEXT_NODE:
-    case CDATA_SECTION_NODE:
-    case ENTITY_REFERENCE_NODE:
-        return true;
-    default:
-        return false;
-    }
-}
-
 void ShadowRoot::recalcStyle(StyleChange change)
 {
     for (Node* n = firstChild(); n; n = n->nextSibling())
index db51b2b..aeccd8a 100644 (file)
 #ifndef ShadowRoot_h
 #define ShadowRoot_h
 
-#include "TreeScope.h"
+#include "DocumentFragment.h"
 
 namespace WebCore {
 
 class Document;
 
-class ShadowRoot : public TreeScope {
+class ShadowRoot : public DocumentFragment {
 public:
     static PassRefPtr<ShadowRoot> create(Document*);
 
@@ -42,12 +42,7 @@ public:
 
 private:
     ShadowRoot(Document*);
-    virtual ~ShadowRoot();
-
     virtual String nodeName() const;
-    virtual NodeType nodeType() const;
-    virtual PassRefPtr<Node> cloneNode(bool deep);
-    virtual bool childTypeAllowed(NodeType) const;
 };
 
 inline PassRefPtr<ShadowRoot> ShadowRoot::create(Document* document)
@@ -55,12 +50,6 @@ inline PassRefPtr<ShadowRoot> ShadowRoot::create(Document* document)
     return adoptRef(new ShadowRoot(document));
 }
 
-inline ShadowRoot* toShadowRoot(Node* node)
-{
-    ASSERT(!node || node->isShadowBoundary());
-    return static_cast<ShadowRoot*>(node);
-}
-
 } // namespace
 
 #endif
index c8dc07e..a995a2d 100644 (file)
@@ -36,12 +36,19 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-TreeScope::TreeScope(Document* document)
-    : ContainerNode(document)
+TreeScope::TreeScope(Document* document, ConstructionType constructionType)
+    : ContainerNode(0, constructionType)
     , m_parentTreeScope(0)
     , m_accessKeyMapValid(false)
     , m_numNodeListCaches(0)
 {
+    m_document = document;
+    if (document != this) {
+        // Assume document as parent scope
+        m_parentTreeScope = document;
+        // FIXME: This branch should be inert until shadow scopes are landed.
+        ASSERT_NOT_REACHED();
+    }
 }
 
 TreeScope::~TreeScope()
@@ -62,6 +69,7 @@ void TreeScope::setParentTreeScope(TreeScope* newParentScope)
     // A document node cannot be re-parented.
     ASSERT(!isDocumentNode());
     // Every scope other than document needs a parent scope.
+    ASSERT(m_parentTreeScope);
     ASSERT(newParentScope);
 
     m_parentTreeScope = newParentScope;
index 1f90a09..6271541 100644 (file)
@@ -39,7 +39,6 @@ class TreeScope : public ContainerNode {
 
 public:
     TreeScope* parentTreeScope() const { return m_parentTreeScope; }
-    void setParentTreeScope(TreeScope*);
 
     Element* getElementById(const AtomicString&) const;
     bool hasElementWithId(AtomicStringImpl* id) const;
@@ -66,11 +65,14 @@ public:
     Element* findAnchor(const String& name);
 
 protected:
-    TreeScope(Document*);
+    TreeScope(Document*, ConstructionType = CreateContainer);
+
     virtual ~TreeScope();
 
     void destroyTreeScopeData();
 
+    void setParentTreeScope(TreeScope*);
+
 private:
     TreeScope* m_parentTreeScope;
 
index 07d325e..ad5ab1a 100644 (file)
@@ -37,7 +37,7 @@
 #include "RenderLayer.h"
 #include "RenderTheme.h"
 #include "RenderView.h"
-#include "ShadowRoot.h"
+#include "ShadowElement.h"
 #include "SliderThumbElement.h"
 #include "StepRange.h"
 #include <wtf/MathExtras.h>