Retry generating RTCNetwork serialization #18984

achristensen07 · 2023-10-12T05:26:21Z

`1a1ce78`

Retry generating RTCNetwork serialization
https://bugs.webkit.org/show_bug.cgi?id=263060
rdar://116847094

Reviewed by Youenn Fablet.

A few days ago I tried this, but the SocketAddress transformation wasn't lossless so
it was quickly reverted.  This redoes it, but with a very straightforward SocketAddress struct.

* Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/socket_address.h:
* Source/WebKit/CMakeLists.txt:
* Source/WebKit/DerivedSources-input.xcfilelist:
* Source/WebKit/DerivedSources.make:
* Source/WebKit/NetworkProcess/webrtc/NetworkRTCMonitor.cpp:
(WebKit::NetworkManagerWrapper::onNetworksChanged):
* Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:
(WebKit::NetworkRTCProvider::createUDPSocket):
(WebKit::NetworkRTCProvider::createClientTCPSocket):
(WebKit::NetworkRTCProvider::sendToSocket):
* Source/WebKit/Shared/RTCNetwork.cpp:
(WebKit::RTCNetwork::RTCNetwork):
(WebKit::ips):
(WebKit::RTCNetwork::value const):
(WebKit::RTC::Network::IPAddress::IPAddress):
(WebKit::RTC::Network::IPAddress::rtcAddress const):
(WebKit::RTC::Network::SocketAddress::SocketAddress):
(WebKit::RTC::Network::SocketAddress::rtcAddress const):
(WebKit::RTC::Network::InterfaceAddress::InterfaceAddress):
(WebKit::RTC::Network::InterfaceAddress::rtcAddress const):
(WebKit::RTCNetwork::IPAddress::decode): Deleted.
(WebKit::RTCNetwork::IPAddress::encode const): Deleted.
(WebKit::RTCNetwork::SocketAddress::decode): Deleted.
(WebKit::RTCNetwork::SocketAddress::encode const): Deleted.
(WebKit::RTCNetwork::decode): Deleted.
(WebKit::RTCNetwork::encode const): Deleted.
* Source/WebKit/Shared/RTCNetwork.h:
(WebKit::RTC::Network::IPAddress::UnspecifiedFamily::operator== const):
(WebKit::RTC::Network::IPAddress::IPAddress):
(WebKit::RTC::Network::InterfaceAddress::InterfaceAddress):
(WebKit::RTCNetwork::IPAddress::IPAddress): Deleted.
(WebKit::RTCNetwork::SocketAddress::SocketAddress): Deleted.
* Source/WebKit/Shared/RTCNetwork.serialization.in: Added.
* Source/WebKit/WebKit.xcodeproj/project.pbxproj:
* Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetwork.cpp:
(WebKit::LibWebRTCNetwork::signalAddressReady):
(WebKit::LibWebRTCNetwork::signalReadPacket):
* Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetworkManager.cpp:
(WebKit::LibWebRTCNetworkManager::networksChanged):
* Source/WebKit/WebProcess/Network/webrtc/WebRTCResolver.cpp:
(WebKit::WebRTCResolver::setResolvedAddress):

Canonical link: https://commits.webkit.org/269302@main

e6c0609

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 tv-sim
	✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-10-12T05:26:33Z

EWS run on previous version of this PR (hash bb7a931)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/socket_address.h

youennf · 2023-10-12T15:43:03Z

Source/WebKit/Shared/RTCNetwork.serialization.in

+    WebKit::RTC::Network::IPAddress ip;
+    uint16_t port;
+    int scopeID;
+    bool literal;


Why not reuse IsUnresolvedIP here for SocketAddress?
It should allow to map precisely to what the current code does and might not require changing libwebrtc, which is always a bit of a burden.

We could reuse the code from SocketAddress::decode in SocketAddress::rtcAddress()

That caused a test timeout last time for an unknown reason.

webkit-early-warning-system · 2023-10-12T17:59:42Z

EWS run on current version of this PR (hash e6c0609)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 tv-sim
	✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge	✅ 🛠 watch-sim

youennf

LGTM.
Could you file a bug about trying to remove the customer SocketAddress constructor?
I might try to tackle it when I have some time.

