Changeset 143940 in webkit

Timestamp:

Feb 25, 2013 10:02:42 AM (11 years ago)

Author:

Dimitri Glazkov

Message:

Revert r143840 because it caused flaky crashes.
​https://bugs.webkit.org/show_bug.cgi?id=110766

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• WebCore.exp.in (modified) (1 diff)
• dom/Document.cpp (modified) (5 diffs)
• dom/Document.h (modified) (4 diffs)
• dom/DocumentFragment.cpp (modified) (1 diff)
• dom/Node.cpp (modified) (4 diffs)
• dom/Node.h (modified) (1 diff)
• dom/ShadowRoot.cpp (modified) (3 diffs)
• dom/ShadowRoot.h (modified) (1 diff)
• dom/TreeScope.cpp (modified) (8 diffs)
• dom/TreeScope.h (modified) (3 diffs)
• dom/TreeScopeAdopter.cpp (modified) (6 diffs)
• dom/TreeScopeAdopter.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-25  Dimitri Glazkov  <dglazkov@chromium.org>
+
+        Revert r143840 because it caused flaky crashes.
+        https://bugs.webkit.org/show_bug.cgi?id=110766
+
 2013-02-25  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebCore/WebCore.exp.in

@@ -1414 +1414 @@
 __ZNK7WebCore4Node13ownerDocumentEv
 __ZNK7WebCore4Node14isDescendantOfEPKS0_
+__ZNK7WebCore4Node11isTreeScopeEv
 __ZNK7WebCore4Node18getSubresourceURLsERN3WTF11ListHashSetINS_4KURLELm256ENS_8KURLHashEEE
 __ZNK7WebCore4Node31numberOfScopedHTMLStyleChildrenEv

trunk/Source/WebCore/dom/Document.cpp

@@ -418 +418 @@
     : ContainerNode(0, CreateDocument)
     , TreeScope(this)
+    , m_guardRefCount(0)
     , m_styleResolverThrowawayTimer(this, &Document::styleResolverThrowawayTimerFired)
     , m_lastStyleResolverAccessTime(0)
@@ -489 +490 @@
 #endif
 {
+    setTreeScope(this);
+
     m_printing = false;
     m_paginatedForScreen = false;
@@ -588 +591 @@
     ASSERT(!m_styleRecalcTimer.isActive());
     ASSERT(!m_parentTreeScope);
-    ASSERT(!hasGuardRefCount());
+    ASSERT(!m_guardRefCount);
 
 #if ENABLE(TEMPLATE_ELEMENT)
@@ -655 +658 @@
 }
 
