Bug 202400

Summary:

Use strongly typed identifiers for webrtc sockets

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

WebRTC

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

annulen, cdumez, cgarcia, commit-queue, eric.carlson, ews-watchlist, glenn, gyuyoung.kim, hta, jer.noble, philipj, rakuco, ryuan.choi, sergio, tommyw, webkit-bug-importer, youennf, zan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

202403

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch for landing	none
Patch for landing	none

youenn fablet

Reported 2019-10-01 05:20:16 PDT

Use strongly typed identifiers for webrtc sockets

Attachments
Patch (43.36 KB, patch) 2019-10-01 05:32 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (43.36 KB, patch) 2019-10-01 06:17 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (43.59 KB, patch) 2019-10-01 06:26 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (43.45 KB, patch) 2019-10-01 09:31 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (43.42 KB, patch) 2019-10-01 12:51 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch for landing (43.21 KB, patch) 2019-10-02 01:01 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch for landing (43.26 KB, patch) 2019-10-02 01:28 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2019-10-01 05:32:11 PDT

Created attachment 379896 [details] Patch

youenn fablet

Comment 2 2019-10-01 06:17:43 PDT

Created attachment 379897 [details] Patch

youenn fablet

Comment 3 2019-10-01 06:26:05 PDT

Created attachment 379898 [details] Patch

youenn fablet

Comment 4 2019-10-01 09:31:19 PDT

Created attachment 379911 [details] Patch

youenn fablet

Comment 5 2019-10-01 10:36:15 PDT

Any idea why GTK bot has some issues?

Chris Dumez

Comment 6 2019-10-01 12:51:55 PDT

Created attachment 379934 [details] Patch

Chris Dumez

Comment 7 2019-10-01 12:52:25 PDT

(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

Comment 8 2019-10-01 13:04:34 PDT

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 9 2019-10-01 13:08:13 PDT

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

Comment 10 2019-10-02 00:48:10 PDT

(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

Comment 11 2019-10-02 01:01:04 PDT

Created attachment 379998 [details] Patch for landing

youenn fablet

Comment 12 2019-10-02 01:28:47 PDT

Created attachment 379999 [details] Patch for landing

Zan Dobersek

Comment 13 2019-10-02 01:39:14 PDT

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

Comment 14 2019-10-02 01:58:13 PDT

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

Comment 15 2019-10-02 02:12:54 PDT

(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 16 2019-10-02 02:42:06 PDT

Comment on attachment 379999 [details] Patch for landing Clearing flags on attachment: 379999 Committed r250599: <https://trac.webkit.org/changeset/250599>

WebKit Commit Bot

Comment 17 2019-10-02 02:42:08 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 18 2019-10-02 02:43:20 PDT

<rdar://problem/55904598>

Note You need to log in before you can comment on or make changes to this bug.