2011-04-06 Roland Steiner <rolandsteiner@chromium.org>
authorrolandsteiner@chromium.org <rolandsteiner@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Apr 2011 23:52:52 +0000 (23:52 +0000)
committerrolandsteiner@chromium.org <rolandsteiner@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Apr 2011 23:52:52 +0000 (23:52 +0000)
        Reviewed by Dimitri Glazkov.

        Bug 57994 - Move guardRef functionality back to Document
        https://bugs.webkit.org/show_bug.cgi?id=57994

        Move the relevant code parts from TreeScope back into Document.

        No new tests. (no new functionality)

        * dom/Document.cpp:
        (WebCore::Document::removedLastRef):
        * dom/Document.h:
        * dom/TreeScope.cpp:
        (WebCore::TreeScope::destroyTreeScopeData):
        * dom/TreeScope.h:

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/TreeScope.cpp
Source/WebCore/dom/TreeScope.h

index 9ca3999..1350a25 100644 (file)
@@ -1,3 +1,21 @@
+2011-04-06  Roland Steiner  <rolandsteiner@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        Bug 57994 - Move guardRef functionality back to Document
+        https://bugs.webkit.org/show_bug.cgi?id=57994
+
+        Move the relevant code parts from TreeScope back into Document.
+
+        No new tests. (no new functionality)
+
+        * dom/Document.cpp:
+        (WebCore::Document::removedLastRef):
+        * dom/Document.h:
+        * dom/TreeScope.cpp:
+        (WebCore::TreeScope::destroyTreeScopeData):
+        * dom/TreeScope.h:
+
 2011-04-06  Ian Henderson  <ianh@apple.com>
 
         Reviewed by Simon Fraser, Antti Koivisto.
index d9ca64d..d293c3f 100644 (file)
@@ -378,6 +378,7 @@ uint64_t Document::s_globalTreeVersion = 0;
 
 Document::Document(Frame* frame, const KURL& url, bool isXHTML, bool isHTML)
     : TreeScope(0)
+    , m_guardRefCount(0)
     , m_compatibilityMode(NoQuirksMode)
     , m_compatibilityModeLocked(false)
     , m_domTreeVersion(++s_globalTreeVersion)
@@ -508,40 +509,6 @@ Document::Document(Frame* frame, const KURL& url, bool isXHTML, bool isHTML)
 #endif
 }
 
-void Document::destroyScope()
-{
-    ASSERT(!m_deletionHasBegun);
-
-    // 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_activeNode = 0;
-    m_titleElement = 0;
-    m_documentElement = 0;
-#if ENABLE(FULLSCREEN_API)
-    m_fullScreenElement = 0;
-#endif
-
-    TreeScope::destroyScope();
-
-    m_markers->detach();
-
-    detachParser();
-
-    m_cssCanvasElements.clear();
-
-#if ENABLE(REQUEST_ANIMATION_FRAME)
-    // FIXME: consider using ActiveDOMObject.
-    m_scriptedAnimationController = 0;
-#endif
-
-#ifndef NDEBUG
-    m_inRemovedLastRefFunction = false;
-#endif
-}
-
 Document::~Document()
 {
     ASSERT(!renderer());
@@ -601,6 +568,57 @@ Document::~Document()
         m_implementation->ownerDocumentDestroyed();
 }
 
+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
+        // removeAllChildren 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_activeNode = 0;
+        m_titleElement = 0;
+        m_documentElement = 0;
+#if ENABLE(FULLSCREEN_API)
+        m_fullScreenElement = 0;
+#endif
+
+        // removeAllChildren() doesn't always unregister IDs,
+        // so tear down scope information upfront to avoid having stale references in the map.
+        destroyTreeScopeData();
+        removeAllChildren();
+
+        m_markers->detach();
+
+        detachParser();
+
+        m_cssCanvasElements.clear();
+
+#if ENABLE(REQUEST_ANIMATION_FRAME)
+        // FIXME: consider using ActiveDOMObject.
+        m_scriptedAnimationController = 0;
+#endif
+
+#ifndef NDEBUG
+        m_inRemovedLastRefFunction = false;
+#endif
+
+        guardDeref();
+    } else {
+#ifndef NDEBUG
+        m_deletionHasBegun = true;
+#endif
+        delete this;
+    }
+}
+
 Element* Document::getElementById(const AtomicString& id) const
 {
     return TreeScope::getElementById(id);
index 2861e1b..357c4a3 100644 (file)
@@ -222,6 +222,31 @@ public:
     using TreeScope::ref;
     using TreeScope::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;
+        }
+    }
+
+    virtual void removedLastRef();
+
     Element* getElementById(const AtomicString& id) const;
 
     // DOM methods & attributes for Document
@@ -1083,8 +1108,6 @@ public:
 protected:
     Document(Frame*, const KURL&, bool isXHTML, bool isHTML);
 
-    virtual void destroyScope();
-
     void clearXMLVersion() { m_xmlVersion = String(); }
 
 private:
@@ -1127,6 +1150,8 @@ private:
 
     void loadEventDelayTimerFired(Timer<Document>*);
 
+    int m_guardRefCount;
+
     OwnPtr<CSSStyleSelector> m_styleSelector;
     bool m_didCalculateStyleSelector;
     bool m_hasDirtyStyleSelector;
index 853846e..89675ee 100644 (file)
@@ -37,32 +37,16 @@ using namespace HTMLNames;
 
 TreeScope::TreeScope(Document* document, ConstructionType constructionType)
     : ContainerNode(document, constructionType)
-    , m_guardRefCount(0)
     , m_accessKeyMapValid(false)
     , m_numNodeListCaches(0)
 {
 }
 
-void TreeScope::removedLastRef()
+void TreeScope::destroyTreeScopeData()
 {
-    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
-        // removeAllChildren returns, so we guard ourselves with an
-        // extra self-only ref.
-        guardRef();
-        destroyScope();
-        guardDeref();
-    } else
-        ContainerNode::removedLastRef();
-}
-
-void TreeScope::destroyScope()
-{
-    // removeAllChildren() doesn't always unregister IDs, do it upfront to avoid having stale references in the map.
     m_elementsById.clear();
-    removeAllChildren();
+    m_imageMapsByName.clear();
+    m_elementsByAccessKey.clear();
 }
 
 Element* TreeScope::getElementById(const AtomicString& elementId) const
index cf4a7a0..64a22ee 100644 (file)
@@ -36,33 +36,6 @@ class HTMLMapElement;
 
 class TreeScope : public ContainerNode {
 public:
-    // 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 scope
-    // 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;
-        }
-    }
-
-    virtual void removedLastRef();
-    virtual void destroyScope();
-
     Element* getElementById(const AtomicString&) const;
     bool hasElementWithId(AtomicStringImpl* id) const;
     bool containsMultipleElementsWithId(const AtomicString& id) const;
@@ -90,9 +63,9 @@ public:
 protected:
     TreeScope(Document*, ConstructionType = CreateContainer);
 
-private:
-    int m_guardRefCount;
+    void destroyTreeScopeData();
 
+private:
     DocumentOrderedMap m_elementsById;
     DocumentOrderedMap m_imageMapsByName;