<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[195605] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/195605">195605</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2016-01-26 11:57:49 -0800 (Tue, 26 Jan 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Make sure a page is still PageCache-able after firing the 'pagehide' events
https://bugs.webkit.org/show_bug.cgi?id=153449
Reviewed by Andreas Kling.
Make sure a page is still PageCache-able after firing the 'pagehide'
events and abort if it isn't. This should improve robustness and it is
easy for pagehide event handlers to do things that would make a Page no
longer PageCache-able and this leads to bugs that are difficult to
investigate.
To achieve this, the 'pagehide' event firing logic was moved out of the
CachedFrame constructor. It now happens earlier in
PageCache::addIfCacheable() after checking if the page is cacheable and
before constructing the CachedPage / CachedFrames. After firing the
'pagehide' event in PageCache::addIfCacheable(), we check again that
the page is still cacheable and we abort early if it is not.
* history/CachedFrame.cpp:
(WebCore::CachedFrame::CachedFrame):
* history/PageCache.cpp:
(WebCore::setInPageCache):
(WebCore::firePageHideEventRecursively):
(WebCore::PageCache::addIfCacheable):
* history/PageCache.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::commitProvisionalLoad):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorehistoryCachedFramecpp">trunk/Source/WebCore/history/CachedFrame.cpp</a></li>
<li><a href="#trunkSourceWebCorehistoryPageCachecpp">trunk/Source/WebCore/history/PageCache.cpp</a></li>
<li><a href="#trunkSourceWebCorehistoryPageCacheh">trunk/Source/WebCore/history/PageCache.h</a></li>
<li><a href="#trunkSourceWebCoreloaderFrameLoadercpp">trunk/Source/WebCore/loader/FrameLoader.cpp</a></li>
<li><a href="#trunkSourceWebCorepagePagecpp">trunk/Source/WebCore/page/Page.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (195604 => 195605)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-01-26 19:33:38 UTC (rev 195604)
+++ trunk/Source/WebCore/ChangeLog 2016-01-26 19:57:49 UTC (rev 195605)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2016-01-26 Chris Dumez <cdumez@apple.com>
+
+ Make sure a page is still PageCache-able after firing the 'pagehide' events
+ https://bugs.webkit.org/show_bug.cgi?id=153449
+
+ Reviewed by Andreas Kling.
+
+ Make sure a page is still PageCache-able after firing the 'pagehide'
+ events and abort if it isn't. This should improve robustness and it is
+ easy for pagehide event handlers to do things that would make a Page no
+ longer PageCache-able and this leads to bugs that are difficult to
+ investigate.
+
+ To achieve this, the 'pagehide' event firing logic was moved out of the
+ CachedFrame constructor. It now happens earlier in
+ PageCache::addIfCacheable() after checking if the page is cacheable and
+ before constructing the CachedPage / CachedFrames. After firing the
+ 'pagehide' event in PageCache::addIfCacheable(), we check again that
+ the page is still cacheable and we abort early if it is not.
+
+ * history/CachedFrame.cpp:
+ (WebCore::CachedFrame::CachedFrame):
+ * history/PageCache.cpp:
+ (WebCore::setInPageCache):
+ (WebCore::firePageHideEventRecursively):
+ (WebCore::PageCache::addIfCacheable):
+ * history/PageCache.h:
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::commitProvisionalLoad):
+
</ins><span class="cx"> 2016-01-26 Beth Dakin <bdakin@apple.com>
</span><span class="cx">
</span><span class="cx"> Rubber-stamped by Tim Horton.
</span></span></pre></div>
<a id="trunkSourceWebCorehistoryCachedFramecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/history/CachedFrame.cpp (195604 => 195605)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/history/CachedFrame.cpp 2016-01-26 19:33:38 UTC (rev 195604)
+++ trunk/Source/WebCore/history/CachedFrame.cpp 2016-01-26 19:57:49 UTC (rev 195605)
</span><span class="lines">@@ -39,7 +39,6 @@
</span><span class="cx"> #include "FrameView.h"
</span><span class="cx"> #include "HistoryController.h"
</span><span class="cx"> #include "HistoryItem.h"
</span><del>-#include "IgnoreOpensDuringUnloadCountIncrementer.h"
</del><span class="cx"> #include "Logging.h"
</span><span class="cx"> #include "MainFrame.h"
</span><span class="cx"> #include "Page.h"
</span><span class="lines">@@ -155,26 +154,13 @@
</span><span class="cx"> // Custom scrollbar renderers will get reattached when the document comes out of the page cache
</span><span class="cx"> m_view->detachCustomScrollbars();
</span><span class="cx">
</span><del>- m_document->setInPageCache(true);
- frame.loader().stopLoading(UnloadEventPolicyUnloadAndPageHide);
</del><ins>+ ASSERT(m_document->inPageCache());
</ins><span class="cx">
</span><del>- {
- // The following will fire the pagehide event in each subframe and the HTML specification states
- // that the parent document's ignore-opens-during-unload counter should be incremented while the
- // pagehide event is being fired in its subframes:
- // https://html.spec.whatwg.org/multipage/browsers.html#unload-a-document
- IgnoreOpensDuringUnloadCountIncrementer ignoreOpensDuringUnloadCountIncrementer(m_document.get());
</del><ins>+ // Create the CachedFrames for all Frames in the FrameTree.
+ for (Frame* child = frame.tree().firstChild(); child; child = child->tree().nextSibling())
+ m_childFrames.append(std::make_unique<CachedFrame>(*child));
</ins><span class="cx">
</span><del>- // Create the CachedFrames for all Frames in the FrameTree.
- for (Frame* child = frame.tree().firstChild(); child; child = child->tree().nextSibling())
- m_childFrames.append(std::make_unique<CachedFrame>(*child));
- }
-
- // Active DOM objects must be suspended before we cache the frame script data,
- // but after we've fired the pagehide event, in case that creates more objects.
- // Suspending must also happen after we've recursed over child frames, in case
- // those create more objects.
-
</del><ins>+ // Active DOM objects must be suspended before we cache the frame script data.
</ins><span class="cx"> m_document->suspend();
</span><span class="cx">
</span><span class="cx"> m_cachedFrameScriptData = std::make_unique<ScriptCachedFrameData>(frame);
</span></span></pre></div>
<a id="trunkSourceWebCorehistoryPageCachecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/history/PageCache.cpp (195604 => 195605)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/history/PageCache.cpp 2016-01-26 19:33:38 UTC (rev 195604)
+++ trunk/Source/WebCore/history/PageCache.cpp 2016-01-26 19:57:49 UTC (rev 195605)
</span><span class="lines">@@ -41,6 +41,7 @@
</span><span class="cx"> #include "FrameLoaderClient.h"
</span><span class="cx"> #include "FrameView.h"
</span><span class="cx"> #include "HistoryController.h"
</span><ins>+#include "IgnoreOpensDuringUnloadCountIncrementer.h"
</ins><span class="cx"> #include "Logging.h"
</span><span class="cx"> #include "MainFrame.h"
</span><span class="cx"> #include "MemoryPressureHandler.h"
</span><span class="lines">@@ -272,22 +273,19 @@
</span><span class="cx"> return globalPageCache;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool PageCache::canCache(Page* page) const
</del><ins>+bool PageCache::canCache(Page& page) const
</ins><span class="cx"> {
</span><del>- if (!page)
- return false;
-
</del><span class="cx"> if (!m_maxSize) {
</span><del>- logPageCacheFailureDiagnosticMessage(page, DiagnosticLoggingKeys::isDisabledKey());
</del><ins>+ logPageCacheFailureDiagnosticMessage(&page, DiagnosticLoggingKeys::isDisabledKey());
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
</span><del>- logPageCacheFailureDiagnosticMessage(page, DiagnosticLoggingKeys::underMemoryPressureKey());
</del><ins>+ logPageCacheFailureDiagnosticMessage(&page, DiagnosticLoggingKeys::underMemoryPressureKey());
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- return canCachePage(*page);
</del><ins>+ return canCachePage(page);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void PageCache::pruneToSizeNow(unsigned size, PruningReason pruningReason)
</span><span class="lines">@@ -374,14 +372,57 @@
</span><span class="cx"> return emptyString();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void PageCache::add(HistoryItem& item, Page& page)
</del><ins>+static void setInPageCache(Page& page, bool isInPageCache)
</ins><span class="cx"> {
</span><del>- ASSERT(canCache(&page));
</del><ins>+ for (Frame* frame = &page.mainFrame(); frame; frame = frame->tree().traverseNext()) {
+ if (auto* document = frame->document())
+ document->setInPageCache(isInPageCache);
+ }
+}
</ins><span class="cx">
</span><del>- // Remove stale cache entry if necessary.
- remove(item);
</del><ins>+static void firePageHideEventRecursively(Frame& frame)
+{
+ auto* document = frame.document();
+ if (!document)
+ return;
</ins><span class="cx">
</span><del>- item.m_cachedPage = std::make_unique<CachedPage>(page);
</del><ins>+ // stopLoading() will fire the pagehide event in each subframe and the HTML specification states
+ // that the parent document's ignore-opens-during-unload counter should be incremented while the
+ // pagehide event is being fired in its subframes:
+ // https://html.spec.whatwg.org/multipage/browsers.html#unload-a-document
+ IgnoreOpensDuringUnloadCountIncrementer ignoreOpensDuringUnloadCountIncrementer(document);
+
+ frame.loader().stopLoading(UnloadEventPolicyUnloadAndPageHide);
+
+ for (RefPtr<Frame> child = frame.tree().firstChild(); child; child = child->tree().nextSibling())
+ firePageHideEventRecursively(*child);
+}
+
+void PageCache::addIfCacheable(HistoryItem& item, Page* page)
+{
+ if (item.isInPageCache())
+ return;
+
+ if (!page || !canCache(*page))
+ return;
+
+ // Make sure all the documents know they are being added to the PageCache.
+ setInPageCache(*page, true);
+
+ // Fire the pagehide event in all frames.
+ firePageHideEventRecursively(page->mainFrame());
+
+ // Check that the page is still page-cacheable after firing the pagehide event. The JS event handlers
+ // could have altered the page in a way that could prevent caching.
+ if (!canCache(*page)) {
+ setInPageCache(*page, false);
+ return;
+ }
+
+ // Make sure we no longer fire any JS events past this point.
+ NoEventDispatchAssertion assertNoEventDispatch;
+
+ item.m_cachedPage = std::make_unique<CachedPage>(*page);
</ins><span class="cx"> item.m_pruningReason = PruningReason::None;
</span><span class="cx"> m_items.add(&item);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorehistoryPageCacheh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/history/PageCache.h (195604 => 195605)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/history/PageCache.h 2016-01-26 19:33:38 UTC (rev 195604)
+++ trunk/Source/WebCore/history/PageCache.h 2016-01-26 19:57:49 UTC (rev 195605)
</span><span class="lines">@@ -46,14 +46,14 @@
</span><span class="cx"> // Function to obtain the global page cache.
</span><span class="cx"> WEBCORE_EXPORT static PageCache& singleton();
</span><span class="cx">
</span><del>- bool canCache(Page*) const;
</del><ins>+ bool canCache(Page&) const;
</ins><span class="cx">
</span><span class="cx"> // Used when memory is low to prune some cached pages.
</span><span class="cx"> WEBCORE_EXPORT void pruneToSizeNow(unsigned maxSize, PruningReason);
</span><span class="cx"> WEBCORE_EXPORT void setMaxSize(unsigned); // number of pages to cache.
</span><span class="cx"> unsigned maxSize() const { return m_maxSize; }
</span><span class="cx">
</span><del>- void add(HistoryItem&, Page&); // Prunes if maxSize() is exceeded.
</del><ins>+ void addIfCacheable(HistoryItem&, Page*); // Prunes if maxSize() is exceeded.
</ins><span class="cx"> WEBCORE_EXPORT void remove(HistoryItem&);
</span><span class="cx"> CachedPage* get(HistoryItem&, Page*);
</span><span class="cx"> std::unique_ptr<CachedPage> take(HistoryItem&, Page*);
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderFrameLoadercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (195604 => 195605)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/FrameLoader.cpp 2016-01-26 19:33:38 UTC (rev 195604)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2016-01-26 19:57:49 UTC (rev 195605)
</span><span class="lines">@@ -1772,9 +1772,8 @@
</span><span class="cx">
</span><span class="cx"> // Check to see if we need to cache the page we are navigating away from into the back/forward cache.
</span><span class="cx"> // We are doing this here because we know for sure that a new page is about to be loaded.
</span><del>- HistoryItem& item = *history().currentItem();
- if (!m_frame.tree().parent() && PageCache::singleton().canCache(m_frame.page()) && !item.isInPageCache())
- PageCache::singleton().add(item, *m_frame.page());
</del><ins>+ if (!m_frame.tree().parent() && history().currentItem())
+ PageCache::singleton().addIfCacheable(*history().currentItem(), m_frame.page());
</ins><span class="cx">
</span><span class="cx"> if (m_loadType != FrameLoadType::Replace)
</span><span class="cx"> closeOldDataSources();
</span></span></pre></div>
<a id="trunkSourceWebCorepagePagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/Page.cpp (195604 => 195605)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/Page.cpp 2016-01-26 19:33:38 UTC (rev 195604)
+++ trunk/Source/WebCore/page/Page.cpp 2016-01-26 19:57:49 UTC (rev 195605)
</span><span class="lines">@@ -1863,7 +1863,7 @@
</span><span class="cx">
</span><span class="cx"> bool Page::canTabSuspend()
</span><span class="cx"> {
</span><del>- return s_tabSuspensionIsEnabled && !m_isPrerender && (m_pageThrottler.activityState() == PageActivityState::NoFlags) && PageCache::singleton().canCache(this);
</del><ins>+ return s_tabSuspensionIsEnabled && !m_isPrerender && (m_pageThrottler.activityState() == PageActivityState::NoFlags) && PageCache::singleton().canCache(*this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void Page::setIsTabSuspended(bool shouldSuspend)
</span></span></pre>
</div>
</div>
</body>
</html>