Changeset 138735 in webkit


Ignore:
Timestamp:
Jan 3, 2013 1:39:11 PM (11 years ago)
Author:
commit-queue@webkit.org
Message:

[Refactoring] Replace Node's Document pointer with a TreeScope pointer
​https://bugs.webkit.org/show_bug.cgi?id=59816

Patch by Elliott Sprehn <Elliott Sprehn> on 2013-01-03
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().

My original patch in r137524 for this did not correctly handle the case
where the Document is being destroyed and we would then call guardDeref on
ourself in the middle of the Document destructor causing asserts. To fix
this we need to go back to the original model where Document passes
null to it's super constructor for the Document pointer and assigns
it's tree scope later, and we also need to clear the tree scope pointer
in Document's destructor.

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::NodeRareDataBase::NodeRareDataBase):
(NodeRareDataBase):
(WebCore::Node::treeScope):
(WebCore::Node::inDocument):
(WebCore::Node::documentInternal):
(WebCore::Node::setTreeScope):
(Node):

  • dom/NodeRareData.cpp:

(SameSizeAsNodeRareData):
(WebCore::NodeRareData::reportMemoryUsage):

  • dom/NodeRareData.h:

(WebCore::NodeRareData::NodeRareData):

  • dom/ShadowRoot.cpp:

(WebCore::ShadowRoot::ShadowRoot):

  • dom/TreeScope.cpp:

(SameSizeAsTreeScope):
(WebCore::TreeScope::TreeScope):
(WebCore::TreeScope::clearDocumentScope):

Needed to clear the document scope during Document destruction
so the Node destructor does not guardDeref the document that is
already being destroyed.

(WebCore::TreeScope::setParentTreeScope):

  • dom/TreeScope.h:

(TreeScope):
(WebCore::TreeScope::documentScope):
(WebCore::TreeScope::noDocumentInstance):

Special shared tree scope that has a document scope that is always
null. This is needed for DocType nodes, and also for Documents
during construction.

(WebCore::TreeScope::setDocumentScope):

  • dom/TreeScopeAdopter.cpp:

(WebCore::TreeScopeAdopter::moveTreeToNewScope):
(WebCore::TreeScopeAdopter::moveNodeToNewDocument):

