- Revision
- 243471
- Author
- [email protected]
- Date
- 2019-03-25 16:30:47 -0700 (Mon, 25 Mar 2019)
Log Message
[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:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (243470 => 243471)
--- 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)
@@ -1,3 +1,26 @@
+2019-03-25 Andy Estes <[email protected]>
+
+ [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:]):
+
2019-03-25 Alex Christensen <[email protected]>
Enable IPC sending and receiving non-default-constructible types
Modified: trunk/Source/WebCore/loader/ResourceLoader.h (243470 => 243471)
--- 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)
@@ -53,7 +53,7 @@
class NetworkLoadMetrics;
class PreviewLoader;
-class ResourceLoader : public RefCounted<ResourceLoader>, protected ResourceHandleClient {
+class ResourceLoader : public CanMakeWeakPtr<ResourceLoader>, public RefCounted<ResourceLoader>, protected ResourceHandleClient {
public:
virtual ~ResourceLoader() = 0;
Modified: trunk/Source/WebCore/loader/ios/PreviewLoader.mm (243470 => 243471)
--- 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)
@@ -42,7 +42,7 @@
using namespace WebCore;
@interface WebPreviewLoader : NSObject {
- RefPtr<ResourceLoader> _resourceLoader;
+ WeakPtr<ResourceLoader> _resourceLoader;
ResourceResponse _response;
RefPtr<PreviewLoaderClient> _client;
std::unique_ptr<PreviewConverter> _converter;
@@ -81,7 +81,7 @@
if (!self)
return nil;
- _resourceLoader = &resourceLoader;
+ _resourceLoader = makeWeakPtr(resourceLoader);
_response = resourceResponse;
_converter = std::make_unique<PreviewConverter>(self, _response);
_bufferedDataArray = adoptNS([[NSMutableArray alloc] init]);
@@ -121,6 +121,9 @@
- (void)_sendDidReceiveResponseIfNecessary
{
+ if (!_resourceLoader)
+ return;
+
ASSERT(!_resourceLoader->reachedTerminalState());
if (_hasSentDidReceiveResponse)
return;
@@ -138,6 +141,9 @@
_resourceLoader->didReceiveResponse(response, [self, retainedSelf = retainPtr(self)] {
_hasProcessedResponse = YES;
+ if (!_resourceLoader)
+ return;
+
if (_resourceLoader->reachedTerminalState())
return;
@@ -158,6 +164,9 @@
- (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data lengthReceived:(long long)lengthReceived
{
+ if (!_resourceLoader)
+ return;
+
ASSERT_UNUSED(connection, !connection);
if (_resourceLoader->reachedTerminalState())
return;
@@ -185,6 +194,9 @@
- (void)connectionDidFinishLoading:(NSURLConnection *)connection
{
+ if (!_resourceLoader)
+ return;
+
ASSERT_UNUSED(connection, !connection);
if (_resourceLoader->reachedTerminalState())
return;
@@ -206,6 +218,9 @@
- (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error
{
+ if (!_resourceLoader)
+ return;
+
ASSERT_UNUSED(connection, !connection);
if (_resourceLoader->reachedTerminalState())
return;
Modified: trunk/Source/WebKitLegacy/mac/ChangeLog (243470 => 243471)
--- 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)
@@ -1,3 +1,25 @@
+2019-03-25 Andy Estes <[email protected]>
+
+ [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:
+
2019-03-25 Gyuyoung Kim <[email protected]>
Remove NavigatorContentUtils in WebCore/Modules
Modified: trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm (243470 => 243471)
--- 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)
@@ -2243,8 +2243,11 @@
if (!filePath)
return nullptr;
+ auto documentWriter = adoptRef(*new QuickLookDocumentWriter(filePath));
+
[m_webFrame provisionalDataSource]._quickLookContent = @{ WebQuickLookFileNameKey : filePath, WebQuickLookUTIKey : uti };
- return adoptRef(*new QuickLookDocumentWriter(filePath));
+ [m_webFrame provisionalDataSource]._quickLookPreviewLoaderClient = documentWriter.ptr();
+ return documentWriter;
}
#endif
Modified: trunk/Source/WebKitLegacy/mac/WebView/WebDataSource.mm (243470 => 243471)
--- 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)
@@ -53,6 +53,7 @@
#import <WebCore/FrameLoader.h>
#import <WebCore/LegacyWebArchive.h>
#import <WebCore/MIMETypeRegistry.h>
+#import <WebCore/PreviewLoaderClient.h>
#import <WebCore/ResourceRequest.h>
#import <WebCore/SharedBuffer.h>
#import <WebCore/WebCoreObjCExtras.h>
@@ -108,6 +109,7 @@
#endif
#if USE(QUICK_LOOK)
RetainPtr<NSDictionary> _quickLookContent;
+ RefPtr<WebCore::PreviewLoaderClient> _quickLookPreviewLoaderClient;
#endif
};
@@ -417,10 +419,20 @@
}
#if USE(QUICK_LOOK)
+- (WebCore::PreviewLoaderClient*)_quickLookPreviewLoaderClient
+{
+ return toPrivate(_private)->_quickLookPreviewLoaderClient.get();
+}
+
- (void)_setQuickLookContent:(NSDictionary *)quickLookContent
{
toPrivate(_private)->_quickLookContent = adoptNS([quickLookContent copy]);
}
+
+- (void)_setQuickLookPreviewLoaderClient:(WebCore::PreviewLoaderClient*)quickLookPreviewLoaderClient
+{
+ toPrivate(_private)->_quickLookPreviewLoaderClient = quickLookPreviewLoaderClient;
+}
#endif
@end
Modified: trunk/Source/WebKitLegacy/mac/WebView/WebDataSourceInternal.h (243470 => 243471)
--- 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)
@@ -30,7 +30,8 @@
#import <wtf/Forward.h>
namespace WebCore {
- class DocumentLoader;
+class DocumentLoader;
+class PreviewLoaderClient;
}
class WebDocumentLoaderMac;
@@ -59,5 +60,6 @@
- (WebCore::DocumentLoader*)_documentLoader;
#if USE(QUICK_LOOK)
@property (nonatomic, copy, setter=_setQuickLookContent:) NSDictionary *_quickLookContent;
+@property (nonatomic, setter=_setQuickLookPreviewLoaderClient:) WebCore::PreviewLoaderClient* _quickLookPreviewLoaderClient;
#endif
@end
