Changeset 18588 in webkit

Timestamp:

Jan 4, 2007 12:52:59 PM (17 years ago)

Author:

beidson

Message:

Reviewed and landed by Brady

• fix ​http://bugs.webkit.org/show_bug.cgi?id=12087 REGRESSION: Reproducible crash going back in Back/Forward history

Allow a PageCache that is pending release to be resurrected by its
HistoryItem instead of creating a new PageCache. This avoids the
the condition of having two PageCaches for the same HistoryItem, one
current and one pending release but not yet released, which was causing
this and other bugs before.

Replaced the vector of PageCaches pending release with a set of
HistoryItems that are pending release of their PageCache.

• history/HistoryItem.cpp: (WebCore::HistoryItem::HistoryItem): (WebCore::HistoryItem::setHasPageCache): Check for an existing PageCache pending release, and if found, resurrect it by canceling the release. (WebCore::HistoryItem::pageCache): Return 0 if the PageCache is pending release. (WebCore::itemsWithPendingPageCacheToRelease): Renamed pendingPageCacheToRelease to this. This method now returns a HashSet of HistoryItems that have scheduled their PageCaches for release. (WebCore::HistoryItem::releasePageCachesOrReschedule): Renamed releasePageCache to this. (WebCore::HistoryItem::releasePageCache): Added. Actually closes and releases the PageCache. (WebCore::HistoryItem::releaseAllPendingPageCaches): Rolled closeObjectsInPendingPageCaches into this method. (WebCore::HistoryItem::scheduleRelease): (WebCore::HistoryItem::cancelRelease): Added.
• history/HistoryItem.h: Removed unused declaration of scheduleReleaseTimer()
• history/HistoryItemTimer.cpp: (WebCore::HistoryItemTimer::HistoryItemTimer): (WebCore::HistoryItemTimer::callReleasePageCachesOrReschedule):
• history/HistoryItemTimer.h:

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• history/HistoryItem.cpp (modified) (11 diffs)
• history/HistoryItem.h (modified) (3 diffs)
• history/HistoryItemTimer.cpp (modified) (2 diffs)
• history/HistoryItemTimer.h (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-01-04  Mitz Pettel  <mitz@webkit.org>
+
+        Reviewed and landed by Brady
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=12087
+          REGRESSION: Reproducible crash going back in Back/Forward history
+
+        Allow a PageCache that is pending release to be resurrected by its
+        HistoryItem instead of creating a new PageCache. This avoids the
+        the condition of having two PageCaches for the same HistoryItem, one
+        current and one pending release but not yet released, which was causing
+        this and other bugs before.
+        
+        Replaced the vector of PageCaches pending release with a set of
+        HistoryItems that are pending release of their PageCache.
+
+        * history/HistoryItem.cpp:
+        (WebCore::HistoryItem::HistoryItem):
+        (WebCore::HistoryItem::setHasPageCache): Check for an existing PageCache
+        pending release, and if found, resurrect it by canceling the release.
+        (WebCore::HistoryItem::pageCache): Return 0 if the PageCache is pending
+        release.
+        (WebCore::itemsWithPendingPageCacheToRelease): Renamed pendingPageCacheToRelease
+        to this. This method now returns a HashSet of HistoryItems that have scheduled
+        their PageCaches for release.
+        (WebCore::HistoryItem::releasePageCachesOrReschedule): Renamed releasePageCache
+        to this.
+        (WebCore::HistoryItem::releasePageCache): Added. Actually closes and releases
+        the PageCache.
+        (WebCore::HistoryItem::releaseAllPendingPageCaches): Rolled closeObjectsInPendingPageCaches
+        into this method.
+        (WebCore::HistoryItem::scheduleRelease):
+        (WebCore::HistoryItem::cancelRelease): Added.
+        * history/HistoryItem.h: Removed unused declaration of scheduleReleaseTimer()
+        * history/HistoryItemTimer.cpp:
+        (WebCore::HistoryItemTimer::HistoryItemTimer):
+        (WebCore::HistoryItemTimer::callReleasePageCachesOrReschedule):
+        * history/HistoryItemTimer.h:
+
 2007-01-04  David Hyatt  <hyatt@apple.com>
 

trunk/WebCore/history/HistoryItem.cpp

@@ -44 +44 @@
 HistoryItem::HistoryItem()
     : m_lastVisitedTime(0)
+    , m_pageCacheIsPendingRelease(false)
     , m_isTargetItem(false)
     , m_alwaysAttemptToUsePageCache(false)
@@ -55 +56 @@
     , m_title(title)
     , m_lastVisitedTime(time)
+    , m_pageCacheIsPendingRelease(false)
     , m_isTargetItem(false)
     , m_alwaysAttemptToUsePageCache(false)
@@ -67 +69 @@
     , m_title(title)
     , m_lastVisitedTime(0)
+    , m_pageCacheIsPendingRelease(false)
     , m_isTargetItem(false)
     , m_alwaysAttemptToUsePageCache(false)
@@ -81 +84 @@
     , m_title(title)
     , m_lastVisitedTime(0)
+    , m_pageCacheIsPendingRelease(false)
     , m_isTargetItem(false)
     , m_alwaysAttemptToUsePageCache(false)
@@ -104 +108 @@
     , m_scrollPoint(item.m_scrollPoint)
     , m_subItems(item.m_subItems.size())
+    , m_pageCacheIsPendingRelease(false)
     , m_isTargetItem(item.m_isTargetItem)
     , m_alwaysAttemptToUsePageCache(item.m_alwaysAttemptToUsePageCache)
@@ -128 +133 @@
     LOG(PageCache, "WebCorePageCache - HistoryItem %p setting has page cache to %s", this, hasCache ? "TRUE" : "FALSE" );
         
-    if (hasCache && !m_pageCache)
-        m_pageCache = new PageCache;
-    if (!hasCache && m_pageCache)
+    if (hasCache) {
+        if (!m_pageCache)
+            m_pageCache = new PageCache;
+        else if (m_pageCacheIsPendingRelease)
+            cancelRelease();
+    } else if (m_pageCache)
         scheduleRelease();
 }
