Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
4 years, 8 months ago by mikescarlett Modified:
4 years, 7 months ago Reviewers:
Taylor Brandstetter, pthatcher1, honghaiz3 CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdd QuicDataChannel and QuicDataTransport classes
QuicDataChannel implements DataChannelInterface. It
replaces SCTP data channels by using a QuicTransportChannel
to create a ReliableQuicStream for each message.
QuicDataChannel only implements unordered, reliable delivery
for the initial implementation and does not send a hello message.
QuicDataTransport is a helper class that dispatches each incoming
ReliableQuicStream to a QuicDataChannel when the remote
peer receives a message by parsing the data channel id and message id
from the message header. It is also responsible for encoding the header
before QuicDataChannel sends the message.
Split from CL https://codereview.chromium.org/1844803002/.
BUG=
Committed: https://crrev.com/9bc517f1237aa77051412c4a9d32cbe9d98120a9
Cr-Commit-Position: refs/heads/master@{#12574}
Patch Set 1 : #
Total comments: 26
Patch Set 2 : #Patch Set 3 : Change QuicDataTransport constructor to make implementing the HELLO message easier #Patch Set 4 : Make QuicDataChannel::Message public so QuicDataTransport can use it for handling messages #
Total comments: 22
Patch Set 5 : Respond to pthatcher's comments #Patch Set 6 : Rename SetBufferedAmount to SetBufferedAmount_w #Patch Set 7 : Sync to upstream #Patch Set 8 : Fix minor mistakes #
Total comments: 2
Patch Set 9 : Make SetTransportChannel_w return the new data channel state #Patch Set 10 : Make changes before committing #
Created: 4 years, 7 months ago
Messages
Total messages: 34 (22 generated)
Description was changed from ========== Add QuicDataChannel and QuicDataTransport classes QuicDataChannel is a data channel that uses a QuicTransportChannel to send messages. QuicDataTransport is a helper class for QuicDataChannel that parses the data channel id and message id from each incoming ReliableQuicStream, then dispatches it to the QuicDataChannel. BUG= ========== to ========== Add QuicDataChannel and QuicDataTransport classes QuicDataChannel is a data channel that uses a QuicTransportChannel to send messages. QuicDataTransport is a helper class for QuicDataChannel that parses the data channel id and message id from each incoming ReliableQuicStream, then dispatches it to the QuicDataChannel. Split from CL https://codereview.chromium.org/1844803002/. BUG= ==========
Description was changed from ========== Add QuicDataChannel and QuicDataTransport classes QuicDataChannel is a data channel that uses a QuicTransportChannel to send messages. QuicDataTransport is a helper class for QuicDataChannel that parses the data channel id and message id from each incoming ReliableQuicStream, then dispatches it to the QuicDataChannel. Split from CL https://codereview.chromium.org/1844803002/. BUG= ========== to ========== Add QuicDataChannel and QuicDataTransport classes QuicDataChannel implements DataChannelInterface. It replaces SCTP data channels by using a QuicTransportChannel to create a ReliableQuicStream for each message. QuicDataChannel only implements unordered, reliable delivery for the initial implementation and does not send a hello message. QuicDataTransport is a helper class that dispatches each incoming ReliableQuicStream to a QuicDataChannel when the remote peer receives a message by parsing the data channel id and message id from the message header. It is also responsible for encoding the header before QuicDataChannel sends the message. Split from CL https://codereview.chromium.org/1844803002/. BUG= ==========
mikescarlett@webrtc.org changed reviewers: + deadbeef@webrtc.org, honghaiz@webrtc.org, pthatcher@webrtc.org
Comments I got through so far while on a flight. Landing now. I'll have to come back to it later. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.cc (right): https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:65: << " refuses to send an empty message."; I think this was just a limitation of SCTP. I don't think we need to emulate it. In other words, we can support empty messages, so let's do so. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:70: message_dispatcher_.EncodeHeader(id_, ++num_sent_messages_, &header); It's strange to me that a class called MessageDispatcher has a method called EncodeHeader. Why not just make EncodeHeader a function separate from any class? https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:76: stream->Write(header.data<char>(), header.size(), false); Don't you need to check the Write call with the header? What if it blocks/fails? https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:78: // there is no more data after this message. Can you add a line like this? bool fin = true; https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.h (right): https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:91: // channel must not already be set. Can you specify in the comment why we can't put this in the constructor? https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:92: void SetTransportChannel(cricket::QuicTransportChannel* channel); Should we return a bool for failure if it fails? https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:95: size_t GetNumWriteBlockedStreams() const; What is this used for? Can you expand the comment to explain? https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:98: size_t GetNumIncomingStreams() const; Same here https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:121: // Callback for |message_dispatcher_|. for message_dispatcher_ or *from* message_dispatcher_? Same for all of the above. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:157: // Invokes asyncronous method calls. This comment isn't needed. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:161: const QuicMessageDispatcher& message_dispatcher_; Why are we storing a const ref instead of a const pointer? https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:183: friend QuicMessageDispatcher; Why is this necessary? https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:195: class QuicMessageDispatcher { I don't quite understand the design of the QuicMessageDispatcher. It might take some more time for me to figure it out.
https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.cc (right): https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:65: << " refuses to send an empty message."; On 2016/04/14 17:21:56, pthatcher1 wrote: > I think this was just a limitation of SCTP. I don't think we need to emulate > it. In other words, we can support empty messages, so let's do so. Ok I'll remove that. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:70: message_dispatcher_.EncodeHeader(id_, ++num_sent_messages_, &header); On 2016/04/14 17:21:56, pthatcher1 wrote: > It's strange to me that a class called MessageDispatcher has a method called > EncodeHeader. Why not just make EncodeHeader a function separate from any > class? Done. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:76: stream->Write(header.data<char>(), header.size(), false); On 2016/04/14 17:21:56, pthatcher1 wrote: > Don't you need to check the Write call with the header? What if it > blocks/fails? I added a check. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:78: // there is no more data after this message. On 2016/04/14 17:21:56, pthatcher1 wrote: > Can you add a line like this? > > bool fin = true; Done. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.h (right): https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:91: // channel must not already be set. On 2016/04/14 17:21:56, pthatcher1 wrote: > Can you specify in the comment why we can't put this in the constructor? Done. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:92: void SetTransportChannel(cricket::QuicTransportChannel* channel); On 2016/04/14 17:21:56, pthatcher1 wrote: > Should we return a bool for failure if it fails? That's ok I changed it. Previously I was asserting that this wouldn't fail, but see why a bool is preferred. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:95: size_t GetNumWriteBlockedStreams() const; On 2016/04/14 17:21:57, pthatcher1 wrote: > What is this used for? Can you expand the comment to explain? This makes it possible to test that write blocked streams and incoming streams are closed. I modified the comment to specify that these are for testing. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:98: size_t GetNumIncomingStreams() const; On 2016/04/14 17:21:57, pthatcher1 wrote: > Same here Done. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:121: // Callback for |message_dispatcher_|. On 2016/04/14 17:21:56, pthatcher1 wrote: > for message_dispatcher_ or *from* message_dispatcher_? > > Same for all of the above. Done. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:157: // Invokes asyncronous method calls. On 2016/04/14 17:21:56, pthatcher1 wrote: > This comment isn't needed. Done. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:161: const QuicMessageDispatcher& message_dispatcher_; On 2016/04/14 17:21:57, pthatcher1 wrote: > Why are we storing a const ref instead of a const pointer? I'm removing this. See explanation below. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:183: friend QuicMessageDispatcher; On 2016/04/14 17:21:57, pthatcher1 wrote: > Why is this necessary? This was for keeping OnIncomingStream private. See explanation below. https://codereview.chromium.org/1886623002/diff/120001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:195: class QuicMessageDispatcher { On 2016/04/14 17:21:56, pthatcher1 wrote: > I don't quite understand the design of the QuicMessageDispatcher. It might take > some more time for me to figure it out. I'm removing this. Its main purpose was to keep OnIncomingStream private (to resolve concerns that OnIncomingStream appears to be an implementation detail and that implementation details should be private) but I'm making OnIncomingStream public again with comments to explain that it is used by QuicDataTransport. I believe this class makes it harder for other people to maintain the code and adversely impacts readability, and that declaring QuicDataTransport a friend class would also be confusing.
I had to modify QuicSession so that when QUIC streams send data while the QuicTransportChannel is unwritable, the data gets sent once the QuicTransportChannel becomes writable. See https://codereview.webrtc.org/1888903002
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
Patchset #1 (id:80001) has been deleted
Patchset #1 (id:100001) has been deleted
Patchset #3 (id:160001) has been deleted
Patchset #3 (id:180001) has been deleted
Patchset #3 (id:200001) has been deleted
Patchset #3 (id:220001) has been deleted
Patchset #2 (id:140001) has been deleted
Patchset #2 (id:240001) has been deleted
Patchset #2 (id:260001) has been deleted
Patchset #4 (id:320001) has been deleted
Mostly naming and nits. But there's a bit with threads to fix. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.cc (right): https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:46: uint64_t tmp; Might as well call that dcid instead of tmp. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:103: EncodeQuicHeader(id_, ++num_sent_messages_, &header); I'd call this next_message_id_. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:108: uint64_t old_queued_bytes = stream->queued_data_bytes(); How could a stream just created have queued_data_bytes()? https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:178: buffered_amount_ -= queued_bytes_written; I'd suggest moving these two lines into a SetBufferedAmount method, and also swap the two lines. Then you can use the method in the two places where this is done. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:180: RTC_DCHECK(kv != write_blocked_quic_streams_.end()); It would be slight better to do this: if (kv == write_blocked_quic_streams_.end()) { RTC_DCHECK(false); return; } https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:215: rtc::Bind(&QuicDataChannel::SetState_s, this, kClosed)); I think the better thing to do would be to have QuicDataChannel::Close() set the state after worker_thread_->Invoke<void>(rtc::Bind(&QuicDataChannel::Close_w, this));. Then it's done immediately after this completes rather than going around the message loop again. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:239: if (quic_transport_channel_->writable()) { quic_transport_channel_->writable() an only be accessed on the worker thread, so this should be part of ConnectTransportChannel_w, or perhaps rename it SetTransportChannel_w. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:296: RTC_DCHECK(kv != incoming_quic_messages_.end()); Again, please make this return so it doesn't fail in release builds. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.h (right): https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:44: rtc::CopyOnWriteBuffer* header); And I'd call this WriteQuicDataChannelMessageHeader. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatatra... File webrtc/api/quicdatatransport.cc (right): https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatatra... webrtc/api/quicdatatransport.cc:109: cricket::ReliableQuicStream* stream = quic_stream_by_id_[id]; It would be better to check for release builds also, and with one lookup: const auto& kv = quic_stream_by_id_.find(id); if (kv == quic_stream_by_id_.end()) { RTC_DCHECK(false); return; } cricket::ReliableQuicStream* stream = kv.second; https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatatra... webrtc/api/quicdatatransport.cc:116: if (!DecodeQuicHeader(data, len, &data_channel_id, &message_id, I'd call this ParseQuicDataMessageHeader instead of DecodeQuicHeader.
https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.cc (right): https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:46: uint64_t tmp; On 2016/04/29 21:08:36, pthatcher1 wrote: > Might as well call that dcid instead of tmp. Renamed. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:103: EncodeQuicHeader(id_, ++num_sent_messages_, &header); On 2016/04/29 21:08:36, pthatcher1 wrote: > I'd call this next_message_id_. Renamed. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:108: uint64_t old_queued_bytes = stream->queued_data_bytes(); On 2016/04/29 21:08:36, pthatcher1 wrote: > How could a stream just created have queued_data_bytes()? Clearly nothing should be queued. Removed old_queued_bytes. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:178: buffered_amount_ -= queued_bytes_written; On 2016/04/29 21:08:36, pthatcher1 wrote: > I'd suggest moving these two lines into a SetBufferedAmount method, and also > swap the two lines. Then you can use the method in the two places where this is > done. Done. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:180: RTC_DCHECK(kv != write_blocked_quic_streams_.end()); On 2016/04/29 21:08:36, pthatcher1 wrote: > It would be slight better to do this: > > if (kv == write_blocked_quic_streams_.end()) { > RTC_DCHECK(false); > return; > } Done. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:215: rtc::Bind(&QuicDataChannel::SetState_s, this, kClosed)); On 2016/04/29 21:08:36, pthatcher1 wrote: > I think the better thing to do would be to have QuicDataChannel::Close() set the > state after worker_thread_->Invoke<void>(rtc::Bind(&QuicDataChannel::Close_w, > this));. Then it's done immediately after this completes rather than going > around the message loop again. Done. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:239: if (quic_transport_channel_->writable()) { On 2016/04/29 21:08:36, pthatcher1 wrote: > quic_transport_channel_->writable() an only be accessed on the worker thread, so > this should be part of ConnectTransportChannel_w, or perhaps rename it > SetTransportChannel_w. Moved to ConnectTransportChannel_w (and renamed ConnectTransportChannel_w to SetTransportChannel_w) https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:296: RTC_DCHECK(kv != incoming_quic_messages_.end()); On 2016/04/29 21:08:36, pthatcher1 wrote: > Again, please make this return so it doesn't fail in release builds. Done. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.h (right): https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.h:44: rtc::CopyOnWriteBuffer* header); On 2016/04/29 21:08:36, pthatcher1 wrote: > And I'd call this WriteQuicDataChannelMessageHeader. Renamed. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatatra... File webrtc/api/quicdatatransport.cc (right): https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatatra... webrtc/api/quicdatatransport.cc:109: cricket::ReliableQuicStream* stream = quic_stream_by_id_[id]; On 2016/04/29 21:08:36, pthatcher1 wrote: > It would be better to check for release builds also, and with one lookup: > > const auto& kv = quic_stream_by_id_.find(id); > if (kv == quic_stream_by_id_.end()) { > RTC_DCHECK(false); > return; > } > cricket::ReliableQuicStream* stream = kv.second; Done. https://codereview.chromium.org/1886623002/diff/340001/webrtc/api/quicdatatra... webrtc/api/quicdatatransport.cc:116: if (!DecodeQuicHeader(data, len, &data_channel_id, &message_id, On 2016/04/29 21:08:36, pthatcher1 wrote: > I'd call this ParseQuicDataMessageHeader instead of DecodeQuicHeader. Done.
It looks good, except for one last thing... https://codereview.chromium.org/1886623002/diff/420001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.cc (right): https://codereview.chromium.org/1886623002/diff/420001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:252: } Hmmm.... I think what we want to do here is have SetTransportChannel_w return the state it should be set to, and then have SetTransportChannel set it to the state. That way, it happens immediately and writable() will be read on the worker thread and state_ accessed on the signaling thread.
https://codereview.chromium.org/1886623002/diff/420001/webrtc/api/quicdatacha... File webrtc/api/quicdatachannel.cc (right): https://codereview.chromium.org/1886623002/diff/420001/webrtc/api/quicdatacha... webrtc/api/quicdatachannel.cc:252: } On 2016/04/29 22:04:17, pthatcher1 wrote: > Hmmm.... I think what we want to do here is have SetTransportChannel_w return > the state it should be set to, and then have SetTransportChannel set it to the > state. That way, it happens immediately and writable() will be read on the > worker thread and state_ accessed on the signaling thread. Good idea. Done.
Patchset #9 (id:440001) has been deleted
lgtm Woo!
The CQ bit was checked by mikescarlett@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from pthatcher@webrtc.org Link to the patchset: https://codereview.webrtc.org/1886623002/#ps480001 (title: "Make changes before committing")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1886623002/480001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1886623002/480001
Message was sent while issue was closed.
Description was changed from ========== Add QuicDataChannel and QuicDataTransport classes QuicDataChannel implements DataChannelInterface. It replaces SCTP data channels by using a QuicTransportChannel to create a ReliableQuicStream for each message. QuicDataChannel only implements unordered, reliable delivery for the initial implementation and does not send a hello message. QuicDataTransport is a helper class that dispatches each incoming ReliableQuicStream to a QuicDataChannel when the remote peer receives a message by parsing the data channel id and message id from the message header. It is also responsible for encoding the header before QuicDataChannel sends the message. Split from CL https://codereview.chromium.org/1844803002/. BUG= ========== to ========== Add QuicDataChannel and QuicDataTransport classes QuicDataChannel implements DataChannelInterface. It replaces SCTP data channels by using a QuicTransportChannel to create a ReliableQuicStream for each message. QuicDataChannel only implements unordered, reliable delivery for the initial implementation and does not send a hello message. QuicDataTransport is a helper class that dispatches each incoming ReliableQuicStream to a QuicDataChannel when the remote peer receives a message by parsing the data channel id and message id from the message header. It is also responsible for encoding the header before QuicDataChannel sends the message. Split from CL https://codereview.chromium.org/1844803002/. BUG= ==========
Message was sent while issue was closed.
Committed patchset #10 (id:480001)
Message was sent while issue was closed.
Description was changed from ========== Add QuicDataChannel and QuicDataTransport classes QuicDataChannel implements DataChannelInterface. It replaces SCTP data channels by using a QuicTransportChannel to create a ReliableQuicStream for each message. QuicDataChannel only implements unordered, reliable delivery for the initial implementation and does not send a hello message. QuicDataTransport is a helper class that dispatches each incoming ReliableQuicStream to a QuicDataChannel when the remote peer receives a message by parsing the data channel id and message id from the message header. It is also responsible for encoding the header before QuicDataChannel sends the message. Split from CL https://codereview.chromium.org/1844803002/. BUG= ========== to ========== Add QuicDataChannel and QuicDataTransport classes QuicDataChannel implements DataChannelInterface. It replaces SCTP data channels by using a QuicTransportChannel to create a ReliableQuicStream for each message. QuicDataChannel only implements unordered, reliable delivery for the initial implementation and does not send a hello message. QuicDataTransport is a helper class that dispatches each incoming ReliableQuicStream to a QuicDataChannel when the remote peer receives a message by parsing the data channel id and message id from the message header. It is also responsible for encoding the header before QuicDataChannel sends the message. Split from CL https://codereview.chromium.org/1844803002/. BUG= Committed: https://crrev.com/9bc517f1237aa77051412c4a9d32cbe9d98120a9 Cr-Commit-Position: refs/heads/master@{#12574} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/9bc517f1237aa77051412c4a9d32cbe9d98120a9 Cr-Commit-Position: refs/heads/master@{#12574} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
