In WK1, the WebDownload class is responsible for writing resume data to the download bundle. WK2 should do the same. <rdar://problem/8753077>
Created attachment 80409 [details] Patch
Attachment 80409 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/network/cf/DownloadBundle.h:30: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/WebCore/platform/network/win/DownloadBundleWin.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/network/win/DownloadBundleWin.cpp:43: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/network/win/DownloadBundleWin.cpp:48: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/network/win/DownloadBundleWin.cpp:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 5 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 80409 [details] did not build on win: Build output: http://queues.webkit.org/results/7629274
Created attachment 80477 [details] Patch v2
Attachment 80477 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/network/cf/DownloadBundle.h:30: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 80477 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=80477&action=review > Source/WebKit/win/WebDownloadCFNet.cpp:253 > + resumeData.adoptCF(CFURLDownloadCopyResumeData(m_download.get())); awesome > Source/WebKit2/WebProcess/Downloads/cf/DownloadCFNet.cpp:171 > +} do you need to null-check download? > Source/WebKit2/WebProcess/Downloads/cf/DownloadCFNet.cpp:179 > + download->didCreateDestination(download->destination()); do you need to null-check download? > Source/WebKit2/WebProcess/Downloads/curl/DownloadCurl.cpp:58 > +} if there's nothing that you know we need to do, you can leave this out. but if we do need to do something, please comment at least briefly about it. > Source/WebKit2/WebProcess/Downloads/mac/DownloadMac.mm:174 > + presumably intentionally empty, and doesn't need notImplemented(), i assume. > Source/WebKit2/WebProcess/Downloads/qt/DownloadQt.cpp:57 > +} if there's nothing that you know we need to do, you can leave this out. but if we do need to do something, please comment at least briefly about it.
(In reply to comment #6) > (From update of attachment 80477 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=80477&action=review > > > Source/WebKit/win/WebDownloadCFNet.cpp:253 > > + resumeData.adoptCF(CFURLDownloadCopyResumeData(m_download.get())); > > awesome > > > Source/WebKit2/WebProcess/Downloads/cf/DownloadCFNet.cpp:171 > > +} > > do you need to null-check download? > > > Source/WebKit2/WebProcess/Downloads/cf/DownloadCFNet.cpp:179 > > + download->didCreateDestination(download->destination()); > > do you need to null-check download? None of the callback functions null check download, so I don't think so. > > > Source/WebKit2/WebProcess/Downloads/curl/DownloadCurl.cpp:58 > > +} > > if there's nothing that you know we need to do, you can leave this out. but if we do need to do something, please comment at least briefly about it. I'm not sure whether other platforms will use this function, but it needs a definition. I'm going to leave the calls to notImplemented() and let someone remove them when downloads are implemented for this platform. > > > Source/WebKit2/WebProcess/Downloads/mac/DownloadMac.mm:174 > > + > > presumably intentionally empty, and doesn't need notImplemented(), i assume. That's right. > > > Source/WebKit2/WebProcess/Downloads/qt/DownloadQt.cpp:57 > > +} > > if there's nothing that you know we need to do, you can leave this out. but if we do need to do something, please comment at least briefly about it. Same response as earlier. Thanks for the review!
Landed in r77055.