[Refactoring] Replace Node's Document pointer with a TreeScope pointer
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Dec 2012 23:20:20 +0000 (23:20 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Dec 2012 23:20:20 +0000 (23:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=59816

Patch by Elliott Sprehn <esprehn@gmail.com> on 2012-12-12
Reviewed by Ryosuke Niwa.

Instead of giving every node in a shadow a rare data, which can be quite
large, we replace the Document pointer in Node with a TreeScope pointer
and we give TreeScope a pointer to it's document scope.

This introduces no branches in document() because in the common
case document() becomes equivalent to m_treeScope->m_documentScope where
the documentScope is actually m_treeScope so this shouldn't introduce a
perf regression.

Note also that TreeScope can never be null after r136328, and the document
pointer is only null for DocumentType nodes so we can use a special
no-document TreeScope for this case that always returns null from
documentScope().

No new tests, no change in behavior.

* dom/Document.cpp:
(WebCore::Document::Document):
(WebCore::Document::~Document):
(WebCore::Document::suggestedMIMEType):
* dom/Document.h:
(WebCore::Node::isDocumentNode):
(WebCore::Node::Node):
* dom/Element.cpp:
(WebCore::Element::createRareData):
* dom/ElementRareData.h:
(ElementRareData):
(WebCore::ElementRareData::ElementRareData):
* dom/Node.cpp:
(WebCore::Node::~Node):
(WebCore::Node::createRareData):
(WebCore::Node::attach):
(WebCore::Node::reportMemoryUsage):
* dom/Node.h:
(WebCore):
(WebCore::NodeRareDataBase::NodeRareDataBase):
(NodeRareDataBase):
(WebCore::Node::treeScope):
(WebCore::Node::inDocument):
(WebCore::Node::documentInternal):
(WebCore::Node::setTreeScope):
(Node):
* dom/NodeRareData.cpp:
(WebCore::NodeRareData::reportMemoryUsage):
* dom/NodeRareData.h:
(WebCore::NodeRareData::NodeRareData):
* dom/ShadowRoot.cpp:
(WebCore::ShadowRoot::ShadowRoot):
* dom/TreeScope.cpp:
(SameSizeAsTreeScope):
(WebCore::TreeScope::TreeScope):
(WebCore::TreeScope::setParentTreeScope):
* dom/TreeScope.h:
(WebCore):
(TreeScope):
(WebCore::TreeScope::documentScope):
(WebCore::TreeScope::noDocumentInstance):
    Returns a special tree scope that has no document for use with
    DocumentType nodes.
(WebCore::TreeScope::setDocumentScope):
* dom/TreeScopeAdopter.cpp:
(WebCore::TreeScopeAdopter::moveTreeToNewScope):
(WebCore::TreeScopeAdopter::moveTreeToNewDocument):
(WebCore::TreeScopeAdopter::moveNodeToNewDocument):

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

13 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/ElementRareData.h
Source/WebCore/dom/Node.cpp
Source/WebCore/dom/Node.h
Source/WebCore/dom/NodeRareData.cpp
Source/WebCore/dom/NodeRareData.h
Source/WebCore/dom/ShadowRoot.cpp
Source/WebCore/dom/TreeScope.cpp
Source/WebCore/dom/TreeScope.h
Source/WebCore/dom/TreeScopeAdopter.cpp

index 5971dd1..08db77f 100644 (file)
@@ -1,3 +1,75 @@
+2012-12-12  Elliott Sprehn  <esprehn@gmail.com>
+
+        [Refactoring] Replace Node's Document pointer with a TreeScope pointer
+        https://bugs.webkit.org/show_bug.cgi?id=59816
+
+        Reviewed by Ryosuke Niwa.
+
+        Instead of giving every node in a shadow a rare data, which can be quite
+        large, we replace the Document pointer in Node with a TreeScope pointer
+        and we give TreeScope a pointer to it's document scope.
+
+        This introduces no branches in document() because in the common
+        case document() becomes equivalent to m_treeScope->m_documentScope where
+        the documentScope is actually m_treeScope so this shouldn't introduce a
+        perf regression.
+
+        Note also that TreeScope can never be null after r136328, and the document
+        pointer is only null for DocumentType nodes so we can use a special
+        no-document TreeScope for this case that always returns null from
+        documentScope().
+
+        No new tests, no change in behavior.
+
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        (WebCore::Document::~Document):
+        (WebCore::Document::suggestedMIMEType):
+        * dom/Document.h:
+        (WebCore::Node::isDocumentNode):
+        (WebCore::Node::Node):
+        * dom/Element.cpp:
+        (WebCore::Element::createRareData):
+        * dom/ElementRareData.h:
+        (ElementRareData):
+        (WebCore::ElementRareData::ElementRareData):
+        * dom/Node.cpp:
+        (WebCore::Node::~Node):
+        (WebCore::Node::createRareData):
+        (WebCore::Node::attach):
+        (WebCore::Node::reportMemoryUsage):
+        * dom/Node.h:
+        (WebCore):
+        (WebCore::NodeRareDataBase::NodeRareDataBase):
+        (NodeRareDataBase):
+        (WebCore::Node::treeScope):
+        (WebCore::Node::inDocument):
+        (WebCore::Node::documentInternal):
+        (WebCore::Node::setTreeScope):
+        (Node):
+        * dom/NodeRareData.cpp:
+        (WebCore::NodeRareData::reportMemoryUsage):
+        * dom/NodeRareData.h:
+        (WebCore::NodeRareData::NodeRareData):
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::ShadowRoot):
+        * dom/TreeScope.cpp:
+        (SameSizeAsTreeScope):
+        (WebCore::TreeScope::TreeScope):
+        (WebCore::TreeScope::setParentTreeScope):
+        * dom/TreeScope.h:
+        (WebCore):
+        (TreeScope):
+        (WebCore::TreeScope::documentScope):
+        (WebCore::TreeScope::noDocumentInstance):
+            Returns a special tree scope that has no document for use with
+            DocumentType nodes.
+        (WebCore::TreeScope::setDocumentScope):
+        * dom/TreeScopeAdopter.cpp:
+        (WebCore::TreeScopeAdopter::moveTreeToNewScope):
+        (WebCore::TreeScopeAdopter::moveTreeToNewDocument):
+        (WebCore::TreeScopeAdopter::moveNodeToNewDocument):
+
 2012-12-12  Mark Lam  <mark.lam@apple.com>
 
         Encapsulate externally used webdatabase APIs in DatabaseManager.
index 154c232..ee18a6d 100644 (file)
@@ -436,7 +436,7 @@ private:
 uint64_t Document::s_globalTreeVersion = 0;
 
 Document::Document(Frame* frame, const KURL& url, bool isXHTML, bool isHTML)
-    : ContainerNode(0, CreateDocument)
+    : ContainerNode(this, CreateDocument)
     , TreeScope(this)
     , m_guardRefCount(0)
     , m_styleResolverThrowawayTimer(this, &Document::styleResolverThrowawayTimerFired)
@@ -510,8 +510,6 @@ Document::Document(Frame* frame, const KURL& url, bool isXHTML, bool isHTML)
     , m_didDispatchViewportPropertiesChanged(false)
 #endif
 {
-    m_document = this;
-
     m_printing = false;
     m_paginatedForScreen = false;
 
@@ -669,8 +667,6 @@ Document::~Document()
     for (unsigned i = 0; i < WTF_ARRAY_LENGTH(m_nodeListCounts); i++)
         ASSERT(!m_nodeListCounts[i]);
 
-    m_document = 0;
-
     InspectorCounters::decrementCounter(InspectorCounters::DocumentCounter);
 }
 
@@ -1345,13 +1341,13 @@ void Document::setContent(const String& content)
 
 String Document::suggestedMIMEType() const
 {
-    if (m_document->isXHTMLDocument())
+    if (isXHTMLDocument())
         return "application/xhtml+xml";
-    if (m_document->isSVGDocument())
+    if (isSVGDocument())
         return "image/svg+xml";
-    if (m_document->xmlStandalone())
+    if (xmlStandalone())
         return "text/xml";
-    if (m_document->isHTMLDocument())
+    if (isHTMLDocument())
         return "text/html";
 
     if (DocumentLoader* documentLoader = loader())
index d994f61..db891d5 100644 (file)
@@ -1559,22 +1559,20 @@ inline void Document::notifyRemovePendingSheetIfNeeded()
 
 inline bool Node::isDocumentNode() const
 {
-    return this == m_document;
-}
-
-inline TreeScope* Node::treeScope() const
-{
-    return hasRareData() ? m_data.m_rareData->treeScope() : documentInternal();
+    return this == documentInternal();
 }
 
 inline Node::Node(Document* document, ConstructionType type)
     : m_nodeFlags(type)
-    , m_document(document)
+    , m_treeScope(document)
     , m_previous(0)
     , m_next(0)
 {
     if (document)
         document->guardRef();
+    else
+        m_treeScope = TreeScope::noDocumentInstance();
+
 #if !defined(NDEBUG) || (defined(DUMP_NODE_STATISTICS) && DUMP_NODE_STATISTICS)
     trackForDebugging();
 #endif
index 4498d32..26ab1e9 100644 (file)
@@ -217,7 +217,7 @@ inline ElementRareData* Element::ensureElementRareData()
 
 PassOwnPtr<NodeRareData> Element::createRareData()
 {
-    return adoptPtr(new ElementRareData(documentInternal()));
+    return adoptPtr(new ElementRareData());
 }
 
 DEFINE_VIRTUAL_ATTRIBUTE_EVENT_LISTENER(Element, blur);
index 7905cb5..88f26bd 100644 (file)
@@ -35,7 +35,7 @@ namespace WebCore {
 
 class ElementRareData : public NodeRareData {
 public:
-    ElementRareData(Document*);
+    ElementRareData();
     virtual ~ElementRareData();
 
     void setPseudoElement(PseudoId, PassRefPtr<PseudoElement>);
@@ -130,9 +130,8 @@ inline IntSize defaultMinimumSizeForResizing()
     return IntSize(LayoutUnit::max(), LayoutUnit::max());
 }
 
-inline ElementRareData::ElementRareData(Document* document)
-    : NodeRareData(document)
-    , m_minimumSizeForResizing(defaultMinimumSizeForResizing())
+inline ElementRareData::ElementRareData()
+    : m_minimumSizeForResizing(defaultMinimumSizeForResizing())
     , m_generatedBefore(0)
     , m_generatedAfter(0)
 {
index 765302a..b4c2fed 100644 (file)
@@ -418,7 +418,7 @@ Node::~Node()
     if (renderer())
         detach();
 
-    Document* doc = m_document;
+    Document* doc = documentInternal();
     if (AXObjectCache::accessibilityEnabled() && doc && doc->axObjectCacheExists() && !isContainerNode())
         doc->axObjectCache()->remove(this);
     
@@ -433,23 +433,6 @@ Node::~Node()
     InspectorCounters::decrementCounter(InspectorCounters::NodeCounter);
 }
 
-void Node::setDocument(Document* document)
-{
-    ASSERT(!inDocument() || m_document == document);
-    if (inDocument() || m_document == document)
-        return;
-
-    m_document = document;
-}
-
-void Node::setTreeScope(TreeScope* scope)
-{
-    if (!hasRareData() && scope->rootNode()->isDocumentNode())
-        return;
-
-    ensureRareData()->setTreeScope(scope);
-}
-
 NodeRareData* Node::rareData() const
 {
     ASSERT(hasRareData());
@@ -471,7 +454,7 @@ NodeRareData* Node::ensureRareData()
 
 PassOwnPtr<NodeRareData> Node::createRareData()
 {
-    return adoptPtr(new NodeRareData(documentInternal()));
+    return adoptPtr(new NodeRareData());
 }
 
 void Node::clearRareData()
@@ -1082,7 +1065,7 @@ void Node::attach()
     setAttached();
     clearNeedsStyleRecalc();
 
-    Document* doc = m_document;
+    Document* doc = documentInternal();
     if (AXObjectCache::accessibilityEnabled() && doc && doc->axObjectCacheExists())
         doc->axObjectCache()->updateCacheAfterNodeIsAttached(this);
 }
@@ -2599,7 +2582,7 @@ void Node::reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
     MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::DOM);
     TreeShared<Node, ContainerNode>::reportMemoryUsage(memoryObjectInfo);
     ScriptWrappable::reportMemoryUsage(memoryObjectInfo);
-    info.addMember(m_document);
+    info.addMember(m_treeScope);
     info.addMember(m_next);
     info.addMember(m_previous);
     info.addMember(this->renderer());
index 50539bb..849dd65 100644 (file)
@@ -33,6 +33,7 @@
 #include "RenderStyleConstants.h"
 #include "ScriptWrappable.h"
 #include "SimulatedClickOptions.h"
+#include "TreeScope.h"
 #include "TreeShared.h"
 #include <wtf/Forward.h>
 #include <wtf/ListHashSet.h>
@@ -85,7 +86,6 @@ class RenderObject;
 class RenderStyle;
 class ShadowRoot;
 class TagNodeList;
-class TreeScope;
 
 #if ENABLE(GESTURE_EVENTS)
 class PlatformGestureEvent;
@@ -115,18 +115,11 @@ public:
     RenderObject* renderer() const { return m_renderer; }
     void setRenderer(RenderObject* renderer) { m_renderer = renderer; }
 
-    TreeScope* treeScope() const { return m_treeScope; }
-    void setTreeScope(TreeScope* scope) { m_treeScope = scope; }
-
     virtual ~NodeRareDataBase() { }
 protected:
-    NodeRareDataBase(TreeScope* scope)
-        : m_treeScope(scope)
-    {
-    }
+    NodeRareDataBase() { }
 private:
     RenderObject* m_renderer;
-    TreeScope* m_treeScope;
 };
 
 class Node : public EventTarget, public ScriptWrappable, public TreeShared<Node, ContainerNode> {
@@ -459,13 +452,13 @@ public:
         return documentInternal();
     }
 
-    TreeScope* treeScope() const;
+    TreeScope* treeScope() const { return m_treeScope; }
 
     // Returns true if this node is associated with a document and is in its associated document's
     // node tree, false otherwise.
     bool inDocument() const 
     { 
-        ASSERT(m_document || !getFlag(InDocumentFlag));
+        ASSERT(documentInternal() || !getFlag(InDocumentFlag));
         return getFlag(InDocumentFlag);
     }
 
@@ -756,17 +749,14 @@ protected:
 
     void setHasCustomCallbacks() { setFlag(true, HasCustomCallbacksFlag); }
 
-    Document* documentInternal() const { return m_document; }
+    Document* documentInternal() const { return treeScope()->documentScope(); }
+    void setTreeScope(TreeScope* scope) { m_treeScope = scope; }
 
 private:
     friend class TreeShared<Node, ContainerNode>;
 
     void removedLastRef();
 
-    // These API should be only used for a tree scope migration.
-    void setTreeScope(TreeScope*);
-    void setDocument(Document*);
-
     enum EditableLevel { Editable, RichlyEditable };
     bool rendererIsEditable(EditableLevel, UserSelectAllTreatment = UserSelectAllIsAlwaysNonEditable) const;
     bool isEditableToAccessibility(EditableLevel) const;
@@ -809,7 +799,7 @@ private:
 #endif
 
     mutable uint32_t m_nodeFlags;
-    Document* m_document;
+    TreeScope* m_treeScope;
     Node* m_previous;
     Node* m_next;
     // When a node has rare data we move the renderer into the rare data.
index 95607bb..2fb4944 100644 (file)
@@ -39,7 +39,7 @@
 namespace WebCore {
 
 struct SameSizeAsNodeRareData {
-    void* m_pointer[4];
+    void* m_pointer[3];
     unsigned m_indicesAndBitfields[2];
 
 #if ENABLE(MUTATION_OBSERVERS)
@@ -64,7 +64,6 @@ void NodeListsNodeData::reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) co
 void NodeRareData::reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
 {
     MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::DOM);
-    info.addMember(treeScope());
     info.addMember(m_nodeLists);
 
 #if ENABLE(MUTATION_OBSERVERS)
index d259371..81a653a 100644 (file)
@@ -246,9 +246,8 @@ class NodeRareData : public NodeRareDataBase {
 #endif
 
 public:    
-    NodeRareData(Document* document)
-        : NodeRareDataBase(document)
-        , m_tabIndex(0)
+    NodeRareData()
+        : m_tabIndex(0)
         , m_childIndex(0)
         , m_tabIndexWasSetExplicitly(false)
         , m_needsFocusAppearanceUpdateSoonAfterAttach(false)
index 897089e..e666ec0 100644 (file)
@@ -65,7 +65,7 @@ COMPILE_ASSERT(sizeof(ShadowRoot) == sizeof(SameSizeAsShadowRoot), shadowroot_sh
 
 ShadowRoot::ShadowRoot(Document* document)
     : DocumentFragment(document, CreateShadowRoot)
-    , TreeScope(this)
+    , TreeScope(this, document)
     , m_prev(0)
     , m_next(0)
     , m_numberOfStyles(0)
@@ -75,12 +75,7 @@ ShadowRoot::ShadowRoot(Document* document)
     , m_registeredWithParentShadowRoot(false)
 {
     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);
+    setTreeScope(this);
 }
 
 ShadowRoot::~ShadowRoot()
index fe19a0f..9ad552b 100644 (file)
@@ -55,19 +55,38 @@ namespace WebCore {
 
 struct SameSizeAsTreeScope {
     virtual ~SameSizeAsTreeScope();
-    void* pointers[7];
+    void* pointers[8];
 };
 
 COMPILE_ASSERT(sizeof(TreeScope) == sizeof(SameSizeAsTreeScope), treescope_should_stay_small);
 
 using namespace HTMLNames;
 
-TreeScope::TreeScope(ContainerNode* rootNode)
+TreeScope::TreeScope(ContainerNode* rootNode, Document* document)
     : m_rootNode(rootNode)
-    , m_parentTreeScope(0)
+    , m_documentScope(document)
+    , m_parentTreeScope(document)
     , m_idTargetObserverRegistry(IdTargetObserverRegistry::create())
 {
     ASSERT(rootNode);
+    ASSERT(document);
+    ASSERT(rootNode != document);
+}
+
+TreeScope::TreeScope(Document* document)
+    : m_rootNode(document)
+    , m_documentScope(document)
+    , m_parentTreeScope(0)
+    , m_idTargetObserverRegistry(IdTargetObserverRegistry::create())
+{
+    ASSERT(document);
+}
+
+TreeScope::TreeScope()
+    : m_rootNode(0)
+    , m_documentScope(0)
+    , m_parentTreeScope(0)
+{
 }
 
 TreeScope::~TreeScope()
@@ -93,6 +112,7 @@ void TreeScope::setParentTreeScope(TreeScope* newParentScope)
     ASSERT(newParentScope);
 
     m_parentTreeScope = newParentScope;
+    setDocumentScope(newParentScope->documentScope());
 }
 
 Element* TreeScope::getElementById(const AtomicString& elementId) const
index cb6690d..c1d04c6 100644 (file)
@@ -35,6 +35,7 @@ namespace WebCore {
 
 class ContainerNode;
 class DOMSelection;
+class Document;
 class Element;
 class HTMLLabelElement;
 class HTMLMapElement;
@@ -46,6 +47,7 @@ class Node;
 // the destructor.
 class TreeScope {
     friend class Document;
+    friend class TreeScopeAdopter;
 
 public:
     TreeScope* parentTreeScope() const { return m_parentTreeScope; }
@@ -58,6 +60,8 @@ public:
     void addElementById(const AtomicString& elementId, Element*);
     void removeElementById(const AtomicString& elementId, Element*);
 
+    Document* documentScope() const { return m_documentScope; }
+
     Node* ancestorInThisScope(Node*) const;
 
     void addImageMap(HTMLMapElement*);
@@ -91,14 +95,30 @@ public:
 
     virtual void reportMemoryUsage(MemoryObjectInfo*) const;
 
+    static TreeScope* noDocumentInstance()
+    {
+        DEFINE_STATIC_LOCAL(TreeScope, instance, ());
+        return &instance;
+    }
+
 protected:
-    explicit TreeScope(ContainerNode*);
+    TreeScope(ContainerNode*, Document*);
+    TreeScope(Document*);
     virtual ~TreeScope();
 
     void destroyTreeScopeData();
+    void setDocumentScope(Document* document)
+    {
+        ASSERT(document);
+        ASSERT(this != noDocumentInstance());
+        m_documentScope = document;
+    }
 
 private:
+    TreeScope();
+
     ContainerNode* m_rootNode;
+    Document* m_documentScope;
     TreeScope* m_parentTreeScope;
 
     OwnPtr<DocumentOrderedMap> m_elementsById;
index 019e67a..4be6f97 100644 (file)
@@ -43,14 +43,15 @@ void TreeScopeAdopter::moveTreeToNewScope(Node* root) const
     // that element may contain stale data as changes made to it will have updated the DOMTreeVersion
     // of the document it was moved to. By increasing the DOMTreeVersion of the donating document here
     // we ensure that the collection cache will be invalidated as needed when the element is moved back.
-    Document* oldDocument = m_oldScope ? m_oldScope->rootNode()->document() : 0;
-    Document* newDocument = m_newScope->rootNode()->document();
+    Document* oldDocument = m_oldScope->documentScope();
+    Document* newDocument = m_newScope->documentScope();
     bool willMoveToNewDocument = oldDocument != newDocument;
     if (oldDocument && willMoveToNewDocument)
         oldDocument->incDOMTreeVersion();
 
     for (Node* node = root; node; node = NodeTraversal::next(node, root)) {
         node->setTreeScope(m_newScope);
+
         if (node->hasRareData()) {
             NodeRareData* rareData = node->rareData();
             if (rareData->nodeLists())
@@ -97,7 +98,8 @@ inline void TreeScopeAdopter::moveNodeToNewDocument(Node* node, Document* oldDoc
     if (oldDocument)
         oldDocument->moveNodeIteratorsToNewDocument(node, newDocument);
 
-    node->setDocument(newDocument);
+    if (node->isShadowRoot())
+        toShadowRoot(node)->setDocumentScope(newDocument);
 
 #ifndef NDEBUG
     didMoveToNewDocumentWasCalled = false;