Bug 21724 – Correct Build Regressions (Win32-Cairo)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brent Fulgham]

The last few months of development have left the Windows Cairo port in bad shape.

The attached patch corrects the build issues.

[Brent Fulgham]

Created attachment 24478 [details]
Patch to correct build regressions in Cairo (Windows) build.

This patch corrects a few build problems due to bit-rot caused by the ongoing development.

[Oliver Hunt]

Comment on attachment 24478 [details]
Patch to correct build regressions in Cairo (Windows) build.

I'm concerned that the project file changes will break the cf build as you replace cf paths with the curl paths instead.  You may want to talk to sfalken about those as he has a much greater understanding of vcproj-fu than i do.  I'd also like you to discuss the changes to WebCore/platform/win/ScrollbarThemeWin.cpp and WebKit/win/WebView.cpp with dhyatt as i suspect they will break the "standard" windows build

[Brent Fulgham]

(In reply to comment #2)
> (From update of attachment 24478 [details] [edit])
> I'm concerned that the project file changes will break the cf build as you
> replace cf paths with the curl paths instead.  You may want to talk to sfalken
> about those as he has a much greater understanding of vcproj-fu than i do.

I am pretty sure these changes are safe.  Visual Studio stores these include paths on a per-configuration basis.  So my modifications for "Debug_Cairo" and "Release_Cairo" should not harm the "Debug" and "Release" targets.  The Cairo variants already have some differences from the 'main' Windows build.

> I'd also like you to discuss the changes to
> WebCore/platform/win/ScrollbarThemeWin.cpp and WebKit/win/WebView.cpp with
> dhyatt as i suspect they will break the "standard" windows build

Sounds good.  I'll see if I can catch sfalken or dhyatt next time they are on line.

Thanks for looking it over! 

[Brent Fulgham]

[11:01am] bfulgham_: dhyatt:  Could you please take a look at https://bugs.webkit.org/attachment.cgi?id=24478, specifically the two small modifications to WebCore/platform/win/ScrollbarThemeWin.cpp and WebKit/win/WebView.cpp?
[11:01am] bfulgham_: dhyatt:  They are needed to address some build problems under Win/Cairo; I *think* they are safe (and correct) for the Apple Windows build, but wanted a second opinion.
[11:03am] dhyatt: bfulgham_: i don't get the change to webview.cpp
[11:29am] bfulgham_: dhyatt:  WebView.cpp is needed because the setShouldPaintNativeControls is #if/def'd in WebCore\page\Settings.h conditionally on SAFARI_THEME.  This just makes WebView match Settings.h; perhaps it should not be conditional in Settings.h?
[11:29am] dhyatt: oh ok
[11:29am] dhyatt: sounds ok

[Adam Roben (:aroben)]

Comment on attachment 24478 [details]
Patch to correct build regressions in Cairo (Windows) build.

 8         * WebCore.vcproj/WebCore.vcproj:  Correct paths for Cairo port.

I think it's helpful to say something along the lines of "Fixed include paths and post-build event for the Debug_Cairo and Release_Cairo configurations and added new Curl files."

 2  * Copyright (C) 2008 Collin Jackson  <[e-mail]>

I don't think this copyright is needed (in DNSCurl.cpp).

 32 struct _CFURLRequest
 33 {
 34    int ignoreMe;
 35 };
 36 
 37 typedef const struct _CFURLRequest* CFURLRequestRef;

I think you can probably just do:

typedef const void* CFURLRequestRef;

Is there a reason why that won't work?

@@ HRESULT WebView::notifyPreferencesChange
41474147     hr = prefsPrivate->shouldPaintNativeControls(&enabled);
41484148     if (FAILED(hr))
41494149         return hr;
 4150 #if USE(SAFARI_THEME)
41504151     settings->setShouldPaintNativeControls(!!enabled);
 4152 #endif

Should we skip the prefsPrivate call when SAFARI_THEME is disabled, too?

r=me, even if you don't address the above.

[Brent Fulgham]

Created attachment 24563 [details]
Patch to correct build regressions in Cairo (Windows) build.

Revised patch based on aroben's comments.

[Adam Roben (:aroben)]

Comment on attachment 24563 [details]
Patch to correct build regressions in Cairo (Windows) build.

 59         CFURLRequestRef cfURLRequest() const   // For compatibility
 60         {
 61            return 0;
 62         }

Normally we put such small inline functions like this on a single line.

r=me

[Adam Roben (:aroben)]

Landed in r37797

[Mihnea Ovidenie]

(In reply to comment #6)
> Created an attachment (id=24563) [edit]
> Patch to correct build regressions in Cairo (Windows) build.
> 
> Revised patch based on aroben's comments.
> 
Hello Brent,
I was trying to build WebKit on windows without Cairo without much success. Should i assume from your patch that you were able to do that? Aside from mentioning --cairo-win32 on the build command, i was modifying WebCore/config.h to define WTF_PLATFORM_CAIRO 1 and undefine WTF_PLATFORM_CG. However, these changes triggered some compile errors in WebCore files. Can you give some insight about your building process?
Regards,
Mihnea

[Brent Fulgham]

(In reply to comment #9)
> I was trying to build WebKit on windows without Cairo without much success.
> Should i assume from your patch that you were able to do that? Aside from
> mentioning --cairo-win32 on the build command, i was modifying WebCore/config.h
> to define WTF_PLATFORM_CAIRO 1 and undefine WTF_PLATFORM_CG. However, these
> changes triggered some compile errors in WebCore files. Can you give some
> insight about your building process?

I have forwarded you a copy of my entire diff against the current source tree.  The things you are probably missing are:

1.  You need the various patches in https://bugs.webkit.org/show_bug.cgi?id=17484.
2.  I have a modified WebCore/config.h:

Index: WebCore/config.h
===================================================================
--- WebCore/config.h	(revision 37798)
+++ WebCore/config.h	(working copy)
@@ -96,9 +96,16 @@
 #endif
 
 #if PLATFORM(WIN)
+#if 0
 #define WTF_PLATFORM_CG 1
 #undef WTF_PLATFORM_CAIRO
 #define WTF_USE_CFNETWORK 1
+#else
+#undef WTF_PLATFORM_CG
+#define WTF_PLATFORM_CAIRO 1
+#undef WTF_USE_CFNETWORK
+#define WTF_USE_CURL 1
+#endif
 #undef WTF_USE_WININET
 #define WTF_PLATFORM_CF 1
 #define WTF_USE_PTHREADS 0

-----------------------------------------------