Bug 216465 - [CMake] Add TestRunnerShared library target
Summary: [CMake] Add TestRunnerShared library target
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CMake (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-13 12:54 PDT by Fujii Hironori
Modified: 2020-09-16 22:36 PDT (History)
7 users (show)

See Also:


Attachments
WIP patch (27.58 KB, patch)
2020-09-13 18:07 PDT, Fujii Hironori
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP patch (29.55 KB, patch)
2020-09-13 18:38 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (15.75 KB, patch)
2020-09-15 19:04 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (15.71 KB, patch)
2020-09-15 23:55 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch for landing (16.48 KB, patch)
2020-09-16 14:22 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch for landing (16.56 KB, patch)
2020-09-16 17:35 PDT, Fujii Hironori
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2020-09-13 12:54:26 PDT
[WinCairo][CMake] Reenable precompiled header for DRT and WTR after r266988
Comment 1 Fujii Hironori 2020-09-13 18:07:07 PDT
Created attachment 408670 [details]
WIP patch
Comment 2 Fujii Hironori 2020-09-13 18:38:53 PDT
Created attachment 408672 [details]
WIP patch
Comment 3 Fujii Hironori 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
Comment 4 Fujii Hironori 2020-09-15 19:04:04 PDT
Created attachment 408886 [details]
Patch
Comment 5 Fujii Hironori 2020-09-15 23:55:22 PDT
Created attachment 408899 [details]
Patch
Comment 6 Fujii Hironori 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.
Comment 7 Don Olmstead 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.
Comment 8 Fujii Hironori 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.
Comment 9 Fujii Hironori 2020-09-16 14:22:49 PDT
Created attachment 408956 [details]
Patch for landing
Comment 10 Fujii Hironori 2020-09-16 17:35:15 PDT
Created attachment 408968 [details]
Patch for landing
Comment 11 Fujii Hironori 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.
Comment 12 Fujii Hironori 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>
Comment 13 Fujii Hironori 2020-09-16 19:06:07 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Darin Adler 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?
Comment 15 Fujii Hironori 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.
Comment 16 Darin Adler 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.