Attribute nw connections to the page bundle identifier
Created attachment 434645 [details] Patch
Created attachment 434743 [details] Patch
<rdar://problem/81415612>
Comment on attachment 434743 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434743&action=review > Source/WebKit/NetworkProcess/NetworkSession.cpp:411 > + return m_attributedBundleIdentifierFromPageIdentifiers.get(identifier); It is worth ASSERTing that the identifier is in the map? > Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:159 > + if (auto socket = NetworkRTCUDPSocketCocoa::createUDPSocket(identifier, *this, address.value, minPort, maxPort, m_ipcConnection.copyRef(), String(attributedBundleIdentifierFromPageIdentifier(pageIdentifier)), isFirstParty, isRelayDisabled, WTFMove(domain))) { Nit: extra space before `WTFMove` Is this called often enough that it would be worth keeping a pageIdentifier -> attributedBundleIdentifier map in the provider instead of doing the synchronous cross-thread hop every time? > Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:202 > + if (auto socket = NetworkRTCTCPSocketCocoa::createClientTCPSocket(identifier, *this, remoteAddress.value, options, attributedBundleIdentifierFromPageIdentifier(pageIdentifier), isRelayDisabled, m_ipcConnection.copyRef())) { Ditto.
Comment on attachment 434743 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434743&action=review > Source/WebKit/ChangeLog:9 > + Use page identifier to get the bundle identifier. attributed bundle identifier, not the bundle identifier. The former is set via SPI in contexts such as SFSafariViewController when the view is made on behalf of another application, the latter is set for the executable. >> Source/WebKit/NetworkProcess/NetworkSession.cpp:411 >> + return m_attributedBundleIdentifierFromPageIdentifiers.get(identifier); > > It is worth ASSERTing that the identifier is in the map? no, it is likely not in the map. > Source/WebKit/NetworkProcess/NetworkSession.h:180 > + String attributedBundleIdentifierFromPageIdentifier(WebPageProxyIdentifier) const; This could return a const String& > Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:149 > + return value; I feel like there should be an isolatedCopy somewhere in here.
Created attachment 434771 [details] Patch for landing
> > Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:159 > > + if (auto socket = NetworkRTCUDPSocketCocoa::createUDPSocket(identifier, *this, address.value, minPort, maxPort, m_ipcConnection.copyRef(), String(attributedBundleIdentifierFromPageIdentifier(pageIdentifier)), isFirstParty, isRelayDisabled, WTFMove(domain))) { > > Nit: extra space before `WTFMove` OK > Is this called often enough that it would be worth keeping a pageIdentifier > -> attributedBundleIdentifier map in the provider instead of doing the > synchronous cross-thread hop every time? Yep, this is in the patch. > > Source/WebKit/ChangeLog:9 > > + Use page identifier to get the bundle identifier. > > attributed bundle identifier, not the bundle identifier. The former is set > via SPI in contexts such as SFSafariViewController when the view is made on > behalf of another application, the latter is set for the executable. OK > > Source/WebKit/NetworkProcess/NetworkSession.h:180 > > + String attributedBundleIdentifierFromPageIdentifier(WebPageProxyIdentifier) const; > > This could return a const String& OK > > Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:149 > > + return value; > > I feel like there should be an isolatedCopy somewhere in here. There is one when adding the NetworkSession string to the NetworkRTCProvider map (line 147). I do not think we need an isolated copy every time we return the string: - The map is only accessed in the background thread. - The sockets keep a ref of the strings but the sockets are deleted in the background thread.
> - The map is only accessed in the background thread. I'll clear the map in the background thread when closing even though this is not strictly needed.
Created attachment 434776 [details] Patch for landing
Created attachment 434778 [details] Patch for landing
Committed r280556 (240180@main): <https://commits.webkit.org/240180@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 434778 [details].