Changeset 25797 in webkit

Timestamp:

Sep 28, 2007, 2:49:55 PM (18 years ago)

Author:

harrison

Message:

Reviewed by Darin Adler.

<rdar://problem/5511128> Crash closing or reloading this SVG

• dom/ContainerNode.cpp: (WebCore::dispatchChildInsertionEvents): (WebCore::dispatchChildRemovalEvents): Use DocPtr instead of RefPtr, since these events are dispatched when the Document is being being torn down by removedLastRef().

• dom/DocPtr.h: (WebCore::DocPtr::DocPtr): Fix longstanding typo in template so that the DocPtr(DocPtr) constructor can be used.

• dom/Document.cpp: (WebCore::Document::Document): (WebCore::Document::removedLastRef):
• dom/Document.h: (WebCore::Document::selfOnlyRef): (WebCore::Document::selfOnlyDeref):
• platform/Shared.h: (WebCore::Shared::Shared): (WebCore::Shared::ref): (WebCore::Shared::deref): (WebCore::Shared::hasOneRef): (WebCore::TreeShared::TreeShared): (WebCore::TreeShared::ref): (WebCore::TreeShared::deref): (WebCore::TreeShared::hasOneRef): (WebCore::TreeShared::refCount): Add debug-only checks for a document being ref-counted while being deleted.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ContainerNode.cpp (modified) (2 diffs)
• dom/DocPtr.h (modified) (1 diff)
• dom/Document.cpp (modified) (3 diffs)
• dom/Document.h (modified) (3 diffs)
• platform/Shared.h (modified) (6 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-09-28  David Harrison  <harrison@apple.com>
+
+        Reviewed by Darin Adler.
+
+        <rdar://problem/5511128> Crash closing or reloading this SVG
+
+        * dom/ContainerNode.cpp:
+        (WebCore::dispatchChildInsertionEvents):
+        (WebCore::dispatchChildRemovalEvents):
+        Use DocPtr instead of RefPtr, since these events are dispatched
+        when the Document is being being torn down by removedLastRef().
+
+        * dom/DocPtr.h:
+        (WebCore::DocPtr::DocPtr):
+        Fix longstanding typo in template so that the DocPtr(DocPtr) constructor can be used.
+
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        (WebCore::Document::removedLastRef):
+        * dom/Document.h:
+        (WebCore::Document::selfOnlyRef):
+        (WebCore::Document::selfOnlyDeref):
+        * platform/Shared.h:
+        (WebCore::Shared::Shared):
+        (WebCore::Shared::ref):
+        (WebCore::Shared::deref):
+        (WebCore::Shared::hasOneRef):
+        (WebCore::TreeShared::TreeShared):
+        (WebCore::TreeShared::ref):
+        (WebCore::TreeShared::deref):
+        (WebCore::TreeShared::hasOneRef):
+        (WebCore::TreeShared::refCount):
+        Add debug-only checks for a document being ref-counted while being deleted.
+
 2007-09-27  Kevin McCullough  <kmccullough@apple.com>
 

trunk/WebCore/dom/ContainerNode.cpp

@@ -886 +886 @@
 
     RefPtr<Node> c = child;
-    RefPtr<Document> doc = child->document();
+    DocPtr<Document> doc = child->document();
 
     if (c->parentNode() && c->parentNode()->inDocument())
@@ -920 +920 @@
 {
     RefPtr<Node> c = child;
-    RefPtr<Document> doc = child->document();
+    DocPtr<Document> doc = child->document();
 
     // update auxiliary doc info (e.g. iterators) to note that node is being removed

trunk/WebCore/dom/DocPtr.h

@@ -30 +30 @@
     DocPtr() : m_ptr(0) {}
     DocPtr(T *ptr) : m_ptr(ptr) { if (ptr) ptr->selfOnlyRef(); }
-    DocPtr(const DocPtr &o) : m_ptr(o.m_ptr) { if (T *ptr = m_ptr) ptr->selfOnlyRefRef(); }
+    DocPtr(const DocPtr &o) : m_ptr(o.m_ptr) { if (T *ptr = m_ptr) ptr->selfOnlyRef(); }
     ~DocPtr() { if (T *ptr = m_ptr) ptr->selfOnlyDeref(); }
     

trunk/WebCore/dom/Document.cpp

@@ -268 +268 @@
     , m_designMode(inherit)
     , m_selfOnlyRefCount(0)
+#ifndef NDEBUG
+    , m_hasDeleted(false)
+#endif
 #if ENABLE(SVG)
     , m_svgExtensions(0)
@@ -342 +345 @@
 void Document::removedLastRef()
 {
+    ASSERT(!m_hasDeleted);
     if (m_selfOnlyRefCount) {
         // if removing a child removes the last self-only ref, we don't
@@ -366 +370 @@
         delete m_tokenizer;
         m_tokenizer = 0;
-    } else
+    } else {
+#ifndef NDEBUG
+        m_hasDeleted = true;
+#endif
         delete this;
+    }
 }
 

trunk/WebCore/dom/Document.h

@@ -138 +138 @@
 public:
     Document(DOMImplementation*, Frame*, bool isXHTML = false);
-    ~Document();
+    virtual ~Document();
 
     virtual void removedLastRef();
@@ -148 +148 @@
     // pointer without introducing reference cycles
 
-    void selfOnlyRef() { ++m_selfOnlyRefCount; }
-    void selfOnlyDeref() {
+    void selfOnlyRef()
+    {
+        ASSERT(!m_hasDeleted);
+        ++m_selfOnlyRefCount;
+    }
+    void selfOnlyDeref()
+    {
+        ASSERT(!m_hasDeleted);
         --m_selfOnlyRefCount;
-        if (!m_selfOnlyRefCount && !refCount())
+        if (!m_selfOnlyRefCount && !refCount()) {
+#ifndef NDEBUG
+            m_hasDeleted = true;
+#endif
             delete this;
+        }
     }
 
@@ -873 +883 @@
     
     int m_selfOnlyRefCount;
+#ifndef NDEBUG
+    bool m_hasDeleted;
+#endif
 
     HTMLFormElement::CheckedRadioButtons m_checkedRadioButtons;

trunk/WebCore/platform/Shared.h

@@ -32 +32 @@
         : m_refCount(0)
 #ifndef NDEBUG
-        , m_inDestructor(0)
+        , m_hasDeleted(false)
 #endif
     {
@@ -39 +39 @@
     void ref()
     {
-        ASSERT(!m_inDestructor);
+        ASSERT(!m_hasDeleted);
         ++m_refCount;
     }
@@ -45 +45 @@
     void deref()
     {
-        ASSERT(!m_inDestructor);
+        ASSERT(!m_hasDeleted);
         if (--m_refCount <= 0) {
 #ifndef NDEBUG
-            m_inDestructor = true;
+            m_hasDeleted = true;
 #endif
             delete static_cast<T*>(this);
@@ -56 +56 @@
     bool hasOneRef()
     {
-        ASSERT(!m_inDestructor);
+        ASSERT(!m_hasDeleted);
         return m_refCount == 1;
     }
@@ -68 +68 @@
     int m_refCount;
 #ifndef NDEBUG
-    bool m_inDestructor;
+    bool m_hasDeleted;
 #endif
 };
 
-template<class T> class TreeShared {
+template<class T> class TreeShared : Noncopyable {
 public:
-    TreeShared() : m_refCount(0), m_parent(0) { }
-    TreeShared(T* parent) : m_refCount(0), m_parent(parent) { }
+    TreeShared()
+        : m_refCount(0)
+        , m_parent(0)
+#ifndef NDEBUG
+        , m_hasRemovedLastRef(false)
+#endif
+    {
+    }
+    TreeShared(T* parent)
+        : m_refCount(0)
+        , m_parent(parent)
+#ifndef NDEBUG
+        , m_hasRemovedLastRef(false)
+#endif
+    {
+    }
     virtual ~TreeShared() { }
 
     virtual void removedLastRef() { delete static_cast<T*>(this); }
 
-    void ref() { ++m_refCount;  }
-    void deref() { if (--m_refCount <= 0 && !m_parent) removedLastRef(); }
-    bool hasOneRef() { return m_refCount == 1; }
-    int refCount() const { return m_refCount; }
+    void ref()
+    {
+        ASSERT(!m_hasRemovedLastRef);
+        ++m_refCount;
+    }
+
+    void deref()
+    {
+        ASSERT(!m_hasRemovedLastRef);
+        if (--m_refCount <= 0 && !m_parent) {
+#ifndef NDEBUG
+            m_hasRemovedLastRef = true;
+#endif
+            removedLastRef();
+        }
+    }
+
+    bool hasOneRef() const
+    {
+        ASSERT(!m_hasRemovedLastRef);
+        return m_refCount == 1;
+    }
+
+    int refCount() const
+    {
+        return m_refCount;
+    }
 
     void setParent(T* parent) { m_parent = parent; }
@@ -91 +128 @@
     int m_refCount;
     T* m_parent;
-
-    TreeShared(const TreeShared&);
-    TreeShared& operator=(const TreeShared&);
+#ifndef NDEBUG
+    bool m_hasRemovedLastRef;
+#endif
 };