@@ -354 +362 @@
 PageCache* HistoryItem::pageCache()
 {
+    if (m_pageCacheIsPendingRelease)
+        return 0;
     return m_pageCache.get();
 }
@@ -427 +437 @@
 }
 
-static Vector<RefPtr<PageCache> >& pendingPageCacheToRelease()
+static HashSet<RefPtr<HistoryItem> >& itemsWithPendingPageCacheToRelease()
 {
     // We keep this on the heap because otherwise, at app shutdown, we run into the "static destruction order fiasco" 
     // where the vector is torn down, the PageCaches destroyed, and all havok may break loose.  Instead, we just leak at shutdown
     // since nothing here persists
-    static Vector<RefPtr<PageCache> >* pendingPageCacheToRelease = new Vector<RefPtr<PageCache> >;
-    return *pendingPageCacheToRelease;
-}
-
-void HistoryItem::releasePageCache()
+    static HashSet<RefPtr<HistoryItem> >* itemsWithPendingPageCacheToRelease = new HashSet<RefPtr<HistoryItem> >;
+    return *itemsWithPendingPageCacheToRelease;
+}
+
+void HistoryItem::releasePageCachesOrReschedule()
 {
     double loadDelta = currentTime() - FrameLoader::timeOfLastCompletedLoad();
@@ -443 +453 @@
     // FIXME: This size of 42 pending caches to release seems awfully arbitrary
     // Wonder if anyone knows the rationalization
-    if ((userDelta < 0.5 || loadDelta < 1.25) && pendingPageCacheToRelease().size() < 42) {
-        LOG(PageCache, "WebCorePageCache: Postponing releasePageCache() - %f since last load, %f since last input, %i objects pending release", loadDelta, userDelta, pendingPageCacheToRelease().size());
+    if ((userDelta < 0.5 || loadDelta < 1.25) && itemsWithPendingPageCacheToRelease().size() < 42) {
+        LOG(PageCache, "WebCorePageCache: Postponing releasePageCachesOrReschedule() - %f since last load, %f since last input, %i objects pending release", loadDelta, userDelta, itemsWithPendingPageCacheToRelease().size());
         timer().schedule();
         return;
     }
 
-    LOG(PageCache, "WebCorePageCache: Releasing page caches - %f seconds since last load, %f since last input, %i objects pending release", loadDelta, userDelta, pendingPageCacheToRelease().size());
+    LOG(PageCache, "WebCorePageCache: Releasing page caches - %f seconds since last load, %f since last input, %i objects pending release", loadDelta, userDelta, itemsWithPendingPageCacheToRelease().size());
     releaseAllPendingPageCaches();
 }
 
-void closeObjectsInPendingPageCaches()
-{
-    size_t size = pendingPageCacheToRelease().size();
-    for (size_t i = 0; i < size; ++i)
-        pendingPageCacheToRelease()[i]->close();
+void HistoryItem::releasePageCache()
+{
+    m_pageCache->close();
+    m_pageCache = 0;
+    m_pageCacheIsPendingRelease = false;
 }
 
@@ -463 +473 @@
 {
     timer().invalidate();
-    closeObjectsInPendingPageCaches();
-    pendingPageCacheToRelease().clear();
+
+    HashSet<RefPtr<HistoryItem> >::iterator i = itemsWithPendingPageCacheToRelease().begin();
+    HashSet<RefPtr<HistoryItem> >::iterator end = itemsWithPendingPageCacheToRelease().end();
+    for (; i != end; ++i)
+        (*i)->releasePageCache();
+
+    itemsWithPendingPageCacheToRelease().clear();
 }
 
@@ -474 +489 @@
         timer().schedule();
 
-    if (m_pageCache) {
-        pendingPageCacheToRelease().append(m_pageCache);
-        m_pageCache = 0;
-    }
-}
-
+    if (m_pageCache && !m_pageCacheIsPendingRelease) {
+        m_pageCacheIsPendingRelease = true;
+        itemsWithPendingPageCacheToRelease().add(this);
+    }
+}
+
+void HistoryItem::cancelRelease()
+{
+    itemsWithPendingPageCacheToRelease().remove(this);
+    m_pageCacheIsPendingRelease = false;
+}
 #ifndef NDEBUG
 void HistoryItem::print() const

