Begin stubbing out the Download class
Created attachment 72061 [details] Patch
Attachment 72061 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/WebProcess/Downloads/qt/DownloadQt.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/WebProcess/Downloads/DownloadManager.h:32: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit2/WebProcess/Downloads/cf/DownloadCFNet.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/WebProcess/Downloads/Download.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 4 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 72061 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=72061&action=review This seems certain to crash if the delegate ever calls back into the Download. > WebKit2/ChangeLog:25 > + * WebProcess/Downloads/mac/DownloadMac.mm: Added. > + (WebKit::Download::start): > + Create an NSURLDownload. > + You seem to have left DownloadCFNet and DownloadQt out of your ChangeLog! > WebKit2/WebProcess/Downloads/DownloadManager.cpp:49 > +void DownloadManager::startDownload(uint64_t downloadID, const ResourceRequest& request) > +{ > + OwnPtr<Download> download = Download::create(downloadID, request); > + download->start(); > +} Is it OK that the download gets deleted right after start() returns? > WebKit2/WebProcess/Downloads/mac/DownloadMac.mm:31 > +@interface WKDownloadAsDelegate : NSObject <NSURLConnectionDelegate> { > + WebKit::Download* m_download; I thought we just used _ for ivars. > WebKit2/WebProcess/Downloads/mac/DownloadMac.mm:68 > +- (void)invalidate > +{ > + m_download = 0; > +} Who calls this? It seems like Download should call it in its destructor (if not earlier). > WebKit2/WebProcess/Downloads/mac/DownloadMac.mm:72 > + NSLog(@"download did begin!"); Did you mean to leave this in here? > WebKit2/win/WebKit2.vcproj:1264 > + <File > + RelativePath="..\WebProcess\Downloads\cf\DownloadCFNet.h" > + > > + </File> I don't think this file exists.
(In reply to comment #3) > (From update of attachment 72061 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=72061&action=review > > This seems certain to crash if the delegate ever calls back into the Download. > Yup, I was going to fix that when I add the first callback handler. > > WebKit2/ChangeLog:25 > > + * WebProcess/Downloads/mac/DownloadMac.mm: Added. > > + (WebKit::Download::start): > > + Create an NSURLDownload. > > + > > You seem to have left DownloadCFNet and DownloadQt out of your ChangeLog! Yup, I didn't see a reason for keeping them when I'm just adding a stubbed out function in each file. > > > WebKit2/WebProcess/Downloads/DownloadManager.cpp:49 > > +void DownloadManager::startDownload(uint64_t downloadID, const ResourceRequest& request) > > +{ > > + OwnPtr<Download> download = Download::create(downloadID, request); > > + download->start(); > > +} > > Is it OK that the download gets deleted right after start() returns? For now it is :) > > > WebKit2/WebProcess/Downloads/mac/DownloadMac.mm:31 > > +@interface WKDownloadAsDelegate : NSObject <NSURLConnectionDelegate> { > > + WebKit::Download* m_download; > > I thought we just used _ for ivars. Copied from ResourceHandleMac, but I can change it. > > > WebKit2/WebProcess/Downloads/mac/DownloadMac.mm:68 > > +- (void)invalidate > > +{ > > + m_download = 0; > > +} > > Who calls this? It seems like Download should call it in its destructor (if not earlier). > Yeah it should be called from the destructor eventually (again, this is just stubbing out the code). > > WebKit2/WebProcess/Downloads/mac/DownloadMac.mm:72 > > + NSLog(@"download did begin!"); > > Did you mean to leave this in here? No > > > WebKit2/win/WebKit2.vcproj:1264 > > + <File > > + RelativePath="..\WebProcess\Downloads\cf\DownloadCFNet.h" > > + > > > + </File> > > I don't think this file exists. Good catch. I'll upload a new patch shortly!
Created attachment 72070 [details] Patch
Attachment 72070 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/WebProcess/Downloads/qt/DownloadQt.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/WebProcess/Downloads/DownloadManager.h:32: Code inside a namespace should not be indented. [whitespace/indent] [4] WebKit2/WebProcess/Downloads/cf/DownloadCFNet.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit2/WebProcess/Downloads/Download.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 4 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r70687: <http://trac.webkit.org/changeset/70687>
http://trac.webkit.org/changeset/70687 might have broken GTK Linux 64-bit Debug