Bug 203663

Summary: [Win][CMake] Build WebCore as an OBJECT library for WinCairo port
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: CMakeAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, bfulgham, commit-queue, don.olmstead, ews-watchlist, gyuyoung.kim, pvollan, ross.kirsling, ryuan.choi, sergio
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP patch for approach #4
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Fujii Hironori 2019-10-30 22:15:58 PDT
[WinCairo] error LNK2019: unresolved external symbol "__declspec(dllimport) public: void __cdecl WebCore::ServiceWorkerThreadProxy::notifyNetworkStateChange(bool)"

WinCairo clang-cl Debug/Release builds can't link DumpRenderTreeLib.dll.

>    Creating library lib64\DumpRenderTreeLib.lib and object lib64\DumpRenderTreeLib.exp
> WebCoreTestSupport.lib(ServiceWorkerInternals.cpp.obj) : error LNK2019: unresolved external symbol "__declspec(dllimport) public: void __cdecl WebCore::ServiceWorkerThreadProxy::notifyNetworkStateChange(bool)" (__imp_?notifyNetworkStateChange@ServiceWorkerThreadProxy@WebCore@@QEAAX_N@Z) referenced in function "public: <auto> __cdecl `public: void __cdecl WebCore::ServiceWorkerInternals::setOnline(bool)'::`1'::<lambda_0>::operator()(void)const " (??R<lambda_0>@?0??setOnline@ServiceWorkerInternals@WebCore@@QEAAX_N@Z@QEBA?A?<auto>@@XZ)
> bin64\DumpRenderTreeLib.dll : fatal error LNK1120: 1 unresolved externals
> [85/214] Building CXX object Tools\TestWebKitAPI\CMakeFiles\TestWebCoreLib.dir\WTFStringUtilities.cpp.obj
> [86/214] Building CXX object Tools\TestWebKitAPI\CMakeFiles\TestWebCoreLib.dir\TestsController.cpp.obj
> [87/214] Building CXX object Source\WebKit\CMakeFiles\WebKit.dir\__\__\WebKit\DerivedSources\unified-sources\UnifiedSource-549

r251774: OK
r251799: NG
Comment 1 Fujii Hironori 2019-10-30 22:22:59 PDT
Any methods of ServiceWorkerThreadProxy which are marked with
WEBCORE_EXPORT and WEBCORE_TESTSUPPORT_EXPORT aren't exported
from WebKit.dll because any symbols in a object file of
ServiceWorkerThreadProxy.cpp are not referenced in WebKit.dll.

This is a similar issue with Bug 196866.

There are 3 ways to solve:

1. bundle SWContextManager.cpp and ServiceWorkerThreadProxy.cpp as Unified source build
2. Use OBJECT library for WebCore (Bug 196866 Comment 4)
3. Use /WHOLEARCHIVE:WebCore.lib (Bug 196866 Comment 4)
4. Uninline some method of ServiceWorkerThreadProxy which are used by SWContextManager
Comment 2 Fujii Hironori 2019-10-30 22:23:34 PDT
Created attachment 382428 [details]
WIP patch for approach #4
Comment 3 Fujii Hironori 2019-10-30 22:34:52 PDT
I prefer OBJECT library approach because it doesn't generate unnecessary WebCore.lib and WebCoreTestSupport.lib, and it can really solve this issue.
However, a caveat of is Apple Internal builds can't use OBJECT library for WebCore because it doesn't generate the static libs.
I'm going to use OBJECT library only for non-intenal builds.
Comment 4 Fujii Hironori 2019-10-30 22:50:39 PDT
Linking TestWebCoreLib.dll reports a lot of warnings because
WebCoreTestSupport is importing WebCore symbols by using dllimport,
but those symbols are not exported by WebCore itself.

[11/11] Linking CXX shared library bin64\TestWebCoreLib.dll
   Creating library lib64\TestWebCoreLib.lib and object lib64\TestWebCoreLib.exp
LINK : warning LNK4217: symbol '?mockScrollbarsEnabled@DeprecatedGlobalSettings@WebCore@@SA_NXZ (public: static bool __cdecl WebCore::DeprecatedGlobalSettings::mockScrollbarsEnabled(void))' defined in 'UnifiedSource-767013ce-3.cpp.obj' is imported by 'InternalSettings.cpp.obj' in function '"public: __cdecl WebCore::InternalSettings::Backup::Backup(class WebCore::Settings &)" (??0Backup@InternalSettings@WebCore@@QEAA@AEAVSettings@2@@Z)'
LINK : warning LNK4217: symbol '?areImagesEnabled@Settings@WebCore@@QEBA_NXZ (public: bool __cdecl WebCore::Settings::areImagesEnabled(void)const )' defined in 'Settings.cpp.obj' is imported by 'InternalSettings.cpp.obj' in function '"public: __cdecl WebCore::InternalSettings::Backup::Backup(class WebCore::Settings &)" (??0Backup@InternalSettings@WebCore@@QEAA@AEAVSettings@2@@Z)'
LINK : warning LNK4286: symbol '?areImagesEnabled@Settings@WebCore@@QEBA_NXZ (public: bool __cdecl WebCore::Settings::areImagesEnabled(void)const )' defined in 'Settings.cpp.obj' is imported by 'InternalSettingsGenerated.cpp.obj'
LINK : warning LNK4217: symbol '?shouldDeactivateAudioSession@PlatformMediaSessionManager@WebCore@@SA_NXZ (public: static bool __cdecl WebCore::PlatformMediaSessionManager::shouldDeactivateAudioSession(void))' defined in 'UnifiedSource-3c72abbe-12.cpp.obj' is imported by 'InternalSettings.cpp.obj' in function '"public: __cdecl WebCore::InternalSettings::Backup::Backup(class WebCore::Settings &)" (??0Backup@InternalSettings@WebCore@@QEAA@AEAVSettings@2@@Z)'
(...)