-void Document::dispose()
-{
-    // We must make sure not to be retaining any of our children through
-    // these extra pointers or we will create a reference cycle.
-    m_docType = 0;
-    m_focusedNode = 0;
-    m_hoverNode = 0;
-    m_activeElement = 0;
-    m_titleElement = 0;
-    m_documentElement = 0;
-    m_contextFeatures = ContextFeatures::defaultSwitch();
-    m_userActionElements.documentDidRemoveLastRef();
+void Document::removedLastRef()
+{
+    ASSERT(!m_deletionHasBegun);
+    if (m_guardRefCount) {
+        // If removing a child removes the last self-only ref, we don't
+        // want the scope to be destructed until after
+        // removeDetachedChildren returns, so we guard ourselves with an
+        // extra self-only ref.
+        guardRef();
+
+        // We must make sure not to be retaining any of our children through
+        // these extra pointers or we will create a reference cycle.
+        m_docType = 0;
+        m_focusedNode = 0;
+        m_hoverNode = 0;
+        m_activeElement = 0;
+        m_titleElement = 0;
+        m_documentElement = 0;
+        m_contextFeatures = ContextFeatures::defaultSwitch();
+        m_userActionElements.documentDidRemoveLastRef();
 #if ENABLE(FULLSCREEN_API)
-    m_fullScreenElement = 0;
-    m_fullScreenElementStack.clear();
-#endif
-
-    detachParser();
+        m_fullScreenElement = 0;
+        m_fullScreenElementStack.clear();
+#endif
+
+        detachParser();
 
 #if ENABLE(CUSTOM_ELEMENTS)
@@ -678 +689 @@
 #endif
 
-    // removeDetachedChildren() doesn't always unregister IDs,
-    // so tear down scope information upfront to avoid having stale references in the map.
-    destroyTreeScopeData();
-    removeDetachedChildren();
-
-    m_markers->detach();
-
-    m_cssCanvasElements.clear();
+        // removeDetachedChildren() doesn't always unregister IDs,
+        // so tear down scope information upfront to avoid having stale references in the map.
+        destroyTreeScopeData();
+        removeDetachedChildren();
+
+        m_markers->detach();
+
+        m_cssCanvasElements.clear();
 
 #if ENABLE(REQUEST_ANIMATION_FRAME)
-    // FIXME: consider using ActiveDOMObject.
-    if (m_scriptedAnimationController)
-        m_scriptedAnimationController->clearDocumentPointer();
-    m_scriptedAnimationController.clear();
-#endif
+        // FIXME: consider using ActiveDOMObject.
+        if (m_scriptedAnimationController)
+            m_scriptedAnimationController->clearDocumentPointer();
+        m_scriptedAnimationController.clear();
+#endif
+
+#ifndef NDEBUG
+        m_inRemovedLastRefFunction = false;
+#endif
+
+        guardDeref();
+    } else {
+#ifndef NDEBUG 
+        m_deletionHasBegun = true; 
+#endif 
+        delete this;
+    }
 }
 

trunk/Source/WebCore/dom/Document.h

@@ -232 +232 @@
     using ContainerNode::deref;
 
+    // Nodes belonging to this document hold guard references -
+    // these are enough to keep the document from being destroyed, but
+    // not enough to keep it from removing its children. This allows a
+    // node that outlives its document to still have a valid document
+    // pointer without introducing reference cycles.
+    void guardRef()
+    {
+        ASSERT(!m_deletionHasBegun);
+        ++m_guardRefCount;
+    }
+
+    void guardDeref()
+    {
+        ASSERT(!m_deletionHasBegun);
+        --m_guardRefCount;
+        if (!m_guardRefCount && !refCount()) {
+#ifndef NDEBUG
+            m_deletionHasBegun = true;
+#endif
+            delete this;
+        }
+    }
+
     Element* getElementById(const AtomicString& id) const;
 
@@ -1203 +1226 @@
     friend class IgnoreDestructiveWriteCountIncrementer;
 
-    virtual void dispose() OVERRIDE;
-
+    void removedLastRef();
+    
     void detachParser();
 
@@ -1271 +1294 @@
     void addListenerType(ListenerType listenerType) { m_listenerTypes |= listenerType; }
     void addMutationEventListenerTypeIfEnabled(ListenerType);
+
+    int m_guardRefCount;
 
     void styleResolverThrowawayTimerFired(Timer<Document>*);
@@ -1591 +1616 @@
     , m_next(0)
 {
-    if (!m_treeScope)
+    if (document)
+        document->guardRef();
+    else
         m_treeScope = TreeScope::noDocumentInstance();
-    m_treeScope->guardRef();
 
 #if !defined(NDEBUG) || (defined(DUMP_NODE_STATISTICS) && DUMP_NODE_STATISTICS)

trunk/Source/WebCore/dom/DocumentFragment.cpp

@@ -35 +35 @@
     : ContainerNode(document, constructionType)
 {
+    ASSERT(document);
 }
 
 PassRefPtr<DocumentFragment> DocumentFragment::create(Document* document)
 {
-    ASSERT(document);
     return adoptRef(new DocumentFragment(document, Node::CreateDocumentFragment));
 }

trunk/Source/WebCore/dom/Node.cpp

@@ -443 +443 @@
         m_next->setPreviousSibling(0);
 
-    m_treeScope->guardDeref();
+    if (doc)
+        doc->guardDeref();
 
     InspectorCounters::decrementCounter(InspectorCounters::NodeCounter);
@@ -890 +891 @@
 
     return true;
+}
+
+bool Node::isTreeScope() const
+{
+    return treeScope()->rootNode() == this;
 }
 
@@ -2556 +2562 @@
 #endif
 
-// This is here for inlining
-inline void TreeScope::removedLastRefToScope()
-{
-    ASSERT(!deletionHasBegun());
-    if (m_guardRefCount) {
-        // If removing a child removes the last self-only ref, we don't
-        // want the scope to be destructed until after
-        // removeDetachedChildren returns, so we guard ourselves with an
-        // extra self-only ref.
-        guardRef();
-        dispose();
-#ifndef NDEBUG
-        // We need to do this right now since guardDeref() can delete this.
-        rootNode()->m_inRemovedLastRefFunction = false;
-#endif
-        guardDeref();
-    } else {
-#ifndef NDEBUG
-        rootNode()->m_inRemovedLastRefFunction = false;
-        beginDeletion();
-#endif
-        delete this;
-    }
-}
-
 // It's important not to inline removedLastRef, because we don't want to inline the code to
 // delete a Node at each deref call site.
@@ -2588 +2569 @@
     // faster for non-Document nodes, and because the call to removedLastRef that is inlined
     // at all deref call sites is smaller if it's a non-virtual function.
-    if (isTreeScope()) {
-        treeScope()->removedLastRefToScope();
+    if (isDocumentNode()) {
+        static_cast<Document*>(this)->removedLastRef();
         return;
     }
-
 #ifndef NDEBUG
     m_deletionHasBegun = true;

trunk/Source/WebCore/dom/Node.h

@@ -249 +249 @@
 
     bool isDocumentNode() const;
-    bool isTreeScope() const { return treeScope()->rootNode() == this; }
+    bool isTreeScope() const;
     bool isDocumentFragment() const { return getFlag(IsDocumentFragmentFlag); }
     bool isShadowRoot() const { return isDocumentFragment() && isTreeScope(); }

trunk/Source/WebCore/dom/ShadowRoot.cpp

@@ -53 +53 @@
 
 ShadowRoot::ShadowRoot(Document* document, ShadowRootType type)
-    : DocumentFragment(0, CreateShadowRoot)
+    : DocumentFragment(document, CreateShadowRoot)
     , TreeScope(this, document)
     , m_prev(0)
@@ -64 +64 @@
 {
     ASSERT(document);
+    setTreeScope(this);
 
 #if PLATFORM(CHROMIUM)
@@ -87 +88 @@
     if (hasRareData())
         clearRareData();
-}
-
-void ShadowRoot::dispose()
-{
-    removeDetachedChildren();
 }
 

trunk/Source/WebCore/dom/ShadowRoot.h

@@ -101 +101 @@
     virtual ~ShadowRoot();
 
-    virtual void dispose() OVERRIDE;
     virtual bool childTypeAllowed(NodeType) const OVERRIDE;
     virtual void childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta) OVERRIDE;

trunk/Source/WebCore/dom/TreeScope.cpp

@@ -60 +60 @@
     virtual ~SameSizeAsTreeScope();
     void* pointers[8];
-    int ints[1];
 };
 
