<!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>[185017] 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/185017">185017</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2015-05-29 16:42:24 -0700 (Fri, 29 May 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>WebContent crash in WebCore::Page::sessionID() const + 0 (Page.cpp:1660)
https://bugs.webkit.org/show_bug.cgi?id=145422
<rdar://problem/20613631>
Reviewed by Brady Eidson.
We sometimes crash when destroying a PageCache CachedFrame because its
DocumentLoader is still loading. This should never happen as we are not
supposed to let pages are still have pending loads into the PageCache.
However, we were using DocumentLoader::isLoadingInAPISense() as check
in PageCache::canCachePageContainingThisFrame() which is not exactly
what we want. isLoadingInAPISense() no longer considers subresource
loads once the frame as loaded. This means if the JS triggers a new
load in a subframe after it has been loaded, then isLoadingInAPISense()
will return false, despite the pending load.
This patch replaces the isLoadingInAPISense() check with isLoading()
as this will consider all pending loads, even after the frame is
loaded.
In most cases, using isLoadingInAPISense() was not an issue because
we call DocumentLoader::stopLoading() in all subframes before starting
a provisional load. However, nothing seems to prevent JS from
triggering a new load after that and before the new load gets committed
(which is when we save the page into PageCache).
No new test as we don't have a reliable reproduction case and the
issue is timing related.
* history/PageCache.cpp:
(WebCore::logCanCacheFrameDecision):
(WebCore::PageCache::canCachePageContainingThisFrame):
* page/DiagnosticLoggingKeys.cpp:
(WebCore::DiagnosticLoggingKeys::isLoading):
(WebCore::DiagnosticLoggingKeys::loadingAPISenseKey): Deleted.
* page/DiagnosticLoggingKeys.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorehistoryPageCachecpp">trunk/Source/WebCore/history/PageCache.cpp</a></li>
<li><a href="#trunkSourceWebCorepageDiagnosticLoggingKeyscpp">trunk/Source/WebCore/page/DiagnosticLoggingKeys.cpp</a></li>
<li><a href="#trunkSourceWebCorepageDiagnosticLoggingKeysh">trunk/Source/WebCore/page/DiagnosticLoggingKeys.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (185016 => 185017)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-05-29 23:20:56 UTC (rev 185016)
+++ trunk/Source/WebCore/ChangeLog 2015-05-29 23:42:24 UTC (rev 185017)
</span><span class="lines">@@ -1,5 +1,45 @@
</span><span class="cx"> 2015-05-29 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><ins>+ WebContent crash in WebCore::Page::sessionID() const + 0 (Page.cpp:1660)
+ https://bugs.webkit.org/show_bug.cgi?id=145422
+ <rdar://problem/20613631>
+
+ Reviewed by Brady Eidson.
+
+ We sometimes crash when destroying a PageCache CachedFrame because its
+ DocumentLoader is still loading. This should never happen as we are not
+ supposed to let pages are still have pending loads into the PageCache.
+
+ However, we were using DocumentLoader::isLoadingInAPISense() as check
+ in PageCache::canCachePageContainingThisFrame() which is not exactly
+ what we want. isLoadingInAPISense() no longer considers subresource
+ loads once the frame as loaded. This means if the JS triggers a new
+ load in a subframe after it has been loaded, then isLoadingInAPISense()
+ will return false, despite the pending load.
+
+ This patch replaces the isLoadingInAPISense() check with isLoading()
+ as this will consider all pending loads, even after the frame is
+ loaded.
+
+ In most cases, using isLoadingInAPISense() was not an issue because
+ we call DocumentLoader::stopLoading() in all subframes before starting
+ a provisional load. However, nothing seems to prevent JS from
+ triggering a new load after that and before the new load gets committed
+ (which is when we save the page into PageCache).
+
+ No new test as we don't have a reliable reproduction case and the
+ issue is timing related.
+
+ * history/PageCache.cpp:
+ (WebCore::logCanCacheFrameDecision):
+ (WebCore::PageCache::canCachePageContainingThisFrame):
+ * page/DiagnosticLoggingKeys.cpp:
+ (WebCore::DiagnosticLoggingKeys::isLoading):
+ (WebCore::DiagnosticLoggingKeys::loadingAPISenseKey): Deleted.
+ * page/DiagnosticLoggingKeys.h:
+
+2015-05-29 Chris Dumez <cdumez@apple.com>
+
</ins><span class="cx"> Consider throttling DOM timers in iframes outside the viewport
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=145465
</span><span class="cx"> <rdar://problem/20768957>
</span></span></pre></div>
<a id="trunkSourceWebCorehistoryPageCachecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/history/PageCache.cpp (185016 => 185017)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/history/PageCache.cpp 2015-05-29 23:20:56 UTC (rev 185016)
+++ trunk/Source/WebCore/history/PageCache.cpp 2015-05-29 23:42:24 UTC (rev 185017)
</span><span class="lines">@@ -86,7 +86,7 @@
</span><span class="cx"> HasSharedWorkers, // FIXME: Remove.
</span><span class="cx"> NoHistoryItem,
</span><span class="cx"> QuickRedirectComing,
</span><del>- IsLoadingInAPISense,
</del><ins>+ IsLoading,
</ins><span class="cx"> IsStopping,
</span><span class="cx"> CannotSuspendActiveDOMObjects,
</span><span class="cx"> DocumentLoaderUsesApplicationCache,
</span><span class="lines">@@ -159,10 +159,10 @@
</span><span class="cx"> logPageCacheFailureDiagnosticMessage(diagnosticLoggingClient, DiagnosticLoggingKeys::quirkRedirectComingKey());
</span><span class="cx"> rejectReasons |= 1 << QuickRedirectComing;
</span><span class="cx"> }
</span><del>- if (frame.loader().documentLoader()->isLoadingInAPISense()) {
- PCLOG(" -DocumentLoader is still loading in API sense");
- logPageCacheFailureDiagnosticMessage(diagnosticLoggingClient, DiagnosticLoggingKeys::loadingAPISenseKey());
- rejectReasons |= 1 << IsLoadingInAPISense;
</del><ins>+ if (frame.loader().documentLoader()->isLoading()) {
+ PCLOG(" -DocumentLoader is still loading");
+ logPageCacheFailureDiagnosticMessage(diagnosticLoggingClient, DiagnosticLoggingKeys::isLoadingKey());
+ rejectReasons |= 1 << IsLoading;
</ins><span class="cx"> }
</span><span class="cx"> if (frame.loader().documentLoader()->isStopping()) {
</span><span class="cx"> PCLOG(" -DocumentLoader is in the middle of stopping");
</span><span class="lines">@@ -308,7 +308,7 @@
</span><span class="cx"> && !(frame.isMainFrame() && document->url().protocolIs("https") && documentLoader->response().cacheControlContainsNoStore())
</span><span class="cx"> && frameLoader.history().currentItem()
</span><span class="cx"> && !frameLoader.quickRedirectComing()
</span><del>- && !documentLoader->isLoadingInAPISense()
</del><ins>+ && !documentLoader->isLoading()
</ins><span class="cx"> && !documentLoader->isStopping()
</span><span class="cx"> && document->canSuspendActiveDOMObjectsForPageCache()
</span><span class="cx"> // FIXME: We should investigating caching frames that have an associated
</span></span></pre></div>
<a id="trunkSourceWebCorepageDiagnosticLoggingKeyscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/DiagnosticLoggingKeys.cpp (185016 => 185017)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/DiagnosticLoggingKeys.cpp 2015-05-29 23:20:56 UTC (rev 185016)
+++ trunk/Source/WebCore/page/DiagnosticLoggingKeys.cpp 2015-05-29 23:42:24 UTC (rev 185017)
</span><span class="lines">@@ -248,9 +248,9 @@
</span><span class="cx"> return ASCIILiteral("reason");
</span><span class="cx"> }
</span><span class="cx">
</span><del>-String DiagnosticLoggingKeys::loadingAPISenseKey()
</del><ins>+String DiagnosticLoggingKeys::isLoadingKey()
</ins><span class="cx"> {
</span><del>- return ASCIILiteral("loadingAPISense");
</del><ins>+ return ASCIILiteral("isLoading");
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> String DiagnosticLoggingKeys::documentLoaderStoppingKey()
</span></span></pre></div>
<a id="trunkSourceWebCorepageDiagnosticLoggingKeysh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/DiagnosticLoggingKeys.h (185016 => 185017)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/DiagnosticLoggingKeys.h 2015-05-29 23:20:56 UTC (rev 185016)
+++ trunk/Source/WebCore/page/DiagnosticLoggingKeys.h 2015-05-29 23:42:24 UTC (rev 185017)
</span><span class="lines">@@ -60,7 +60,7 @@
</span><span class="cx"> WEBCORE_EXPORT static String isReloadIgnoringCacheDataKey();
</span><span class="cx"> static String loadedKey();
</span><span class="cx"> static String loadingKey();
</span><del>- static String loadingAPISenseKey();
</del><ins>+ static String isLoadingKey();
</ins><span class="cx"> static String mainDocumentErrorKey();
</span><span class="cx"> static String mainResourceKey();
</span><span class="cx"> static String mediaKey();
</span></span></pre>
</div>
</div>
</body>
</html>