Diff
Modified: trunk/Source/WebCore/ChangeLog (155149 => 155150)
--- trunk/Source/WebCore/ChangeLog 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/ChangeLog 2013-09-05 21:25:32 UTC (rev 155150)
@@ -1,3 +1,41 @@
+2013-09-05 Andreas Kling <[email protected]>
+
+ Cached Page and Frame don't need to be ref-counted.
+ <https://webkit.org/b/120710>
+
+ Reviewed by Anders Carlsson.
+
+ - CachedPage is owned by HistoryItem.
+ - CachedFrame is owned by CachedPage.
+
+ Remove the ref counting from these objects to make the code less confusing.
+
+ Added a new method:
+
+ - PassOwnPtr<CachedPage> PageCache::take(HistoryItem*)
+
+ ..which is what it looks like - a combined get() and remove() that transfers
+ ownership of the CachedPage to the caller.
+
+ This is used by commitProvisionalLoad() and invalidateCurrentItemCachedPage()
+ to accomplish in one swoop what they used to do in awkwardly spaced steps.
+
+ * history/CachedFrame.h:
+ (WebCore::CachedFrame::create):
+ * history/CachedPage.cpp:
+ (WebCore::CachedPage::create):
+ * history/CachedPage.h:
+ * history/HistoryItem.h:
+ * history/PageCache.cpp:
+ (WebCore::PageCache::take):
+ * history/PageCache.h:
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::commitProvisionalLoad):
+ (WebCore::FrameLoader::transitionToCommitted):
+ * loader/FrameLoader.h:
+ * loader/HistoryController.cpp:
+ (WebCore::HistoryController::invalidateCurrentItemCachedPage):
+
2013-09-05 David Kilzer <[email protected]>
BUILD FIX (r155108): Add SynchronousLoaderClientCFNet.cpp to Xcode project
Modified: trunk/Source/WebCore/history/CachedFrame.h (155149 => 155150)
--- trunk/Source/WebCore/history/CachedFrame.h 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/history/CachedFrame.h 2013-09-05 21:25:32 UTC (rev 155150)
@@ -30,7 +30,6 @@
#include "KURL.h"
#include "ScriptCachedFrameData.h"
#include <wtf/PassOwnPtr.h>
-#include <wtf/RefCounted.h>
#include <wtf/RefPtr.h>
namespace WebCore {
@@ -67,12 +66,12 @@
bool m_isComposited;
#endif
- Vector<RefPtr<CachedFrame>> m_childFrames;
+ Vector<OwnPtr<CachedFrame>> m_childFrames;
};
-class CachedFrame : public RefCounted<CachedFrame>, private CachedFrameBase {
+class CachedFrame : private CachedFrameBase {
public:
- static PassRefPtr<CachedFrame> create(Frame& frame) { return adoptRef(new CachedFrame(frame)); }
+ static PassOwnPtr<CachedFrame> create(Frame& frame) { return adoptPtr(new CachedFrame(frame)); }
void open();
void clear();
Modified: trunk/Source/WebCore/history/CachedPage.cpp (155149 => 155150)
--- trunk/Source/WebCore/history/CachedPage.cpp 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/history/CachedPage.cpp 2013-09-05 21:25:32 UTC (rev 155150)
@@ -45,9 +45,9 @@
DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, cachedPageCounter, ("CachedPage"));
-PassRefPtr<CachedPage> CachedPage::create(Page& page)
+PassOwnPtr<CachedPage> CachedPage::create(Page& page)
{
- return adoptRef(new CachedPage(page));
+ return adoptPtr(new CachedPage(page));
}
CachedPage::CachedPage(Page& page)
Modified: trunk/Source/WebCore/history/CachedPage.h (155149 => 155150)
--- trunk/Source/WebCore/history/CachedPage.h 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/history/CachedPage.h 2013-09-05 21:25:32 UTC (rev 155150)
@@ -27,7 +27,6 @@
#define CachedPage_h
#include "CachedFrame.h"
-#include <wtf/RefCounted.h>
namespace WebCore {
@@ -35,9 +34,9 @@
class DocumentLoader;
class Page;
-class CachedPage : public RefCounted<CachedPage> {
+class CachedPage {
public:
- static PassRefPtr<CachedPage> create(Page&);
+ static PassOwnPtr<CachedPage> create(Page&);
~CachedPage();
void restore(Page&);
@@ -67,7 +66,7 @@
double m_timeStamp;
double m_expirationTime;
- RefPtr<CachedFrame> m_cachedMainFrame;
+ OwnPtr<CachedFrame> m_cachedMainFrame;
bool m_needStyleRecalcForVisitedLinks;
bool m_needsFullStyleRecalc;
bool m_needsCaptionPreferencesChanged;
Modified: trunk/Source/WebCore/history/HistoryItem.h (155149 => 155150)
--- trunk/Source/WebCore/history/HistoryItem.h 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/history/HistoryItem.h 2013-09-05 21:25:32 UTC (rev 155150)
@@ -286,7 +286,7 @@
// PageCache controls these fields.
HistoryItem* m_next;
HistoryItem* m_prev;
- RefPtr<CachedPage> m_cachedPage;
+ OwnPtr<CachedPage> m_cachedPage;
#if PLATFORM(MAC)
RetainPtr<id> m_viewState;
Modified: trunk/Source/WebCore/history/PageCache.cpp (155149 => 155150)
--- trunk/Source/WebCore/history/PageCache.cpp 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/history/PageCache.cpp 2013-09-05 21:25:32 UTC (rev 155150)
@@ -442,6 +442,29 @@
prune();
}
+PassOwnPtr<CachedPage> PageCache::take(HistoryItem* item)
+{
+ if (!item)
+ return nullptr;
+
+ OwnPtr<CachedPage> cachedPage = item->m_cachedPage.release();
+
+ removeFromLRUList(item);
+ --m_size;
+
+ item->deref(); // Balanced in add().
+
+ if (!cachedPage)
+ return nullptr;
+
+ if (cachedPage->hasExpired()) {
+ LOG(PageCache, "Not restoring page for %s from back/forward cache because cache entry has expired", item->url().string().ascii().data());
+ return nullptr;
+ }
+
+ return cachedPage.release();
+}
+
CachedPage* PageCache::get(HistoryItem* item)
{
if (!item)
Modified: trunk/Source/WebCore/history/PageCache.h (155149 => 155150)
--- trunk/Source/WebCore/history/PageCache.h 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/history/PageCache.h 2013-09-05 21:25:32 UTC (rev 155150)
@@ -51,6 +51,7 @@
void add(PassRefPtr<HistoryItem>, Page&); // Prunes if capacity() is exceeded.
void remove(HistoryItem*);
CachedPage* get(HistoryItem* item);
+ PassOwnPtr<CachedPage> take(HistoryItem*);
int pageCount() const { return m_size; }
int frameCount() const;
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (155149 => 155150)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2013-09-05 21:25:32 UTC (rev 155150)
@@ -1699,10 +1699,13 @@
void FrameLoader::commitProvisionalLoad()
{
- RefPtr<CachedPage> cachedPage = m_loadingFromCachedPage ? pageCache()->get(history().provisionalItem()) : 0;
RefPtr<DocumentLoader> pdl = m_provisionalDocumentLoader;
Ref<Frame> protect(m_frame);
+ OwnPtr<CachedPage> cachedPage;
+ if (m_loadingFromCachedPage)
+ cachedPage = pageCache()->take(history().provisionalItem());
+
LOG(PageCache, "WebCoreLoading %s: About to commit provisional load from previous URL '%s' to new URL '%s'", m_frame.tree().uniqueName().string().utf8().data(),
m_frame.document() ? m_frame.document()->url().stringCenterEllipsizedToLength().utf8().data() : "",
pdl ? pdl->url().stringCenterEllipsizedToLength().utf8().data() : "<no provisional DocumentLoader>");
@@ -1721,7 +1724,7 @@
if (!cachedPage && !m_stateMachine.creatingInitialEmptyDocument())
m_client.makeRepresentation(pdl.get());
- transitionToCommitted(cachedPage);
+ transitionToCommitted(cachedPage.get());
if (pdl && m_documentLoader) {
// Check if the destination page is allowed to access the previous page's timing information.
@@ -1738,11 +1741,10 @@
if (cachedPage && cachedPage->document()) {
prepareForCachedPageRestore();
+
+ // FIXME: This API should be turned around so that we ground CachedPage into the Page.
cachedPage->restore(*m_frame.page());
- // The page should be removed from the cache immediately after a restoration in order for the PageCache to be consistent.
- pageCache()->remove(history().currentItem());
-
dispatchDidCommitLoad();
// If we have a title let the WebView know about it.
@@ -1751,11 +1753,8 @@
m_client.dispatchDidReceiveTitle(title);
checkCompleted();
- } else {
- if (cachedPage)
- pageCache()->remove(history().currentItem());
+ } else
didOpenURL();
- }
LOG(Loading, "WebCoreLoading %s: Finished committing provisional load to URL %s", m_frame.tree().uniqueName().string().utf8().data(),
m_frame.document() ? m_frame.document()->url().stringCenterEllipsizedToLength().utf8().data() : "");
@@ -1789,7 +1788,7 @@
}
}
-void FrameLoader::transitionToCommitted(PassRefPtr<CachedPage> cachedPage)
+void FrameLoader::transitionToCommitted(CachedPage* cachedPage)
{
ASSERT(m_client.hasWebView());
ASSERT(m_state == FrameStateProvisional);
Modified: trunk/Source/WebCore/loader/FrameLoader.h (155149 => 155150)
--- trunk/Source/WebCore/loader/FrameLoader.h 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/loader/FrameLoader.h 2013-09-05 21:25:32 UTC (rev 155150)
@@ -308,7 +308,7 @@
void addExtraFieldsToRequest(ResourceRequest&, FrameLoadType, bool isMainResource);
void clearProvisionalLoad();
- void transitionToCommitted(PassRefPtr<CachedPage>);
+ void transitionToCommitted(CachedPage*);
void frameLoadCompleted();
SubstituteData defaultSubstituteDataForURL(const KURL&);
Modified: trunk/Source/WebCore/loader/HistoryController.cpp (155149 => 155150)
--- trunk/Source/WebCore/loader/HistoryController.cpp 2013-09-05 21:19:09 UTC (rev 155149)
+++ trunk/Source/WebCore/loader/HistoryController.cpp 2013-09-05 21:25:32 UTC (rev 155150)
@@ -224,21 +224,21 @@
void HistoryController::invalidateCurrentItemCachedPage()
{
- // When we are pre-commit, the currentItem is where the pageCache data resides
- CachedPage* cachedPage = pageCache()->get(currentItem());
+ // When we are pre-commit, the currentItem is where any page cache data resides.
+ if (!pageCache()->get(currentItem()))
+ return;
+ OwnPtr<CachedPage> cachedPage = pageCache()->take(currentItem());
+
// FIXME: This is a grotesque hack to fix <rdar://problem/4059059> Crash in RenderFlow::detach
// Somehow the PageState object is not properly updated, and is holding onto a stale document.
// Both Xcode and FileMaker see this crash, Safari does not.
- ASSERT(!cachedPage || cachedPage->document() == m_frame.document());
- if (cachedPage && cachedPage->document() == m_frame.document()) {
+ ASSERT(cachedPage->document() == m_frame.document());
+ if (cachedPage->document() == m_frame.document()) {
cachedPage->document()->setInPageCache(false);
cachedPage->clear();
}
-
- if (cachedPage)
- pageCache()->remove(currentItem());
}
bool HistoryController::shouldStopLoadingForHistoryItem(HistoryItem* targetItem) const
