Add promise-based addIceCandidate, setLocalDescription and setRemoteDescription to RTCPeerConnection

third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp

Index: third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp
diff --git a/third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp b/third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp
index 854a5babc9d35c6b9a4521f59b7bde8c605c4088..b31aff3c5546617b6308f72ab628a7a770e5b623 100644
--- a/third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp
+++ b/third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp
@@ -35,10 +35,15 @@
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/Nullable.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
+#include "bindings/core/v8/ScriptState.h"
+#include "bindings/core/v8/ScriptValue.h"
+#include "bindings/core/v8/V8ThrowException.h"
+#include "bindings/modules/v8/UnionTypesModules.h"
 #include "bindings/modules/v8/V8RTCCertificate.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContext.h"
+#include "core/dom/Microtask.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/UseCounter.h"
 #include "core/html/VoidCallback.h"
@@ -54,10 +59,12 @@
 #include "modules/mediastream/RTCIceCandidateEvent.h"
 #include "modules/mediastream/RTCSessionDescription.h"
 #include "modules/mediastream/RTCSessionDescriptionCallback.h"
+#include "modules/mediastream/RTCSessionDescriptionInit.h"
 #include "modules/mediastream/RTCSessionDescriptionRequestImpl.h"
 #include "modules/mediastream/RTCStatsCallback.h"
 #include "modules/mediastream/RTCStatsRequestImpl.h"
 #include "modules/mediastream/RTCVoidRequestImpl.h"
+#include "modules/mediastream/RTCVoidRequestPromiseImpl.h"
 #include "platform/mediastream/RTCConfiguration.h"
 #include "platform/mediastream/RTCOfferOptions.h"
 #include "public/platform/Platform.h"
