RESOLVED FIXED 216465
[CMake] Add TestRunnerShared library target
https://bugs.webkit.org/show_bug.cgi?id=216465
Summary [CMake] Add TestRunnerShared library target
Fujii Hironori
Reported 2020-09-13 12:54:26 PDT
[WinCairo][CMake] Reenable precompiled header for DRT and WTR after r266988
Attachments
WIP patch (27.58 KB, patch)
2020-09-13 18:07 PDT, Fujii Hironori
ews-feeder: commit-queue-
WIP patch (29.55 KB, patch)
2020-09-13 18:38 PDT, Fujii Hironori
no flags
Patch (15.75 KB, patch)
2020-09-15 19:04 PDT, Fujii Hironori
no flags
Patch (15.71 KB, patch)
2020-09-15 23:55 PDT, Fujii Hironori
no flags
Patch for landing (16.48 KB, patch)
2020-09-16 14:22 PDT, Fujii Hironori
no flags
Patch for landing (16.56 KB, patch)
2020-09-16 17:35 PDT, Fujii Hironori
ews-feeder: commit-queue-
Fujii Hironori
Comment 1 2020-09-13 18:07:07 PDT
Created attachment 408670 [details] WIP patch
Fujii Hironori
Comment 2 2020-09-13 18:38:53 PDT
Created attachment 408672 [details] WIP patch
Fujii Hironori
Comment 3 2020-09-13 20:17:04 PDT
I'm going to split the patch to decouple TestRunnerShared from WTR and DRT. Bug 216470 – [TestRunnerShared] Make UIScriptContext not directly call UIScriptController::create which are defined in DRT and WTR Bug 216469 – [TestRunnerShared] Make UIScriptController::windowIsKey and UIScriptController::setWindowIsKey virtual
Fujii Hironori
Comment 4 2020-09-15 19:04:04 PDT
Fujii Hironori
Comment 5 2020-09-15 23:55:22 PDT
Fujii Hironori
Comment 6 2020-09-16 00:59:05 PDT
AppleWin EWS failed. > LINK : fatal error LNK1181: cannot open input file 'C:\cygwin\home\buildbot\worker\Windows-EWS\build\WebKitBuild\Release\Tools\TestRunnerShared\TestRunnerShared.dir\Release\ReftestFunctions.obj' [C:\cygwin\home\buildbot\worker\Windows-EWS\build\WebKitBuild\Release\Tools\DumpRenderTree\DumpRenderTreeLib.vcxproj] I also saw the same linkage error of AppleWin on my PC for the incremental build, but I confirmed clean build succeeded to link.
Don Olmstead
Comment 7 2020-09-16 11:04:44 PDT
Comment on attachment 408899 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408899&action=review r=me with nits > Source/cmake/WebKitFS.cmake:54 > +set(TestRunnerShared_DERIVED_SOURCES_DIR "${CMAKE_BINARY_DIR}/DerivedSources/TestRunnerShared") For WinCairo can you override this according to the directory structure there? > Tools/DumpRenderTree/CMakeLists.txt:23 > + TestRunnerShared Any reason you're not using WebKit::TestRunnerShared? You declared it by doing WEBKIT_FRAMEWORK.
Fujii Hironori
Comment 8 2020-09-16 14:05:27 PDT
Comment on attachment 408899 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408899&action=review >> Source/cmake/WebKitFS.cmake:54 >> +set(TestRunnerShared_DERIVED_SOURCES_DIR "${CMAKE_BINARY_DIR}/DerivedSources/TestRunnerShared") > > For WinCairo can you override this according to the directory structure there? Will fix. >> Tools/DumpRenderTree/CMakeLists.txt:23 >> + TestRunnerShared > > Any reason you're not using WebKit::TestRunnerShared? You declared it by doing WEBKIT_FRAMEWORK. I need more boilerplate code setting TestRunnerShared_INTERFACE_* variables and adding WEBKIT_FRAMEWORK_TARGET() to use WebKit::TestRunnerShared. It doesn't look nice to me.
Fujii Hironori
Comment 9 2020-09-16 14:22:49 PDT
Created attachment 408956 [details] Patch for landing
Fujii Hironori
Comment 10 2020-09-16 17:35:15 PDT
Created attachment 408968 [details] Patch for landing
Fujii Hironori
Comment 11 2020-09-16 19:02:08 PDT
AppleWin EWS reported some tests crashing. I can reproduce the crash for incremental build of AppleWin, and I confirmed clean build doesn't crash.
Fujii Hironori
Comment 12 2020-09-16 19:06:03 PDT
Comment on attachment 408968 [details] Patch for landing Clearing flags on attachment: 408968 Committed r267177: <https://trac.webkit.org/changeset/267177>
Fujii Hironori
Comment 13 2020-09-16 19:06:07 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 14 2020-09-16 21:47:47 PDT
Comment on attachment 408968 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=408968&action=review > ChangeLog:6 > + WinCairo port is using both DRT and WTR. However, it has a problem Why is it using both? Can we stop using one or the other?
Fujii Hironori
Comment 15 2020-09-16 22:25:32 PDT
Comment on attachment 408968 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=408968&action=review >> ChangeLog:6 >> + WinCairo port is using both DRT and WTR. However, it has a problem > > Why is it using both? > > Can we stop using one or the other? WinCairo port is building both DRT and WTR because it is supporting WK1 and WK2 as well as Mac port is. WinCairo is still maintaining WK1 so that WinCairo developers don't break AppleWin WK1 because AppleWin port is still using WK1.
Darin Adler
Comment 16 2020-09-16 22:36:07 PDT
(In reply to Fujii Hironori from comment #15) > WinCairo port is building both DRT and WTR because it is supporting WK1 and > WK2 as well as Mac port is. Sure, but the Mac and iOS ports are forced to support the legacy WebKit because of all the legacy apps on the Mac and iOS platforms that rely on it. > WinCairo is still maintaining WK1 so that WinCairo developers don't break > AppleWin WK1 because AppleWin port is still using WK1. OK.
Note You need to log in before you can comment on or make changes to this bug.