We currently have an API to start a new download: WK_EXPORT WKDownloadRef WKContextDownloadURLRequest(WKContextRef context, const WKURLRequestRef request); but there's no API to resume a partially completed download. Presumably, such an API would take the data returned by WKDownloadGetResumeData() as well as a path which can be used by NSURLDownload.
<rdar://problem/18576259>
Created attachment 239494 [details] Patch
Comment on attachment 239494 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239494&action=review > Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm:99 > + m_nsURLDownload = [[NSURLDownload alloc] initWithResumeData:nsData.get() delegate:m_delegate.get() path:path]; Nit: Needs adoptNS(...) here.
Comment on attachment 239494 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239494&action=review > Source/WebKit2/WebProcess/WebProcess.messages.in:64 > + # three messages. There are other messages files with fewer messages; since you're having to change the comment I wonder whether this would be an opportune time to spin off a Download.messages.in.
Comment on attachment 239494 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239494&action=review >> Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm:99 >> + m_nsURLDownload = [[NSURLDownload alloc] initWithResumeData:nsData.get() delegate:m_delegate.get() path:path]; > > Nit: Needs adoptNS(...) here. I'll fix this. >> Source/WebKit2/WebProcess/WebProcess.messages.in:64 >> + # three messages. > > There are other messages files with fewer messages; since you're having to change the comment I wonder whether this would be an opportune time to spin off a Download.messages.in. Fair point. I'll do that with a separate bug.
Comment on attachment 239494 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239494&action=review >>> Source/WebKit2/WebProcess/WebProcess.messages.in:64 >>> + # three messages. >> >> There are other messages files with fewer messages; since you're having to change the comment I wonder whether this would be an opportune time to spin off a Download.messages.in. > > Fair point. I'll do that with a separate bug. I filed bug 137744 to track this.
Created attachment 239891 [details] Patch
Comment on attachment 239494 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239494&action=review > Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm:98 > + RetainPtr<NSData> nsData = adoptNS([[NSData alloc] initWithBytes:resumeData.data() length:resumeData.size()]); Can use auto nsData = > Source/WebKit2/UIProcess/API/C/WKContext.cpp:285 > +WKDownloadRef WKContextResumeDownload(WKContextRef contextRef, WKDataRef resumeData, WKStringRef path) I'm not in love with this name, since it reads like a getter. I don't have a better idea though.
Comment on attachment 239494 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239494&action=review >> Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm:98 >> + RetainPtr<NSData> nsData = adoptNS([[NSData alloc] initWithBytes:resumeData.data() length:resumeData.size()]); > > Can use auto nsData = Fixed. >> Source/WebKit2/UIProcess/API/C/WKContext.cpp:285 >> +WKDownloadRef WKContextResumeDownload(WKContextRef contextRef, WKDataRef resumeData, WKStringRef path) > > I'm not in love with this name, since it reads like a getter. I don't have a better idea though. I don't, either.
Created attachment 240135 [details] Patch
Comment on attachment 240135 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240135&action=review > Source/WebKit2/NetworkProcess/NetworkProcess.h:36 > +#include "SandboxExtension.h" Not needed, because DownloadManager.h now includes this header too. > Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm:102 > + auto nsData = adoptNS([[NSData alloc] initWithBytes:resumeData.data() length:resumeData.size()]); Is this always how we turn an IPC::DataReference into an NSData? I would have expected a more streamlined idiom. > Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm:105 > + m_request = ResourceRequest([m_nsURLDownload request]); Is the explicit constructor needed here? > Source/WebKit2/UIProcess/API/C/WKContext.h:113 > WK_EXPORT WKDownloadRef WKContextDownloadURLRequest(WKContextRef context, const WKURLRequestRef request); Not new to this patch: This const is a mistake. > Source/WebKit2/UIProcess/WebContext.cpp:908 > + Should leave out this blank line, I think.
Comment on attachment 240135 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240135&action=review >> Source/WebKit2/NetworkProcess/NetworkProcess.h:36 >> +#include "SandboxExtension.h" > > Not needed, because DownloadManager.h now includes this header too. Removed. >> Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm:102 >> + auto nsData = adoptNS([[NSData alloc] initWithBytes:resumeData.data() length:resumeData.size()]); > > Is this always how we turn an IPC::DataReference into an NSData? I would have expected a more streamlined idiom. As far as I can tell, yes. I see this idiom used in other places. >> Source/WebKit2/Shared/Downloads/mac/DownloadMac.mm:105 >> + m_request = ResourceRequest([m_nsURLDownload request]); > > Is the explicit constructor needed here? It isn't. I'll remove the explicit constructor. >> Source/WebKit2/UIProcess/API/C/WKContext.h:113 >> WK_EXPORT WKDownloadRef WKContextDownloadURLRequest(WKContextRef context, const WKURLRequestRef request); > > Not new to this patch: This const is a mistake. I'll fix this in a separate patch. >> Source/WebKit2/UIProcess/WebContext.cpp:908 >> + > > Should leave out this blank line, I think. Done.
Committed r174987: <http://trac.webkit.org/changeset/174987>