@@ -81,16 +88,85 @@ namespace blink {
 
 namespace {
 
-static bool throwExceptionIfSignalingStateClosed(RTCPeerConnection::SignalingState state, ExceptionState& exceptionState)
+const char kSignalingStateClosedMsg[] = "The RTCPeerConnection's signalingState is 'closed'.";

[haraken, 2016/02/13 15:07:18]

Msg => Message (Blink prefers a fully qualified name.)

[Guido Urdaneta, 2016/02/15 16:56:51]

Done.

+
+// TODO(guidou): Update exception codes once the error-handling aspects of the spec are finalized. crbug.com/585621

[philipj_slow, 2016/02/15 10:00:44]

AFAICT the spec now uses only existing DOMException names:
https://github.com/w3c/webrtc-pc/issues/495#issuecomment-184057583

[Guido Urdaneta, 2016/02/15 16:56:51]

Done.

+const ExceptionCode kDefaultErrorName = AbortError;
+const ExceptionCode kSessionDescriptionErrorName = AbortError;
+
+bool throwExceptionIfSignalingStateClosed(RTCPeerConnection::SignalingState state, ExceptionState& exceptionState)
 {
     if (state == RTCPeerConnection::SignalingStateClosed) {
-        exceptionState.throwDOMException(InvalidStateError, "The RTCPeerConnection's signalingState is 'closed'.");
+        exceptionState.throwDOMException(InvalidStateError, kSignalingStateClosedMsg);
         return true;
     }
 
     return false;
 }
 
+// Helper class for running error callbacks asynchronously
+class ErrorCallbackTask : public WebTaskRunner::Task {
+public:
+    static PassOwnPtr<ErrorCallbackTask> create(RTCErrorCallback* errorCallback, const String& errorMsg)

[haraken, 2016/02/13 15:07:18]

Msg => Message

The same comment for other parts in this file.

[Guido Urdaneta, 2016/02/15 16:56:51]

Done.

+    {
+        return adoptPtr(new ErrorCallbackTask(errorCallback, errorMsg));
+    }
+
+    ~ErrorCallbackTask() override = default;
+
+    void run() override
+    {
+        m_errorCallback->handleEvent(m_errorMsg);
+    }
+
+private:
+    ErrorCallbackTask(RTCErrorCallback* errorCallback, const String& errorMsg)
+        : m_errorCallback(errorCallback)
+        , m_errorMsg(errorMsg) {}

[haraken, 2016/02/13 15:07:18]

Shall we add ASSERT(m_errorCallback)?

[Guido Urdaneta, 2016/02/15 16:56:51]

Done.

+
+    Persistent<RTCErrorCallback> m_errorCallback;
+    String m_errorMsg;
+};
+
+void asyncCallErrorCallback(RTCErrorCallback* errorCallback, const String& errorMsg)
+{
+    Microtask::enqueueMicrotask(ErrorCallbackTask::create(errorCallback, errorMsg));
+}
+
+bool callErrorCallbackIfSignalingStateClosed(RTCPeerConnection::SignalingState state, RTCErrorCallback* errorCallback)
+{
+    if (state == RTCPeerConnection::SignalingStateClosed) {
+        if (errorCallback)
+            asyncCallErrorCallback(errorCallback, kSignalingStateClosedMsg);
+
+        return true;
+    }
+
+    return false;
+}
+
+bool isIceCandidateMissingSdp(const RTCIceCandidateInitOrRTCIceCandidate& candidate)
+{
+    if (candidate.isRTCIceCandidateInit()) {
+        const RTCIceCandidateInit& iceCandidateInit = candidate.getAsRTCIceCandidateInit();
+        return !iceCandidateInit.hasSdpMid() && !iceCandidateInit.hasSdpMLineIndex();
+    }
+
+    ASSERT(candidate.isRTCIceCandidate());
+    return false;
+}
+
+WebRTCICECandidate convertToWebRTCIceCandidate(const RTCIceCandidateInitOrRTCIceCandidate& candidate)
+{
+    if (candidate.isRTCIceCandidateInit()) {
+        const RTCIceCandidateInit& iceCandidateInit = candidate.getAsRTCIceCandidateInit();
+        return WebRTCICECandidate(iceCandidateInit.candidate(), iceCandidateInit.sdpMid(), iceCandidateInit.sdpMLineIndex());
+    }
+
+    ASSERT(candidate.isRTCIceCandidate());
+    return candidate.getAsRTCIceCandidate()->webCandidate();
+}
+
 // Helper class for RTCPeerConnection::generateCertificate.
 class WebRTCCertificateObserver : public WebCallbacks<WebRTCCertificate*, void> {
 public:
@@ -460,8 +536,21 @@ void RTCPeerConnection::createAnswer(ExecutionContext* context, RTCSessionDescri
     m_peerHandler->createAnswer(request, constraints);
 }
 
-void RTCPeerConnection::setLocalDescription(ExecutionContext* context, RTCSessionDescription* sessionDescription, VoidCallback* successCallback, RTCErrorCallback* errorCallback, ExceptionState& exceptionState)
+ScriptPromise RTCPeerConnection::setLocalDescription(ScriptState* scriptState, const RTCSessionDescriptionInit& sessionDescriptionInit)
 {
+    if (m_signalingState == SignalingStateClosed)
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, kSignalingStateClosedMsg));
+
+    ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
+    ScriptPromise promise = resolver->promise();
+    RTCVoidRequest* request = RTCVoidRequestPromiseImpl::create(this, resolver, kSessionDescriptionErrorName);
+    m_peerHandler->setLocalDescription(request, WebRTCSessionDescription(sessionDescriptionInit.type(), sessionDescriptionInit.sdp()));
+    return promise;
+}
+
+ScriptPromise RTCPeerConnection::setLocalDescription(ScriptState* scriptState, RTCSessionDescription* sessionDescription, VoidCallback* successCallback, RTCErrorCallback* errorCallback)

[haraken, 2016/02/13 15:07:18]

This method always returns an undefined ScriptPromise. Is it expected? Or is it
going to be changed in the future?

[philipj_slow, 2016/02/15 10:00:44]

That is expected. The methods that take success/error callbacks return a dummy
promises because they have the same name as the newer promise-vending versions.
I can't find where WebIDL says that it must be so, but it does somewhere and the
bindings generator checks for it.

[Guido Urdaneta, 2016/02/15 16:56:51]

To further clarify, the reason all methods with the same name have to return
promises is that if the method was called arguments that are invalid for all
overloads, it would not be possible to decide whether to throw a TypeError or
return a rejected Promise.

+{
+    ExecutionContext* context = scriptState->executionContext();
     if (successCallback && errorCallback) {
         UseCounter::count(context, UseCounter::RTCPeerConnectionSetLocalDescriptionLegacyCompliant);
     } else {
@@ -471,13 +560,14 @@ void RTCPeerConnection::setLocalDescription(ExecutionContext* context, RTCSessio
             UseCounter::count(context, UseCounter::RTCPeerConnectionSetLocalDescriptionLegacyNoFailureCallback);
     }
 
-    if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
-        return;
+    if (callErrorCallbackIfSignalingStateClosed(m_signalingState, errorCallback))
+        return ScriptPromise::cast(scriptState, v8::Undefined(scriptState->isolate()));

[haraken, 2016/02/13 15:07:18]

Can we just use ScriptPromise()?

[Guido Urdaneta, 2016/02/15 16:56:51]

Done. That is what the CL had originally.

 
     ASSERT(sessionDescription);
 
     RTCVoidRequest* request = RTCVoidRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     m_peerHandler->setLocalDescription(request, sessionDescription->webSessionDescription());
+    return ScriptPromise::cast(scriptState, v8::Undefined(scriptState->isolate()));
 }
 
 RTCSessionDescription* RTCPeerConnection::localDescription()
@@ -489,8 +579,21 @@ RTCSessionDescription* RTCPeerConnection::localDescription()
     return RTCSessionDescription::create(webSessionDescription);
 }
 
-void RTCPeerConnection::setRemoteDescription(ExecutionContext* context, RTCSessionDescription* sessionDescription, VoidCallback* successCallback, RTCErrorCallback* errorCallback, ExceptionState& exceptionState)
+ScriptPromise RTCPeerConnection::setRemoteDescription(ScriptState* scriptState, const RTCSessionDescriptionInit& sessionDescriptionInit)
+{
+    if (m_signalingState == SignalingStateClosed)
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, kSignalingStateClosedMsg));
+
+    ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
+    ScriptPromise promise = resolver->promise();
+    RTCVoidRequest* request = RTCVoidRequestPromiseImpl::create(this, resolver, kSessionDescriptionErrorName);
+    m_peerHandler->setRemoteDescription(request, WebRTCSessionDescription(sessionDescriptionInit.type(), sessionDescriptionInit.sdp()));
+    return promise;
+}
+
+ScriptPromise RTCPeerConnection::setRemoteDescription(ScriptState* scriptState, RTCSessionDescription* sessionDescription, VoidCallback* successCallback, RTCErrorCallback* errorCallback)
 {
+    ExecutionContext* context = scriptState->executionContext();
     if (successCallback && errorCallback) {
         UseCounter::count(context, UseCounter::RTCPeerConnectionSetRemoteDescriptionLegacyCompliant);
     } else {
@@ -500,13 +603,14 @@ void RTCPeerConnection::setRemoteDescription(ExecutionContext* context, RTCSessi
             UseCounter::count(context, UseCounter::RTCPeerConnectionSetRemoteDescriptionLegacyNoFailureCallback);
     }
 
-    if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
-        return;
+    if (callErrorCallbackIfSignalingStateClosed(m_signalingState, errorCallback))
+        return ScriptPromise::cast(scriptState, v8::Undefined(scriptState->isolate()));
 
     ASSERT(sessionDescription);
 
     RTCVoidRequest* request = RTCVoidRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     m_peerHandler->setRemoteDescription(request, sessionDescription->webSessionDescription());
+    return ScriptPromise::cast(scriptState, v8::Undefined(scriptState->isolate()));
 }
 
 RTCSessionDescription* RTCPeerConnection::remoteDescription()
@@ -608,33 +712,41 @@ ScriptPromise RTCPeerConnection::generateCertificate(ScriptState* scriptState, c
     return promise;
 }
 
-void RTCPeerConnection::addIceCandidate(RTCIceCandidate* iceCandidate, ExceptionState& exceptionState)
+ScriptPromise RTCPeerConnection::addIceCandidate(ScriptState* scriptState, const RTCIceCandidateInitOrRTCIceCandidate& candidate)
 {
-    if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
-        return;
+    if (m_signalingState == SignalingStateClosed)
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, kSignalingStateClosedMsg));
 
