Bug 196845 – WebKit should build successfully even with -DENABLE_UNIFIED_BUILDS=OFF

• Component: New Bugs
• Status: RESOLVED FIXED

[Ross Kirsling]

WebKit should build successfully even with -DENABLE_UNIFIED_BUILDS=OFF

[Ross Kirsling]

Created attachment 367273 [details]
Patch (with linking errors for Tools on WinCairo)

[Ross Kirsling]

Confirmed that this patch builds successfully on GTK. Yay!

Using lld as a linker, this patch almost builds successfully on WinCairo, but some strange linking errors occur at the very end:

> lld-link.exe: error: undefined symbol: __imp_?toJS@WebCore@@YA?AVJSValue@JSC@@PEAVExecState@3@PEAVJSDOMGlobalObject@1@AEAVTextTrackCueGeneric@1@@Z
> >>> referenced by C:\GitHub\neko\WebKitBuild\Release\DerivedSources\WebCore\JSInternals.cpp:6803
> >>>               WebCoreTestSupport.lib(JSInternals.cpp.obj):(?jsInternalsPrototypeFunctionCreateGenericCue@WebCore@@YA_JPEAVExecState@JSC@@@Z)
> 
> 
> lld-link.exe: error: undefined symbol: ?notifyNetworkStateChange@ServiceWorkerThreadProxy@WebCore@@QEAAX_N@Z
> >>> referenced by C:\GitHub\neko\WebKitBuild\Release\WTF\Headers\wtf\Function.h:102
> >>>               WebCoreTestSupport.lib(ServiceWorkerInternals.cpp.obj):(?call@?$CallableWrapper@V<lambda_0>@?0??setOnline@ServiceWorkerInternals@WebCore@@QEAAX_N@Z@@?$Function@$$A6AXXZ@WTF@@UEAAXXZ)
> 
> 
> lld-link.exe: error: undefined symbol: __imp_?parseHEVCCodecParameters@WebCore@@YA?AV?$Optional@UHEVCParameterSet@WebCore@@@WTF@@AEBVString@3@@Z
> >>> referenced by C:\GitHub\neko\Source\WebCore\testing\Internals.cpp:4962
> >>>               WebCoreTestSupport.lib(Internals.cpp.obj):(?parseHEVCCodecParameters@Internals@WebCore@@QEAA?AV?$Optional@UHEVCParameterSet@WebCore@@@WTF@@AEBVString@4@@Z)
> 
> 
> lld-link.exe: error: undefined symbol: __imp_?doDispatchMessageOnFrontendPage@InspectorClient@WebCore@@SAXPEAVPage@2@AEBVString@WTF@@@Z
> >>> referenced by C:\GitHub\neko\Source\WebCore\testing\Internals.cpp:372
> >>>               WebCoreTestSupport.lib(Internals.cpp.obj):(?sendMessageToFrontend@InspectorStubFrontend@WebCore@@EEAAXAEBVString@WTF@@@Z)

I spent the entire afternoon trying to understand how these are possible but I remain confounded.

[Ross Kirsling]

Created attachment 367274 [details]
Patch (fails to link Tools on WinCairo)

[Ryosuke Niwa]

Comment on attachment 367274 [details]
Patch (fails to link Tools on WinCairo)

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

> Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:325
> -inline void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle)
> +void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle)

Why are you removing inline keywords here? That seems unrelated.

> Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.h:29
> +#include <wtf/Forward.h>

Why do we need Forward.h?

[Ross Kirsling]

(In reply to Ryosuke Niwa from comment #4)
> Comment on attachment 367274 [details]
> Patch (fails to link Tools on WinCairo)
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=367274&action=review
> 
> > Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:325
> > -inline void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle)
> > +void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle)
> 
> Why are you removing inline keywords here? That seems unrelated.

All of them are demanded by linking errors like this:

lib/libWebCoreGTK.a(lib/../Source/WebCore/CMakeFiles/WebCore.dir/html/canvas/CanvasRenderingContext2D.cpp.o):CanvasRenderingContext2D.cpp:function WebCore::CanvasRenderingContext2D::font() const: error: undefined reference to 'WebCore::CanvasRenderingContext2DBase::FontProxy::fontDescription() const'

Seems that an inline specifier in an .cpp file interferes with the creation of a symbol.

> > Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.h:29
> > +#include <wtf/Forward.h>
> 
> Why do we need Forward.h?

We need forward declarations for String and CompletionHandler -- I thought this was the preferred style?

[Ryosuke Niwa]

(In reply to Ross Kirsling from comment #5)
> (In reply to Ryosuke Niwa from comment #4)
> > Comment on attachment 367274 [details]
> > Patch (fails to link Tools on WinCairo)
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=367274&action=review
> > 
> > > Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:325
> > > -inline void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle)
> > > +void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle)
> > 
> > Why are you removing inline keywords here? That seems unrelated.
> 
> All of them are demanded by linking errors like this:
> 
> lib/libWebCoreGTK.a(lib/../Source/WebCore/CMakeFiles/WebCore.dir/html/canvas/
> CanvasRenderingContext2D.cpp.o):CanvasRenderingContext2D.cpp:function
> WebCore::CanvasRenderingContext2D::font() const: error: undefined reference
> to 'WebCore::CanvasRenderingContext2DBase::FontProxy::fontDescription()
> const'
> 
> Seems that an inline specifier in an .cpp file interferes with the creation
> of a symbol.

I see.

> > > Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.h:29
> > > +#include <wtf/Forward.h>
> > 
> > Why do we need Forward.h?
> 
> We need forward declarations for String and CompletionHandler -- I thought
> this was the preferred style?


Okay. I just wanted the clarification as to why it's needed. I think a comment explaining why in the change log helps.

[Ross Kirsling]

Absolutely, I'll make the ChangeLog more informative before landing. Thanks!

[Ross Kirsling]

Opened bug 196866 to track the issue mentioned in comment 2.

[Ross Kirsling]

Created attachment 367331 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 367331 [details]
Patch for landing

Clearing flags on attachment: 367331

Committed r244225: <https://trac.webkit.org/changeset/244225>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/49857545>