@@ -71 +70 @@
     , m_documentScope(document)
     , m_parentTreeScope(document)
-    , m_guardRefCount(0)
     , m_idTargetObserverRegistry(IdTargetObserverRegistry::create())
 {
@@ -77 +75 @@
     ASSERT(document);
     ASSERT(rootNode != document);
-    m_parentTreeScope->guardRef();
-    m_rootNode->setTreeScope(this);
 }
 
@@ -85 +81 @@
     , m_documentScope(document)
     , m_parentTreeScope(0)
-    , m_guardRefCount(0)
     , m_idTargetObserverRegistry(IdTargetObserverRegistry::create())
 {
     ASSERT(document);
-    m_rootNode->setTreeScope(this);
 }
 
@@ -96 +90 @@
     , m_documentScope(0)
     , m_parentTreeScope(0)
-    , m_guardRefCount(0)
 {
 }
@@ -102 +95 @@
 TreeScope::~TreeScope()
 {
-    ASSERT(!m_guardRefCount);
-    m_rootNode->setTreeScope(noDocumentInstance());
-
     if (m_selection) {
         m_selection->clearTreeScope();
         m_selection = 0;
     }
-
-    if (m_parentTreeScope)
-        m_parentTreeScope->guardDeref();
 }
 
@@ -134 +121 @@
     ASSERT(newParentScope);
 
-    newParentScope->guardRef();
-    if (m_parentTreeScope)
-        m_parentTreeScope->guardDeref();
     m_parentTreeScope = newParentScope;
     setDocumentScope(newParentScope->documentScope());
