Changeset 143840 in webkit


Ignore:
Timestamp:
Feb 23, 2013 2:49:49 AM (11 years ago)
Author:
morrita@google.com
Message:

ShadowRoot needs guardRef() and guardDeref()
​https://bugs.webkit.org/show_bug.cgi?id=109777

Reviewed by Dimitri Glazkov.

This change moves m_guardRefCount from Document to TreeScope,
which allows ShadowRoot to be guarded by guardRef() mechanism as
Document. After r137524, Node referes TreeScope instead of
Document. This is natural consequence of the change: It no longer
makes sense to guardRef() Document pointer from Node.

Detail:

  • Document::m_guardRefCount and related funcdtions are moved to TreeScope
  • Document::removedLastRef is factored out into TreeScope::removedLastRefToScope(), TreeScope::dispose() and Docuent::dispose(). ShadowRoot also got its own dispose() implementation.
  • Moved guardRef() and guardDeref() calls to TreeScope and Node. Note that there are two "guarded" TreeScope references. One is Node::m_treeScope and another is TreeScope::m_parentTreeScope. The guarded-ref management is now encapsulated in these two classes.

No new tests. Covered by existing tests.

  • WebCore.exp.in:
  • dom/Document.cpp:

(WebCore::Document::Document):
(WebCore::Document::~Document):
(WebCore::Document::dispose): Extracted from removedLastRef()

  • dom/Document.h:

(WebCore::Node::isTreeScope):
(WebCore::Node::Node):

  • dom/DocumentFragment.cpp:

(WebCore::DocumentFragment::DocumentFragment): Remove ASSERT() and move it to ...
(WebCore::DocumentFragment::create): ... here, to allow NULL document from ShadowRoot.

  • dom/Node.cpp:

(WebCore::Node::~Node):
(WebCore::Node::removedLastRef):

  • dom/Node.h:

(WebCore::Node::setTreeScope):

  • dom/ShadowRoot.cpp:

(WebCore::ShadowRoot::ShadowRoot): Passed NULL document to superclass. This aligns what Document is doing.
(WebCore::ShadowRoot::dispose): Added.

  • dom/ShadowRoot.h:

(ShadowRoot):

  • dom/TreeScope.cpp:

(SameSizeAsTreeScope):
(WebCore::TreeScope::TreeScope):
(WebCore::TreeScope::~TreeScope):
(WebCore::TreeScope::dispose): Added.
(WebCore::TreeScope::setParentTreeScope):
(WebCore::TreeScope::deletionHasBegun):
(WebCore::TreeScope::beginDeletion):
(WebCore::TreeScope::refCount): Added.

  • dom/TreeScope.h: Turned m_rootNode to Node* from ContainerNode* for Node::isTreeScope to be inlined.

(WebCore::TreeScope::guardRef): Pulled up from Document.
(WebCore::TreeScope::guardDeref): Ditto.
(WebCore::TreeScope::hasGuardRefCount): Added to hide m_guardRefCount.
(WebCore::TreeScope::deletionHasBegun): Added.
(WebCore::TreeScope::beginDeletion): Added.
(WebCore::TreeScope::removedLastRefToScope): Pulled up from Document.

  • dom/TreeScopeAdopter.cpp:

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

