Bug 177202 – [WinCairo] Fix forwarding header conflict of wincairo webkit

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Yousuke Kimoto]

PlatformWin.cmake in JavaScript, WTF and WebCore copy simply header to directories which are called forwarding headers,
but for webkit build, many redefinition errors occur due to loading the same file which is detected on two different paths.
To avoid such errors, two ways to copy headers are defined; one is for WebKitLegacy and  the other one is for WebKit.

[Yousuke Kimoto]

Created attachment 322314 [details]
[PATCH] To generate a solution file for WinCairo WebKit

[Yousuke Kimoto]

Comment on attachment 322314 [details]
[PATCH] To generate a solution file for WinCairo WebKit

The attached patch has gotten old, I'll refine it.

[Fujii Hironori]

Created attachment 332216 [details]
WIP patch

[Fujii Hironori]

Created attachment 332218 [details]
WIP patch

[Fujii Hironori]

Filed Bug 182081 for the template friend class NeverDestroyed issue.

[Basuke Suzuki]

Created attachment 332420 [details]
PATCH

[Basuke Suzuki]

We've modified the cmake file to copy headers simply as Mac port do.

[Alex Christensen]

Comment on attachment 332420 [details]
PATCH

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

> Source/WebKit/PlatformWin.cmake:89
> +    "${WEBKIT_DIR}/UIProcess/WebCoreSupport/curl"

This should go in PlatformWinCairo.cmake.

> Source/WebKit/PlatformWin.cmake:164
> +set(WIN32 0)
> +WEBKIT_CREATE_FORWARDING_HEADERS(WebKit DIRECTORIES ${WebKit_FORWARDING_HEADERS_DIRECTORIES})
> +set(WIN32 1)

This is no good.

[Basuke Suzuki]

(In reply to Alex Christensen from comment #8)
> Comment on attachment 332420 [details]
> PATCH
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332420&action=review
> 
> > Source/WebKit/PlatformWin.cmake:89
> > +    "${WEBKIT_DIR}/UIProcess/WebCoreSupport/curl"
> 
> This should go in PlatformWinCairo.cmake.

I don't know the detail, but after including WebKitCommon, whole project shares same PlatformXXX.cmake file among AppleWin and WinCairo. Is it okay to put this in a same file within if wincairo block?


> > Source/WebKit/PlatformWin.cmake:164
> > +set(WIN32 0)
> > +WEBKIT_CREATE_FORWARDING_HEADERS(WebKit DIRECTORIES ${WebKit_FORWARDING_HEADERS_DIRECTORIES})
> > +set(WIN32 1)
> 
> This is no good.

I know ... And I knew you say so.

[Basuke Suzuki]

Created attachment 332453 [details]
PATCH

[EWS Watchlist]

Comment on attachment 332453 [details]
PATCH

Attachment 332453 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/6229686

New failing tests:
compositing/debug-borders-dynamic.html

[EWS Watchlist]

Created attachment 332459 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[Basuke Suzuki]

Created attachment 332461 [details]
patch again

[Yousuke Kimoto]

Created attachment 332537 [details]
[PATCH] To build wincairo webkit

I modified basuke's patch to build wincairo webkit. The following command is to generate the build configuration files.

> perl Tools\Scripts\build-webkit --no-ninja --wincairo --64-bit --cmakeargs="-DENABLE_WEBKIT_LEGACY=OFF" --generate-project-only

Currently many compilation error occur because Windows SDK doesn't match the SDK like UNIX.

Basuke, could you check this patch?

[Basuke Suzuki]

Comment on attachment 332537 [details]
[PATCH] To build wincairo webkit

I don't agree with your change on this patch. Departing from Win is good, but not like this. With this patch, PlatformWinCairo is loaded by PlatformWin. It should be included by WebKitCommon as others are. So that means if we depart from Win, other projects also be separated because WebKitCommon is shared by every project. And basically that is the right direction. We'd better to open another bug for that objective.

[Alex Christensen]

Comment on attachment 332461 [details]
patch again

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

> Source/WebCore/PlatformWin.cmake:245
> +        Modules/applepay

We shouldn't need to copy apple pay headers on windows.

> Source/WebKit/ChangeLog:10
> [phone]  Basuke Suzuki  <[e-mail]>

duplicate changelog entry.

[Basuke Suzuki]

Created attachment 332558 [details]
Fix

Fixed.

[Basuke Suzuki]

The bug for separating PlatformWinCairo was filed here. https://bugs.webkit.org/show_bug.cgi?id=182254

[Alex Christensen]

Comment on attachment 332558 [details]
Fix

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

> Source/cmake/WebKitMacros.cmake:-184
> -    # On Windows, we copy the entire contents of forwarding headers.

We still need to copy the entire contents of the forwarding headers in the AppleWin internal build.

[Basuke Suzuki]

Created attachment 332605 [details]
Switch to use WEBKIT_MAKE_FORWARDING_HEADERS macro

We switched to new WEBKIT_MAKE_FORWARDING_HEADERS which AppleWin does not use at all. Also this is in WebKit so that also AppleWin won't be affected.

[WebKit Commit Bot]

Comment on attachment 332605 [details]
Switch to use WEBKIT_MAKE_FORWARDING_HEADERS macro

Clearing flags on attachment: 332605

Committed r227778: <https://trac.webkit.org/changeset/227778>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37023603>

[Yusuke Suzuki]

Committed r227783: <https://trac.webkit.org/changeset/227783>