Bug 106558 – [Windows] Compile fix.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

The Windows CF build fails with:

CFNetwork/CFURLConnection.h(336) : error C2061: syntax error : identifier 'dispatch_queue_t'

Created attachment 182124 [details]
Patch

[WebKit Review Bot]

Attachment 182124 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit/win/WebDownload.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/win/DefaultDownloadDelegate.h:32:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 182340 [details]
Patch

The reason for these compile errors might be that the function

void CFURLConnectionSetDelegateDispatchQueue(CFURLConnectionRef conn, dispatch_queue_t q)

which uses the type dispatch_queue_t, has been added to WebKitLibraries/win/include/CFNetwork/CFURLConnection.h.
It seems my WebKitSupportLibraryVersion is 4.

[Darin Adler]

Comment on attachment 182340 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182340&action=review

Fine to add these includes. But none of these includes should be inside #if. They are all safe to do on non-Windows platforms so they should be unconditional.

> Source/WebCore/platform/network/cf/AuthenticationCF.cpp:39
> +#endif
>  // This header must come before all other CFNetwork headers to work around a CFNetwork bug. It can

Missing blank line here.

[Darin Adler]

(In reply to comment #4)
> The reason for these compile errors might be that the function
> 
> void CFURLConnectionSetDelegateDispatchQueue(CFURLConnectionRef conn, dispatch_queue_t q)
> 
> which uses the type dispatch_queue_t, has been added to WebKitLibraries/win/include/CFNetwork/CFURLConnection.h.
> It seems my WebKitSupportLibraryVersion is 4.

The correct fix is for that header file to include the correct header. We should see if we can get this fixed instead of working around it in WebKit.

Created attachment 205244 [details]
Patch

(In reply to comment #5)
> (From update of attachment 182340 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182340&action=review
> 

Thanks for reviewing ;)

I removed the platform ifdefs in the new patch.

> Fine to add these includes. But none of these includes should be inside #if. They are all safe to do on non-Windows platforms so they should be unconditional.
> 
> > Source/WebCore/platform/network/cf/AuthenticationCF.cpp:39
> > +#endif
> >  // This header must come before all other CFNetwork headers to work around a CFNetwork bug. It can
> 
> Missing blank line here.

[Brent Fulgham]

Is this something you see on a WinCairo build? Because the Windows builds all seem to be working properly without this patch.

(In reply to comment #9)
> Is this something you see on a WinCairo build? Because the Windows builds all seem to be working properly without this patch.

No, this is in the WinApple build. I haven't been able to build WinApple for a long time because of these compile errors. I'm not sure why I am the only one with these problems. Maybe I have the wrong version of the file WebKitLibraries/win/include/CFNetwork/CFURLConnection.h, as I mentioned in an earlier comment. The function void CFURLConnectionSetDelegateDispatchQueue(CFURLConnectionRef conn, dispatch_queue_t q) uses the type dispatch_queue_t, which is not declared at this point. I'm on  WebKitSupportLibraryVersion 4, it seems.

[Roger Fong]

I updated the downloadable support libraries recently (yesterday actually), on developer.apple.com/opensource.

Care to try those new ones out?

(In reply to comment #11)
> I updated the downloadable support libraries recently (yesterday actually), on developer.apple.com/opensource.
> 
> Care to try those new ones out?

Thanks, I'll try them out ;)

[Vivek Galatage]

(In reply to comment #11)
> I updated the downloadable support libraries recently (yesterday actually), on developer.apple.com/opensource.
> 
> Care to try those new ones out?

I tried to downloading the WebKitSupportLibs.zip and tried extracting it using the script ./Tools/Scripts/update-webkit-support-libs. It throws out-of-date error to re-download it again. So when I tried to cross verify what the error was, I got this:

The WebKitSupportLibraryVersion included in the zip file has the value '5' whereas https://developer.apple.com/opensource/internet/WebKitSupportLibraryVersion return '4' due to which the installation fails.

Also I am getting a lot of errors while building (due to new missing headers in the WebKitSupportLibs) as -

Cannot open include file: 'Security/Security.h': No such file or directory
Cannot open include file: 'ImageIO/ImageIOBase.h': No such file or directory

I think there is some issue with the version.

[Vivek Galatage]

(In reply to comment #13)
> (In reply to comment #11)
> > I updated the downloadable support libraries recently (yesterday actually), on developer.apple.com/opensource.
> > 
> > Care to try those new ones out?
> 
> I tried to downloading the WebKitSupportLibs.zip and tried extracting it using the script ./Tools/Scripts/update-webkit-support-libs. It throws out-of-date error to re-download it again. So when I tried to cross verify what the error was, I got this:
> 
> The WebKitSupportLibraryVersion included in the zip file has the value '5' whereas https://developer.apple.com/opensource/internet/WebKitSupportLibraryVersion return '4' due to which the installation fails.
> 
> Also I am getting a lot of errors while building (due to new missing headers in the WebKitSupportLibs) as -
> 
> Cannot open include file: 'Security/Security.h': No such file or directory
> Cannot open include file: 'ImageIO/ImageIOBase.h': No such file or directory
> 
> I think there is some issue with the version.


Created a separate bug for the above mentioned error here: https://bugs.webkit.org/show_bug.cgi?id=118229

[Alexey Proskuryakov]

Comment on attachment 205244 [details]
Patch

Can it be added to some shared prefix header instead, to better match Mac?

Created attachment 205910 [details]
Patch

(In reply to comment #15)
> (From update of attachment 205244 [details])
> Can it be added to some shared prefix header instead, to better match Mac?

Thanks, that's a good idea, updated patch.

[Alexey Proskuryakov]

Why did you choose config.h, and not WebCore/WebCorePrefix.h?

Just curious, I'll leave final review to Brent and Roger.

Created attachment 206307 [details]
Patch

(In reply to comment #18)
> Why did you choose config.h, and not WebCore/WebCorePrefix.h?
> 

I agree, WebCorePrefix.h is a better place for it, updated patch ;)

[Brent Fulgham]

Comment on attachment 206307 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 206307 [details]
Patch

Clearing flags on attachment: 206307

Committed r152495: <http://trac.webkit.org/changeset/152495>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

(In reply to comment #21)
> (From update of attachment 206307 [details])
> r=me

Thanks for reviewing :)