trunk/WebCore/history/HistoryItem.h

@@ -144 +144 @@
 
     void scheduleRelease();
+    void cancelRelease();
+    void releasePageCache();  
     
 #ifndef NDEBUG
@@ -151 +153 @@
 private:
     HistoryItem(const HistoryItem&);
-    static void scheduleReleaseTimer();
-    static void releasePageCache();
+    static void releasePageCachesOrReschedule();
     
     String m_urlString;
@@ -167 +168 @@
     
     HistoryItemVector m_subItems;
+    bool m_pageCacheIsPendingRelease;
     RefPtr<PageCache> m_pageCache;
     

trunk/WebCore/history/HistoryItemTimer.cpp

@@ -34 +34 @@
 
 HistoryItemTimer::HistoryItemTimer()
-    : m_timer(this, &HistoryItemTimer::callReleasePageCache)
+    : m_timer(this, &HistoryItemTimer::callReleasePageCachesOrReschedule)
 {
 }
@@ -53 +53 @@
 }
 
-void HistoryItemTimer::callReleasePageCache(Timer<HistoryItemTimer>*)
+void HistoryItemTimer::callReleasePageCachesOrReschedule(Timer<HistoryItemTimer>*)
 {
-    HistoryItem::releasePageCache();
+    HistoryItem::releasePageCachesOrReschedule();
 }
 

trunk/WebCore/history/HistoryItemTimer.h

@@ -42 +42 @@
 
 private:
-    void callReleasePageCache(Timer<HistoryItemTimer>*);
+    void callReleasePageCachesOrReschedule(Timer<HistoryItemTimer>*);
     Timer<HistoryItemTimer> m_timer;
 };