@@ -432 +416 @@
 }
 
-#ifndef NDEBUG
-bool TreeScope::deletionHasBegun()
-{
-    return rootNode() && rootNode()->m_deletionHasBegun;
-}
-
-void TreeScope::beginDeletion()
-{
-    ASSERT(this != noDocumentInstance());
-    rootNode()->m_deletionHasBegun = true;
-}
-#endif
-
-int TreeScope::refCount() const
-{
-    if (Node* root = rootNode())
-        return root->refCount();
-    return 0;
-}
-
 } // namespace WebCore

trunk/Source/WebCore/dom/TreeScope.h

@@ -93 +93 @@
     void adoptIfNeeded(Node*);
 
-    Node* rootNode() const { return m_rootNode; }
+    ContainerNode* rootNode() const { return m_rootNode; }
 
     IdTargetObserverRegistry& idTargetObserverRegistry() const { return *m_idTargetObserverRegistry.get(); }
@@ -104 +104 @@
         return &instance;
     }
-
-    // Nodes belonging to this scope hold guard references -
-    // these are enough to keep the scope from being destroyed, but
-    // not enough to keep it from removing its children. This allows a
-    // node that outlives its scope to still have a valid document
-    // pointer without introducing reference cycles.
-    void guardRef()
-    {
-        ASSERT(!deletionHasBegun());
-        ++m_guardRefCount;
-    }
-
-    void guardDeref()
-    {
-        ASSERT(!deletionHasBegun());
-        --m_guardRefCount;
-        if (!m_guardRefCount && !refCount() && this != noDocumentInstance()) {
-            beginDeletion();
-            delete this;
-        }
-    }
-
-    void removedLastRefToScope();
 
 protected:
@@ -142 +119 @@
     }
 
-    bool hasGuardRefCount() const { return m_guardRefCount; }
-
 private:
     TreeScope();
 
-    virtual void dispose() { }
-
-    int refCount() const;
-#ifndef NDEBUG
-    bool deletionHasBegun();
-    void beginDeletion();
-#else
-    bool deletionHasBegun() { return false; }
-    void beginDeletion() { }
-#endif
-
-    Node* m_rootNode;
+    ContainerNode* m_rootNode;
     Document* m_documentScope;
     TreeScope* m_parentTreeScope;
-    int m_guardRefCount;
 
     OwnPtr<DocumentOrderedMap> m_elementsById;

trunk/Source/WebCore/dom/TreeScopeAdopter.cpp

@@ -41 +41 @@
     ASSERT(needsScopeChange());
 
-    m_oldScope->guardRef();
-
     // If an element is moved from a document and then eventually back again the collection cache for
     // that element may contain stale data as changes made to it will have updated the DOMTreeVersion
@@ -54 +52 @@
 
     for (Node* node = root; node; node = NodeTraversal::next(node, root)) {
-        updateTreeScope(node);
+        node->setTreeScope(m_newScope);
 
         if (willMoveToNewDocument)
@@ -79 +77 @@
         }
     }
-
-    m_oldScope->guardDeref();
 }
 
@@ -104 +100 @@
 #endif
 
-inline void TreeScopeAdopter::updateTreeScope(Node* node) const
-{
-    ASSERT(!node->isTreeScope());
-    ASSERT(node->treeScope() == m_oldScope);
-    m_newScope->guardRef();
-    m_oldScope->guardDeref();
-    node->setTreeScope(m_newScope);
-}
-
 inline void TreeScopeAdopter::moveNodeToNewDocument(Node* node, Document* oldDocument, Document* newDocument) const
 {
@@ -123 +110 @@
     }
 
+    newDocument->guardRef();
     if (oldDocument)
         oldDocument->moveNodeIteratorsToNewDocument(node, newDocument);
@@ -136 +124 @@
     node->didMoveToNewDocument(oldDocument);
     ASSERT(didMoveToNewDocumentWasCalled);
+    
+    if (oldDocument)
+        oldDocument->guardDeref();
 }
 

trunk/Source/WebCore/dom/TreeScopeAdopter.h

@@ -46 +46 @@
 
 private:
-    void updateTreeScope(Node*) const;
     void moveTreeToNewScope(Node*) const;
     void moveTreeToNewDocument(Node*, Document* oldDocument, Document* newDocument) const;