<!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>[243471] 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/243471">243471</a></dd>
<dt>Author</dt> <dd>aestes@apple.com</dd>
<dt>Date</dt> <dd>2019-03-25 16:30:47 -0700 (Mon, 25 Mar 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>[iOS] Break a reference cycle between PreviewLoader and ResourceLoader
https://bugs.webkit.org/show_bug.cgi?id=194964
<rdar://problem/48279441>

Reviewed by Alex Christensen.

Source/WebCore:

When a document's QuickLook preview is loaded, a reference cycle is created between
WebPreviewLoader and ResourceLoader. Break the cycle by changing WebPreviewLoader to hold a
WeakPtr to its ResourceLoader ResourceLoader::releaseResources().

Fixes leaks detected by `run-webkit-tests --leaks LayoutTests/quicklook`. Also covered by
existing API tests.

* loader/ResourceLoader.h:
* loader/ios/PreviewLoader.mm:
(-[WebPreviewLoader initWithResourceLoader:resourceResponse:]):
(-[WebPreviewLoader _sendDidReceiveResponseIfNecessary]):
(-[WebPreviewLoader connection:didReceiveData:lengthReceived:]):
(-[WebPreviewLoader connectionDidFinishLoading:]):
(-[WebPreviewLoader connection:didFailWithError:]):

Source/WebKitLegacy/mac:

The WebDataSource._quickLookContent SPI accidentally relied on PreviewLoaders being leaked
to keep the temporary file referenced by WebQuickLookFileNameKey in existence. Since
PreviewLoaders are now being deleted properly, we teach WebDataSource to keep the
PreviewLoaderClient alive for its lifetime. This ensures that as long as
WebDataSource._quickLookContent can be called, the associated temp file will not be deleted
by WebKit.

* WebCoreSupport/WebFrameLoaderClient.mm:
(WebFrameLoaderClient::createPreviewLoaderClient):
* WebView/WebDataSource.mm:
(-[WebDataSource _quickLookPreviewLoaderClient]):
(-[WebDataSource _setQuickLookPreviewLoaderClient:]):
* WebView/WebDataSourceInternal.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreloaderResourceLoaderh">trunk/Source/WebCore/loader/ResourceLoader.h</a></li>
<li><a href="#trunkSourceWebCoreloaderiosPreviewLoadermm">trunk/Source/WebCore/loader/ios/PreviewLoader.mm</a></li>
<li><a href="#trunkSourceWebKitLegacymacChangeLog">trunk/Source/WebKitLegacy/mac/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitLegacymacWebCoreSupportWebFrameLoaderClientmm">trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm</a></li>
<li><a href="#trunkSourceWebKitLegacymacWebViewWebDataSourcemm">trunk/Source/WebKitLegacy/mac/WebView/WebDataSource.mm</a></li>
<li><a href="#trunkSourceWebKitLegacymacWebViewWebDataSourceInternalh">trunk/Source/WebKitLegacy/mac/WebView/WebDataSourceInternal.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (243470 => 243471)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2019-03-25 23:18:48 UTC (rev 243470)
+++ trunk/Source/WebCore/ChangeLog      2019-03-25 23:30:47 UTC (rev 243471)
</span><span class="lines">@@ -1,3 +1,26 @@
</span><ins>+2019-03-25  Andy Estes  <aestes@apple.com>
+
+        [iOS] Break a reference cycle between PreviewLoader and ResourceLoader
+        https://bugs.webkit.org/show_bug.cgi?id=194964
+        <rdar://problem/48279441>
+
+        Reviewed by Alex Christensen.
+
+        When a document's QuickLook preview is loaded, a reference cycle is created between
+        WebPreviewLoader and ResourceLoader. Break the cycle by changing WebPreviewLoader to hold a
+        WeakPtr to its ResourceLoader ResourceLoader::releaseResources().
+
+        Fixes leaks detected by `run-webkit-tests --leaks LayoutTests/quicklook`. Also covered by
+        existing API tests.
+
+        * loader/ResourceLoader.h:
+        * loader/ios/PreviewLoader.mm:
+        (-[WebPreviewLoader initWithResourceLoader:resourceResponse:]):
+        (-[WebPreviewLoader _sendDidReceiveResponseIfNecessary]):
+        (-[WebPreviewLoader connection:didReceiveData:lengthReceived:]):
+        (-[WebPreviewLoader connectionDidFinishLoading:]):
+        (-[WebPreviewLoader connection:didFailWithError:]):
+
</ins><span class="cx"> 2019-03-25  Alex Christensen  <achristensen@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         Enable IPC sending and receiving non-default-constructible types
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderResourceLoaderh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/ResourceLoader.h (243470 => 243471)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/ResourceLoader.h     2019-03-25 23:18:48 UTC (rev 243470)
+++ trunk/Source/WebCore/loader/ResourceLoader.h        2019-03-25 23:30:47 UTC (rev 243471)
</span><span class="lines">@@ -53,7 +53,7 @@
</span><span class="cx"> class NetworkLoadMetrics;
</span><span class="cx"> class PreviewLoader;
</span><span class="cx"> 
</span><del>-class ResourceLoader : public RefCounted<ResourceLoader>, protected ResourceHandleClient {
</del><ins>+class ResourceLoader : public CanMakeWeakPtr<ResourceLoader>, public RefCounted<ResourceLoader>, protected ResourceHandleClient {
</ins><span class="cx"> public:
</span><span class="cx">     virtual ~ResourceLoader() = 0;
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderiosPreviewLoadermm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/ios/PreviewLoader.mm (243470 => 243471)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/ios/PreviewLoader.mm 2019-03-25 23:18:48 UTC (rev 243470)
+++ trunk/Source/WebCore/loader/ios/PreviewLoader.mm    2019-03-25 23:30:47 UTC (rev 243471)
</span><span class="lines">@@ -42,7 +42,7 @@
</span><span class="cx"> using namespace WebCore;
</span><span class="cx"> 
</span><span class="cx"> @interface WebPreviewLoader : NSObject {
</span><del>-    RefPtr<ResourceLoader> _resourceLoader;
</del><ins>+    WeakPtr<ResourceLoader> _resourceLoader;
</ins><span class="cx">     ResourceResponse _response;
</span><span class="cx">     RefPtr<PreviewLoaderClient> _client;
</span><span class="cx">     std::unique_ptr<PreviewConverter> _converter;
</span><span class="lines">@@ -81,7 +81,7 @@
</span><span class="cx">     if (!self)
</span><span class="cx">         return nil;
</span><span class="cx"> 
</span><del>-    _resourceLoader = &resourceLoader;
</del><ins>+    _resourceLoader = makeWeakPtr(resourceLoader);
</ins><span class="cx">     _response = resourceResponse;
</span><span class="cx">     _converter = std::make_unique<PreviewConverter>(self, _response);
</span><span class="cx">     _bufferedDataArray = adoptNS([[NSMutableArray alloc] init]);
</span><span class="lines">@@ -121,6 +121,9 @@
</span><span class="cx"> 
</span><span class="cx"> - (void)_sendDidReceiveResponseIfNecessary
</span><span class="cx"> {
</span><ins>+    if (!_resourceLoader)
+        return;
+
</ins><span class="cx">     ASSERT(!_resourceLoader->reachedTerminalState());
</span><span class="cx">     if (_hasSentDidReceiveResponse)
</span><span class="cx">         return;
</span><span class="lines">@@ -138,6 +141,9 @@
</span><span class="cx">     _resourceLoader->didReceiveResponse(response, [self, retainedSelf = retainPtr(self)] {
</span><span class="cx">         _hasProcessedResponse = YES;
</span><span class="cx"> 
</span><ins>+        if (!_resourceLoader)
+            return;
+
</ins><span class="cx">         if (_resourceLoader->reachedTerminalState())
</span><span class="cx">             return;
</span><span class="cx"> 
</span><span class="lines">@@ -158,6 +164,9 @@
</span><span class="cx"> 
</span><span class="cx"> - (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data lengthReceived:(long long)lengthReceived
</span><span class="cx"> {
</span><ins>+    if (!_resourceLoader)
+        return;
+
</ins><span class="cx">     ASSERT_UNUSED(connection, !connection);
</span><span class="cx">     if (_resourceLoader->reachedTerminalState())
</span><span class="cx">         return;
</span><span class="lines">@@ -185,6 +194,9 @@
</span><span class="cx"> 
</span><span class="cx"> - (void)connectionDidFinishLoading:(NSURLConnection *)connection
</span><span class="cx"> {
</span><ins>+    if (!_resourceLoader)
+        return;
+
</ins><span class="cx">     ASSERT_UNUSED(connection, !connection);
</span><span class="cx">     if (_resourceLoader->reachedTerminalState())
</span><span class="cx">         return;
</span><span class="lines">@@ -206,6 +218,9 @@
</span><span class="cx"> 
</span><span class="cx"> - (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error
</span><span class="cx"> {
</span><ins>+    if (!_resourceLoader)
+        return;
+
</ins><span class="cx">     ASSERT_UNUSED(connection, !connection);
</span><span class="cx">     if (_resourceLoader->reachedTerminalState())
</span><span class="cx">         return;
</span></span></pre></div>
<a id="trunkSourceWebKitLegacymacChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKitLegacy/mac/ChangeLog (243470 => 243471)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKitLegacy/mac/ChangeLog  2019-03-25 23:18:48 UTC (rev 243470)
+++ trunk/Source/WebKitLegacy/mac/ChangeLog     2019-03-25 23:30:47 UTC (rev 243471)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2019-03-25  Andy Estes  <aestes@apple.com>
+
+        [iOS] Break a reference cycle between PreviewLoader and ResourceLoader
+        https://bugs.webkit.org/show_bug.cgi?id=194964
+        <rdar://problem/48279441>
+
+        Reviewed by Alex Christensen.
+
+        The WebDataSource._quickLookContent SPI accidentally relied on PreviewLoaders being leaked
+        to keep the temporary file referenced by WebQuickLookFileNameKey in existence. Since
+        PreviewLoaders are now being deleted properly, we teach WebDataSource to keep the
+        PreviewLoaderClient alive for its lifetime. This ensures that as long as
+        WebDataSource._quickLookContent can be called, the associated temp file will not be deleted
+        by WebKit.
+
+        * WebCoreSupport/WebFrameLoaderClient.mm:
+        (WebFrameLoaderClient::createPreviewLoaderClient):
+        * WebView/WebDataSource.mm:
+        (-[WebDataSource _quickLookPreviewLoaderClient]):
+        (-[WebDataSource _setQuickLookPreviewLoaderClient:]):
+        * WebView/WebDataSourceInternal.h:
+
</ins><span class="cx"> 2019-03-25  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         Remove NavigatorContentUtils in WebCore/Modules
</span></span></pre></div>
<a id="trunkSourceWebKitLegacymacWebCoreSupportWebFrameLoaderClientmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm (243470 => 243471)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm     2019-03-25 23:18:48 UTC (rev 243470)
+++ trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm        2019-03-25 23:30:47 UTC (rev 243471)
</span><span class="lines">@@ -2243,8 +2243,11 @@
</span><span class="cx">     if (!filePath)
</span><span class="cx">         return nullptr;
</span><span class="cx"> 
</span><ins>+    auto documentWriter = adoptRef(*new QuickLookDocumentWriter(filePath));
+
</ins><span class="cx">     [m_webFrame provisionalDataSource]._quickLookContent = @{ WebQuickLookFileNameKey : filePath, WebQuickLookUTIKey : uti };
</span><del>-    return adoptRef(*new QuickLookDocumentWriter(filePath));
</del><ins>+    [m_webFrame provisionalDataSource]._quickLookPreviewLoaderClient = documentWriter.ptr();
+    return documentWriter;
</ins><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebKitLegacymacWebViewWebDataSourcemm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKitLegacy/mac/WebView/WebDataSource.mm (243470 => 243471)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKitLegacy/mac/WebView/WebDataSource.mm   2019-03-25 23:18:48 UTC (rev 243470)
+++ trunk/Source/WebKitLegacy/mac/WebView/WebDataSource.mm      2019-03-25 23:30:47 UTC (rev 243471)
</span><span class="lines">@@ -53,6 +53,7 @@
</span><span class="cx"> #import <WebCore/FrameLoader.h>
</span><span class="cx"> #import <WebCore/LegacyWebArchive.h>
</span><span class="cx"> #import <WebCore/MIMETypeRegistry.h>
</span><ins>+#import <WebCore/PreviewLoaderClient.h>
</ins><span class="cx"> #import <WebCore/ResourceRequest.h>
</span><span class="cx"> #import <WebCore/SharedBuffer.h>
</span><span class="cx"> #import <WebCore/WebCoreObjCExtras.h>
</span><span class="lines">@@ -108,6 +109,7 @@
</span><span class="cx"> #endif
</span><span class="cx"> #if USE(QUICK_LOOK)
</span><span class="cx">     RetainPtr<NSDictionary> _quickLookContent;
</span><ins>+    RefPtr<WebCore::PreviewLoaderClient> _quickLookPreviewLoaderClient;
</ins><span class="cx"> #endif
</span><span class="cx"> };
</span><span class="cx"> 
</span><span class="lines">@@ -417,10 +419,20 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> #if USE(QUICK_LOOK)
</span><ins>+- (WebCore::PreviewLoaderClient*)_quickLookPreviewLoaderClient
+{
+    return toPrivate(_private)->_quickLookPreviewLoaderClient.get();
+}
+
</ins><span class="cx"> - (void)_setQuickLookContent:(NSDictionary *)quickLookContent
</span><span class="cx"> {
</span><span class="cx">     toPrivate(_private)->_quickLookContent = adoptNS([quickLookContent copy]);
</span><span class="cx"> }
</span><ins>+
+- (void)_setQuickLookPreviewLoaderClient:(WebCore::PreviewLoaderClient*)quickLookPreviewLoaderClient
+{
+    toPrivate(_private)->_quickLookPreviewLoaderClient = quickLookPreviewLoaderClient;
+}
</ins><span class="cx"> #endif
</span><span class="cx"> 
</span><span class="cx"> @end
</span></span></pre></div>
<a id="trunkSourceWebKitLegacymacWebViewWebDataSourceInternalh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKitLegacy/mac/WebView/WebDataSourceInternal.h (243470 => 243471)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKitLegacy/mac/WebView/WebDataSourceInternal.h    2019-03-25 23:18:48 UTC (rev 243470)
+++ trunk/Source/WebKitLegacy/mac/WebView/WebDataSourceInternal.h       2019-03-25 23:30:47 UTC (rev 243471)
</span><span class="lines">@@ -30,7 +30,8 @@
</span><span class="cx"> #import <wtf/Forward.h>
</span><span class="cx"> 
</span><span class="cx"> namespace WebCore {
</span><del>-    class DocumentLoader;
</del><ins>+class DocumentLoader;
+class PreviewLoaderClient;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> class WebDocumentLoaderMac;
</span><span class="lines">@@ -59,5 +60,6 @@
</span><span class="cx"> - (WebCore::DocumentLoader*)_documentLoader;
</span><span class="cx"> #if USE(QUICK_LOOK)
</span><span class="cx"> @property (nonatomic, copy, setter=_setQuickLookContent:) NSDictionary *_quickLookContent;
</span><ins>+@property (nonatomic, setter=_setQuickLookPreviewLoaderClient:) WebCore::PreviewLoaderClient* _quickLookPreviewLoaderClient;
</ins><span class="cx"> #endif
</span><span class="cx"> @end
</span></span></pre>
</div>
</div>

</body>
</html>