For Curl, the WebDownload class (WebDownloadCurl.cpp) is not implemented.
Created attachment 201829 [details] Patch
Created attachment 203200 [details] Patch
Modified the patch slightly by including a crash fix, and creating a new header file.
Comment on attachment 203200 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203200&action=review Very nice! I had a few minor style/nit-picky comments, but this looks great. I do feel that we should place the "CurlDownload" class should be in its own set of files. It might make sense for the CurlDownload class to live in WebCore (platform/network/curl). > Source/WebKit/win/ChangeLog:11 > + (CurlDownloadManager): I would get rid of all of these method call-outs. The whole file is new, so no point listing them individually. > Source/WebKit/win/ChangeLog:33 > + (CurlDownloadManager::CurlDownloadManager): I think this should be in its own file. I know this means we touch the project files, but I think putting them in the same file is worse. > Source/WebKit/win/WebDownloadCurl.cpp:208 > +// CurlDownloadManager ------------------------------------------------------------------- This should be in its own file. > Source/WebKit/win/WebDownloadCurl.cpp:458 > + if (!m_destination.isEmpty()) Preference: I really prefer to handle the "no work" case as an early return. I know this method only has one line, but in my mind early return clarifies that if the destination file name is empty, we do nothing. As it's written, it's clear that we don't move the file when the name is empty, but it leaves open the possibility that other work might be done in this method if the file name was empty. > Source/WebKit/win/WebDownloadCurl.cpp:459 > + MoveFile(m_tempPath.charactersWithNullTermination(), m_destination.charactersWithNullTermination()); Nit: We try to expose Win API calls with the global namespace, i.e., "::MoveFile"
Created attachment 203435 [details] Patch
(In reply to comment #4) > (From update of attachment 203200 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=203200&action=review > Thanks for reviewing! I updated the patch according to your comments. The files CurlDownload.h/cpp were added to the WebCore/platform/network/curl folder.
Comment on attachment 203435 [details] Patch Looks great! Thanks for making the changes.
Comment on attachment 203435 [details] Patch Clearing flags on attachment: 203435 Committed r151067: <http://trac.webkit.org/changeset/151067>
All reviewed patches have been landed. Closing bug.