From 10721227e4626a4cab3c7a5308aa21c748c60aa8 Mon Sep 17 00:00:00 2001
From: Victor Boivie <boivie@webrtc.org>
Date: Fri, 27 Aug 2021 16:11:48 +0200
Subject: [PATCH] Revert "Propagate socket write errors for DtlsTransport"

There is a suspicion that it causes OpenSSL errors:
  [openssl_stream_adapter.cc(961)]
       OpenSSLStreamAdapter::Error(SSL_write, 5, 0)

This commit does change the interaction with OpenSSL as propagating the
socket write errors as SR_BLOCK results in calling BIO_set_retry_write,
as part of current implementation of OpenSSLStreamAdapter.

Testing this regression has proven to be hard to do manually.

This reverts commit edfaaef086ccff2dbff29d64c9a8d9f633637c57.

Bug: webrtc:12943
Change-Id: Ib6767bd4af68c59fd3b7cb051341876f175bb921
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/230420
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34875}
---
 p2p/base/dtls_transport.cc          | 15 +++++----------
 p2p/base/dtls_transport_unittest.cc | 10 ----------
 p2p/base/fake_ice_transport.h       | 17 +----------------
 3 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/p2p/base/dtls_transport.cc b/p2p/base/dtls_transport.cc
index ce2793cb46..172d06188f 100644
--- a/p2p/base/dtls_transport.cc
+++ b/p2p/base/dtls_transport.cc
@@ -93,17 +93,12 @@ rtc::StreamResult StreamInterfaceChannel::Write(const void* data,
                                                 size_t* written,
                                                 int* error) {
   RTC_DCHECK_RUN_ON(&sequence_checker_);
+  // Always succeeds, since this is an unreliable transport anyway.
+  // TODO(zhihuang): Should this block if ice_transport_'s temporarily
+  // unwritable?
   rtc::PacketOptions packet_options;
-  int sent = ice_transport_->SendPacket(static_cast<const char*>(data),
-                                        data_len, packet_options);
-  if (sent < 0) {
-    if (written) {
-      *written = 0;
-    }
-    return rtc::IsBlockingError(ice_transport_->GetError()) ? rtc::SR_BLOCK
-                                                            : rtc::SR_ERROR;
-  }
-
+  ice_transport_->SendPacket(static_cast<const char*>(data), data_len,
+                             packet_options);
   if (written) {
     *written = data_len;
   }
diff --git a/p2p/base/dtls_transport_unittest.cc b/p2p/base/dtls_transport_unittest.cc
index 4957b46a50..851c1ea131 100644
--- a/p2p/base/dtls_transport_unittest.cc
+++ b/p2p/base/dtls_transport_unittest.cc
@@ -421,16 +421,6 @@ TEST_F(DtlsTransportTest, TestTransferDtls) {
   TestTransfer(1000, 100, /*srtp=*/false);
 }
 
-// Connect with DTLS, and fail to write, to ensure errors are propagated.
-TEST_F(DtlsTransportTest, TestWriteFailsOverDtls) {
-  PrepareDtls(rtc::KT_DEFAULT);
-  ASSERT_TRUE(Connect());
-  client1_.fake_ice_transport()->SetError(EAGAIN);
-  int res = client1_.dtls_transport()->SendPacket("hello", 5,
-                                                  rtc::PacketOptions(), 0);
-  EXPECT_EQ(res, -1);
-}
-
 // Connect with DTLS, combine multiple DTLS records into one packet.
 // Our DTLS implementation doesn't do this, but other implementations may;
 // see https://tools.ietf.org/html/rfc6347#section-4.1.1.
diff --git a/p2p/base/fake_ice_transport.h b/p2p/base/fake_ice_transport.h
index 7f934446e4..8b52fe934c 100644
--- a/p2p/base/fake_ice_transport.h
+++ b/p2p/base/fake_ice_transport.h
@@ -302,10 +302,6 @@ class FakeIceTransport : public IceTransportInternal {
       return -1;
     }
 
-    if (error_ != 0) {
-      return -1;
-    }
-
     send_packet_.AppendData(data, len);
     if (!combine_outgoing_packets_ || send_packet_.size() > len) {
       rtc::CopyOnWriteBuffer packet(std::move(send_packet_));
@@ -342,17 +338,7 @@ class FakeIceTransport : public IceTransportInternal {
     }
   }
 
-  // Sets the error that is returned by `GetError`. If an error is set (i.e.
-  // non-zero), `SendPacket` will return -1.
-  void SetError(int error) {
-    RTC_DCHECK_RUN_ON(network_thread_);
-    error_ = error;
-  }
-
-  int GetError() override {
-    RTC_DCHECK_RUN_ON(network_thread_);
-    return error_;
-  }
+  int GetError() override { return 0; }
 
   rtc::CopyOnWriteBuffer last_sent_packet() {
     RTC_DCHECK_RUN_ON(network_thread_);
@@ -435,7 +421,6 @@ class FakeIceTransport : public IceTransportInternal {
   rtc::CopyOnWriteBuffer last_sent_packet_ RTC_GUARDED_BY(network_thread_);
   rtc::Thread* const network_thread_;
   webrtc::ScopedTaskSafetyDetached task_safety_;
-  int error_ RTC_GUARDED_BY(network_thread_) = 0;
 };
 
 class FakeIceTransportWrapper : public webrtc::IceTransportInterface {