Summary: | [WebRTC] Add support for libwebrtc data channel | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||
Component: | WebCore Misc. | Assignee: | youenn fablet <youennf> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, commit-queue, eric.carlson, jonlee, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 143211 | ||||||||||
Attachments: |
|
Description
youenn fablet
2017-01-26 21:46:54 PST
Created attachment 299945 [details]
Patch
Comment on attachment 299945 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299945&action=review > Source/WebCore/ChangeLog:10 > + ntroducing LibWebRTCDataChannelHandler as the integration layer between WebCore (RTCDataChannel) Introducing > Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:-68 > - m_handler->setClient(this); Does moving this accomplish anything? > Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp:52 > + return m_channel->Send({rtc::CopyOnWriteBuffer(text.utf8().data(), text.length()), false}); The CString returned by utf8() goes out of scope immediately. Will that cause a problem with the lifetime of CopyOnWriteBuffer? Will it ever read after the CString has been freed? > Source/WebCore/platform/mediastream/RTCDataChannelHandlerClient.h:34 > +class RTCDataChannelHandlerClient : public RefCounted<RTCDataChannelHandlerClient> { This should be ThreadSafeRefCounted because we are calling ref and deref from different threads. Created attachment 299980 [details]
Patch
Created attachment 299981 [details]
Patch
Thanks for the review. > > Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:-68 > > - m_handler->setClient(this); > > Does moving this accomplish anything? The call to setClient triggers taking a ref of the channel at libwebrtc stack. If done within constructor, it will happen before the pointer being adopted. > > Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp:52 > > + return m_channel->Send({rtc::CopyOnWriteBuffer(text.utf8().data(), text.length()), false}); > > The CString returned by utf8() goes out of scope immediately. Will that > cause a problem with the lifetime of CopyOnWriteBuffer? Will it ever read > after the CString has been freed? CopyOnWrite is doing its own copy of the data. > > Source/WebCore/platform/mediastream/RTCDataChannelHandlerClient.h:34 > > +class RTCDataChannelHandlerClient : public RefCounted<RTCDataChannelHandlerClient> { > > This should be ThreadSafeRefCounted because we are calling ref and deref > from different threads. Right! Comment on attachment 299981 [details] Patch Clearing flags on attachment: 299981 Committed r211371: <http://trac.webkit.org/changeset/211371> All reviewed patches have been landed. Closing bug. |