Changeset 91725 in webkit

Timestamp:

Jul 25, 2011 4:57:43 PM (13 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION (r39725?): Resources removed from document can not be freed until the document is deleted
​https://bugs.webkit.org/show_bug.cgi?id=61006

Patch by Scott Graham <​scottmg@chromium.org> on 2011-07-25
Reviewed by Nate Chapin.

Rollback previous patch. Occasional crashes seen caused by
CachedResource being deleted twice.

• loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::CachedResourceLoader):
(WebCore::CachedResourceLoader::loadDone):

• loader/cache/CachedResourceLoader.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/cache/CachedResource.h (modified) (1 diff)
• loader/cache/CachedResourceLoader.cpp (modified) (2 diffs)
• loader/cache/CachedResourceLoader.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-07-25  Scott Graham  <scottmg@chromium.org>
+
+        REGRESSION (r39725?): Resources removed from document can not be freed until the document is deleted
+        https://bugs.webkit.org/show_bug.cgi?id=61006
+
+        Reviewed by Nate Chapin.
+
+        Rollback previous patch. Occasional crashes seen caused by
+        CachedResource being deleted twice.
+
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::CachedResourceLoader):
+        (WebCore::CachedResourceLoader::loadDone):
+        * loader/cache/CachedResourceLoader.h:
+
 2011-07-25  Chris Rogers  <crogers@google.com>
 

trunk/Source/WebCore/loader/cache/CachedResource.h

@@ -183 +183 @@
 
     bool canDelete() const { return !hasClients() && !m_request && !m_preloadCount && !m_handleCount && !m_resourceToRevalidate && !m_proxyResource; }
-    bool hasOneHandle() const { return m_handleCount == 1; }
 
     bool isExpired() const;

trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

@@ -87 +87 @@
     : m_document(document)
     , m_requestCount(0)
-    , m_garbageCollectDocumentResourcesTimer(this, &CachedResourceLoader::garbageCollectDocumentResourcesTimerFired)
     , m_autoLoadImages(true)
     , m_loadFinishing(false)
@@ -574 +573 @@
         frame()->loader()->loadDone();
     performPostLoadActions();
-
-    if (!m_garbageCollectDocumentResourcesTimer.isActive())
-        m_garbageCollectDocumentResourcesTimer.startOneShot(0);
-}
-
-// Garbage collecting m_documentResources is a workaround for the
-// CachedResourceHandles on the RHS being strong references. Ideally this
-// would be a weak map, however CachedResourceHandles perform additional
-// bookkeeping on CachedResources, so instead pseudo-GC them -- when the
-// reference count reaches 1, m_documentResources is the only reference, so
-// remove it from the map.
-void CachedResourceLoader::garbageCollectDocumentResourcesTimerFired(Timer<CachedResourceLoader>* timer)
-{
-    ASSERT_UNUSED(timer, timer == &m_garbageCollectDocumentResourcesTimer);
-
-    Vector<String, 10> toDelete;
-
-    for (DocumentResourceMap::iterator it = m_documentResources.begin(); it != m_documentResources.end(); ++it) {
-        if (it->second->hasOneHandle())
-            toDelete.append(it->first);
-    }
-
-    for (Vector<String, 10>::const_iterator idel = toDelete.begin(); idel != toDelete.end(); ++idel)
-        m_documentResources.remove(*idel);
 }
 

trunk/Source/WebCore/loader/cache/CachedResourceLoader.h

@@ -31 +31 @@
 #include "CachePolicy.h"
 #include "ResourceLoadPriority.h"
-#include "Timer.h"
 #include <wtf/Deque.h>
 #include <wtf/HashMap.h>
@@ -119 +118 @@
     bool canRequest(CachedResource::Type, const KURL&, bool forPreload = false);
 
-    void garbageCollectDocumentResourcesTimerFired(Timer<CachedResourceLoader>*);
     void performPostLoadActions();
     
@@ -136 +134 @@
     Deque<PendingPreload> m_pendingPreloads;
 
-    Timer<CachedResourceLoader> m_garbageCollectDocumentResourcesTimer;
-    
     //29 bits left
     bool m_autoLoadImages : 1;