JDK-8270479 : WebKit 612.1 build fails with Visual Studio 2017
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 8,openjfx11,openjfx18
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2021-07-14
  • Updated: 2021-12-18
  • Resolved: 2021-07-20
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 Other
8u311Fixed openjfx11.0.13Fixed
Related Reports
Relates :  
JDK-8268849 - Update to 612.1 version of WebKit
Relates :  
JDK-8278983 - WebKit 613.1 build fails with Visual Studio 2017
Description
The latest version of WebKit, v612.1, no longer builds with VS 2017. There is one compilation error that is trivial to resolve, but after fixing that locally, it fails during linking.

Here is the compilation error:

$ gradle -PCOMPILE_WEBKIT=true sdk
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1222): error C2398: Element '1': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1229): error C2398: Element '2': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1230): error C2398: Element '2': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1232): error C2398: Element '1': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1232): error C2398: Element '2': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1233): error C2398: Element '2': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1240): error C2398: Element '2': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1249): error C2398: Element '1': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1250): error C2398: Element '1': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1250): error C2398: Element '2': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1252): error C2398: Element '1': conversion from 'const int' to 'float' requires a narrowing conversion
../jfx/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp(1253): error C2398: Element '1': conversion from 'const int' to 'float' requires a narrowing conversion

The fix is simply the following:

--- a/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp
+++ b/modules/javafx.web/src/main/native/Source/WebCore/inspector/InspectorOverlay.cpp
@@ -1195,8 +1195,8 @@ void InspectorOverlay::drawLayoutLabel(GraphicsContext& context, String label, F
     FontCascade font(WTFMove(fontDescription), 0, 0);
     font.update(nullptr);

-    constexpr auto padding = 4;
-    constexpr auto arrowSize = 4;
+    constexpr auto padding = 4.0f;
+    constexpr auto arrowSize = 4.0f;
     float textHeight = font.fontMetrics().floatHeight();
     float textDescent = font.fontMetrics().floatDescent();

After applying that fix, here is the link error that we get:

[1967/1981] Linking CXX shared library bin/jfxwebkit.dll
FAILED: bin/jfxwebkit.dll lib/jfxwebkit.lib 
cmd.exe /C "cd . && jfx/buildSrc/build/build-tools/cmake-3.13.3-win32-x86/cmake-3.13.3-win32-x86/bin/cmake.exe -E vs_link_dll --intdir=Source/WebKitLegacy/CMakeFiles/WebKitLegacy.dir --manifests  -- jfx/buildSrc/build/build-tools/devkit-windows_x64-VS2017-15.9.24+1.0.tar/VC/bin/x64/link.exe /nologo @CMakeFiles/WebKitLegacy.rsp  /out:bin/jfxwebkit.dll /implib:lib/jfxwebkit.lib /pdb:bin/jfxwebkit.pdb /dll /version:0.0 /machine:x64 /DEBUG /OPT:ICF /OPT:REF /INCREMENTAL:NO /INCREMENTAL:NO jfx/modules/javafx.web/build/win/Release/WebCore/obj/version.res   && cd ."
LINK: command "jfx/buildSrc/build/build-tools/devkit-windows_x64-VS2017-15.9.24+1.0.tar/VC/bin/x64/link.exe /nologo @CMakeFiles/WebKitLegacy.rsp /out:bin/jfxwebkit.dll /implib:lib/jfxwebkit.lib /pdb:bin/jfxwebkit.pdb /dll /version:0.0 /machine:x64 /DEBUG /OPT:ICF /OPT:REF /INCREMENTAL:NO /INCREMENTAL:NO jfx/modules/javafx.web/build/win/Release/WebCore/obj/version.res /MANIFEST /MANIFESTFILE:bin/jfxwebkit.dll.manifest" failed (exit code 1169) with the following output:
WebStorageNamespaceProvider.cpp.obj : error LNK2005: "public: static class WebCore::PartialOrdering const WebCore::PartialOrdering::less" (?less@PartialOrdering@WebCore@@2V12@B) already defined in StorageAreaImpl.cpp.obj
WebStorageNamespaceProvider.cpp.obj : error LNK2005: "public: static class WebCore::PartialOrdering const WebCore::PartialOrdering::equivalent" (?equivalent@PartialOrdering@WebCore@@2V12@B) already defined in StorageAreaImpl.cpp.obj
...
WebCoreTestSupport.lib(MockPageOverlay.cpp.obj) : error LNK2005: "public: static class WebCore::PartialOrdering const WebCore::PartialOrdering::greater" (?greater@PartialOrdering@WebCore@@2V12@B) already defined in StorageAreaImpl.cpp.obj
WebCoreTestSupport.lib(MockPageOverlay.cpp.obj) : error LNK2005: "public: static class WebCore::PartialOrdering const WebCore::PartialOrdering::unordered" (?unordered@PartialOrdering@WebCore@@2V12@B) already defined in StorageAreaImpl.cpp.obj
Creating library lib/jfxwebkit.lib and object lib/jfxwebkit.exp
bin/jfxwebkit.dll : fatal error LNK1169: one or more multiply defined symbols found
ninja: build stopped: subcommand failed.

See the attached log for the complete list of link errors.

It's possibly related to the following comment:

https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/WebCore/dom/Node.h#L746

There are two possible solutions for jfx11u:

1. Find a solution to the VS 2017 link issue. We would then fix it in mainline (and 8u) and backport it to 11 along with the backport of WebKit 612.1.
2. Update jfx11u to use VS 2019. This will require solving a long-standing problem where we don't ship the microsoft DLLs in our jmod bundles to avoid the jlink error described in JDK-8207015. Basically our solution to that was to rely on the DLLs in the JDK, which only works if the JDK has the same or newer version of Visual Studio. This means if we update to VS 2019 a jlinked application using JDK 11.0.x and JavaFX 11.0.x will no longer run unless we ship the DLLs (in a separate directory to avoid reintroducing JDK-8207015).

Solution 1 will likely be the easiest, so is probably what we should do for October. We eventually need solution 2 anyway, since it will become increasingly difficult, to build newer WebKit libraries with VS 2017.
Comments
Changeset: 8aaacb5b Author: Kevin Rushforth <kcr@openjdk.org> Date: 2021-07-20 14:44:13 +0000 URL: https://git.openjdk.java.net/jfx/commit/8aaacb5bd5368d544c4c468f85ce9ed4dbf26d07
20-07-2021
It seems that VS 2017 doesn't fully support inline constants, even though it claims to support C++17 which is where they were introduced. They specifically claim that support for inline constant variables was added in VS2017 15.5 [1]. [1] https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-160
14-07-2021
I'm testing a trivial fix now, which is to move the definition of the 4 constant variables in question to Node.cpp as runtime constants.
14-07-2021
The following inline constant variables in Node.h are the ones that the VS 2017 linker is complaining about: inline constexpr PartialOrdering PartialOrdering::less(Type::Less); inline constexpr PartialOrdering PartialOrdering::equivalent(Type::Equivalent); inline constexpr PartialOrdering PartialOrdering::greater(Type::Greater); inline constexpr PartialOrdering PartialOrdering::unordered(Type::Unordered);
14-07-2021