https://bugs.webkit.org/show_bug.cgi?id=263060 rdar://116847094 Reviewed by Youenn Fablet. A few days ago I tried this, but the SocketAddress transformation wasn't lossless so it was quickly reverted. This redoes it, but with a very straightforward SocketAddress struct. * Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/socket_address.h: * Source/WebKit/CMakeLists.txt: * Source/WebKit/DerivedSources-input.xcfilelist: * Source/WebKit/DerivedSources.make: * Source/WebKit/NetworkProcess/webrtc/NetworkRTCMonitor.cpp: (WebKit::NetworkManagerWrapper::onNetworksChanged): * Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp: (WebKit::NetworkRTCProvider::createUDPSocket): (WebKit::NetworkRTCProvider::createClientTCPSocket): (WebKit::NetworkRTCProvider::sendToSocket): * Source/WebKit/Shared/RTCNetwork.cpp: (WebKit::RTCNetwork::RTCNetwork): (WebKit::ips): (WebKit::RTCNetwork::value const): (WebKit::RTC::Network::IPAddress::IPAddress): (WebKit::RTC::Network::IPAddress::rtcAddress const): (WebKit::RTC::Network::SocketAddress::SocketAddress): (WebKit::RTC::Network::SocketAddress::rtcAddress const): (WebKit::RTC::Network::InterfaceAddress::InterfaceAddress): (WebKit::RTC::Network::InterfaceAddress::rtcAddress const): (WebKit::RTCNetwork::IPAddress::decode): Deleted. (WebKit::RTCNetwork::IPAddress::encode const): Deleted. (WebKit::RTCNetwork::SocketAddress::decode): Deleted. (WebKit::RTCNetwork::SocketAddress::encode const): Deleted. (WebKit::RTCNetwork::decode): Deleted. (WebKit::RTCNetwork::encode const): Deleted. * Source/WebKit/Shared/RTCNetwork.h: (WebKit::RTC::Network::IPAddress::UnspecifiedFamily::operator== const): (WebKit::RTC::Network::IPAddress::IPAddress): (WebKit::RTC::Network::InterfaceAddress::InterfaceAddress): (WebKit::RTCNetwork::IPAddress::IPAddress): Deleted. (WebKit::RTCNetwork::SocketAddress::SocketAddress): Deleted. * Source/WebKit/Shared/RTCNetwork.serialization.in: Added. * Source/WebKit/WebKit.xcodeproj/project.pbxproj: * Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetwork.cpp: (WebKit::LibWebRTCNetwork::signalAddressReady): (WebKit::LibWebRTCNetwork::signalReadPacket): * Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetworkManager.cpp: (WebKit::LibWebRTCNetworkManager::networksChanged): * Source/WebKit/WebProcess/Network/webrtc/WebRTCResolver.cpp: (WebKit::WebRTCResolver::setResolvedAddress): Canonical link: https://commits.webkit.org/269302@main

webkit-commit-queue · 2023-10-13T16:02:49Z

Committed 269302@main (1a1ce78): https://commits.webkit.org/269302@main

Reviewed commits have been landed. Closing PR #18984 and removing active labels.

achristensen07 · 2023-10-13T16:10:37Z

I filed rdar://116922531 for future investigation

achristensen07 requested a review from cdumez as a code owner October 12, 2023 05:26

achristensen07 self-assigned this Oct 12, 2023

achristensen07 added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Oct 12, 2023

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 12, 2023

youennf reviewed Oct 12, 2023

View reviewed changes

achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Oct 12, 2023

achristensen07 force-pushed the eng/Retry-generating-RTCNetwork-serialization branch from bb7a931 to e6c0609 Compare October 12, 2023 17:59

youennf approved these changes Oct 13, 2023

View reviewed changes

achristensen07 added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Oct 13, 2023

webkit-commit-queue force-pushed the eng/Retry-generating-RTCNetwork-serialization branch from e6c0609 to 1a1ce78 Compare October 13, 2023 16:02

webkit-commit-queue merged commit 1a1ce78 into WebKit:main Oct 13, 2023

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Oct 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retry generating RTCNetwork serialization #18984

Retry generating RTCNetwork serialization #18984

achristensen07 commented Oct 12, 2023 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Oct 12, 2023 •

edited

Loading

youennf Oct 12, 2023

achristensen07 Oct 12, 2023

webkit-early-warning-system commented Oct 12, 2023 •

edited

Loading

youennf left a comment

webkit-commit-queue commented Oct 13, 2023

achristensen07 commented Oct 13, 2023

Retry generating RTCNetwork serialization #18984

Retry generating RTCNetwork serialization #18984

Conversation

achristensen07 commented Oct 12, 2023 • edited by webkit-early-warning-system Loading

1a1ce78

webkit-early-warning-system commented Oct 12, 2023 • edited Loading

youennf Oct 12, 2023

Choose a reason for hiding this comment

achristensen07 Oct 12, 2023

Choose a reason for hiding this comment

webkit-early-warning-system commented Oct 12, 2023 • edited Loading

youennf left a comment

Choose a reason for hiding this comment

webkit-commit-queue commented Oct 13, 2023

achristensen07 commented Oct 13, 2023

achristensen07 commented Oct 12, 2023 •

edited by webkit-early-warning-system

Loading

`1a1ce78`

webkit-early-warning-system commented Oct 12, 2023 •

edited

Loading

webkit-early-warning-system commented Oct 12, 2023 •

edited

Loading