Reviewed by Maciej.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Oct 2007 01:54:09 +0000 (01:54 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Oct 2007 01:54:09 +0000 (01:54 +0000)
        - tone down the assertion I asked Harrison to include in his fix for
          <rdar://problem/5511128>; it's OK to re-ref and deref the document
          as long as you do so after the children are done being destroyed

        No effect on release builds. Assertion change only.

        Besides the changes listed below, renamed m_hasDeleted flag to
        m_deletionHasBegun.

        * dom/ContainerNode.cpp: (WebCore::ContainerNode::removeAllChildren):
        Added code to set the m_deletionHasBegun flag and some assertions
        that test its state.

        * dom/Document.h: Removed m_hasDeleted -- we now use m_deletionHasBegun
        in the base class TreeShared.
        * dom/Document.cpp:
        (WebCore::Document::Document): Removed initialization of m_hasDeleted.
        (WebCore::Document::removedLastRef): Added code to clear
        m_inRemovedLastRefFunction if we end up deciding not to delete this.

        * platform/Shared.h:
        (WebCore::TreeShared::TreeShared): Added m_deletionHasBegun in addition to
        m_inRemovedLastRefFunction (formerly named m_hasRemovedLastRef).
        (WebCore::TreeShared::~TreeShared): Assert that m_deletionHasBegun is true.
        (WebCore::TreeShared::ref): Assert neither flag is true.
        (WebCore::TreeShared::deref): Ditto.
        (WebCore::TreeShared::hasOneRef): Ditto.
        (WebCore::TreeShared::removedLastRef): Made private. Added code to
        set m_hasDeleted to true. Also removed cast; since this class template
        has a virtual destructor, we don't need to cast before calling delete.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@25813 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/dom/ContainerNode.cpp
WebCore/dom/Document.cpp
WebCore/dom/Document.h
WebCore/platform/Shared.h

index 0dcb99a2bc517df5b405831f0a0d52ca7c900d57..ec6591083b81b295d66b7949e33564d2e5f11f37 100644 (file)
@@ -1,3 +1,38 @@
+2007-09-30  Darin Adler  <darin@apple.com>
+
+        Reviewed by Maciej.
+
+        - tone down the assertion I asked Harrison to include in his fix for
+          <rdar://problem/5511128>; it's OK to re-ref and deref the document
+          as long as you do so after the children are done being destroyed
+
+        No effect on release builds. Assertion change only.
+
+        Besides the changes listed below, renamed m_hasDeleted flag to
+        m_deletionHasBegun.
+
+        * dom/ContainerNode.cpp: (WebCore::ContainerNode::removeAllChildren):
+        Added code to set the m_deletionHasBegun flag and some assertions
+        that test its state.
+
+        * dom/Document.h: Removed m_hasDeleted -- we now use m_deletionHasBegun
+        in the base class TreeShared.
+        * dom/Document.cpp:
+        (WebCore::Document::Document): Removed initialization of m_hasDeleted.
+        (WebCore::Document::removedLastRef): Added code to clear
+        m_inRemovedLastRefFunction if we end up deciding not to delete this.
+
+        * platform/Shared.h:
+        (WebCore::TreeShared::TreeShared): Added m_deletionHasBegun in addition to
+        m_inRemovedLastRefFunction (formerly named m_hasRemovedLastRef).
+        (WebCore::TreeShared::~TreeShared): Assert that m_deletionHasBegun is true.
+        (WebCore::TreeShared::ref): Assert neither flag is true.
+        (WebCore::TreeShared::deref): Ditto.
+        (WebCore::TreeShared::hasOneRef): Ditto.
+        (WebCore::TreeShared::removedLastRef): Made private. Added code to
+        set m_hasDeleted to true. Also removed cast; since this class template
+        has a virtual destructor, we don't need to cast before calling delete.
+
 2007-09-29  Holger Hans Peter Freyther  <zecke@selfish.org>
 
         Reviewed by Mark.
         Reviewed by Darin.
 
         - <rdar://5261371> Nothing downloaded when exporting bookmarks from iGoogle web history
-        - Implemented IWebHTTPURLResponse::allHeaderFields so that if the content disposition is "attachment" we will download the file instead of display it. Also implemented some missing functionality.
+
+        Function for use by WebKit. Currently used only on Windows.
 
         * platform/network/ResourceResponse.cpp:
         (WebCore::ResourceResponse::isAttachment):
index f4f3cc8add3db098efa8ce79e1943ecf2acb935a..341b42b9bf227484ce6babbfece2d9af1e2f952c 100644 (file)
@@ -62,22 +62,26 @@ void ContainerNode::removeAllChildren()
     bool topLevel = !alreadyInsideDestructor;
     if (topLevel)
         alreadyInsideDestructor = true;
-    
+
     // List of nodes to be deleted.
-    static Node *head;
-    static Node *tail;
-    
+    static Nodehead;
+    static Nodetail;
+
     // We have to tell all children that their parent has died.
-    Node *n;
-    Node *next;
+    Node* n;
+    Node* next;
+    for (n = m_firstChild; n != 0; n = next) {
+        ASSERT(!n->m_deletionHasBegun);
 
-    for (n = m_firstChild; n != 0; n = next ) {
         next = n->nextSibling();
         n->setPreviousSibling(0);
         n->setNextSibling(0);
         n->setParent(0);
         
-        if ( !n->refCount() ) {
+        if (!n->refCount()) {
+#ifndef NDEBUG
+            n->m_deletionHasBegun = true;
+#endif
             // Add the node to the list of nodes to be deleted.
             // Reuse the nextSibling pointer for this purpose.
             if (tail)
@@ -88,20 +92,22 @@ void ContainerNode::removeAllChildren()
         } else if (n->inDocument())
             n->removedFromDocument();
     }
-    
+
     // Only for the top level call, do the actual deleting.
     if (topLevel) {
         while ((n = head) != 0) {
+            ASSERT(n->m_deletionHasBegun);
+
             next = n->nextSibling();
             n->setNextSibling(0);
 
             head = next;
             if (next == 0)
                 tail = 0;
-            
+
             delete n;
         }
-        
+
         alreadyInsideDestructor = false;
         m_firstChild = 0;
         m_lastChild = 0;
index 432f5acffb425830a2bedf9615c9a2c3909918f2..21b06947f2c05f04cfb053eb1acb16c1850155ff 100644 (file)
@@ -267,9 +267,6 @@ Document::Document(DOMImplementation* impl, Frame* frame, bool isXHTML)
     , m_secureForms(0)
     , m_designMode(inherit)
     , m_selfOnlyRefCount(0)
-#ifndef NDEBUG
-    , m_hasDeleted(false)
-#endif
 #if ENABLE(SVG)
     , m_svgExtensions(0)
 #endif
@@ -344,17 +341,17 @@ Document::Document(DOMImplementation* impl, Frame* frame, bool isXHTML)
 
 void Document::removedLastRef()
 {
-    ASSERT(!m_hasDeleted);
+    ASSERT(!m_deletionHasBegun);
     if (m_selfOnlyRefCount) {
-        // if removing a child removes the last self-only ref, we don't
+        // If removing a child removes the last self-only ref, we don't
         // want the document to be destructed until after
         // removeAllChildren returns, so we guard ourselves with an
-        // extra self-only ref
+        // extra self-only ref.
 
         DocPtr<Document> guard(this);
 
-        // we must make sure not to be retaining any of our children through
-        // these extra pointers or we will create a reference cycle
+        // 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;
@@ -369,9 +366,13 @@ void Document::removedLastRef()
 
         delete m_tokenizer;
         m_tokenizer = 0;
+
+#ifndef NDEBUG
+        m_inRemovedLastRefFunction = false;
+#endif
     } else {
 #ifndef NDEBUG
-        m_hasDeleted = true;
+        m_deletionHasBegun = true;
 #endif
         delete this;
     }
index 668cb1d4b139851bf17717ffedd35981a391ad37..c1ef1f4df8629c8794a80e85fec92c8ab9213e54 100644 (file)
@@ -149,16 +149,16 @@ public:
 
     void selfOnlyRef()
     {
-        ASSERT(!m_hasDeleted);
+        ASSERT(!m_deletionHasBegun);
         ++m_selfOnlyRefCount;
     }
     void selfOnlyDeref()
     {
-        ASSERT(!m_hasDeleted);
+        ASSERT(!m_deletionHasBegun);
         --m_selfOnlyRefCount;
         if (!m_selfOnlyRefCount && !refCount()) {
 #ifndef NDEBUG
-            m_hasDeleted = true;
+            m_deletionHasBegun = true;
 #endif
             delete this;
         }
@@ -882,9 +882,6 @@ private:
     InheritedBool m_designMode;
     
     int m_selfOnlyRefCount;
-#ifndef NDEBUG
-    bool m_hasDeleted;
-#endif
 
     HTMLFormElement::CheckedRadioButtons m_checkedRadioButtons;
     
index 1a0077673e33876a17111ea5ff1c7d9e90f6e838..4f0800abe4d6594167ae888b1e5d9174437ff8f9 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006 Apple Computer, Inc.
+ * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -31,23 +31,23 @@ public:
     Shared()
         : m_refCount(0)
 #ifndef NDEBUG
-        , m_hasDeleted(false)
+        , m_deletionHasBegun(false)
 #endif
     {
     }
 
     void ref()
     {
-        ASSERT(!m_hasDeleted);
+        ASSERT(!m_deletionHasBegun);
         ++m_refCount;
     }
 
     void deref()
     {
-        ASSERT(!m_hasDeleted);
+        ASSERT(!m_deletionHasBegun);
         if (--m_refCount <= 0) {
 #ifndef NDEBUG
-            m_hasDeleted = true;
+            m_deletionHasBegun = true;
 #endif
             delete static_cast<T*>(this);
         }
@@ -55,7 +55,7 @@ public:
 
     bool hasOneRef()
     {
-        ASSERT(!m_hasDeleted);
+        ASSERT(!m_deletionHasBegun);
         return m_refCount == 1;
     }
 
@@ -67,7 +67,7 @@ public:
 private:
     int m_refCount;
 #ifndef NDEBUG
-    bool m_hasDeleted;
+    bool m_deletionHasBegun;
 #endif
 };
 
@@ -76,35 +76,40 @@ public:
     TreeShared()
         : m_refCount(0)
         , m_parent(0)
+    {
 #ifndef NDEBUG
-        , m_hasRemovedLastRef(false)
+        m_deletionHasBegun = false;
+        m_inRemovedLastRefFunction = false;
 #endif
-    {
     }
     TreeShared(T* parent)
         : m_refCount(0)
-        , m_parent(parent)
+        , m_parent(0)
+    {
 #ifndef NDEBUG
-        , m_hasRemovedLastRef(false)
+        m_deletionHasBegun = false;
+        m_inRemovedLastRefFunction = false;
 #endif
+    }
+    virtual ~TreeShared()
     {
+        ASSERT(m_deletionHasBegun);
     }
-    virtual ~TreeShared() { }
-
-    virtual void removedLastRef() { delete static_cast<T*>(this); }
 
     void ref()
     {
-        ASSERT(!m_hasRemovedLastRef);
+        ASSERT(!m_deletionHasBegun);
+        ASSERT(!m_inRemovedLastRefFunction);
         ++m_refCount;
     }
 
     void deref()
     {
-        ASSERT(!m_hasRemovedLastRef);
+        ASSERT(!m_deletionHasBegun);
+        ASSERT(!m_inRemovedLastRefFunction);
         if (--m_refCount <= 0 && !m_parent) {
 #ifndef NDEBUG
-            m_hasRemovedLastRef = true;
+            m_inRemovedLastRefFunction = true;
 #endif
             removedLastRef();
         }
@@ -112,7 +117,8 @@ public:
 
     bool hasOneRef() const
     {
-        ASSERT(!m_hasRemovedLastRef);
+        ASSERT(!m_deletionHasBegun);
+        ASSERT(!m_inRemovedLastRefFunction);
         return m_refCount == 1;
     }
 
@@ -124,12 +130,22 @@ public:
     void setParent(T* parent) { m_parent = parent; }
     T* parent() const { return m_parent; }
 
+#ifndef NDEBUG
+    bool m_deletionHasBegun;
+    bool m_inRemovedLastRefFunction;
+#endif
+
 private:
-    int m_refCount;
-    T* m_parent;
+    virtual void removedLastRef()
+    {
 #ifndef NDEBUG
-    bool m_hasRemovedLastRef;
+        m_deletionHasBegun = true;
 #endif
+        delete this;
+    }
+
+    int m_refCount;
+    T* m_parent;
 };
 
 }