Report errors in RTCPeerConnection legacy functions via the the failure callback instead of excepti…

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

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 83 matching lines @@
 {
     if (state == RTCPeerConnection::SignalingStateClosed) {
         exceptionState.throwDOMException(InvalidStateError, kSignalingStateClosedMessage);
         return true;
     }
 
     return false;
 }
 
 // Helper class for running error callbacks asynchronously
 class ErrorCallbackTask : public WebTaskRunner::Task {

[philipj_slow, 2016/03/01 04:53:05]

This class is gone, needs rebase.

[Guido Urdaneta, 2016/03/09 15:04:35]

Done.

 public:
     static PassOwnPtr<ErrorCallbackTask> create(RTCPeerConnectionErrorCallback* errorCallback, DOMException* exception)
     {
         return adoptPtr(new ErrorCallbackTask(errorCallback, exception));
     }
 
     ~ErrorCallbackTask() override = default;
 
     void run() override
     {
@@ skipping 237 matching lines @@
                 return 0;
             }
 
             rtcConfiguration->appendCertificate(certificate->certificateShallowCopy());
         }
     }
 
     return rtcConfiguration;
 }
 
-RTCOfferOptions* RTCPeerConnection::parseOfferOptions(const Dictionary& options, ExceptionState& exceptionState)
+RTCOfferOptions* RTCPeerConnection::parseOfferOptions(const Dictionary& options)
 {
     if (options.isUndefinedOrNull())
         return 0;
 
     Vector<String> propertyNames;
     options.getPropertyNames(propertyNames);
 
     // Treat |options| as MediaConstraints if it is empty or has "optional" or "mandatory" properties for compatibility.
     // TODO(jiayl): remove constraints when RTCOfferOptions reaches Stable and client code is ready.
     if (propertyNames.isEmpty() || propertyNames.contains("optional") || propertyNames.contains("mandatory"))
         return 0;
 
     int32_t offerToReceiveVideo = -1;
     int32_t offerToReceiveAudio = -1;
     bool voiceActivityDetection = true;
     bool iceRestart = false;
 
-    if (DictionaryHelper::get(options, "offerToReceiveVideo", offerToReceiveVideo) && offerToReceiveVideo < 0) {
-        exceptionState.throwTypeError("Invalid offerToReceiveVideo");
-        return 0;
-    }
-
-    if (DictionaryHelper::get(options, "offerToReceiveAudio", offerToReceiveAudio) && offerToReceiveAudio < 0) {
-        exceptionState.throwTypeError("Invalid offerToReceiveAudio");
-        return 0;
-    }
-
+    if (DictionaryHelper::get(options, "offerToReceiveVideo", offerToReceiveVideo) && offerToReceiveVideo < 0)
+        offerToReceiveVideo = 0;
+    if (DictionaryHelper::get(options, "offerToReceiveAudio", offerToReceiveAudio) && offerToReceiveAudio < 0)
+        offerToReceiveAudio = 0;
     DictionaryHelper::get(options, "voiceActivityDetection", voiceActivityDetection);
     DictionaryHelper::get(options, "iceRestart", iceRestart);
 
     RTCOfferOptions* rtcOfferOptions = RTCOfferOptions::create(offerToReceiveVideo, offerToReceiveAudio, voiceActivityDetection, iceRestart);
     return rtcOfferOptions;
 }
 
 RTCPeerConnection* RTCPeerConnection::create(ExecutionContext* context, const Dictionary& rtcConfiguration, const Dictionary& mediaConstraints, ExceptionState& exceptionState)
 {
     if (mediaConstraints.isObject())
@@ skipping 58 matching lines @@
     }
 }
 
 RTCPeerConnection::~RTCPeerConnection()
 {
     // This checks that close() or stop() is called before the destructor.
     // We are assuming that a wrapper is always created when RTCPeerConnection is created.
     ASSERT(m_closed || m_stopped);
 }
 
