<!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>[171647] trunk/Source</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/171647">171647</a></dd>
<dt>Author</dt> <dd>timothy_horton@apple.com</dd>
<dt>Date</dt> <dd>2014-07-26 11:45:04 -0700 (Sat, 26 Jul 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>Crash in Web Content Process under ~PDFDocument under clearTouchEventListeners at topDocument()
https://bugs.webkit.org/show_bug.cgi?id=135319
<rdar://problem/17315168>
Reviewed by Darin Adler and Antti Koivisto.
* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::committedLoad):
Allow data through to WebCore for frames with custom content providers;
the only custom content provider currently implemented is main frame PDF
on iOS, which will end up creating a PDFDocument in WebCore, which drops all
data on the floor immediately, so this won't result in WebCore doing anything
with the data, but makes sure that more of the normal document lifecycle is maintained.
In the future, we might want to consider ensuring that all custom content providers
end up creating a SinkDocument or something similarly generic to ensure that
WebCore doesn't try to do anything with their data, but for now, the only client is covered.
* dom/Document.h:
* dom/Document.cpp:
(WebCore::Document::Document):
(WebCore::Document::prepareForDestruction):
Add a flag on Document, m_hasPreparedForDestruction, which ensures
that each Document only goes through prepareForDestruction() once.
prepareForDestruction() can be called a number of times during teardown,
but it's only necessary to actually execute it once.
This was previously achieved by virtue of all callers of prepareForDestruction()
first checking hasLivingRenderTree, and prepareForDestruction() tearing down
the render tree, but that meant that prepareForDestruction() was not called
for Documents who never had a render tree in the first place.
The only part of prepareForDestruction() that is now predicated on hasLivingRenderTree()
is the call to destroyRenderTree(); the rest of the function has the potential to be relevant
for non-rendered placeholder documents and can safely deal with them in other ways.
It is important to call prepareForDestruction() on non-rendered placeholder documents
because some of the cleanup (like disconnectFromFrame()) is critical to safe destruction.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::clear):
Call prepareForDestruction() even if we don't have a living render tree.
For the sake of minimizing change, removeFocusedNodeOfSubtree still
depends on having a living render tree before calling prepareForDestruction().
* page/Frame.cpp:
(WebCore::Frame::setView):
(WebCore::Frame::setDocument):
Call prepareForDestruction() even if we don't have a living render tree.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomDocumentcpp">trunk/Source/WebCore/dom/Document.cpp</a></li>
<li><a href="#trunkSourceWebCoredomDocumenth">trunk/Source/WebCore/dom/Document.h</a></li>
<li><a href="#trunkSourceWebCoreloaderFrameLoadercpp">trunk/Source/WebCore/loader/FrameLoader.cpp</a></li>
<li><a href="#trunkSourceWebCorepageFramecpp">trunk/Source/WebCore/page/Frame.cpp</a></li>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebCoreSupportWebFrameLoaderClientcpp">trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (171646 => 171647)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-07-26 16:44:19 UTC (rev 171646)
+++ trunk/Source/WebCore/ChangeLog 2014-07-26 18:45:04 UTC (rev 171647)
</span><span class="lines">@@ -1,3 +1,43 @@
</span><ins>+2014-07-26 Timothy Horton <timothy_horton@apple.com>
+
+ Crash in Web Content Process under ~PDFDocument under clearTouchEventListeners at topDocument()
+ https://bugs.webkit.org/show_bug.cgi?id=135319
+ <rdar://problem/17315168>
+
+ Reviewed by Darin Adler and Antti Koivisto.
+
+ * dom/Document.h:
+ * dom/Document.cpp:
+ (WebCore::Document::Document):
+ (WebCore::Document::prepareForDestruction):
+ Add a flag on Document, m_hasPreparedForDestruction, which ensures
+ that each Document only goes through prepareForDestruction() once.
+ prepareForDestruction() can be called a number of times during teardown,
+ but it's only necessary to actually execute it once.
+
+ This was previously achieved by virtue of all callers of prepareForDestruction()
+ first checking hasLivingRenderTree, and prepareForDestruction() tearing down
+ the render tree, but that meant that prepareForDestruction() was not called
+ for Documents who never had a render tree in the first place.
+
+ The only part of prepareForDestruction() that is now predicated on hasLivingRenderTree()
+ is the call to destroyRenderTree(); the rest of the function has the potential to be relevant
+ for non-rendered placeholder documents and can safely deal with them in other ways.
+
+ It is important to call prepareForDestruction() on non-rendered placeholder documents
+ because some of the cleanup (like disconnectFromFrame()) is critical to safe destruction.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::clear):
+ Call prepareForDestruction() even if we don't have a living render tree.
+ For the sake of minimizing change, removeFocusedNodeOfSubtree still
+ depends on having a living render tree before calling prepareForDestruction().
+
+ * page/Frame.cpp:
+ (WebCore::Frame::setView):
+ (WebCore::Frame::setDocument):
+ Call prepareForDestruction() even if we don't have a living render tree.
+
</ins><span class="cx"> 2014-07-25 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> Merge r170090, r170092, r170129, r170141, r170161, r170215, r170275, r170375, r170376, r170382, r170383, r170399, r170436, r170489, r170490, r170556 from ftlopt.
</span></span></pre></div>
<a id="trunkSourceWebCoredomDocumentcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Document.cpp (171646 => 171647)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Document.cpp 2014-07-26 16:44:19 UTC (rev 171646)
+++ trunk/Source/WebCore/dom/Document.cpp 2014-07-26 18:45:04 UTC (rev 171647)
</span><span class="lines">@@ -515,6 +515,7 @@
</span><span class="cx"> , m_disabledFieldsetElementsCount(0)
</span><span class="cx"> , m_hasInjectedPlugInsScript(false)
</span><span class="cx"> , m_renderTreeBeingDestroyed(false)
</span><ins>+ , m_hasPreparedForDestruction(false)
</ins><span class="cx"> , m_hasStyleWithViewportUnits(false)
</span><span class="cx"> {
</span><span class="cx"> allDocuments().add(this);
</span><span class="lines">@@ -2047,6 +2048,9 @@
</span><span class="cx">
</span><span class="cx"> void Document::prepareForDestruction()
</span><span class="cx"> {
</span><ins>+ if (m_hasPreparedForDestruction)
+ return;
+
</ins><span class="cx"> #if ENABLE(TOUCH_EVENTS) && PLATFORM(IOS)
</span><span class="cx"> clearTouchEventListeners();
</span><span class="cx"> #endif
</span><span class="lines">@@ -2055,7 +2059,8 @@
</span><span class="cx"> if (m_domWindow && m_frame)
</span><span class="cx"> m_domWindow->willDetachDocumentFromFrame();
</span><span class="cx">
</span><del>- destroyRenderTree();
</del><ins>+ if (hasLivingRenderTree())
+ destroyRenderTree();
</ins><span class="cx">
</span><span class="cx"> if (isPluginDocument())
</span><span class="cx"> toPluginDocument(this)->detachFromPluginElement();
</span><span class="lines">@@ -2087,6 +2092,8 @@
</span><span class="cx"> m_mediaQueryMatcher->documentDestroyed();
</span><span class="cx">
</span><span class="cx"> disconnectFromFrame();
</span><ins>+
+ m_hasPreparedForDestruction = true;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void Document::removeAllEventListeners()
</span></span></pre></div>
<a id="trunkSourceWebCoredomDocumenth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Document.h (171646 => 171647)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Document.h 2014-07-26 16:44:19 UTC (rev 171646)
+++ trunk/Source/WebCore/dom/Document.h 2014-07-26 18:45:04 UTC (rev 171647)
</span><span class="lines">@@ -1695,6 +1695,7 @@
</span><span class="cx">
</span><span class="cx"> bool m_hasInjectedPlugInsScript;
</span><span class="cx"> bool m_renderTreeBeingDestroyed;
</span><ins>+ bool m_hasPreparedForDestruction;
</ins><span class="cx">
</span><span class="cx"> bool m_hasStyleWithViewportUnits;
</span><span class="cx"> };
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderFrameLoadercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (171646 => 171647)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/FrameLoader.cpp 2014-07-26 16:44:19 UTC (rev 171646)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2014-07-26 18:45:04 UTC (rev 171647)
</span><span class="lines">@@ -613,10 +613,10 @@
</span><span class="cx"> if (!m_frame.document()->inPageCache()) {
</span><span class="cx"> m_frame.document()->cancelParsing();
</span><span class="cx"> m_frame.document()->stopActiveDOMObjects();
</span><del>- if (m_frame.document()->hasLivingRenderTree()) {
- m_frame.document()->prepareForDestruction();
</del><ins>+ bool hadLivingRenderTree = m_frame.document()->hasLivingRenderTree();
+ m_frame.document()->prepareForDestruction();
+ if (hadLivingRenderTree)
</ins><span class="cx"> m_frame.document()->removeFocusedNodeOfSubtree(m_frame.document());
</span><del>- }
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // Do this after detaching the document so that the unload event works.
</span></span></pre></div>
<a id="trunkSourceWebCorepageFramecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/Frame.cpp (171646 => 171647)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/Frame.cpp 2014-07-26 16:44:19 UTC (rev 171646)
+++ trunk/Source/WebCore/page/Frame.cpp 2014-07-26 18:45:04 UTC (rev 171647)
</span><span class="lines">@@ -251,7 +251,7 @@
</span><span class="cx"> // Prepare for destruction now, so any unload event handlers get run and the DOMWindow is
</span><span class="cx"> // notified. If we wait until the view is destroyed, then things won't be hooked up enough for
</span><span class="cx"> // these calls to work.
</span><del>- if (!view && m_doc && m_doc->hasLivingRenderTree() && !m_doc->inPageCache())
</del><ins>+ if (!view && m_doc && !m_doc->inPageCache())
</ins><span class="cx"> m_doc->prepareForDestruction();
</span><span class="cx">
</span><span class="cx"> if (m_view)
</span><span class="lines">@@ -271,7 +271,7 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(!newDocument || newDocument->frame() == this);
</span><span class="cx">
</span><del>- if (m_doc && m_doc->hasLivingRenderTree() && !m_doc->inPageCache())
</del><ins>+ if (m_doc && !m_doc->inPageCache())
</ins><span class="cx"> m_doc->prepareForDestruction();
</span><span class="cx">
</span><span class="cx"> m_doc = newDocument.get();
</span></span></pre></div>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (171646 => 171647)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2014-07-26 16:44:19 UTC (rev 171646)
+++ trunk/Source/WebKit2/ChangeLog 2014-07-26 18:45:04 UTC (rev 171647)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2014-07-26 Timothy Horton <timothy_horton@apple.com>
+
+ Crash in Web Content Process under ~PDFDocument under clearTouchEventListeners at topDocument()
+ https://bugs.webkit.org/show_bug.cgi?id=135319
+ <rdar://problem/17315168>
+
+ Reviewed by Darin Adler and Antti Koivisto.
+
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+ (WebKit::WebFrameLoaderClient::committedLoad):
+ Allow data through to WebCore for frames with custom content providers;
+ the only custom content provider currently implemented is main frame PDF
+ on iOS, which will end up creating a PDFDocument in WebCore, which drops all
+ data on the floor immediately, so this won't result in WebCore doing anything
+ with the data, but makes sure that more of the normal document lifecycle is maintained.
+
+ In the future, we might want to consider ensuring that all custom content providers
+ end up creating a SinkDocument or something similarly generic to ensure that
+ WebCore doesn't try to do anything with their data, but for now, the only client is covered.
+
</ins><span class="cx"> 2014-07-25 Jeremy Jones <jeremyj@apple.com>
</span><span class="cx">
</span><span class="cx"> Parent fullscreen from window instead of view
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebCoreSupportWebFrameLoaderClientcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (171646 => 171647)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2014-07-26 16:44:19 UTC (rev 171646)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2014-07-26 18:45:04 UTC (rev 171647)
</span><span class="lines">@@ -887,10 +887,6 @@
</span><span class="cx">
</span><span class="cx"> void WebFrameLoaderClient::committedLoad(DocumentLoader* loader, const char* data, int length)
</span><span class="cx"> {
</span><del>- // If we're loading a custom representation, we don't want to hand off the data to WebCore.
- if (m_frameHasCustomContentProvider)
- return;
-
</del><span class="cx"> if (!m_pluginView)
</span><span class="cx"> loader->commitData(data, length);
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>