Call send asynchronously from RTCDataChannel.

The underlying data channel always returns true on sending,
asynchronously invoking a callback to notify that the message was sent,
so there's no reason not to call it asynchronously at the blink level.
This will make the main thread more responsive.

Bug: webrtc:11547
Change-Id: I8a8fd518bf25d14242561501a3b05c614be0a3db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2255317
Commit-Queue: Tommi <tommi@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Reviewed-by: Tommi <tommi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782015}

third_party/blink/renderer/modules/peerconnection/adapters/web_rtc_cross_thread_copier.h

@@ -34,6 +34,7 @@ class MediaStreamInterface;
 class RtpReceiverInterface;
 class SctpTransportInformation;
 class VideoTrackInterface;
+struct DataBuffer;
 }
 
 namespace blink {
@@ -171,6 +172,12 @@ struct CrossThreadCopier<rtc::scoped_refptr<webrtc::VideoTrackInterface>>
   STATIC_ONLY(CrossThreadCopier);
 };
 
+template <>
+struct CrossThreadCopier<webrtc::DataBuffer>
+    : public CrossThreadCopierPassThrough<webrtc::DataBuffer> {
+  STATIC_ONLY(CrossThreadCopier);
+};
+
 }  // namespace WTF
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_MODULES_PEERCONNECTION_ADAPTERS_WEB_RTC_CROSS_THREAD_COPIER_H_

third_party/blink/renderer/modules/peerconnection/rtc_data_channel.cc

@@ -36,6 +36,7 @@
 #include "third_party/blink/renderer/core/fileapi/blob.h"
 #include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"
 #include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer_view.h"
+#include "third_party/blink/renderer/modules/peerconnection/adapters/web_rtc_cross_thread_copier.h"
 #include "third_party/blink/renderer/modules/peerconnection/rtc_error_event.h"
 #include "third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h"
 #include "third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.h"
@@ -45,6 +46,17 @@
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
 #include "third_party/blink/renderer/platform/wtf/thread_safe_ref_counted.h"
 
+namespace WTF {
+
+template <>
+struct CrossThreadCopier<scoped_refptr<webrtc::DataChannelInterface>>
+    : public CrossThreadCopierPassThrough<
+          scoped_refptr<webrtc::DataChannelInterface>> {
+  STATIC_ONLY(CrossThreadCopier);
+};
+
+}  // namespace WTF
+
 namespace blink {
 
 namespace {
@@ -110,6 +122,12 @@ void RecordMessageSent(const webrtc::DataChannelInterface& channel,
   }
 }
 
+void SendOnSignalingThread(
+    const scoped_refptr<webrtc::DataChannelInterface> channel,
+    const webrtc::DataBuffer data_buffer) {
+  channel->Send(data_buffer);
+}
+
 }  // namespace
 
 static void ThrowNotOpenException(ExceptionState* exception_state) {
@@ -223,10 +241,12 @@ RTCDataChannel::RTCDataChannel(
       buffered_amount_(0U),
       stopped_(false),
       closed_from_owner_(false),
+      is_rtp_data_channel_(peer_connection_handler->enable_rtp_data_channel()),
       observer_(base::MakeRefCounted<Observer>(
           context->GetTaskRunner(TaskType::kNetworking),
           this,
-          channel)) {
+          channel)),
+      signaling_thread_(peer_connection_handler->signaling_thread()) {
   DCHECK(peer_connection_handler);
 
   // Register observer and get state update to make up for state change updates
@@ -364,9 +384,7 @@ void RTCDataChannel::send(const String& data, ExceptionState& exception_state) {
   }
   buffered_amount_ += data_buffer.size();
   RecordMessageSent(*channel().get(), data_buffer.size());
-  if (!channel()->Send(data_buffer)) {
-    // TODO(https://crbug.com/937848): Don't throw an exception if data is
-    // queued.
+  if (!SendDataBuffer(std::move(data_buffer))) {
     ThrowCouldNotSendDataException(&exception_state);
   }
 }
@@ -613,7 +631,20 @@ bool RTCDataChannel::SendRawData(const char* data, size_t length) {
   rtc::CopyOnWriteBuffer buffer(data, length);
   webrtc::DataBuffer data_buffer(buffer, true);
   RecordMessageSent(*channel().get(), data_buffer.size());
-  return channel()->Send(data_buffer);
+  return SendDataBuffer(std::move(data_buffer));
+}
+
+bool RTCDataChannel::SendDataBuffer(webrtc::DataBuffer data_buffer) {
+  // RTP data channels return false on failure to send. SCTP data channels
+  // queue the packet on failure and always return true, so Send can be
+  // called asynchronously for them.
+  if (is_rtp_data_channel_) {
+    return channel()->Send(data_buffer);
+  }
+  PostCrossThreadTask(*signaling_thread_.get(), FROM_HERE,
+                      CrossThreadBindOnce(&SendOnSignalingThread, channel(),
+                                          std::move(data_buffer)));
+  return true;
 }
 
 }  // namespace blink

third_party/blink/renderer/modules/peerconnection/rtc_data_channel.h

@@ -165,6 +165,7 @@ class MODULES_EXPORT RTCDataChannel final
 
   const scoped_refptr<webrtc::DataChannelInterface>& channel() const;
   bool SendRawData(const char* data, size_t length);
+  bool SendDataBuffer(webrtc::DataBuffer data_buffer);
 
   webrtc::DataChannelInterface::DataState state_;
 
@@ -182,7 +183,9 @@ class MODULES_EXPORT RTCDataChannel final
   unsigned buffered_amount_;
   bool stopped_;
   bool closed_from_owner_;
+  bool is_rtp_data_channel_;
   scoped_refptr<Observer> observer_;
+  scoped_refptr<base::SingleThreadTaskRunner> signaling_thread_;
   THREAD_CHECKER(thread_checker_);
 };
 