Location:
trunk/Source/WebCore
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r143838 r143840  
     12013-02-23  Hajime Morrita  <morrita@google.com>
     2
     3        ShadowRoot needs guardRef() and guardDeref()
     4        https://bugs.webkit.org/show_bug.cgi?id=109777
     5
     6        Reviewed by Dimitri Glazkov.
     7
     8        This change moves m_guardRefCount from Document to TreeScope,
     9        which allows ShadowRoot to be guarded by guardRef() mechanism as
     10        Document. After r137524, Node referes TreeScope instead of
     11        Document. This is natural consequence of the change: It no longer
     12        makes sense to guardRef() Document pointer from Node.
     13
     14        Detail:
     15
     16        - Document::m_guardRefCount and related funcdtions are moved to TreeScope
     17        - Document::removedLastRef is factored out into TreeScope::removedLastRefToScope(),
     18          TreeScope::dispose() and Docuent::dispose(). ShadowRoot also got its own dispose() implementation.
     19        - Moved guardRef() and guardDeref() calls to TreeScope and Node.
     20          Note that there are two "guarded" TreeScope references. One is
     21          Node::m_treeScope and another is TreeScope::m_parentTreeScope.
     22          The guarded-ref management is now encapsulated in these two classes.
     23
     24        No new tests. Covered by existing tests.
     25
     26        * WebCore.exp.in:
     27        * dom/Document.cpp:
     28        (WebCore::Document::Document):
     29        (WebCore::Document::~Document):
     30        (WebCore::Document::dispose): Extracted from removedLastRef()
     31        * dom/Document.h:
     32        (WebCore::Node::isTreeScope):
     33        (WebCore::Node::Node):
     34        * dom/DocumentFragment.cpp:
     35        (WebCore::DocumentFragment::DocumentFragment): Remove ASSERT() and move it to ...
     36        (WebCore::DocumentFragment::create): ... here, to allow NULL document from ShadowRoot.
     37        * dom/Node.cpp:
     38        (WebCore::Node::~Node):
     39        (WebCore::Node::removedLastRef):
     40        * dom/Node.h:
     41        (WebCore::Node::setTreeScope):
     42        * dom/ShadowRoot.cpp:
     43        (WebCore::ShadowRoot::ShadowRoot): Passed NULL document to superclass. This aligns what Document is doing.
     44        (WebCore::ShadowRoot::dispose): Added.
     45        * dom/ShadowRoot.h:
     46        (ShadowRoot):
     47        * dom/TreeScope.cpp:
     48        (SameSizeAsTreeScope):
     49        (WebCore::TreeScope::TreeScope):
     50        (WebCore::TreeScope::~TreeScope):
     51        (WebCore::TreeScope::dispose): Added.
     52        (WebCore::TreeScope::setParentTreeScope):
     53        (WebCore::TreeScope::deletionHasBegun):
     54        (WebCore::TreeScope::beginDeletion):
     55        (WebCore::TreeScope::refCount): Added.
     56        * dom/TreeScope.h: Turned m_rootNode to Node* from ContainerNode* for Node::isTreeScope to be inlined.
     57        (WebCore::TreeScope::guardRef): Pulled up from Document.
     58        (WebCore::TreeScope::guardDeref): Ditto.
     59        (WebCore::TreeScope::hasGuardRefCount): Added to hide m_guardRefCount.
     60        (WebCore::TreeScope::deletionHasBegun): Added.
     61        (WebCore::TreeScope::beginDeletion): Added.
     62        (WebCore::TreeScope::removedLastRefToScope): Pulled up from Document.
     63        * dom/TreeScopeAdopter.cpp:
     64        (WebCore::TreeScopeAdopter::moveTreeToNewScope):
     65        (WebCore::TreeScopeAdopter::moveNodeToNewDocument):
     66
    1672013-02-23  Alexey Proskuryakov  <ap@apple.com>
    268

  • trunk/Source/WebCore/WebCore.exp.in

    r143816 r143840  
    14141414__ZNK7WebCore4Node13ownerDocumentEv
    14151415__ZNK7WebCore4Node14isDescendantOfEPKS0_
    1416 __ZNK7WebCore4Node11isTreeScopeEv
    14171416__ZNK7WebCore4Node18getSubresourceURLsERN3WTF11ListHashSetINS_4KURLELm256ENS_8KURLHashEEE
    14181417__ZNK7WebCore4Node31numberOfScopedHTMLStyleChildrenEv

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

    r143806 r143840  
    414414    : ContainerNode(0, CreateDocument)
    415415    , TreeScope(this)
    416     , m_guardRefCount(0)
    417416    , m_styleResolverThrowawayTimer(this, &Document::styleResolverThrowawayTimerFired)
    418417    , m_lastStyleResolverAccessTime(0)
    … …  
    486485#endif
    487486{
    488     setTreeScope(this);
    489 
    490487    m_printing = false;
    491488    m_paginatedForScreen = false;
    … …  
    587584    ASSERT(!m_styleRecalcTimer.isActive());
    588585    ASSERT(!m_parentTreeScope);
    589     ASSERT(!m_guardRefCount);
     586    ASSERT(!hasGuardRefCount());
    590587
    591588#if ENABLE(TEMPLATE_ELEMENT)
    … …  
    654651}
    655652
    656 void Document::removedLastRef()
    657 {
    658     ASSERT(!m_deletionHasBegun);
    659     if (m_guardRefCount) {
    660         // If removing a child removes the last self-only ref, we don't
    661         // want the scope to be destructed until after
    662         // removeDetachedChildren returns, so we guard ourselves with an
    663         // extra self-only ref.
    664         guardRef();
    665 
    666         // We must make sure not to be retaining any of our children through
    667         // these extra pointers or we will create a reference cycle.
    668         m_docType = 0;
    669         m_focusedNode = 0;
    670         m_hoverNode = 0;
    671         m_activeElement = 0;
    672         m_titleElement = 0;
    673         m_documentElement = 0;
    674         m_contextFeatures = ContextFeatures::defaultSwitch();
    675         m_userActionElements.documentDidRemoveLastRef();
     653void Document::dispose()
     654{
     655    // We must make sure not to be retaining any of our children through
     656    // these extra pointers or we will create a reference cycle.
     657    m_docType = 0;
     658    m_focusedNode = 0;
     659    m_hoverNode = 0;
     660    m_activeElement = 0;
     661    m_titleElement = 0;
     662    m_documentElement = 0;
     663    m_contextFeatures = ContextFeatures::defaultSwitch();
     664    m_userActionElements.documentDidRemoveLastRef();
    676665#if ENABLE(FULLSCREEN_API)
    677         m_fullScreenElement = 0;
    678         m_fullScreenElementStack.clear();
    679 #endif
    680 
    681         detachParser();
    682 
    683         // removeDetachedChildren() doesn't always unregister IDs,
    684         // so tear down scope information upfront to avoid having stale references in the map.
    685         destroyTreeScopeData();
    686         removeDetachedChildren();
    687 
    688         m_markers->detach();
    689 
    690         m_cssCanvasElements.clear();
     666    m_fullScreenElement = 0;
     667    m_fullScreenElementStack.clear();
     668#endif
     669
     670    detachParser();
     671
     672    // removeDetachedChildren() doesn't always unregister IDs,
     673    // so tear down scope information upfront to avoid having stale references in the map.
     674    destroyTreeScopeData();
     675    removeDetachedChildren();
     676
     677    m_markers->detach();
     678
     679    m_cssCanvasElements.clear();
    691680
    692681#if ENABLE(REQUEST_ANIMATION_FRAME)
    693         // FIXME: consider using ActiveDOMObject.
    694         if (m_scriptedAnimationController)
    695             m_scriptedAnimationController->clearDocumentPointer();
    696         m_scriptedAnimationController.clear();
    697 #endif
    698 
    699 #ifndef NDEBUG
    700         m_inRemovedLastRefFunction = false;
    701 #endif
    702 
    703         guardDeref();
    704     } else {
    705 #ifndef NDEBUG
    706         m_deletionHasBegun = true;
    707 #endif
    708         delete this;
    709     }
     682    // FIXME: consider using ActiveDOMObject.
     683    if (m_scriptedAnimationController)
     684        m_scriptedAnimationController->clearDocumentPointer();
     685    m_scriptedAnimationController.clear();
     686#endif
    710687}
    711688

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

    r142977 r143840  
    230230    using ContainerNode::deref;
    231231
    232     // Nodes belonging to this document hold guard references -
    233     // these are enough to keep the document from being destroyed, but
    234     // not enough to keep it from removing its children. This allows a
    235     // node that outlives its document to still have a valid document
    236     // pointer without introducing reference cycles.
    237     void guardRef()
    238     {
    239         ASSERT(!m_deletionHasBegun);
    240         ++m_guardRefCount;
    241     }
    242 
    243     void guardDeref()
    244     {
    245         ASSERT(!m_deletionHasBegun);
    246         --m_guardRefCount;
    247         if (!m_guardRefCount && !refCount()) {
    248 #ifndef NDEBUG
    249             m_deletionHasBegun = true;
    250 #endif
    251             delete this;
    252         }
    253     }
    254 
    255232    Element* getElementById(const AtomicString& id) const;
    256233
    … …  
    12181195    friend class IgnoreDestructiveWriteCountIncrementer;
    12191196
    1220     void removedLastRef();
    1221    
     1197    virtual void dispose() OVERRIDE;
     1198
    12221199    void detachParser();
    12231200
    … …  
    12861263    void addListenerType(ListenerType listenerType) { m_listenerTypes |= listenerType; }
    12871264    void addMutationEventListenerTypeIfEnabled(ListenerType);
    1288 
    1289     int m_guardRefCount;
    12901265
    12911266    void styleResolverThrowawayTimerFired(Timer<Document>*);
    … …  
    16041579    , m_next(0)
    16051580{
    1606     if (document)
    1607         document->guardRef();
    1608     else
     1581    if (!m_treeScope)
    16091582        m_treeScope = TreeScope::noDocumentInstance();
     1583    m_treeScope->guardRef();
    16101584
    16111585#if !defined(NDEBUG) || (defined(DUMP_NODE_STATISTICS) && DUMP_NODE_STATISTICS)

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

    r140399 r143840  
    3535    : ContainerNode(document, constructionType)
    3636{
    37     ASSERT(document);
    3837}
    3938
    4039PassRefPtr<DocumentFragment> DocumentFragment::create(Document* document)
    4140{
     41    ASSERT(document);
    4242    return adoptRef(new DocumentFragment(document, Node::CreateDocumentFragment));
    4343}

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

    r143239 r143840  
    443443        m_next->setPreviousSibling(0);
    444444
    445     if (doc)
    446         doc->guardDeref();
     445    m_treeScope->guardDeref();
    447446
    448447    InspectorCounters::decrementCounter(InspectorCounters::NodeCounter);
    … …  
    891890
    892891    return true;
    893 }
    894 
    895 bool Node::isTreeScope() const
    896 {
    897     return treeScope()->rootNode() == this;
    898892}
    899893
    … …  
    25622556#endif
    25632557
     2558// This is here for inlining
     2559inline void TreeScope::removedLastRefToScope()
     2560{
     2561    ASSERT(!deletionHasBegun());
     2562    if (m_guardRefCount) {
     2563        // If removing a child removes the last self-only ref, we don't
     2564        // want the scope to be destructed until after
     2565        // removeDetachedChildren returns, so we guard ourselves with an
     2566        // extra self-only ref.
     2567        guardRef();
     2568        dispose();
     2569#ifndef NDEBUG
     2570        // We need to do this right now since guardDeref() can delete this.
     2571        rootNode()->m_inRemovedLastRefFunction = false;
     2572#endif
     2573        guardDeref();
     2574    } else {
     2575#ifndef NDEBUG
     2576        rootNode()->m_inRemovedLastRefFunction = false;
     2577        beginDeletion();
     2578#endif
     2579        delete this;
     2580    }
     2581}
     2582
    25642583// It's important not to inline removedLastRef, because we don't want to inline the code to
    25652584// delete a Node at each deref call site.
    … …  
    25692588    // faster for non-Document nodes, and because the call to removedLastRef that is inlined
    25702589    // at all deref call sites is smaller if it's a non-virtual function.
    2571     if (isDocumentNode()) {
    2572         static_cast<Document*>(this)->removedLastRef();
     2590    if (isTreeScope()) {
     2591        treeScope()->removedLastRefToScope();
    25732592        return;
    25742593    }
     2594
    25752595#ifndef NDEBUG
    25762596    m_deletionHasBegun = true;

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

    r143089 r143840  
    249249
    250250    bool isDocumentNode() const;
    251     bool isTreeScope() const;
     251    bool isTreeScope() const { return treeScope()->rootNode() == this; }
    252252    bool isDocumentFragment() const { return getFlag(IsDocumentFragmentFlag); }
    253253    bool isShadowRoot() const { return isDocumentFragment() && isTreeScope(); }

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

    r143089 r143840  
    5353
    5454ShadowRoot::ShadowRoot(Document* document, ShadowRootType type)
    55     : DocumentFragment(document, CreateShadowRoot)
     55    : DocumentFragment(0, CreateShadowRoot)
    5656    , TreeScope(this, document)
    5757    , m_prev(0)
    … …  
    6464{
    6565    ASSERT(document);
    66     setTreeScope(this);
    6766
    6867#if PLATFORM(CHROMIUM)
    … …  
    8887    if (hasRareData())
    8988        clearRareData();
     89}
     90
     91void ShadowRoot::dispose()
     92{
     93    removeDetachedChildren();
    9094}
    9195

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

    r141524 r143840  
    101101    virtual ~ShadowRoot();
    102102
     103    virtual void dispose() OVERRIDE;
    103104    virtual bool childTypeAllowed(NodeType) const OVERRIDE;
    104105    virtual void childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta) OVERRIDE;

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

    r143422 r143840  
    6060    virtual ~SameSizeAsTreeScope();
    6161    void* pointers[8];
     62    int ints[1];
    6263};
    6364
    … …  
    7071    , m_documentScope(document)
    7172    , m_parentTreeScope(document)
     73    , m_guardRefCount(0)
    7274    , m_idTargetObserverRegistry(IdTargetObserverRegistry::create())
    7375{
    … …  
    7577    ASSERT(document);
    7678    ASSERT(rootNode != document);
     79    m_parentTreeScope->guardRef();
     80    m_rootNode->setTreeScope(this);
    7781}
    7882
    … …  
    8185    , m_documentScope(document)
    8286    , m_parentTreeScope(0)
     87    , m_guardRefCount(0)
    8388    , m_idTargetObserverRegistry(IdTargetObserverRegistry::create())
    8489{
    8590    ASSERT(document);
     91    m_rootNode->setTreeScope(this);
    8692}
    8793
    … …  
    9096    , m_documentScope(0)
    9197    , m_parentTreeScope(0)
     98    , m_guardRefCount(0)
    9299{
    93100}
    … …  
    95102TreeScope::~TreeScope()
    96103{
     104    ASSERT(!m_guardRefCount);
     105    m_rootNode->setTreeScope(noDocumentInstance());
     106
    97107    if (m_selection) {
    98108        m_selection->clearTreeScope();
    99109        m_selection = 0;
    100110    }
     111
     112    if (m_parentTreeScope)
     113        m_parentTreeScope->guardDeref();
    101114}
    102115
    … …  
    121134    ASSERT(newParentScope);
    122135
     136    newParentScope->guardRef();
     137    if (m_parentTreeScope)
     138        m_parentTreeScope->guardDeref();
    123139    m_parentTreeScope = newParentScope;
    124140    setDocumentScope(newParentScope->documentScope());
    … …  
    416432}
    417433
     434#ifndef NDEBUG
     435bool TreeScope::deletionHasBegun()
     436{
     437    return rootNode() && rootNode()->m_deletionHasBegun;
     438}
     439
     440void TreeScope::beginDeletion()
     441{
     442    ASSERT(this != noDocumentInstance());
     443    rootNode()->m_deletionHasBegun = true;
     444}
     445#endif
     446
     447int TreeScope::refCount() const
     448{
     449    if (Node* root = rootNode())
     450        return root->refCount();
     451    return 0;
     452}
     453
    418454} // namespace WebCore

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

    r138735 r143840  
    9393    void adoptIfNeeded(Node*);
    9494
    95     ContainerNode* rootNode() const { return m_rootNode; }
     95    Node* rootNode() const { return m_rootNode; }
    9696
    9797    IdTargetObserverRegistry& idTargetObserverRegistry() const { return *m_idTargetObserverRegistry.get(); }
    … …  
    104104        return &instance;
    105105    }
     106
     107    // Nodes belonging to this scope hold guard references -
     108    // these are enough to keep the scope from being destroyed, but
     109    // not enough to keep it from removing its children. This allows a
     110    // node that outlives its scope to still have a valid document
     111    // pointer without introducing reference cycles.
     112    void guardRef()
     113    {
     114        ASSERT(!deletionHasBegun());
     115        ++m_guardRefCount;
     116    }
     117
     118    void guardDeref()
     119    {
     120        ASSERT(!deletionHasBegun());
     121        --m_guardRefCount;
     122        if (!m_guardRefCount && !refCount() && this != noDocumentInstance()) {
     123            beginDeletion();
     124            delete this;
     125        }
     126    }
     127
     128    void removedLastRefToScope();
    106129
    107130protected:
    … …  
    119142    }
    120143
     144    bool hasGuardRefCount() const { return m_guardRefCount; }
     145
    121146private:
    122147    TreeScope();
    123148
    124     ContainerNode* m_rootNode;
     149    virtual void dispose() { }
     150
     151    int refCount() const;
     152#ifndef NDEBUG
     153    bool deletionHasBegun();
     154    void beginDeletion();
     155#else
     156    bool deletionHasBegun() { return false; }
     157    void beginDeletion() { }
     158#endif
     159
     160    Node* m_rootNode;
    125161    Document* m_documentScope;
    126162    TreeScope* m_parentTreeScope;
     163    int m_guardRefCount;
    127164
    128165    OwnPtr<DocumentOrderedMap> m_elementsById;

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

    r140103 r143840  
    4141    ASSERT(needsScopeChange());
    4242
     43    m_oldScope->guardRef();
     44
    4345    // If an element is moved from a document and then eventually back again the collection cache for
    4446    // that element may contain stale data as changes made to it will have updated the DOMTreeVersion
    … …  
    5254
    5355    for (Node* node = root; node; node = NodeTraversal::next(node, root)) {
    54         node->setTreeScope(m_newScope);
     56        updateTreeScope(node);
    5557
    5658        if (willMoveToNewDocument)
    … …  
    7779        }
    7880    }
     81
     82    m_oldScope->guardDeref();
    7983}
    8084
    … …  
    100104#endif
    101105
     106inline void TreeScopeAdopter::updateTreeScope(Node* node) const
     107{
     108    ASSERT(!node->isTreeScope());
     109    ASSERT(node->treeScope() == m_oldScope);
     110    m_newScope->guardRef();
     111    m_oldScope->guardDeref();
     112    node->setTreeScope(m_newScope);
     113}
     114
    102115inline void TreeScopeAdopter::moveNodeToNewDocument(Node* node, Document* oldDocument, Document* newDocument) const
    103116{
    … …  
    110123    }
    111124
    112     newDocument->guardRef();
    113125    if (oldDocument)
    114126        oldDocument->moveNodeIteratorsToNewDocument(node, newDocument);
    … …  
    124136    node->didMoveToNewDocument(oldDocument);
    125137    ASSERT(didMoveToNewDocumentWasCalled);
    126    
    127     if (oldDocument)
    128         oldDocument->guardDeref();
    129138}
    130139

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

    r127300 r143840  
    4646
    4747private:
     48    void updateTreeScope(Node*) const;
    4849    void moveTreeToNewScope(Node*) const;
    4950    void moveTreeToNewDocument(Node*, Document* oldDocument, Document* newDocument) const;
Note: See TracChangeset for help on using the changeset viewer.