Introduce CreateDataChannelOrError

Deprecate CreateDataChannel, and make it a simple wrapper function.

Bug: webrtc:12796
Change-Id: I053d75a264596ba87ca734a29df9241de93a80c3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219784
Reviewed-by: Xavier Lepaul‎ <xalep@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34130}

api/peer_connection_interface.h

@@ -920,9 +920,24 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
   // Also, calling CreateDataChannel is the only way to get a data "m=" section
   // in SDP, so it should be done before CreateOffer is called, if the
   // application plans to use data channels.
+  virtual RTCErrorOr<rtc::scoped_refptr<DataChannelInterface>>
+  CreateDataChannelOrError(const std::string& label,
+                           const DataChannelInit* config) {
+    return RTCError(RTCErrorType::INTERNAL_ERROR, "dummy function called");
+  }
+  // TODO(crbug.com/788659): Remove "virtual" below and default implementation
+  // above once mock in Chrome is fixed.
+  ABSL_DEPRECATED("Use CreateDataChannelOrError")
   virtual rtc::scoped_refptr<DataChannelInterface> CreateDataChannel(
       const std::string& label,
-      const DataChannelInit* config) = 0;
+      const DataChannelInit* config) {
+    auto result = CreateDataChannelOrError(label, config);
+    if (!result.ok()) {
+      return nullptr;
+    } else {
+      return result.MoveValue();
+    }
+  }
 
   // NOTE: For the following 6 methods, it's only safe to dereference the
   // SessionDescriptionInterface on signaling_thread() (for example, calling

api/peer_connection_proxy.h

@@ -76,8 +76,8 @@ PROXY_METHOD2(void,
               rtc::scoped_refptr<RtpReceiverInterface>,
               rtc::scoped_refptr<RTCStatsCollectorCallback>)
 PROXY_METHOD0(void, ClearStatsCache)
