Summary: | [Mac] ResourceRequest's nsURLRequest() does not differentiate null and empty URLs with CFNetwork | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||
Component: | Page Loading | Assignee: | Benjamin Poulain <benjamin> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ddkilzer, kling, menard, ossy, psolanki, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Benjamin Poulain
2011-11-03 22:19:48 PDT
Created attachment 113622 [details]
Patch
The next step is to get rid of copyToBuffer() (we are the only one using that) and fix the warning. I did not do that here to not fix two bugs in one patch. Comment on attachment 113622 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113622&action=review r=me, but setting the r- flag since you wanted this to go through the commit queue. (Also, it seems the win EWS bot is building for Mac OS X?!) > Source/WebCore/platform/cf/KURLCFNet.cpp:82 > + CharBuffer buffer; I don't think you can use CharBuffer here, can you? It only seems to be typedef-ed in KURL.cpp. Might be useful to move that typedef to KURL.h in another patch, though. > Source/WebCore/platform/mac/KURLMac.mm:75 > + return (CFURLRef)[[NSURL alloc] initWithString:@""]; Can you use reinterpret_cast<CFURLRef>() instead of a C-style cast here? Created attachment 113717 [details]
Patch
> > Source/WebCore/platform/cf/KURLCFNet.cpp:82 > > + CharBuffer buffer; > > I don't think you can use CharBuffer here, can you? It only seems to be typedef-ed in KURL.cpp. Yep, good catch. > Might be useful to move that typedef to KURL.h in another patch, though. I would prefer not to spread the CharBuffer typedef. I am trying to fix the 2 FIXME in a follow up and that should get rid of the CharBuffer in the Mac specific files. Comment on attachment 113717 [details]
Patch
r=me!
Comment on attachment 113717 [details]
Patch
Patch looks good to me as well.
Comment on attachment 113717 [details] Patch Clearing flags on attachment: 113717 Committed r99334: <http://trac.webkit.org/changeset/99334> All reviewed patches have been landed. Closing bug. Comment on attachment 113717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113717&action=review It broke QtWebKit on Mac: ld: duplicate symbol WebCore::KURL::createCFURL() constin /buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/WebKitBuild/Release/Source/WebCore/release/libwebcore.a(KURLCFNet.o) and /buildbot/snowleopard-qt-release/snowleopard-qt-intel-release/build/WebKitBuild/Release/Source/WebCore/release/libwebcore.a(KURLMac.o) > Source/WebCore/platform/cf/KURLCFNet.cpp:79 > +#if !PLATFORM(MAC) > +CFURLRef KURL::createCFURL() const It is true if you build QtWebKit on Mac. > Source/WebCore/platform/mac/KURLMac.mm:70 > +CFURLRef KURL::createCFURL() const . (In reply to comment #11) > (From update of attachment 113717 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=113717&action=review > > It broke QtWebKit on Mac: Do you need this file on QtWebKit? It is to convert KURL to/from CFURL. Maybe this was added by accident? (In reply to comment #12) > (In reply to comment #11) > > (From update of attachment 113717 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=113717&action=review > > > > It broke QtWebKit on Mac: > > Do you need this file on QtWebKit? It is to convert KURL to/from CFURL. Maybe this was added by accident? Alexis, have you got any idea? You usually hack QtWebKit on Mac. (In reply to comment #12) > (In reply to comment #11) > > (From update of attachment 113717 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=113717&action=review > > > > It broke QtWebKit on Mac: > > Do you need this file on QtWebKit? It is to convert KURL to/from CFURL. Maybe this was added by accident? We use it when we use the QuickTime media backend :/ Landed a buildfix in http://trac.webkit.org/changeset/99429 |