Bug 24779

Summary: Conditionalize CFNetwork-Specific Code in WebView
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Exclude CFNetwork-specific calls in non-Apple builds. aroben: review+

Description Brent Fulgham 2009-03-24 11:22:15 PDT
WebView.cpp makes calls to various CFNetwork-specific routines that are not present in Windows builds based on other network backends.

As a first step, add #if/def blocks around the two blocks of code that make CFNetwork calls.  Later, cURL or Google network API calls can be added here.
Comment 1 Brent Fulgham 2009-03-24 11:32:28 PDT
Created attachment 28893 [details]
Exclude CFNetwork-specific calls in non-Apple builds.
Comment 2 Adam Roben (:aroben) 2009-03-24 11:35:50 PDT
Comment on attachment 28893 [details]
Exclude CFNetwork-specific calls in non-Apple builds.

> +++ WebKit/win/WebView.cpp	(working copy)
> @@ -50,7 +50,9 @@
>  #include "WebNotificationCenter.h"
>  #include "WebPreferences.h"
>  #pragma warning( push, 0 )
> +#if PLATFORM(CG)
>  #include <CoreGraphics/CGContext.h>
> +#endif

It would be better to move the conditionally-included headers down below the unconditionally-included headers. Each #if/#endif pair can form its own paragraph.

> +#include <CoreFoundation/CoreFoundation.h>

Should this be inside #if PLATFORM(CF)/#endif?

r=me
Comment 3 Brent Fulgham 2009-03-24 11:40:52 PDT
(In reply to comment #2)
> (From update of attachment 28893 [details] [review])
> > +++ WebKit/win/WebView.cpp	(working copy)
> > @@ -50,7 +50,9 @@
> >  #include "WebNotificationCenter.h"
> >  #include "WebPreferences.h"
> >  #pragma warning( push, 0 )
> > +#if PLATFORM(CG)
> >  #include <CoreGraphics/CGContext.h>
> > +#endif
> 
> It would be better to move the conditionally-included headers down below the
> unconditionally-included headers. Each #if/#endif pair can form its own
> paragraph.

Will do.

> > +#include <CoreFoundation/CoreFoundation.h>
> 
> Should this be inside #if PLATFORM(CF)/#endif?

Currently, all Windows builds use CoreFoundation, so if this was not included I'm not sure if anything would build.  But it certainly won't hurt to add the check, and might help some future porter so I'll change it when I commit.

> r=me
> 

Comment 4 Brent Fulgham 2009-03-24 13:57:24 PDT
Committed in @r41949.