-PROXY_METHOD2(rtc::scoped_refptr<DataChannelInterface>,
+PROXY_METHOD2(RTCErrorOr<rtc::scoped_refptr<DataChannelInterface>>,
-              CreateDataChannel,
+              CreateDataChannelOrError,
               const std::string&,
               const DataChannelInit*)
 PROXY_CONSTMETHOD0(const SessionDescriptionInterface*, local_description)

api/test/dummy_peer_connection.h

@@ -114,10 +114,10 @@ class DummyPeerConnection : public PeerConnectionInterface {
   }
   void ClearStatsCache() override {}
 
-  rtc::scoped_refptr<DataChannelInterface> CreateDataChannel(
+  RTCErrorOr<rtc::scoped_refptr<DataChannelInterface>> CreateDataChannelOrError(
       const std::string& label,
       const DataChannelInit* config) override {
-    return nullptr;
+    return RTCError(RTCErrorType::INTERNAL_ERROR, "Dummy function called");
   }
 
   const SessionDescriptionInterface* local_description() const override {

api/test/mock_peerconnectioninterface.h

@@ -100,8 +100,8 @@ class MockPeerConnectionInterface
               GetSctpTransport,
               (),
               (const override));
-  MOCK_METHOD(rtc::scoped_refptr<DataChannelInterface>,
+  MOCK_METHOD(RTCErrorOr<rtc::scoped_refptr<DataChannelInterface>>,
-              CreateDataChannel,
+              CreateDataChannelOrError,
               (const std::string&, const DataChannelInit*),
               (override));
   MOCK_METHOD(const SessionDescriptionInterface*,

examples/unityplugin/simple_peer_connection.cc

@@ -497,8 +497,9 @@ bool SimplePeerConnection::CreateDataChannel() {
   struct webrtc::DataChannelInit init;
   init.ordered = true;
   init.reliable = true;
-  data_channel_ = peer_connection_->CreateDataChannel("Hello", &init);
+  auto result = peer_connection_->CreateDataChannelOrError("Hello", &init);
-  if (data_channel_.get()) {
+  if (result.ok()) {
+    data_channel_ = result.MoveValue();
     data_channel_->RegisterObserver(this);
     RTC_LOG(LS_INFO) << "Succeeds to create data channel";
     return true;

pc/peer_connection.cc

@@ -1279,8 +1279,8 @@ absl::optional<bool> PeerConnection::can_trickle_ice_candidates() {
       "trickle");
 }
 
-rtc::scoped_refptr<DataChannelInterface> PeerConnection::CreateDataChannel(
+RTCErrorOr<rtc::scoped_refptr<DataChannelInterface>>
-    const std::string& label,
+PeerConnection::CreateDataChannelOrError(const std::string& label,
                                          const DataChannelInit* config) {
   RTC_DCHECK_RUN_ON(signaling_thread());
   TRACE_EVENT0("webrtc", "PeerConnection::CreateDataChannel");
@@ -1291,11 +1291,13 @@ rtc::scoped_refptr<DataChannelInterface> PeerConnection::CreateDataChannel(
   if (config) {
     internal_config.reset(new InternalDataChannelInit(*config));
   }
+  // TODO(bugs.webrtc.org/12796): Return a more specific error.
   rtc::scoped_refptr<DataChannelInterface> channel(
       data_channel_controller_.InternalCreateDataChannelWithProxy(
           label, internal_config.get()));
   if (!channel.get()) {
-    return nullptr;
+    return RTCError(RTCErrorType::INTERNAL_ERROR,
+                    "Data channel creation failed");
   }
 
   // Trigger the onRenegotiationNeeded event for

pc/peer_connection.h

@@ -167,7 +167,7 @@ class PeerConnection : public PeerConnectionInternal,
   std::vector<rtc::scoped_refptr<RtpTransceiverInterface>> GetTransceivers()
       const override;
 
-  rtc::scoped_refptr<DataChannelInterface> CreateDataChannel(
+  RTCErrorOr<rtc::scoped_refptr<DataChannelInterface>> CreateDataChannelOrError(
       const std::string& label,
       const DataChannelInit* config) override;
   // WARNING: LEGACY. See peerconnectioninterface.h

pc/peer_connection_wrapper.cc

@@ -306,7 +306,14 @@ rtc::scoped_refptr<RtpSenderInterface> PeerConnectionWrapper::AddVideoTrack(
 
 rtc::scoped_refptr<DataChannelInterface>
 PeerConnectionWrapper::CreateDataChannel(const std::string& label) {
-  return pc()->CreateDataChannel(label, nullptr);
+  auto result = pc()->CreateDataChannelOrError(label, nullptr);
+  if (!result.ok()) {
+    RTC_LOG(LS_ERROR) << "CreateDataChannel failed: "
+                      << ToString(result.error().type()) << " "
+                      << result.error().message();
+    return nullptr;
+  }
+  return result.MoveValue();
 }
 
 PeerConnectionInterface::SignalingState

pc/test/fake_peer_connection_base.h

@@ -120,10 +120,11 @@ class FakePeerConnectionBase : public PeerConnectionInternal {
     return nullptr;
   }
 
-  rtc::scoped_refptr<DataChannelInterface> CreateDataChannel(
+  RTCErrorOr<rtc::scoped_refptr<DataChannelInterface>> CreateDataChannelOrError(
       const std::string& label,
       const DataChannelInit* config) override {
-    return nullptr;
+    return RTCError(RTCErrorType::UNSUPPORTED_OPERATION,
+                    "Fake function called");
   }
 
   const SessionDescriptionInterface* local_description() const override {

pc/test/peer_connection_test_wrapper.cc

@@ -140,7 +140,14 @@ rtc::scoped_refptr<webrtc::DataChannelInterface>
 PeerConnectionTestWrapper::CreateDataChannel(
     const std::string& label,
     const webrtc::DataChannelInit& init) {
-  return peer_connection_->CreateDataChannel(label, &init);
+  auto result = peer_connection_->CreateDataChannelOrError(label, &init);
+  if (!result.ok()) {
+    RTC_LOG(LS_ERROR) << "CreateDataChannel failed: "
+                      << ToString(result.error().type()) << " "
+                      << result.error().message();
+    return nullptr;
+  }
+  return result.MoveValue();
 }
 
 void PeerConnectionTestWrapper::WaitForNegotiation() {

sdk/android/src/jni/pc/peer_connection.cc

@@ -538,10 +538,12 @@ static ScopedJavaLocalRef<jobject> JNI_PeerConnection_CreateDataChannel(
     const JavaParamRef<jstring>& j_label,
     const JavaParamRef<jobject>& j_init) {
   DataChannelInit init = JavaToNativeDataChannelInit(jni, j_init);
-  rtc::scoped_refptr<DataChannelInterface> channel(
+  auto result = ExtractNativePC(jni, j_pc)->CreateDataChannelOrError(
-      ExtractNativePC(jni, j_pc)->CreateDataChannel(
+      JavaToNativeString(jni, j_label), &init);
-          JavaToNativeString(jni, j_label), &init));
+  if (!result.ok()) {
-  return WrapNativeDataChannel(jni, channel);
+    return WrapNativeDataChannel(jni, nullptr);
+  }
+  return WrapNativeDataChannel(jni, result.MoveValue());
 }
 
 static void JNI_PeerConnection_CreateOffer(

sdk/objc/api/peerconnection/RTCPeerConnection+DataChannel.mm

@@ -23,14 +23,12 @@
   std::string labelString = [NSString stdStringForString:label];
   const webrtc::DataChannelInit nativeInit =
       configuration.nativeDataChannelInit;
-  rtc::scoped_refptr<webrtc::DataChannelInterface> dataChannel =
+  auto result = self.nativePeerConnection->CreateDataChannelOrError(labelString, &nativeInit);
-      self.nativePeerConnection->CreateDataChannel(labelString,
+  if (!result.ok()) {
-                                                   &nativeInit);
-  if (!dataChannel) {
     return nil;
   }
   return [[RTC_OBJC_TYPE(RTCDataChannel) alloc] initWithFactory:self.factory
-                                              nativeDataChannel:dataChannel];
+                                              nativeDataChannel:result.MoveValue()];
 }
 
 @end