Location:
trunk/Source/WebCore
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r138731 r138735  
     12013-01-03  Elliott Sprehn  <esprehn@gmail.com>
     2
     3        [Refactoring] Replace Node's Document pointer with a TreeScope pointer
     4        https://bugs.webkit.org/show_bug.cgi?id=59816
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Instead of giving every node in a shadow a rare data, which can be quite
     9        large, we replace the Document pointer in Node with a TreeScope pointer
     10        and we give TreeScope a pointer to it's document scope.
     11
     12        This introduces no branches in document() because in the common
     13        case document() becomes equivalent to m_treeScope->m_documentScope where
     14        the documentScope is actually m_treeScope so this shouldn't introduce a
     15        perf regression.
     16
     17        Note also that TreeScope can never be null after r136328, and the document
     18        pointer is only null for DocumentType nodes so we can use a special
     19        no-document TreeScope for this case that always returns null from
     20        documentScope().
     21
     22        My original patch in r137524 for this did not correctly handle the case
     23        where the Document is being destroyed and we would then call guardDeref on
     24        ourself in the middle of the Document destructor causing asserts. To fix
     25        this we need to go back to the original model where Document passes
     26        null to it's super constructor for the Document pointer and assigns
     27        it's tree scope later, and we also need to clear the tree scope pointer
     28        in Document's destructor.
     29
     30        No new tests, no change in behavior.
     31
     32        * dom/Document.cpp:
     33        (WebCore::Document::Document):
     34        (WebCore::Document::~Document):
     35        (WebCore::Document::suggestedMIMEType):
     36        * dom/Document.h:
     37        (WebCore::Node::isDocumentNode):
     38        (WebCore::Node::Node):
     39        * dom/Element.cpp:
     40        (WebCore::Element::createRareData):
     41        * dom/ElementRareData.h:
     42        (ElementRareData):
     43        (WebCore::ElementRareData::ElementRareData):
     44        * dom/Node.cpp:
     45        (WebCore::Node::~Node):
     46        (WebCore::Node::createRareData):
     47        (WebCore::Node::attach):
     48        (WebCore::Node::reportMemoryUsage):
     49        * dom/Node.h:
     50        (WebCore::NodeRareDataBase::NodeRareDataBase):
     51        (NodeRareDataBase):
     52        (WebCore::Node::treeScope):
     53        (WebCore::Node::inDocument):
     54        (WebCore::Node::documentInternal):
     55        (WebCore::Node::setTreeScope):
     56        (Node):
     57        * dom/NodeRareData.cpp:
     58        (SameSizeAsNodeRareData):
     59        (WebCore::NodeRareData::reportMemoryUsage):
     60        * dom/NodeRareData.h:
     61        (WebCore::NodeRareData::NodeRareData):
     62        * dom/ShadowRoot.cpp:
     63        (WebCore::ShadowRoot::ShadowRoot):
     64        * dom/TreeScope.cpp:
     65        (SameSizeAsTreeScope):
     66        (WebCore::TreeScope::TreeScope):
     67        (WebCore::TreeScope::clearDocumentScope):
     68            Needed to clear the document scope during Document destruction
     69            so the Node destructor does not guardDeref the document that is
     70            already being destroyed.
     71        (WebCore::TreeScope::setParentTreeScope):
     72        * dom/TreeScope.h:
     73        (TreeScope):
     74        (WebCore::TreeScope::documentScope):
     75        (WebCore::TreeScope::noDocumentInstance):
     76            Special shared tree scope that has a document scope that is always
     77            null. This is needed for DocType nodes, and also for Documents
     78            during construction.
     79        (WebCore::TreeScope::setDocumentScope):
     80        * dom/TreeScopeAdopter.cpp:
     81        (WebCore::TreeScopeAdopter::moveTreeToNewScope):
     82        (WebCore::TreeScopeAdopter::moveNodeToNewDocument):
     83
    1842013-01-03  Adam Klein  <adamk@chromium.org>
    285

  • trunk/Source/WebCore/dom/Document.cpp

    r138730 r138735  
    508508#endif
    509509{
    510     m_document = this;
     510    setTreeScope(this);
    511511
    512512    m_printing = false;
    … …  
    671671        ASSERT(!m_nodeListCounts[i]);
    672672
    673     m_document = 0;
     673    clearDocumentScope();
    674674
    675675    InspectorCounters::decrementCounter(InspectorCounters::DocumentCounter);
    … …  
    13471347String Document::suggestedMIMEType() const
    13481348{
    1349     if (m_document->isXHTMLDocument())
     1349    if (isXHTMLDocument())
    13501350        return "application/xhtml+xml";
    1351     if (m_document->isSVGDocument())
     1351    if (isSVGDocument())
    13521352        return "image/svg+xml";
    1353     if (m_document->xmlStandalone())
     1353    if (xmlStandalone())
    13541354        return "text/xml";
    1355     if (m_document->isHTMLDocument())
     1355    if (isHTMLDocument())
    13561356        return "text/html";
    13571357

  • trunk/Source/WebCore/dom/Document.h

    r138730 r138735  
    15871587inline bool Node::isDocumentNode() const
    15881588{
    1589     return this == m_document;
    1590 }
    1591 
    1592 inline TreeScope* Node::treeScope() const
    1593 {
    1594     return hasRareData() ? m_data.m_rareData->treeScope() : documentInternal();
     1589    return this == documentInternal();
    15951590}
    15961591
    15971592inline Node::Node(Document* document, ConstructionType type)
    15981593    : m_nodeFlags(type)
    1599     , m_document(document)
     1594    , m_treeScope(document)
    16001595    , m_previous(0)
    16011596    , m_next(0)
    … …  
    16031598    if (document)
    16041599        document->guardRef();
     1600    else
     1601        m_treeScope = TreeScope::noDocumentInstance();
     1602
    16051603#if !defined(NDEBUG) || (defined(DUMP_NODE_STATISTICS) && DUMP_NODE_STATISTICS)
    16061604    trackForDebugging();

  • trunk/Source/WebCore/dom/Element.cpp

    r138691 r138735  
    218218PassOwnPtr<NodeRareData> Element::createRareData()
    219219{
    220     return adoptPtr(new ElementRareData(documentInternal()));
     220    return adoptPtr(new ElementRareData());
    221221}
    222222

  • trunk/Source/WebCore/dom/ElementRareData.h

    r138691 r138735  
    3636class ElementRareData : public NodeRareData {
    3737public:
    38     ElementRareData(Document*);
     38    ElementRareData();
    3939    virtual ~ElementRareData();
    4040
    … …  
    142142}
    143143
    144 inline ElementRareData::ElementRareData(Document* document)
    145     : NodeRareData(document)
    146     , m_minimumSizeForResizing(defaultMinimumSizeForResizing())
     144inline ElementRareData::ElementRareData()
     145    : m_minimumSizeForResizing(defaultMinimumSizeForResizing())
    147146    , m_generatedBefore(0)
    148147    , m_generatedAfter(0)

  • trunk/Source/WebCore/dom/Node.cpp

    r138730 r138735  
    415415        clearRareData();
    416416
     417    Document* doc = documentInternal();
     418
    417419    if (hasEventTargetData()) {
    418420#if ENABLE(TOUCH_EVENT_TRACKING)
    419         if (m_document)
    420             m_document->didRemoveEventTargetNode(this);
     421        if (doc)
     422            doc->didRemoveEventTargetNode(this);
    421423#endif
    422424        clearEventTargetData();
    … …  
    426428        detach();
    427429
    428     Document* doc = m_document;
    429430    if (AXObjectCache::accessibilityEnabled() && doc && doc->axObjectCacheExists() && !isContainerNode())
    430431        doc->axObjectCache()->remove(this);
    … …  
    439440
    440441    InspectorCounters::decrementCounter(InspectorCounters::NodeCounter);
    441 }
    442 
    443 void Node::setDocument(Document* document)
    444 {
    445     ASSERT(!inDocument() || m_document == document);
    446     if (inDocument() || m_document == document)
    447         return;
    448 
    449     m_document = document;
    450 }
    451 
    452 void Node::setTreeScope(TreeScope* scope)
    453 {
    454     ASSERT(!isShadowRoot());
    455 
    456     if (!hasRareData() && scope->rootNode()->isDocumentNode())
    457         return;
    458 
    459     ensureRareData()->setTreeScope(scope);
    460442}
    461443
    … …  
    481463PassOwnPtr<NodeRareData> Node::createRareData()
    482464{
    483     return adoptPtr(new NodeRareData(documentInternal()));
     465    return adoptPtr(new NodeRareData());
    484466}
    485467
    … …  
    11061088    clearNeedsStyleRecalc();
    11071089
    1108     Document* doc = m_document;
     1090    Document* doc = documentInternal();
    11091091    if (AXObjectCache::accessibilityEnabled() && doc && doc->axObjectCacheExists())
    11101092        doc->axObjectCache()->updateCacheAfterNodeIsAttached(this);
    … …  
    26072589    TreeShared<Node, ContainerNode>::reportMemoryUsage(memoryObjectInfo);
    26082590    ScriptWrappable::reportMemoryUsage(memoryObjectInfo);
    2609     info.addMember(m_document);
     2591    info.addMember(m_treeScope);
    26102592    info.addMember(m_next);
    26112593    info.addMember(m_previous);

  • trunk/Source/WebCore/dom/Node.h

    r138730 r138735  
    3434#include "ScriptWrappable.h"
    3535#include "SimulatedClickOptions.h"
     36#include "TreeScope.h"
    3637#include "TreeShared.h"
    3738#include <wtf/Forward.h>
    … …  
    8687class ShadowRoot;
    8788class TagNodeList;
    88 class TreeScope;
    8989
    9090#if ENABLE(GESTURE_EVENTS)
    … …  
    116116    void setRenderer(RenderObject* renderer) { m_renderer = renderer; }
    117117
    118     TreeScope* treeScope() const { return m_treeScope; }
    119     void setTreeScope(TreeScope* scope) { m_treeScope = scope; }
    120 
    121118    virtual ~NodeRareDataBase() { }
    122119protected:
    123     NodeRareDataBase(TreeScope* scope)
    124         : m_treeScope(scope)
    125     {
    126     }
     120    NodeRareDataBase() { }
    127121private:
    128122    RenderObject* m_renderer;
    129     TreeScope* m_treeScope;
    130123};
    131124
    … …  
    469462    }
    470463
    471     TreeScope* treeScope() const;
     464    TreeScope* treeScope() const { return m_treeScope; }
    472465
    473466    // Returns true if this node is associated with a document and is in its associated document's
    … …  
    475468    bool inDocument() const
    476469    {
    477         ASSERT(m_document || !getFlag(InDocumentFlag));
     470        ASSERT(documentInternal() || !getFlag(InDocumentFlag));
    478471        return getFlag(InDocumentFlag);
    479472    }
    … …  
    771764    void setHasCustomCallbacks() { setFlag(true, HasCustomCallbacksFlag); }
    772765
    773     Document* documentInternal() const { return m_document; }
     766    Document* documentInternal() const { return treeScope()->documentScope(); }
     767    void setTreeScope(TreeScope* scope) { m_treeScope = scope; }
    774768
    775769private:
    … …  
    777771
    778772    void removedLastRef();
    779 
    780     // These API should be only used for a tree scope migration.
    781     void setTreeScope(TreeScope*);
    782     void setDocument(Document*);
    783773
    784774    enum EditableLevel { Editable, RichlyEditable };
    … …  
    824814
    825815    mutable uint32_t m_nodeFlags;
    826     Document* m_document;
     816    TreeScope* m_treeScope;
    827817    Node* m_previous;
    828818    Node* m_next;

  • trunk/Source/WebCore/dom/NodeRareData.cpp

    r137535 r138735  
    4040
    4141struct SameSizeAsNodeRareData {
    42     void* m_pointer[4];
     42    void* m_pointer[3];
    4343    unsigned m_indicesAndBitfields[2];
    4444
    … …  
    6565{
    6666    MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::DOM);
    67     info.addMember(treeScope());
    6867    info.addMember(m_nodeLists);
    6968

  • trunk/Source/WebCore/dom/NodeRareData.h

    r137955 r138735  
    247247
    248248public:   
    249     NodeRareData(Document* document)
    250         : NodeRareDataBase(document)
    251         , m_tabIndex(0)
     249    NodeRareData()
     250        : m_tabIndex(0)
    252251        , m_childIndex(0)
    253252        , m_tabIndexWasSetExplicitly(false)

  • trunk/Source/WebCore/dom/ShadowRoot.cpp

    r138404 r138735  
    6666ShadowRoot::ShadowRoot(Document* document)
    6767    : DocumentFragment(document, CreateShadowRoot)
    68     , TreeScope(this)
     68    , TreeScope(this, document)
    6969    , m_prev(0)
    7070    , m_next(0)
    … …  
    7676{
    7777    ASSERT(document);
    78    
    79     // Assume document as parent scope.
    80     setParentTreeScope(document);
    81     // Shadow tree scopes have the scope pointer point to themselves.
    82     // This way, direct children will receive the correct scope pointer.
    83     ensureRareData()->setTreeScope(this);
     78    setTreeScope(this);
    8479}
    8580

  • trunk/Source/WebCore/dom/TreeScope.cpp

    r138379 r138735  
    5959struct SameSizeAsTreeScope {
    6060    virtual ~SameSizeAsTreeScope();
    61     void* pointers[7];
     61    void* pointers[8];
    6262};
    6363
    … …  
    6666using namespace HTMLNames;
    6767
    68 TreeScope::TreeScope(ContainerNode* rootNode)
     68TreeScope::TreeScope(ContainerNode* rootNode, Document* document)
    6969    : m_rootNode(rootNode)
     70    , m_documentScope(document)
     71    , m_parentTreeScope(document)
     72    , m_idTargetObserverRegistry(IdTargetObserverRegistry::create())
     73{
     74    ASSERT(rootNode);
     75    ASSERT(document);
     76    ASSERT(rootNode != document);
     77}
     78
     79TreeScope::TreeScope(Document* document)
     80    : m_rootNode(document)
     81    , m_documentScope(document)
    7082    , m_parentTreeScope(0)
    7183    , m_idTargetObserverRegistry(IdTargetObserverRegistry::create())
    7284{
    73     ASSERT(rootNode);
     85    ASSERT(document);
     86}
     87
     88TreeScope::TreeScope()
     89    : m_rootNode(0)
     90    , m_documentScope(0)
     91    , m_parentTreeScope(0)
     92{
    7493}
    7594
    … …  
    87106    m_imageMapsByName.clear();
    88107    m_labelsByForAttribute.clear();
     108}
     109
     110void TreeScope::clearDocumentScope()
     111{
     112    ASSERT(rootNode()->isDocumentNode());
     113    m_documentScope = 0;
    89114}
    90115
    … …  
    97122
    98123    m_parentTreeScope = newParentScope;
     124    setDocumentScope(newParentScope->documentScope());
    99125}
    100126

  • trunk/Source/WebCore/dom/TreeScope.h

    r138379 r138735  
    4949class TreeScope {
    5050    friend class Document;
     51    friend class TreeScopeAdopter;
    5152
    5253public:
    … …  
    6061    void addElementById(const AtomicString& elementId, Element*);
    6162    void removeElementById(const AtomicString& elementId, Element*);
     63
     64    Document* documentScope() const { return m_documentScope; }
    6265
    6366    Node* ancestorInThisScope(Node*) const;
    … …  
    9699    virtual void reportMemoryUsage(MemoryObjectInfo*) const;
    97100
     101    static TreeScope* noDocumentInstance()
     102    {
     103        DEFINE_STATIC_LOCAL(TreeScope, instance, ());
     104        return &instance;
     105    }
     106
    98107protected:
    99     explicit TreeScope(ContainerNode*);
     108    TreeScope(ContainerNode*, Document*);
     109    TreeScope(Document*);
    100110    virtual ~TreeScope();
    101111
    102112    void destroyTreeScopeData();
     113    void clearDocumentScope();
     114    void setDocumentScope(Document* document)
     115    {
     116        ASSERT(document);
     117        ASSERT(this != noDocumentInstance());
     118        m_documentScope = document;
     119    }
    103120
    104121private:
     122    TreeScope();
     123
    105124    ContainerNode* m_rootNode;
     125    Document* m_documentScope;
    106126    TreeScope* m_parentTreeScope;
    107127

  • trunk/Source/WebCore/dom/TreeScopeAdopter.cpp

    r137535 r138735  
    4444    // of the document it was moved to. By increasing the DOMTreeVersion of the donating document here
    4545    // we ensure that the collection cache will be invalidated as needed when the element is moved back.
    46     Document* oldDocument = m_oldScope ? m_oldScope->rootNode()->document() : 0;
    47     Document* newDocument = m_newScope->rootNode()->document();
     46    Document* oldDocument = m_oldScope->documentScope();
     47    Document* newDocument = m_newScope->documentScope();
    4848    bool willMoveToNewDocument = oldDocument != newDocument;
    4949    if (oldDocument && willMoveToNewDocument)
    … …  
    5252    for (Node* node = root; node; node = NodeTraversal::next(node, root)) {
    5353        node->setTreeScope(m_newScope);
     54
    5455        if (node->hasRareData()) {
    5556            NodeRareData* rareData = node->rareData();
    … …  
    9899        oldDocument->moveNodeIteratorsToNewDocument(node, newDocument);
    99100
    100     node->setDocument(newDocument);
     101    if (node->isShadowRoot())
     102        toShadowRoot(node)->setDocumentScope(newDocument);
    101103
    102104#ifndef NDEBUG
Note: See TracChangeset for help on using the changeset viewer.