third_party/blink/renderer/modules/peerconnection/rtc_data_channel_test.cc

@@ -51,6 +51,11 @@ class MockPeerConnectionHandler : public MockRTCPeerConnectionHandlerPlatform {
       scoped_refptr<base::TestSimpleTaskRunner> signaling_thread)
       : signaling_thread_(signaling_thread) {}
 
+  scoped_refptr<base::SingleThreadTaskRunner> signaling_thread()
+      const override {
+    return signaling_thread_;
+  }
+
   void RunSynchronousOnceClosureOnSignalingThread(
       CrossThreadOnceClosure closure,
       const char* trace_event_name) override {
@@ -253,6 +258,9 @@ TEST_F(RTCDataChannelTest, BufferedAmount) {
   String message(std::string(100, 'A').c_str());
   channel->send(message, IGNORE_EXCEPTION_FOR_TESTING);
   EXPECT_EQ(100U, channel->bufferedAmount());
+  // The actual send operation is posted to the signaling thread; wait for it
+  // to run to avoid a memory leak.
+  signaling_thread()->RunUntilIdle();
 }
 
 TEST_F(RTCDataChannelTest, BufferedAmountLow) {
@@ -272,6 +280,9 @@ TEST_F(RTCDataChannelTest, BufferedAmountLow) {
   ASSERT_EQ(1U, channel->scheduled_events_.size());
   EXPECT_EQ("bufferedamountlow",
             channel->scheduled_events_.back()->type().Utf8());
+  // The actual send operation is posted to the signaling thread; wait for it
+  // to run to avoid a memory leak.
+  signaling_thread()->RunUntilIdle();
 }
 
 TEST_F(RTCDataChannelTest, Open) {

third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.h

@@ -225,6 +225,9 @@ class MODULES_EXPORT RTCPeerConnectionHandler {
   // WebRTC event log fragments sent back from PeerConnection land here.
   void OnWebRtcEventLogWrite(const WTF::Vector<uint8_t>& output);
 
+  // Virtual for testing purposes.
+  virtual scoped_refptr<base::SingleThreadTaskRunner> signaling_thread() const;
+
   bool force_encoded_audio_insertable_streams() {
     return force_encoded_audio_insertable_streams_;
   }
@@ -233,6 +236,10 @@ class MODULES_EXPORT RTCPeerConnectionHandler {
     return force_encoded_video_insertable_streams_;
   }
 
+  bool enable_rtp_data_channel() const {
+    return configuration_.enable_rtp_data_channel;
+  }
+
  protected:
   // Constructor to be used for constructing mocks only.
   explicit RTCPeerConnectionHandler(
@@ -373,8 +380,6 @@ class MODULES_EXPORT RTCPeerConnectionHandler {
       blink::RtpTransceiverState transceiver_state,
       blink::TransceiverStateUpdateMode update_mode);
 
-  scoped_refptr<base::SingleThreadTaskRunner> signaling_thread() const;
-
   // Initialize() is never expected to be called more than once, even if the
   // first call fails.
   bool initialize_called_ = false;