Changeset 243471 in webkit


Ignore:
Timestamp:
Mar 25, 2019 4:30:47 PM (5 years ago)
Author:
aestes@apple.com
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:
Location:
trunk/Source
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r243460 r243471  
     12019-03-25  Andy Estes  <aestes@apple.com>
     2
     3        [iOS] Break a reference cycle between PreviewLoader and ResourceLoader
     4        https://bugs.webkit.org/show_bug.cgi?id=194964
     5        <rdar://problem/48279441>
     6
     7        Reviewed by Alex Christensen.
     8
     9        When a document's QuickLook preview is loaded, a reference cycle is created between
     10        WebPreviewLoader and ResourceLoader. Break the cycle by changing WebPreviewLoader to hold a
     11        WeakPtr to its ResourceLoader ResourceLoader::releaseResources().
     12
     13        Fixes leaks detected by `run-webkit-tests --leaks LayoutTests/quicklook`. Also covered by
     14        existing API tests.
     15
     16        * loader/ResourceLoader.h:
     17        * loader/ios/PreviewLoader.mm:
     18        (-[WebPreviewLoader initWithResourceLoader:resourceResponse:]):
     19        (-[WebPreviewLoader _sendDidReceiveResponseIfNecessary]):
     20        (-[WebPreviewLoader connection:didReceiveData:lengthReceived:]):
     21        (-[WebPreviewLoader connectionDidFinishLoading:]):
     22        (-[WebPreviewLoader connection:didFailWithError:]):
     23
    1242019-03-25  Alex Christensen  <achristensen@webkit.org>
    225
  • trunk/Source/WebCore/loader/ResourceLoader.h

    r238933 r243471  
    5454class PreviewLoader;
    5555
    56 class ResourceLoader : public RefCounted<ResourceLoader>, protected ResourceHandleClient {
     56class ResourceLoader : public CanMakeWeakPtr<ResourceLoader>, public RefCounted<ResourceLoader>, protected ResourceHandleClient {
    5757public:
    5858    virtual ~ResourceLoader() = 0;
  • trunk/Source/WebCore/loader/ios/PreviewLoader.mm

    r232179 r243471  
    4343
    4444@interface WebPreviewLoader : NSObject {
    45     RefPtr<ResourceLoader> _resourceLoader;
     45    WeakPtr<ResourceLoader> _resourceLoader;
    4646    ResourceResponse _response;
    4747    RefPtr<PreviewLoaderClient> _client;
     
    8282        return nil;
    8383
    84     _resourceLoader = &resourceLoader;
     84    _resourceLoader = makeWeakPtr(resourceLoader);
    8585    _response = resourceResponse;
    8686    _converter = std::make_unique<PreviewConverter>(self, _response);
     
    122122- (void)_sendDidReceiveResponseIfNecessary
    123123{
     124    if (!_resourceLoader)
     125        return;
     126
    124127    ASSERT(!_resourceLoader->reachedTerminalState());
    125128    if (_hasSentDidReceiveResponse)
     
    138141    _resourceLoader->didReceiveResponse(response, [self, retainedSelf = retainPtr(self)] {
    139142        _hasProcessedResponse = YES;
     143
     144        if (!_resourceLoader)
     145            return;
    140146
    141147        if (_resourceLoader->reachedTerminalState())
     
    159165- (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data lengthReceived:(long long)lengthReceived
    160166{
     167    if (!_resourceLoader)
     168        return;
     169
    161170    ASSERT_UNUSED(connection, !connection);
    162171    if (_resourceLoader->reachedTerminalState())
     
    186195- (void)connectionDidFinishLoading:(NSURLConnection *)connection
    187196{
     197    if (!_resourceLoader)
     198        return;
     199
    188200    ASSERT_UNUSED(connection, !connection);
    189201    if (_resourceLoader->reachedTerminalState())
     
    207219- (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error
    208220{
     221    if (!_resourceLoader)
     222        return;
     223
    209224    ASSERT_UNUSED(connection, !connection);
    210225    if (_resourceLoader->reachedTerminalState())
  • trunk/Source/WebKitLegacy/mac/ChangeLog

    r243433 r243471  
     12019-03-25  Andy Estes  <aestes@apple.com>
     2
     3        [iOS] Break a reference cycle between PreviewLoader and ResourceLoader
     4        https://bugs.webkit.org/show_bug.cgi?id=194964
     5        <rdar://problem/48279441>
     6
     7        Reviewed by Alex Christensen.
     8
     9        The WebDataSource._quickLookContent SPI accidentally relied on PreviewLoaders being leaked
     10        to keep the temporary file referenced by WebQuickLookFileNameKey in existence. Since
     11        PreviewLoaders are now being deleted properly, we teach WebDataSource to keep the
     12        PreviewLoaderClient alive for its lifetime. This ensures that as long as
     13        WebDataSource._quickLookContent can be called, the associated temp file will not be deleted
     14        by WebKit.
     15
     16        * WebCoreSupport/WebFrameLoaderClient.mm:
     17        (WebFrameLoaderClient::createPreviewLoaderClient):
     18        * WebView/WebDataSource.mm:
     19        (-[WebDataSource _quickLookPreviewLoaderClient]):
     20        (-[WebDataSource _setQuickLookPreviewLoaderClient:]):
     21        * WebView/WebDataSourceInternal.h:
     22
    1232019-03-25  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
    224
  • trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm

    r242920 r243471  
    22442244        return nullptr;
    22452245
     2246    auto documentWriter = adoptRef(*new QuickLookDocumentWriter(filePath));
     2247
    22462248    [m_webFrame provisionalDataSource]._quickLookContent = @{ WebQuickLookFileNameKey : filePath, WebQuickLookUTIKey : uti };
    2247     return adoptRef(*new QuickLookDocumentWriter(filePath));
     2249    [m_webFrame provisionalDataSource]._quickLookPreviewLoaderClient = documentWriter.ptr();
     2250    return documentWriter;
    22482251}
    22492252#endif
  • trunk/Source/WebKitLegacy/mac/WebView/WebDataSource.mm

    r239709 r243471  
    5454#import <WebCore/LegacyWebArchive.h>
    5555#import <WebCore/MIMETypeRegistry.h>
     56#import <WebCore/PreviewLoaderClient.h>
    5657#import <WebCore/ResourceRequest.h>
    5758#import <WebCore/SharedBuffer.h>
     
    109110#if USE(QUICK_LOOK)
    110111    RetainPtr<NSDictionary> _quickLookContent;
     112    RefPtr<WebCore::PreviewLoaderClient> _quickLookPreviewLoaderClient;
    111113#endif
    112114};
     
    418420
    419421#if USE(QUICK_LOOK)
     422- (WebCore::PreviewLoaderClient*)_quickLookPreviewLoaderClient
     423{
     424    return toPrivate(_private)->_quickLookPreviewLoaderClient.get();
     425}
     426
    420427- (void)_setQuickLookContent:(NSDictionary *)quickLookContent
    421428{
    422429    toPrivate(_private)->_quickLookContent = adoptNS([quickLookContent copy]);
     430}
     431
     432- (void)_setQuickLookPreviewLoaderClient:(WebCore::PreviewLoaderClient*)quickLookPreviewLoaderClient
     433{
     434    toPrivate(_private)->_quickLookPreviewLoaderClient = quickLookPreviewLoaderClient;
    423435}
    424436#endif
  • trunk/Source/WebKitLegacy/mac/WebView/WebDataSourceInternal.h

    r216816 r243471  
    3131
    3232namespace WebCore {
    33     class DocumentLoader;
     33class DocumentLoader;
     34class PreviewLoaderClient;
    3435}
    3536
     
    6061#if USE(QUICK_LOOK)
    6162@property (nonatomic, copy, setter=_setQuickLookContent:) NSDictionary *_quickLookContent;
     63@property (nonatomic, setter=_setQuickLookPreviewLoaderClient:) WebCore::PreviewLoaderClient* _quickLookPreviewLoaderClient;
    6264#endif
    6365@end
Note: See TracChangeset for help on using the changeset viewer.