[Webkit-unassigned] [Bug 129322] [iOS] Download support by CFURLDownloadRef under USE(CFNETWORK).

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=129322





--- Comment #12 from Andy Estes <aestes at apple.com>  2014-03-21 18:13:28 PST ---
(In reply to comment #6)
> (In reply to comment #4)
> > (From update of attachment 225185 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=225185&action=review
> > 
> > > Source/WebKit2/Shared/Downloads/ios/DownloadIOS.mm:170
> > > +    CFURLConnectionRef connection = handle->connection();
> > > +    m_download = CFURLDownloadCreateAndStartWithLoadingConnection(NULL, connection, m_request.cfURLRequest(UpdateHTTPBody), response.cfURLResponse(), &client);
> > > +    handle->releaseConnectionForDownload();
> > > +    CFRelease(connection);
> > 
> > This is a really awkward construction. It seems that ResourceHandle::releaseConnectionForDownload should return a RetainPtr of the connection. An explicit CFRelease like this seems both unclear and dangerous.
> 
> I agree. 
> 
> This pattern is copied from WebKit1 WebFrameLoaderClient:  https://trac.webkit.org/changeset/91198/trunk/Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm
> 
> If ResourceHandle::releaseConnectionForDownload returns RetainPtr then we won't need to call CFRelease.  It currently returns CFURLConnectionRef (not RetainPtr).  This could be misleading since all releaseConnectionForDownload does is clear the connection data member (a RetainPtr) by calling leadRef() in ResourceHandleCFNet.cpp:
> 
>     return d->m_connection.leakRef();
> 
> I will file another bug to track this.

I'm going to fix this as part of this bug.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.