-    ASSERT(iceCandidate);
+    if (isIceCandidateMissingSdp(candidate))
+        return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(scriptState->isolate(), "Candidate missing values for both sdpMid and sdpMLineIndex"));
 
-    bool valid = m_peerHandler->addICECandidate(iceCandidate->webCandidate());
-    if (!valid)
-        exceptionState.throwDOMException(SyntaxError, "The ICE candidate could not be added.");
+    ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
+    ScriptPromise promise = resolver->promise();
+    RTCVoidRequest* request = RTCVoidRequestPromiseImpl::create(this, resolver, kDefaultErrorName);
+    WebRTCICECandidate webCandidate = convertToWebRTCIceCandidate(candidate);
+    bool implemented = m_peerHandler->addICECandidate(request, webCandidate);
+    // TODO(guidou): replace NotSupportedError when error handling in the spec is finalized. crbug.com/585621
+    if (!implemented)
+        resolver->reject(DOMException::create(NotSupportedError, "This method is not yet implemented."));
+
+    return promise;
 }
 
-void RTCPeerConnection::addIceCandidate(RTCIceCandidate* iceCandidate, VoidCallback* successCallback, RTCErrorCallback* errorCallback, ExceptionState& exceptionState)
+ScriptPromise RTCPeerConnection::addIceCandidate(ScriptState* scriptState, RTCIceCandidate* iceCandidate, VoidCallback* successCallback, RTCErrorCallback* errorCallback)
 {
-    if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
-        return;
-
     ASSERT(iceCandidate);
     ASSERT(successCallback);
     ASSERT(errorCallback);
 
-    RTCVoidRequest* request = RTCVoidRequestImpl::create(executionContext(), this, successCallback, errorCallback);
+    if (callErrorCallbackIfSignalingStateClosed(m_signalingState, errorCallback))
+        return ScriptPromise::cast(scriptState, v8::Undefined(scriptState->isolate()));
 
+    RTCVoidRequest* request = RTCVoidRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     bool implemented = m_peerHandler->addICECandidate(request, iceCandidate->webCandidate());
-    if (!implemented) {
-        exceptionState.throwDOMException(NotSupportedError, "This method is not yet implemented.");
-    }
+    if (!implemented)
+        asyncCallErrorCallback(errorCallback, "This method is not yet implemented.");
+
+    return ScriptPromise::cast(scriptState, v8::Undefined(scriptState->isolate()));
 }
 
 String RTCPeerConnection::signalingState() const