datachannel: Merge SendDataParams and DMT types with webrtc equivalent cricket::SendDataParams is replaced by webrtc::SendDataParams. cricket::DataMessageType is replaced by webrtc::DataMessageType. The sid member from cricket::SendDataParams is now passed as an argument to functions that used one when necessary. Bug: webrtc:7484 Change-Id: Ia4a89c9651fb54ab9a084a6098d49130b6319e1b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217761 Commit-Queue: Florent Castelli <orphis@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/master@{#33966}
diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc index 7e6dc4b..d8e6b39 100644 --- a/pc/data_channel_controller.cc +++ b/pc/data_channel_controller.cc
@@ -31,11 +31,12 @@ return !sctp_data_channels_.empty(); } -bool DataChannelController::SendData(const cricket::SendDataParams& params, +bool DataChannelController::SendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) { if (data_channel_transport()) - return DataChannelSendData(params, payload, result); + return DataChannelSendData(sid, params, payload, result); RTC_LOG(LS_ERROR) << "SendData called before transport is ready"; return false; } @@ -106,7 +107,7 @@ RTC_DCHECK_RUN_ON(network_thread()); cricket::ReceiveDataParams params; params.sid = channel_id; - params.type = ToCricketDataMessageType(type); + params.type = type; signaling_thread()->PostTask( ToQueuedTask([self = weak_factory_.GetWeakPtr(), params, buffer] { if (self) { @@ -222,7 +223,7 @@ bool DataChannelController::HandleOpenMessage_s( const cricket::ReceiveDataParams& params, const rtc::CopyOnWriteBuffer& buffer) { - if (params.type == cricket::DMT_CONTROL && IsOpenMessage(buffer)) { + if (params.type == DataMessageType::kControl && IsOpenMessage(buffer)) { // Received OPEN message; parse and signal that a new data channel should // be created. std::string label; @@ -386,7 +387,8 @@ } bool DataChannelController::DataChannelSendData( - const cricket::SendDataParams& params, + int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) { // TODO(bugs.webrtc.org/11547): Expect method to be called on the network @@ -395,16 +397,9 @@ RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK(data_channel_transport()); - SendDataParams send_params; - send_params.type = ToWebrtcDataMessageType(params.type); - send_params.ordered = params.ordered; - send_params.max_rtx_count = params.max_rtx_count; - send_params.max_rtx_ms = params.max_rtx_ms; - RTCError error = network_thread()->Invoke<RTCError>( - RTC_FROM_HERE, [this, params, send_params, payload] { - return data_channel_transport()->SendData(params.sid, send_params, - payload); + RTC_FROM_HERE, [this, sid, params, payload] { + return data_channel_transport()->SendData(sid, params, payload); }); if (error.ok()) {
diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h index 4c42b8a..05fcff0 100644 --- a/pc/data_channel_controller.h +++ b/pc/data_channel_controller.h
@@ -53,7 +53,8 @@ // Implements // SctpDataChannelProviderInterface. - bool SendData(const cricket::SendDataParams& params, + bool SendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) override; bool ConnectDataChannel(SctpDataChannel* webrtc_data_channel) override; @@ -131,7 +132,8 @@ RTC_RUN_ON(signaling_thread()); // Called from SendData when data_channel_transport() is true. - bool DataChannelSendData(const cricket::SendDataParams& params, + bool DataChannelSendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result);
diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc index 2dc003f..98c44f2 100644 --- a/pc/data_channel_unittest.cc +++ b/pc/data_channel_unittest.cc
@@ -286,8 +286,9 @@ SetChannelReady(); EXPECT_GE(webrtc_data_channel_->id(), 0); - EXPECT_EQ(cricket::DMT_CONTROL, provider_->last_send_data_params().type); - EXPECT_EQ(provider_->last_send_data_params().sid, webrtc_data_channel_->id()); + EXPECT_EQ(webrtc::DataMessageType::kControl, + provider_->last_send_data_params().type); + EXPECT_EQ(provider_->last_sid(), webrtc_data_channel_->id()); } TEST_F(SctpDataChannelTest, QueuedOpenMessageSent) { @@ -295,8 +296,9 @@ SetChannelReady(); provider_->set_send_blocked(false); - EXPECT_EQ(cricket::DMT_CONTROL, provider_->last_send_data_params().type); - EXPECT_EQ(provider_->last_send_data_params().sid, webrtc_data_channel_->id()); + EXPECT_EQ(webrtc::DataMessageType::kControl, + provider_->last_send_data_params().type); + EXPECT_EQ(provider_->last_sid(), webrtc_data_channel_->id()); } // Tests that the DataChannel created after transport gets ready can enter OPEN @@ -333,7 +335,7 @@ // Emulates receiving an OPEN_ACK message. cricket::ReceiveDataParams params; params.sid = init.id; - params.type = cricket::DMT_CONTROL; + params.type = webrtc::DataMessageType::kControl; rtc::CopyOnWriteBuffer payload; webrtc::WriteDataChannelOpenAckMessage(&payload); dc->OnDataReceived(params, payload); @@ -359,7 +361,7 @@ // Emulates receiving a DATA message. cricket::ReceiveDataParams params; params.sid = init.id; - params.type = cricket::DMT_TEXT; + params.type = webrtc::DataMessageType::kText; webrtc::DataBuffer buffer("data"); dc->OnDataReceived(params, buffer.data); @@ -380,7 +382,8 @@ provider_->set_send_blocked(false); EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, webrtc_data_channel_->state(), 1000); - EXPECT_EQ(cricket::DMT_CONTROL, provider_->last_send_data_params().type); + EXPECT_EQ(webrtc::DataMessageType::kControl, + provider_->last_send_data_params().type); } // Tests that close first makes sure all queued data gets sent. @@ -401,7 +404,8 @@ EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed, webrtc_data_channel_->state(), 1000); EXPECT_TRUE(webrtc_data_channel_->error().ok()); - EXPECT_EQ(cricket::DMT_TEXT, provider_->last_send_data_params().type); + EXPECT_EQ(webrtc::DataMessageType::kText, + provider_->last_send_data_params().type); } // Tests that messages are sent with the right id. @@ -410,7 +414,7 @@ SetChannelReady(); webrtc::DataBuffer buffer("data"); EXPECT_TRUE(webrtc_data_channel_->Send(buffer)); - EXPECT_EQ(1, provider_->last_send_data_params().sid); + EXPECT_EQ(1, provider_->last_sid()); } // Tests that the incoming messages with wrong ids are rejected. @@ -457,7 +461,7 @@ rtc::Thread::Current(), rtc::Thread::Current()); EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, dc->state(), 1000); - EXPECT_EQ(0, provider_->last_send_data_params().sid); + EXPECT_EQ(0, provider_->last_sid()); } // Tests that DataChannel::messages_received() and DataChannel::bytes_received() @@ -522,8 +526,9 @@ EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, dc->state(), 1000); - EXPECT_EQ(config.id, provider_->last_send_data_params().sid); - EXPECT_EQ(cricket::DMT_CONTROL, provider_->last_send_data_params().type); + EXPECT_EQ(config.id, provider_->last_sid()); + EXPECT_EQ(webrtc::DataMessageType::kControl, + provider_->last_send_data_params().type); } // Tests the OPEN_ACK role assigned by InternalDataChannelInit.
diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc index 2c90661..682d768 100644 --- a/pc/sctp_data_channel.cc +++ b/pc/sctp_data_channel.cc
@@ -406,7 +406,7 @@ return; } - if (params.type == cricket::DMT_CONTROL) { + if (params.type == DataMessageType::kControl) { if (handshake_state_ != kHandshakeWaitingForAck) { // Ignore it if we are not expecting an ACK message. RTC_LOG(LS_WARNING) @@ -427,8 +427,8 @@ return; } - RTC_DCHECK(params.type == cricket::DMT_BINARY || - params.type == cricket::DMT_TEXT); + RTC_DCHECK(params.type == DataMessageType::kBinary || + params.type == DataMessageType::kText); RTC_LOG(LS_VERBOSE) << "DataChannel received DATA message, sid = " << params.sid; @@ -439,7 +439,7 @@ handshake_state_ = kHandshakeReady; } - bool binary = (params.type == cricket::DMT_BINARY); + bool binary = (params.type == webrtc::DataMessageType::kBinary); auto buffer = std::make_unique<DataBuffer>(payload, binary); if (state_ == kOpen && observer_) { ++messages_received_; @@ -620,7 +620,7 @@ bool SctpDataChannel::SendDataMessage(const DataBuffer& buffer, bool queue_if_blocked) { RTC_DCHECK_RUN_ON(signaling_thread_); - cricket::SendDataParams send_params; + SendDataParams send_params; send_params.ordered = config_.ordered; // Send as ordered if it is still going through OPEN/ACK signaling. @@ -633,11 +633,12 @@ send_params.max_rtx_count = config_.maxRetransmits; send_params.max_rtx_ms = config_.maxRetransmitTime; - send_params.sid = config_.id; - send_params.type = buffer.binary ? cricket::DMT_BINARY : cricket::DMT_TEXT; + send_params.type = + buffer.binary ? DataMessageType::kBinary : DataMessageType::kText; cricket::SendDataResult send_result = cricket::SDR_SUCCESS; - bool success = provider_->SendData(send_params, buffer.data, &send_result); + bool success = + provider_->SendData(config_.id, send_params, buffer.data, &send_result); if (success) { ++messages_sent_; @@ -703,16 +704,16 @@ bool is_open_message = handshake_state_ == kHandshakeShouldSendOpen; RTC_DCHECK(!is_open_message || !config_.negotiated); - cricket::SendDataParams send_params; - send_params.sid = config_.id; + SendDataParams send_params; // Send data as ordered before we receive any message from the remote peer to // make sure the remote peer will not receive any data before it receives the // OPEN message. send_params.ordered = config_.ordered || is_open_message; - send_params.type = cricket::DMT_CONTROL; + send_params.type = DataMessageType::kControl; cricket::SendDataResult send_result = cricket::SDR_SUCCESS; - bool retval = provider_->SendData(send_params, buffer, &send_result); + bool retval = + provider_->SendData(config_.id, send_params, buffer, &send_result); if (retval) { RTC_LOG(LS_VERBOSE) << "Sent CONTROL message on channel " << config_.id;
diff --git a/pc/sctp_data_channel.h b/pc/sctp_data_channel.h index ddb8565..1d7a3c7 100644 --- a/pc/sctp_data_channel.h +++ b/pc/sctp_data_channel.h
@@ -40,7 +40,8 @@ class SctpDataChannelProviderInterface { public: // Sends the data to the transport. - virtual bool SendData(const cricket::SendDataParams& params, + virtual bool SendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) = 0; // Connects to the transport signals.
diff --git a/pc/sctp_data_channel_transport.cc b/pc/sctp_data_channel_transport.cc index 786931c..bb81156 100644 --- a/pc/sctp_data_channel_transport.cc +++ b/pc/sctp_data_channel_transport.cc
@@ -39,17 +39,8 @@ int channel_id, const SendDataParams& params, const rtc::CopyOnWriteBuffer& buffer) { - // Map webrtc::SendDataParams to cricket::SendDataParams. - // TODO(mellem): See about unifying these structs. - cricket::SendDataParams sd_params; - sd_params.sid = channel_id; - sd_params.type = ToCricketDataMessageType(params.type); - sd_params.ordered = params.ordered; - sd_params.max_rtx_count = params.max_rtx_count; - sd_params.max_rtx_ms = params.max_rtx_ms; - cricket::SendDataResult result; - sctp_transport_->SendData(sd_params, buffer, &result); + sctp_transport_->SendData(channel_id, params, buffer, &result); // TODO(mellem): See about changing the interfaces to not require mapping // SendDataResult to RTCError and back again. @@ -94,8 +85,7 @@ const cricket::ReceiveDataParams& params, const rtc::CopyOnWriteBuffer& buffer) { if (sink_) { - sink_->OnDataReceived(params.sid, ToWebrtcDataMessageType(params.type), - buffer); + sink_->OnDataReceived(params.sid, params.type, buffer); } }
diff --git a/pc/sctp_transport_unittest.cc b/pc/sctp_transport_unittest.cc index 8ab4482..b4618ed 100644 --- a/pc/sctp_transport_unittest.cc +++ b/pc/sctp_transport_unittest.cc
@@ -38,7 +38,8 @@ } bool OpenStream(int sid) override { return true; } bool ResetStream(int sid) override { return true; } - bool SendData(const cricket::SendDataParams& params, + bool SendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result = nullptr) override { return true;
diff --git a/pc/sctp_utils.cc b/pc/sctp_utils.cc index 9d46cc4..f745840 100644 --- a/pc/sctp_utils.cc +++ b/pc/sctp_utils.cc
@@ -230,33 +230,4 @@ payload->SetData(&data, sizeof(data)); } -cricket::DataMessageType ToCricketDataMessageType(DataMessageType type) { - switch (type) { - case DataMessageType::kText: - return cricket::DMT_TEXT; - case DataMessageType::kBinary: - return cricket::DMT_BINARY; - case DataMessageType::kControl: - return cricket::DMT_CONTROL; - default: - return cricket::DMT_NONE; - } - return cricket::DMT_NONE; -} - -DataMessageType ToWebrtcDataMessageType(cricket::DataMessageType type) { - switch (type) { - case cricket::DMT_TEXT: - return DataMessageType::kText; - case cricket::DMT_BINARY: - return DataMessageType::kBinary; - case cricket::DMT_CONTROL: - return DataMessageType::kControl; - case cricket::DMT_NONE: - default: - RTC_NOTREACHED(); - } - return DataMessageType::kControl; -} - } // namespace webrtc
diff --git a/pc/sctp_utils.h b/pc/sctp_utils.h index 44225cf..da85445 100644 --- a/pc/sctp_utils.h +++ b/pc/sctp_utils.h
@@ -40,10 +40,6 @@ void WriteDataChannelOpenAckMessage(rtc::CopyOnWriteBuffer* payload); -cricket::DataMessageType ToCricketDataMessageType(DataMessageType type); - -DataMessageType ToWebrtcDataMessageType(cricket::DataMessageType type); - } // namespace webrtc #endif // PC_SCTP_UTILS_H_
diff --git a/pc/test/fake_data_channel_provider.h b/pc/test/fake_data_channel_provider.h index 6a063f8..f9e9e91 100644 --- a/pc/test/fake_data_channel_provider.h +++ b/pc/test/fake_data_channel_provider.h
@@ -26,7 +26,8 @@ transport_error_(false) {} virtual ~FakeDataChannelProvider() {} - bool SendData(const cricket::SendDataParams& params, + bool SendData(int sid, + const webrtc::SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) override { RTC_CHECK(ready_to_send_); @@ -41,6 +42,7 @@ return false; } + last_sid_ = sid; last_send_data_params_ = params; return true; } @@ -127,7 +129,8 @@ void set_transport_error() { transport_error_ = true; } - cricket::SendDataParams last_send_data_params() const { + int last_sid() const { return last_sid_; } + const webrtc::SendDataParams& last_send_data_params() const { return last_send_data_params_; } @@ -144,7 +147,8 @@ } private: - cricket::SendDataParams last_send_data_params_; + int last_sid_; + webrtc::SendDataParams last_send_data_params_; bool send_blocked_; bool transport_available_; bool ready_to_send_;