Bug 202400 – Use strongly typed identifiers for webrtc sockets

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

Use strongly typed identifiers for webrtc sockets

[youenn fablet]

Created attachment 379896 [details]
Patch

[youenn fablet]

Created attachment 379897 [details]
Patch

[youenn fablet]

Created attachment 379898 [details]
Patch

[youenn fablet]

Created attachment 379911 [details]
Patch

[youenn fablet]

Any idea why GTK bot has some issues?

[Chris Dumez]

Created attachment 379934 [details]
Patch

[Chris Dumez]

(In reply to youenn fablet from comment #5)
> Any idea why GTK bot has some issues?

Attempted to re-upload the patch to see if it fixes it. The error seems to refer to outdated code...

[Zan Dobersek]

I think the problem is with the `using namespace WebCore` statements being placed outside of the WebKit namespace. With unified sources, the modified source files get clobbered together with others, and the additional namespace leaks produce collisions with names coming from various X11 headers.

AFAIK in general the `using namespace` statements have to be limited to some other namespace for the unified sources build to avoid errors like these, so I'd recommend shifting those statements a few lines downwards.

[Chris Dumez]

Comment on attachment 379934 [details]
Patch

r=me but please make GTK port happy before landing. Zan is likely right about the naming conflict due to your new "using namespace WebCore;".

[youenn fablet]

(In reply to Zan Dobersek from comment #8)
> I think the problem is with the `using namespace WebCore` statements being
> placed outside of the WebKit namespace. With unified sources, the modified
> source files get clobbered together with others, and the additional
> namespace leaks produce collisions with names coming from various X11
> headers.
> 
> AFAIK in general the `using namespace` statements have to be limited to some
> other namespace for the unified sources build to avoid errors like these, so
> I'd recommend shifting those statements a few lines downwards.

Thanks, will do!

[youenn fablet]

Created attachment 379998 [details]
Patch for landing

[youenn fablet]

Created attachment 379999 [details]
Patch for landing

[Zan Dobersek]

Remaining WPE build failure is due to LibWebRTCSocketIdentifier being used in LibWebRTCSocketFactory.cpp without the using-namespace declaration for WebCore. Unclear to me why only WPE trips up on it.

[youenn fablet]

Latest wpe error is related to missing symbols:
[3210/3305] Linking CXX shared library lib/libWPEWebKit-1.0.so.3.6.0
WebKit::NetworkProcess::NetworkProcess(WebKit::AuxiliaryProcessInitializationParameters&&): error: undefined reference to 'WebKit::LegacyCustomProtocolManager::LegacyCustomProtocolManager(WebKit::NetworkProcess&)'
Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-sources/UnifiedSource-72468c22-2.cpp.o:UnifiedSource-72468c22-2.cpp:function WebKit::NetworkProcess::NetworkProcess(WebKit::AuxiliaryProcessInitializationParameters&&): error: undefined reference to 'WebKit::LegacyCustomProtocolManager::supplementName()'
Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-sources/UnifiedSource-88d1702b-20.cpp.o:UnifiedSource-88d1702b-20.cpp:function WebKit::NetworkProcessProxy::didClose(IPC::Connection&): error: undefined reference to 'WebKit::LegacyCustomProtocolManagerProxy::invalidate()'
Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-sources/UnifiedSource-88d1702b-20.cpp.o:UnifiedSource-88d1702b-20.cpp:function non-virtual thunk to WebKit::NetworkProcessProxy::didClose(IPC::Connection&): error: undefined reference to 'WebKit::LegacyCustomProtocolManagerProxy::invalidate()'
Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-sources/UnifiedSource-88d1702b-20.cpp.o:UnifiedSource-88d1702b-20.cpp:function WebKit::NetworkProcessProxy::NetworkProcessProxy(WebKit::WebProcessPool&): error: undefined reference to 'WebKit::LegacyCustomProtocolManagerProxy::LegacyCustomProtocolManagerProxy(WebKit::NetworkProcessProxy&)'
Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-sources/UnifiedSource-88d1702b-20.cpp.o:UnifiedSource-88d1702b-20.cpp:function WebKit::NetworkProcessProxy::~NetworkProcessProxy(): error: undefined reference to 'WebKit::LegacyCustomProtocolManagerProxy::~LegacyCustomProtocolManagerProxy()'
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

This seems related with https://trac.webkit.org/changeset/250597.
I'll land this patch and will see if it breaks WPE/GTK on webkit console.

[Carlos Garcia Campos]

(In reply to youenn fablet from comment #14)
> Latest wpe error is related to missing symbols:
> [3210/3305] Linking CXX shared library lib/libWPEWebKit-1.0.so.3.6.0
> WebKit::NetworkProcess::NetworkProcess(WebKit::
> AuxiliaryProcessInitializationParameters&&): error: undefined reference to
> 'WebKit::LegacyCustomProtocolManager::LegacyCustomProtocolManager(WebKit::
> NetworkProcess&)'
> Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-
> sources/UnifiedSource-72468c22-2.cpp.o:UnifiedSource-72468c22-2.cpp:function
> WebKit::NetworkProcess::NetworkProcess(WebKit::
> AuxiliaryProcessInitializationParameters&&): error: undefined reference to
> 'WebKit::LegacyCustomProtocolManager::supplementName()'
> Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-
> sources/UnifiedSource-88d1702b-20.cpp.o:UnifiedSource-88d1702b-20.cpp:
> function WebKit::NetworkProcessProxy::didClose(IPC::Connection&): error:
> undefined reference to
> 'WebKit::LegacyCustomProtocolManagerProxy::invalidate()'
> Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-
> sources/UnifiedSource-88d1702b-20.cpp.o:UnifiedSource-88d1702b-20.cpp:
> function non-virtual thunk to
> WebKit::NetworkProcessProxy::didClose(IPC::Connection&): error: undefined
> reference to 'WebKit::LegacyCustomProtocolManagerProxy::invalidate()'
> Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-
> sources/UnifiedSource-88d1702b-20.cpp.o:UnifiedSource-88d1702b-20.cpp:
> function
> WebKit::NetworkProcessProxy::NetworkProcessProxy(WebKit::WebProcessPool&):
> error: undefined reference to
> 'WebKit::LegacyCustomProtocolManagerProxy::
> LegacyCustomProtocolManagerProxy(WebKit::NetworkProcessProxy&)'
> Source/WebKit/CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-
> sources/UnifiedSource-88d1702b-20.cpp.o:UnifiedSource-88d1702b-20.cpp:
> function WebKit::NetworkProcessProxy::~NetworkProcessProxy(): error:
> undefined reference to
> 'WebKit::LegacyCustomProtocolManagerProxy::
> ~LegacyCustomProtocolManagerProxy()'
> collect2: error: ld returned 1 exit status
> ninja: build stopped: subcommand failed.
> 
> This seems related with https://trac.webkit.org/changeset/250597.
> I'll land this patch and will see if it breaks WPE/GTK on webkit console.

Those get fixed by doing a clean build. I don't think it's possible to trigger a clean build in EWS, no?

[WebKit Commit Bot]

Comment on attachment 379999 [details]
Patch for landing

Clearing flags on attachment: 379999

Committed r250599: <https://trac.webkit.org/changeset/250599>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55904598>