Support for Curl in Windows-Cairo build
Created attachment 19615 [details] patch to allow curl in windows-cario build
Created attachment 19616 [details] webcore.vcproj file
Created attachment 19625 [details] now, includes ChangeLog
Comment on attachment 19625 [details] now, includes ChangeLog Please read over <http://webkit.org/coding/coding-style.html> if you haven't already. Many of the comments below are due to style problems. +2008-03-09 U-IBM-X30\Daniel Zucker <set EMAIL_ADDRESS environment variable> You should set the CHANGE_LOG_NAME and CHANGE_LOG_EMAIL_ADDRESS environment variables so that this first line will be generated correctly. We typically put comments by each file/function we've modified to explain the changes. This is particularly important for changes to .vcproj/.sln files where it's very hard to read the diff. -#define WTF_PLATFORM_CG 1 -#undef WTF_PLATFORM_CAIRO -#define WTF_USE_CFNETWORK 1 +//DFZ +//#define WTF_PLATFORM_CG 1 +//#undef WTF_PLATFORM_CAIRO +#undef WTF_PLATFORM_CG +#define WTF_PLATFORM_CAIRO 1 +//#define WTF_USE_CFNETWORK 1 +#undef WTF_USE_CFNETWORK +#define WTF_USE_CURL 1 This change will break the CG-based builds. We need some way of having both configurations co-existing. We also don't land commented-out code. + <Filter + Name="curl" + > + <File + RelativePath="..\platform\network\curl\AuthenticationChallenge.h" + > + </File> + <File + RelativePath="..\platform\network\curl\ResourceError.h" + > + </File> + <File + RelativePath="..\platform\network\curl\ResourceHandleCurl.cpp" + > + </File> + <File + RelativePath="..\platform\network\curl\ResourceHandleManager.cpp" + > + </File> + <File + RelativePath="..\platform\network\curl\ResourceHandleManager.h" + > + </File> + <File + RelativePath="..\platform\network\curl\ResourceRequest.h" + > + </File> + <File + RelativePath="..\platform\network\curl\ResourceResponse.h" + > + </File> + </Filter> Every file in this new filter needs to be excluded from the three CG-based configurations: Debug, Release, and Debug_Internal. @@ -118,18 +118,22 @@ String cookies(const Document* /*documen return String(); Vector<UChar> buffer(count); - InternetGetCookie(buffer.data(), 0, buffer, &count); - buffer.shrink(count - 1); // Ignore the null terminator. + InternetGetCookie(str.charactersWithNullTermination(), 0, buffer.data(), &count); + buffer.shrink(count - 1); // Ignore the null terminator. return String::adopt(buffer); #endif Please put the justification for this change in your ChangeLog. Please put the justification for the ForEachCoClass.h changes in you ChangeLog. +++ WebKit/win/WebDataSource.cpp (working copy) @@ -27,6 +27,8 @@ #include "WebKitDLL.h" #include "WebDataSource.h" +#include <wtf/RetainPtr.h> + #include "WebKit.h" #include "MemoryStream.h" #include "WebDocumentLoader.h" This new #include is misplaced. It should be placed in ASCII order with the second paragraph of #includes. Ditto for WebError.cpp, WebMutableURLRequest.cpp, WebURLAuthenticationChallenge.cpp. @@ -204,11 +205,13 @@ HRESULT STDMETHODCALLTYPE WebError::sslP return E_POINTER; *result = 0; - if (!m_cfErrorUserInfoDict) { +#if USE(CFNETWORK) + if (!m_cfErrorUserInfoDict) { // copy userinfo from CFErrorRef CFErrorRef cfError = m_error; m_cfErrorUserInfoDict.adoptCF(CFErrorCopyUserInfo(cfError)); } +#endif It looks like the indentation of the if line is wrong. We use 4-space indents, no tabs. This occurs elsewhere throughout your patch. +#include "notImplemented.h" The case of the filename is "NotImplemented.h". Please change your #includes to match. +++ WebKit/win/WebKit.vcproj/WebKit.sln (working copy) It looks like this patch adds WinLauncher.vcproj to WebKit.sln. I believe we already have another bug for doing that, so I'd rather it not be included in this patch. I'm not sure it's good to have so many configuration-specific #ifs in WebKit source files. I think we need to figure out some better way of handling this. Thanks for the patch!. r- so that the style issues can be fixed and the ChangeLogs can be made more thorough, and so that we can sort out the issue of the configuration-specific WebKit code.
Dan -- welcome to the world of peer-reviewed source patches! Don't lose hope -- the result will be better than you expect, and well worth the revisions. WebKit/win/WebError.cpp (WebError::sslPeerCertificate): In addition to the region you alread excluded, I notice that a few lines further down we have a call to 'wkGetSSLPeerCertificateData'. This is an internal platform API which we should also excluded. I have discovered the same problem with some of my CoreGraphics changes, as there are a few font-handling routines in the 'wk' namespace I must get rid of. We might want to break this into three files: WebError (common stuff), WebErrorCF (CFNetwork stuff) and WebErrorCurl. WebKit/win/WebFrame.cpp: Again, we have a fairly small chunk of this file that is CFNetwork-specific. Maybe we should move the download chunk into its own file (WebDownloadCF.cpp and WebDownloadCurl.cpp)? It also looks like some of my temporary CoreGraphics diff is in this file, as the "spoolPages" has a #if(CG) around it when you use my manual patch; it looks like the tail end of that is in this revision. WebKit/win/WebMutableURLRequest.cpp: This whole patch might go away if we just stub out something in the Curl-based ResourceHandle to issue a "NotImplemented" when calling ResourceHandle::setHostAllowsAnyHTTPSCertificate. Similarly, maybe we should move the certificate stuff into its own file so that we can just stub out the Curl things until we have valid implementations. WebKit/win/WebURLAuthenticationChallenge.cpp: WebKit/win/WebURLAuthenticationChallengeSender.cpp: It seems like both of these changes could go away if we provided stub implementations in the WebURLAuthenticationChallenge stuff. WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp: ditto above
(In reply to comment #4) > It looks like this patch adds WinLauncher.vcproj to WebKit.sln. I believe we > already have another bug for doing that, so I'd rather it not be included in > this patch. See http://bugs.webkit.org/show_bug.cgi?id=17715.
Created attachment 19640 [details] updated Curl patch Fixes Problems identified by Aroben: (does not yet include response to bfulgham's comments) You should set the CHANGE_LOG_NAME and CHANGE_LOG_EMAIL_ADDRESS environment variables so that this first line will be generated correctly. >>done This change will break the CG-based builds. We need some way of having both configurations co-existing. We also don't land commented-out code. >>done. config.h is reverted. Every file in this new filter needs to be excluded from the three CG-based configurations: Debug, Release, and Debug_Internal. >>done Please put the justification for this change in your ChangeLog. >>done Please put the justification for the ForEachCoClass.h changes in you ChangeLog. >>done This new #include is misplaced. It should be placed in ASCII order with the second paragraph of #includes. Ditto for WebError.cpp, WebMutableURLRequest.cpp, WebURLAuthenticationChallenge.cpp. >>done It looks like the indentation of the if line is wrong. We use 4-space indents, no tabs. This occurs elsewhere throughout your patch. >>done. I fixed all the problem areas I could find. Please let me know if I missed any. The case of the filename is "NotImplemented.h". Please change your #includes to match. >>done It looks like this patch adds WinLauncher.vcproj to WebKit.sln. I believe we already have another bug for doing that, so I'd rather it not be included in this patch. >>done (removed WinLauncher.vcproj)
Portion of this patch moved to 17783.
Please note the changes in http://bugs.webkit.org/show_bug.cgi?id=17788, which split CookieJarWin.cpp. The Cairo (Curl) build will need to compile CookieJarWin.cpp, rather than the new CookieJarCFNetWin.cpp which is now the default in the project file.
Please see http://bugs.webkit.org/show_bug.cgi?id=17837 for another sub-patch of this work, revised to use multiple files (rather than #ifdef's).
Please note that a final set of VCPROJ updates have been presented under http://bugs.webkit.org/show_bug.cgi?id=17985 for review.
Due to the other subsequent patches submitted (See Bug 17730 depends on:..), the attached patch is obsolete. However, all dependent bugs must be fixed to get the build to work with Curl. As of the time of this comment, bug 18468 is still open.
This bug is no longer relevant. The cURL components are now fully building and running under the redistributable Windows port.