[Webkit-unassigned] [Bug 17730] Allows Windows version to use Curl

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=17730


aroben at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #19625|review?                     |review-
               Flag|                            |




------- Comment #4 from aroben at apple.com  2008-03-10 08:42 PDT -------
(From update of attachment 19625)
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.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.