-void RTCPeerConnection::createOffer(ExecutionContext* context, RTCSessionDescriptionCallback* successCallback, RTCPeerConnectionErrorCallback* errorCallback, const Dictionary& rtcOfferOptions, ExceptionState& exceptionState)
+void RTCPeerConnection::createOffer(ExecutionContext* context, RTCSessionDescriptionCallback* successCallback, RTCPeerConnectionErrorCallback* errorCallback, const Dictionary& rtcOfferOptions)
 {
-    if (errorCallback)
-        UseCounter::count(context, UseCounter::RTCPeerConnectionCreateOfferLegacyFailureCallback);
-    else
-        Deprecation::countDeprecation(context, UseCounter::RTCPeerConnectionCreateOfferLegacyNoFailureCallback);
-
-    if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
+    ASSERT(successCallback);
+    ASSERT(errorCallback);
+    UseCounter::count(context, UseCounter::RTCPeerConnectionCreateOfferLegacyFailureCallback);
+    if (callErrorCallbackIfSignalingStateClosed(m_signalingState, errorCallback))
         return;
 
-    ASSERT(successCallback);
-
-    RTCOfferOptions* offerOptions = parseOfferOptions(rtcOfferOptions, exceptionState);
-    if (exceptionState.hadException())
-        return;
-
+    RTCOfferOptions* offerOptions = parseOfferOptions(rtcOfferOptions);
     RTCSessionDescriptionRequest* request = RTCSessionDescriptionRequestImpl::create(executionContext(), this, successCallback, errorCallback);
 
     if (offerOptions) {
         if (offerOptions->offerToReceiveAudio() != -1 || offerOptions->offerToReceiveVideo() != -1)
             UseCounter::count(context, UseCounter::RTCPeerConnectionCreateOfferLegacyOfferOptions);
-        else if (errorCallback)
+        else
             UseCounter::count(context, UseCounter::RTCPeerConnectionCreateOfferLegacyCompliant);
 
         m_peerHandler->createOffer(request, offerOptions);
     } else {
         MediaErrorState mediaErrorState;
         WebMediaConstraints constraints = MediaConstraintsImpl::create(context, rtcOfferOptions, mediaErrorState);
-        if (mediaErrorState.hadException()) {
-            mediaErrorState.raiseException(exceptionState);
+        if (mediaErrorState.canGenerateException()) {

[philipj_slow, 2016/03/01 04:53:05]

It's a bit subtle why this works. Isn't it a bug in MediaErrorState.cpp that
canGenerateException returns false for ConstraintError when raiseException in
fact *can* generate an exception for it?

I was assuming you'd remove the error cases from MediaConstraintsImpl.cpp
entirely, then you could also remove those bits from MediaErrorState. Is there a
reason to avoid doing that?

[hta - Chromium, 2016/03/07 15:15:47]

The reason for canGenerateException is that the DOMException class can't be used
for a constraintError - it doesn't have a constraintname field, and isn't
extensible to add one. However, the error callback *can* pass a constraintError,
which has the constraintName field.

The spec says that the name of the failed constraint has to be reported. So we
must use a mechanism that is able to pass it - ie the callback. (We can also
reject the promise with a constraintError, given the lack of strong typing on
promise rejection - up to you whether you consider that an ECMAscript bug or a
feature.)

But we can't use DOMException::create() to create the error argument.

[philipj_slow, 2016/03/09 13:15:31]

OK, so this is related to NavigatorUserMediaError and the fact that
MediaErrorState is also used for getUserMedia().

Maybe an isContraintError() and createConstraintError() pair would make this
more obvious at the call sites.

I guess changing getUserMedia at the same time would be needless risk, but can
you comment which errors this is suppressing and what "canGenerateException" is
used to mean in this context?

[Guido Urdaneta, 2016/03/09 15:04:35]

Done.

+            String errorMsg = mediaErrorState.getErrorMessage();
+            asyncCallErrorCallback(errorCallback, DOMException::create(OperationError, errorMsg));
             return;
         }
 
         if (!constraints.isEmpty())
             UseCounter::count(context, UseCounter::RTCPeerConnectionCreateOfferLegacyConstraints);
-        else if (errorCallback)
+        else
             UseCounter::count(context, UseCounter::RTCPeerConnectionCreateOfferLegacyCompliant);
 
         m_peerHandler->createOffer(request, constraints);
     }
 }
 
-void RTCPeerConnection::createAnswer(ExecutionContext* context, RTCSessionDescriptionCallback* successCallback, RTCPeerConnectionErrorCallback* errorCallback, const Dictionary& mediaConstraints, ExceptionState& exceptionState)
+void RTCPeerConnection::createAnswer(ExecutionContext* context, RTCSessionDescriptionCallback* successCallback, RTCPeerConnectionErrorCallback* errorCallback, const Dictionary& mediaConstraints)
 {
-    if (errorCallback)
-        UseCounter::count(context, UseCounter::RTCPeerConnectionCreateAnswerLegacyFailureCallback);
-    else
-        Deprecation::countDeprecation(context, UseCounter::RTCPeerConnectionCreateAnswerLegacyNoFailureCallback);
-
+    ASSERT(successCallback);
+    ASSERT(errorCallback);
+    UseCounter::count(context, UseCounter::RTCPeerConnectionCreateAnswerLegacyFailureCallback);
     if (mediaConstraints.isObject())
         UseCounter::count(context, UseCounter::RTCPeerConnectionCreateAnswerLegacyConstraints);
-    else if (errorCallback)
+    else
         UseCounter::count(context, UseCounter::RTCPeerConnectionCreateAnswerLegacyCompliant);
 
-    if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
+    if (callErrorCallbackIfSignalingStateClosed(m_signalingState, errorCallback))
         return;
 
-    ASSERT(successCallback);
-
     MediaErrorState mediaErrorState;
     WebMediaConstraints constraints = MediaConstraintsImpl::create(context, mediaConstraints, mediaErrorState);
-    if (mediaErrorState.hadException()) {
-        mediaErrorState.raiseException(exceptionState);
+    if (mediaErrorState.canGenerateException()) {
+        String errorMsg = mediaErrorState.getErrorMessage();
+        asyncCallErrorCallback(errorCallback, DOMException::create(OperationError, errorMsg));
         return;
     }
 
     RTCSessionDescriptionRequest* request = RTCSessionDescriptionRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     m_peerHandler->createAnswer(request, constraints);
 }
 
 ScriptPromise RTCPeerConnection::setLocalDescription(ScriptState* scriptState, const RTCSessionDescriptionInit& sessionDescriptionInit)
 {
     if (m_signalingState == SignalingStateClosed)
@@ skipping 618 matching lines @@
 {
     visitor->trace(m_localStreams);
     visitor->trace(m_remoteStreams);
     visitor->trace(m_dispatchScheduledEventRunner);
     visitor->trace(m_scheduledEvents);
     RefCountedGarbageCollectedEventTargetWithInlineData<RTCPeerConnection>::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }
 
 } // namespace blink