TestWebCoreLib shouldn't link WebCoreTestSupport.
Comment 5 Fujii Hironori 2019-10-30 23:26:00 PDT
Created attachment 382431 [details]
Patch
Comment 6 Fujii Hironori 2019-10-31 02:19:32 PDT
GTK and WPE port EWS can't compile TestWebCore.

> DerivedSources/ForwardingHeaders/WebCore/GUniquePtrSoup.h:26:10: fatal error: libsoup/soup.h: No such file or directory

> DerivedSources/ForwardingHeaders/WebCore/HbUniquePtr.h:29:10: fatal error: hb.h: No such file or directory

WebCoreTestSupport has a public include path to WebCore_SYSTEM_INCLUDE_DIRECTORIES.

> target_include_directories(WebCoreTestSupport SYSTEM PUBLIC ${WebCoreTestSupport_SYSTEM_INCLUDE_DIRECTORIES} ${WebCore_SYSTEM_INCLUDE_DIRECTORIES})

Hmm..
Comment 7 Fujii Hironori 2019-10-31 04:25:30 PDT
Created attachment 382451 [details]
Patch
Comment 8 Fujii Hironori 2019-10-31 21:43:29 PDT
Committed r251908: <https://trac.webkit.org/changeset/251908>
Comment 9 Fujii Hironori 2019-10-31 21:44:38 PDT
committed a workaround patch.
Comment 10 Fujii Hironori 2019-10-31 23:04:41 PDT
Created attachment 382562 [details]
Patch
Comment 11 Ross Kirsling 2019-11-01 10:40:06 PDT
Comment on attachment 382562 [details]
Patch

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

> Source/WebCore/CMakeLists.txt:1237
> +if (NOT WTF_OS_WINDOWS)
> +    list(APPEND WebCoreTestSupport_LIBRARIES WebCore)
> +endif ()

Is this part not needed for Apple-internal Windows builds?
Comment 12 Fujii Hironori 2019-11-04 19:46:21 PST
Comment on attachment 382562 [details]
Patch

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

>> Source/WebCore/CMakeLists.txt:1237
>> +endif ()
> 
> Is this part not needed for Apple-internal Windows builds?

Nope. Apple internal builds invokes CMake under each sub-directory. This is irrelevant for it.
Comment 13 Fujii Hironori 2019-11-04 19:46:48 PST
AppleWin EWS failed:

>   WebCoreTestSupport.vcxproj -> C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCoreTestSupport.dir\Release\WebCoreTestSupport.lib
>      Creating library C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/WebKit.lib and object C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/WebKit.exp
> StructuredExceptionHandlerSuppressor.obj : error LNK2019: unresolved external symbol _exceptionHandlerThunk@0 referenced in function "public: __thiscall WebCore::StructuredExceptionHandlerSuppressor::StructuredExceptionHandlerSuppressor(struct WebCore::ExceptionRegistration &)" (??0StructuredExceptionHandlerSuppressor@WebCore@@QAE@AAUExceptionRegistration@1@@Z) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebKitLegacy\WebKitLegacy.vcxproj]
> C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\bin32\WebKit.dll : fatal error LNK1120: 1 unresolved externals [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebKitLegacy\WebKitLegacy.vcxproj]
Comment 14 Fujii Hironori 2019-11-04 19:55:38 PST
Created attachment 382801 [details]
Patch
Comment 15 Fujii Hironori 2019-11-05 03:01:44 PST
AppleWin build fails because WebCore Object library is missing makesafeseh.obj.

> if (CMAKE_SIZEOF_VOID_P EQUAL 4)
>     list(APPEND WebCore_SOURCES ${WebCore_DERIVED_SOURCES_DIR}/makesafeseh.obj)
>     add_custom_command(
>         OUTPUT ${WebCore_DERIVED_SOURCES_DIR}/makesafeseh.obj
>         DEPENDS ${WEBCORE_DIR}/platform/win/makesafeseh.asm
>         COMMAND ml /safeseh /c /Fo ${WebCore_DERIVED_SOURCES_DIR}/makesafeseh.obj ${WEBCORE_DIR}/platform/win/makesafeseh.asm
>         VERBATIM)
> endif ()

If enable_language(ASM_MASM) is used, the problem of r194434 will reoccur.
Comment 16 Fujii Hironori 2019-11-05 03:18:18 PST
Created attachment 382815 [details]
Patch
Comment 17 WebKit Commit Bot 2019-11-05 14:57:53 PST
Comment on attachment 382815 [details]
Patch

Clearing flags on attachment: 382815

Committed r252086: <https://trac.webkit.org/changeset/252086>
Comment 18 WebKit Commit Bot 2019-11-05 14:57:55 PST
All reviewed patches have been landed.  Closing bug.
Comment 19 Fujii Hironori 2019-11-05 18:04:41 PST
Committed r252118: <https://trac.webkit.org/changeset/252118>
Comment 20 Fujii Hironori 2019-11-05 21:55:53 PST
r252086 causes a bug that WinCairo DumpRenderTree.exe and WebKitTestRunner.exe can't start at all.

  Bug 203879 – [Win] DumpRenderTree.exe and WebKitTestRunner.exe shouldn't link with WebCoreTestSupport