Changeset 144735 in webkit


Ignore:
Timestamp:
Mar 5, 2013 12:08:15 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/Element.cpp:

(WebCore::Element::ensureAttr): This has been wrong and is fixed in this revision since the incorrectness is unveiled by this change.

  • 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:
14 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r144732 r144735  
     12013-03-05  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/Element.cpp:
     43        (WebCore::Element::ensureAttr): This has been wrong and is fixed in this revision since the incorrectness is unveiled by this change.
     44        * dom/ShadowRoot.cpp:
     45        (WebCore::ShadowRoot::ShadowRoot): Passed NULL document to superclass. This aligns what Document is doing.
     46        (WebCore::ShadowRoot::dispose): Added.
     47        * dom/ShadowRoot.h:
     48        (ShadowRoot):
     49        * dom/TreeScope.cpp:
     50        (SameSizeAsTreeScope):
     51        (WebCore::TreeScope::TreeScope):
     52        (WebCore::TreeScope::~TreeScope):
     53        (WebCore::TreeScope::dispose): Added.
     54        (WebCore::TreeScope::setParentTreeScope):
     55        (WebCore::TreeScope::deletionHasBegun):
     56        (WebCore::TreeScope::beginDeletion):
     57        (WebCore::TreeScope::refCount): Added.
     58        * dom/TreeScope.h: Turned m_rootNode to Node* from ContainerNode* for Node::isTreeScope to be inlined.
     59        (WebCore::TreeScope::guardRef): Pulled up from Document.
     60        (WebCore::TreeScope::guardDeref): Ditto.
     61        (WebCore::TreeScope::hasGuardRefCount): Added to hide m_guardRefCount.
     62        (WebCore::TreeScope::deletionHasBegun): Added.
     63        (WebCore::TreeScope::beginDeletion): Added.
     64        (WebCore::TreeScope::removedLastRefToScope): Pulled up from Document.
     65        * dom/TreeScopeAdopter.cpp:
     66        (WebCore::TreeScopeAdopter::moveTreeToNewScope):
     67        (WebCore::TreeScopeAdopter::moveNodeToNewDocument):
     68
    1692013-03-04  Uday Kiran  <udaykiran@motorola.com>
    270
  • trunk/Source/WebCore/WebCore.exp.in

    r144696 r144735  
    14241424__ZNK7WebCore4Node13ownerDocumentEv
    14251425__ZNK7WebCore4Node14isDescendantOfEPKS0_
    1426 __ZNK7WebCore4Node11isTreeScopeEv
    14271426__ZNK7WebCore4Node18getSubresourceURLsERN3WTF11ListHashSetINS_4KURLELm256ENS_8KURLHashEEE
    14281427__ZNK7WebCore4Node31numberOfScopedHTMLStyleChildrenEv
  • trunk/Source/WebCore/dom/Document.cpp

    r144696 r144735  
    418418    : ContainerNode(0, CreateDocument)
    419419    , TreeScope(this)
    420     , m_guardRefCount(0)
    421420    , m_styleResolverThrowawayTimer(this, &Document::styleResolverThrowawayTimerFired)
    422421    , m_lastStyleResolverAccessTime(0)
     
    490489#endif
    491490{
    492     setTreeScope(this);
    493 
    494491    m_printing = false;
    495492    m_paginatedForScreen = false;
     
    591588    ASSERT(!m_styleRecalcTimer.isActive());
    592589    ASSERT(!m_parentTreeScope);
    593     ASSERT(!m_guardRefCount);
     590    ASSERT(!hasGuardRefCount());
    594591
    595592#if ENABLE(TEMPLATE_ELEMENT)
     
    658655}
    659656
    660 void Document::removedLastRef()
     657void Document::dispose()
    661658{
    662659    ASSERT(!m_deletionHasBegun);
    663     if (m_guardRefCount) {
    664         // If removing a child removes the last self-only ref, we don't
    665         // want the scope to be destructed until after
    666         // removeDetachedChildren returns, so we guard ourselves with an
    667         // extra self-only ref.
    668         guardRef();
    669 
    670         // We must make sure not to be retaining any of our children through
    671         // these extra pointers or we will create a reference cycle.
    672         m_docType = 0;
    673         m_focusedNode = 0;
    674         m_hoverNode = 0;
    675         m_activeElement = 0;
    676         m_titleElement = 0;
    677         m_documentElement = 0;
    678         m_contextFeatures = ContextFeatures::defaultSwitch();
    679         m_userActionElements.documentDidRemoveLastRef();
     660    // We must make sure not to be retaining any of our children through
     661    // these extra pointers or we will create a reference cycle.
     662    m_docType = 0;
     663    m_focusedNode = 0;
     664    m_hoverNode = 0;
     665    m_activeElement = 0;
     666    m_titleElement = 0;
     667    m_documentElement = 0;
     668    m_contextFeatures = ContextFeatures::defaultSwitch();
     669    m_userActionElements.documentDidRemoveLastRef();
    680670#if ENABLE(FULLSCREEN_API)
    681         m_fullScreenElement = 0;
    682         m_fullScreenElementStack.clear();
    683 #endif
    684 
    685         detachParser();
     671    m_fullScreenElement = 0;
     672    m_fullScreenElementStack.clear();
     673#endif
     674
     675    detachParser();
    686676
    687677#if ENABLE(CUSTOM_ELEMENTS)
    688         m_registry.clear();
    689 #endif
    690 
    691         // removeDetachedChildren() doesn't always unregister IDs,
    692         // so tear down scope information upfront to avoid having stale references in the map.
    693         destroyTreeScopeData();
    694         removeDetachedChildren();
    695 
    696         m_markers->detach();
    697 
    698         m_cssCanvasElements.clear();
     678    m_registry.clear();
     679#endif
     680
     681    // removeDetachedChildren() doesn't always unregister IDs,
     682    // so tear down scope information upfront to avoid having stale references in the map.
     683    destroyTreeScopeData();
     684    removeDetachedChildren();
     685
     686    m_markers->detach();
     687
     688    m_cssCanvasElements.clear();
    699689
    700690#if ENABLE(REQUEST_ANIMATION_FRAME)
    701         // FIXME: consider using ActiveDOMObject.
    702         if (m_scriptedAnimationController)
    703             m_scriptedAnimationController->clearDocumentPointer();
    704         m_scriptedAnimationController.clear();
    705 #endif
    706 
    707 #ifndef NDEBUG
    708         m_inRemovedLastRefFunction = false;
    709 #endif
    710 
    711         guardDeref();
    712     } else {
    713 #ifndef NDEBUG
    714         m_deletionHasBegun = true;
    715 #endif
    716         delete this;
    717     }
     691    // FIXME: consider using ActiveDOMObject.
     692    if (m_scriptedAnimationController)
     693        m_scriptedAnimationController->clearDocumentPointer();
     694    m_scriptedAnimationController.clear();
     695#endif
    718696}
    719697
  • trunk/Source/WebCore/dom/Document.h

    r144696 r144735  
    232232    using ContainerNode::deref;
    233233
    234     // Nodes belonging to this document hold guard references -
    235     // these are enough to keep the document from being destroyed, but
    236     // not enough to keep it from removing its children. This allows a
    237     // node that outlives its document to still have a valid document
    238     // pointer without introducing reference cycles.
    239     void guardRef()
    240     {
    241         ASSERT(!m_deletionHasBegun);
    242         ++m_guardRefCount;
    243     }
    244 
    245     void guardDeref()
    246     {
    247         ASSERT(!m_deletionHasBegun);
    248         --m_guardRefCount;
    249         if (!m_guardRefCount && !refCount()) {
    250 #ifndef NDEBUG
    251             m_deletionHasBegun = true;
    252 #endif
    253             delete this;
    254         }
    255     }
    256 
    257234    Element* getElementById(const AtomicString& id) const;
    258235
     
    12271204    friend class IgnoreDestructiveWriteCountIncrementer;
    12281205
    1229     void removedLastRef();
    1230    
     1206    virtual void dispose() OVERRIDE;
     1207
    12311208    void detachParser();
    12321209
     
    12951272    void addListenerType(ListenerType listenerType) { m_listenerTypes |= listenerType; }
    12961273    void addMutationEventListenerTypeIfEnabled(ListenerType);
    1297 
    1298     int m_guardRefCount;
    12991274
    13001275    void styleResolverThrowawayTimerFired(Timer<Document>*);
     
    16171592    , m_next(0)
    16181593{
    1619     if (document)
    1620         document->guardRef();
    1621     else
     1594    if (!m_treeScope)
    16221595        m_treeScope = TreeScope::noDocumentInstance();
     1596    m_treeScope->guardRef();
    16231597
    16241598#if !defined(NDEBUG) || (defined(DUMP_NODE_STATISTICS) && DUMP_NODE_STATISTICS)
  • trunk/Source/WebCore/dom/DocumentFragment.cpp

    r143940 r144735  
    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/Element.cpp

    r144702 r144735  
    27542754    if (!attrNode) {
    27552755        attrNode = Attr::create(this, name);
     2756        treeScope()->adoptIfNeeded(attrNode.get());
    27562757        attrNodeList->append(attrNode);
    27572758    }
  • trunk/Source/WebCore/dom/Node.cpp

    r144526 r144735  
    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
     
    25682562#endif
    25692563
     2564// This is here for inlining
     2565inline void TreeScope::removedLastRefToScope()
     2566{
     2567    ASSERT(!deletionHasBegun());
     2568    if (m_guardRefCount) {
     2569        // If removing a child removes the last self-only ref, we don't
     2570        // want the scope to be destructed until after
     2571        // removeDetachedChildren returns, so we guard ourselves with an
     2572        // extra self-only ref.
     2573        guardRef();
     2574        dispose();
     2575#ifndef NDEBUG
     2576        // We need to do this right now since guardDeref() can delete this.
     2577        rootNode()->m_inRemovedLastRefFunction = false;
     2578#endif
     2579        guardDeref();
     2580    } else {
     2581#ifndef NDEBUG
     2582        rootNode()->m_inRemovedLastRefFunction = false;
     2583        beginDeletion();
     2584#endif
     2585        delete this;
     2586    }
     2587}
     2588
    25702589// It's important not to inline removedLastRef, because we don't want to inline the code to
    25712590// delete a Node at each deref call site.
     
    25752594    // faster for non-Document nodes, and because the call to removedLastRef that is inlined
    25762595    // at all deref call sites is smaller if it's a non-virtual function.
    2577     if (isDocumentNode()) {
    2578         static_cast<Document*>(this)->removedLastRef();
     2596    if (isTreeScope()) {
     2597        treeScope()->removedLastRefToScope();
    25792598        return;
    25802599    }
     2600
    25812601#ifndef NDEBUG
    25822602    m_deletionHasBegun = true;
  • trunk/Source/WebCore/dom/Node.h

    r143940 r144735  
    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

    r143940 r144735  
    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

    r143940 r144735  
    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

    r143940 r144735  
    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

    r143940 r144735  
    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

    r143940 r144735  
    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

    r143940 r144735  
    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.