Summary: | Make RTCDataChannel transferable | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||
Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | alecflett, beidson, cdumez, eric.carlson, esprehn+autocc, ews-watchlist, glenn, hta, jer.noble, jsbell, kondapallykalyan, philipj, sergio, tommyw, webkit-bug-importer, youennf | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Local Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
youenn fablet
2021-03-09 00:39:34 PST
Created attachment 422675 [details]
Patch
Created attachment 422914 [details]
Patch
Comment on attachment 422914 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422914&action=review > Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:68 > + if (!channel->m_handler) > + return; Is this unexpected? If so, it may be worth logging an error to help track down errors later. > Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:270 > + ASSERT(m_isDetachable); Maybe ASSERT(canDetach())? > Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:295 > + if (identifier.processIdentifier != Process::identifier()) { > + // FIXME: Add support for out-of-process data channels. > + return createClosedChannel(context, WTFMove(label), WTFMove(options)); > + } Maybe log an error here to help diagnose error reports? > Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:301 > + if (!handler) > + return createClosedChannel(context, WTFMove(label), WTFMove(options)); Ditto Comment on attachment 422914 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422914&action=review >> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:68 >> + return; > > Is this unexpected? If so, it may be worth logging an error to help track down errors later. It is not unexpected, in case a channel is created and closed immediately. >> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:270 >> + ASSERT(m_isDetachable); > > Maybe ASSERT(canDetach())? Yep, sounds much better! And I will add that bufferedAmount is zero as well. >> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:295 >> + } > > Maybe log an error here to help diagnose error reports? OK, will do. >> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:301 >> + return createClosedChannel(context, WTFMove(label), WTFMove(options)); > > Ditto For this one, it would be an error right now, but in the future if the connection gets closed before the transfer happens, we could end up in that case. Created attachment 423025 [details]
Patch for landing
Committed r274351: <https://commits.webkit.org/r274351> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423025 [details]. |