MediaDevices.getUserMedia should reject promise instead of throwing exceptions
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 May 2015 08:10:00 +0000 (08:10 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 May 2015 08:10:00 +0000 (08:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145282

Reviewed by Darin Adler.

Source/WebCore:

Ensuring at the custom binding level that all potential errors are used to reject promise.
Cleaned up the wrappers by removing unneeded RefPtr.

Covered by modified test.

* Modules/mediastream/MediaDevices.cpp:
(WebCore::MediaDevices::getUserMedia):
* Modules/mediastream/MediaDevices.h:
* Modules/mediastream/UserMediaRequest.cpp:
(WebCore::UserMediaRequest::create):
* bindings/js/JSMediaDevicesCustom.cpp:
(WebCore::JSMediaDevices::getUserMedia):

LayoutTests:

Updating test to expect rejection and not error throwing.

* fast/mediastream/MediaDevices-getUserMedia-expected.txt:
* fast/mediastream/MediaDevices-getUserMedia.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@184984 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/mediastream/MediaDevices-getUserMedia-expected.txt
LayoutTests/fast/mediastream/MediaDevices-getUserMedia.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/MediaDevices.cpp
Source/WebCore/Modules/mediastream/MediaDevices.h
Source/WebCore/Modules/mediastream/UserMediaRequest.cpp
Source/WebCore/bindings/js/JSMediaDevicesCustom.cpp

index 8aa59b2..de4df72 100644 (file)
@@ -1,3 +1,15 @@
+2015-05-29  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        MediaDevices.getUserMedia should reject promise instead of throwing exceptions
+        https://bugs.webkit.org/show_bug.cgi?id=145282
+
+        Reviewed by Darin Adler.
+
+        Updating test to expect rejection and not error throwing.
+
+        * fast/mediastream/MediaDevices-getUserMedia-expected.txt:
+        * fast/mediastream/MediaDevices-getUserMedia.html:
+
 2015-05-28  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
 
         [EFL] Unreviewed gardening on 29th May
index b5ae81b..e059e31 100644 (file)
@@ -3,10 +3,10 @@ Tests getUserMedia (Promise-based version on navigator.mediaDevices)
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS navigator.mediaDevices.getUserMedia(); threw exception TypeError: First argument of getUserMedia must be a valid Dictionary.
-PASS navigator.mediaDevices.getUserMedia({}); threw exception Error: NotSupportedError: DOM Exception 9.
-PASS navigator.mediaDevices.webkitGetUserMedia({audio:true}); threw exception TypeError: navigator.mediaDevices.webkitGetUserMedia is not a function. (In 'navigator.mediaDevices.webkitGetUserMedia({audio:true})', 'navigator.mediaDevices.webkitGetUserMedia' is undefined).
+PASS typeof navigator.mediaDevices.webkitGetUserMedia is 'undefined'
 PASS navigator.mediaDevices.getUserMedia({audio:true}).then(gotStream1); did not throw exception.
+PASS navigator.mediaDevices.getUserMedia() rejected with error: TypeError: First argument of getUserMedia must be a valid Dictionary
+PASS  navigator.mediaDevices.getUserMedia({}) rejected with error: Error: NotSupportedError: DOM Exception 9
 PASS Stream generated.
 PASS stream.getAudioTracks().length is 1
 PASS stream.getVideoTracks().length is 0
index 8065015..07e81e5 100644 (file)
                 shouldNotThrow("navigator.mediaDevices.getUserMedia({video:true}).then(gotStream2);")
             }
 
-            shouldThrow("navigator.mediaDevices.getUserMedia();");
-            shouldThrow("navigator.mediaDevices.getUserMedia({});");
-            shouldThrow("navigator.mediaDevices.webkitGetUserMedia({audio:true});");
+            navigator.mediaDevices.getUserMedia().then(invalidGotStream, function(error) {
+                testPassed("navigator.mediaDevices.getUserMedia() rejected with error: " + error);
+            });
+            navigator.mediaDevices.getUserMedia({}).then(invalidGotStream, function(error) {
+                testPassed(" navigator.mediaDevices.getUserMedia({}) rejected with error: " + error);
+            });
+            shouldBe("typeof navigator.mediaDevices.webkitGetUserMedia", "'undefined'");
             setUserMediaPermission(true);
             shouldNotThrow("navigator.mediaDevices.getUserMedia({audio:true}).then(gotStream1);");
 
index 0deb857..ae239ab 100644 (file)
@@ -1,3 +1,23 @@
+2015-05-29  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        MediaDevices.getUserMedia should reject promise instead of throwing exceptions
+        https://bugs.webkit.org/show_bug.cgi?id=145282
+
+        Reviewed by Darin Adler.
+
+        Ensuring at the custom binding level that all potential errors are used to reject promise.
+        Cleaned up the wrappers by removing unneeded RefPtr.
+
+        Covered by modified test.
+
+        * Modules/mediastream/MediaDevices.cpp:
+        (WebCore::MediaDevices::getUserMedia):
+        * Modules/mediastream/MediaDevices.h:
+        * Modules/mediastream/UserMediaRequest.cpp:
+        (WebCore::UserMediaRequest::create):
+        * bindings/js/JSMediaDevicesCustom.cpp:
+        (WebCore::JSMediaDevices::getUserMedia):
+
 2015-05-28  Hunseop Jeong  <hs85.jeong@samsung.com>
 
         Replaced 0 with nullptr in WebCore/accessibility.
index 0e760d9..02fbc86 100644 (file)
@@ -64,6 +64,7 @@ void MediaDevices::getUserMedia(const Dictionary& options, ResolveCallback resol
 {
     UserMediaController* userMedia = UserMediaController::from(document() ? document()->page() : nullptr);
     if (!userMedia) {
+        // FIXME: We probably want to return a MediaStreamError here using the rejectCallback, and get rid off the ExceptionCode parameter.
         ec = NOT_SUPPORTED_ERR;
         return;
     }
index 19ccddb..74a781c 100644 (file)
@@ -55,8 +55,8 @@ public:
 
     Document* document() const;
 
-    typedef std::function<void(RefPtr<MediaStream>)> ResolveCallback;
-    typedef std::function<void(RefPtr<NavigatorUserMediaError>)> RejectCallback;
+    typedef std::function<void(MediaStream&)> ResolveCallback;
+    typedef std::function<void(NavigatorUserMediaError&)> RejectCallback;
 
     void getUserMedia(const Dictionary&, ResolveCallback, RejectCallback, ExceptionCode&) const;
 
index f0fc63a..7d6064c 100644 (file)
@@ -75,12 +75,12 @@ RefPtr<UserMediaRequest> UserMediaRequest::create(ScriptExecutionContext* contex
 {
     ASSERT(successCallback);
 
-    auto resolveCallback = [successCallback](RefPtr<MediaStream> stream) mutable {
-        successCallback->handleEvent(stream.get());
+    auto resolveCallback = [successCallback](MediaStream& stream) mutable {
+        successCallback->handleEvent(&stream);
     };
-    auto rejectCallback = [errorCallback](RefPtr<NavigatorUserMediaError> error) mutable {
+    auto rejectCallback = [errorCallback](NavigatorUserMediaError& error) mutable {
         if (errorCallback)
-            errorCallback->handleEvent(error.get());
+            errorCallback->handleEvent(&error);
     };
 
     return UserMediaRequest::create(context, controller, options, WTF::move(resolveCallback), WTF::move(rejectCallback), ec);
@@ -177,7 +177,7 @@ void UserMediaRequest::didCreateStream(PassRefPtr<MediaStreamPrivate> privateStr
         for (auto& track : stream->getVideoTracks())
             track->applyConstraints(protectedThis->m_videoConstraints);
 
-        protectedThis->m_resolveCallback(stream.get());
+        protectedThis->m_resolveCallback(*stream);
     });
 }
 
@@ -190,7 +190,7 @@ void UserMediaRequest::failedToCreateStreamWithConstraintsError(const String& co
     RefPtr<UserMediaRequest> protectedThis(this);
     RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(NavigatorUserMediaError::constraintNotSatisfiedErrorName(), constraintName);
     callOnMainThread([protectedThis, error] {
-        protectedThis->m_rejectCallback(error.get());
+        protectedThis->m_rejectCallback(*error);
     });
 }
 
@@ -203,7 +203,7 @@ void UserMediaRequest::failedToCreateStreamWithPermissionError()
     // FIXME: Replace NavigatorUserMediaError with MediaStreamError (see bug 143335)
     RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(NavigatorUserMediaError::permissionDeniedErrorName(), emptyString());
     callOnMainThread([protectedThis, error] {
-        protectedThis->m_rejectCallback(error.get());
+        protectedThis->m_rejectCallback(*error);
     });
 }
 
index e401ab0..91149d8 100644 (file)
@@ -46,29 +46,31 @@ namespace WebCore {
 
 JSValue JSMediaDevices::getUserMedia(ExecState* exec)
 {
+    DeferredWrapper wrapper(exec, globalObject());
+
     Dictionary options(exec, exec->argument(0));
-    if (exec->hadException())
-        return jsUndefined();
+    if (exec->hadException()) {
+        wrapper.reject(exec->exception());
+        return wrapper.promise();
+    }
 
     if (!options.isObject()) {
-        throwVMError(exec, createTypeError(exec, "First argument of getUserMedia must be a valid Dictionary"));
-        return jsUndefined();
+        JSValue error = createTypeError(exec, "First argument of getUserMedia must be a valid Dictionary");
+        wrapper.reject(error);
+        return wrapper.promise();
     }
 
-    DeferredWrapper wrapper(exec, globalObject());
-    auto resolveCallback = [wrapper](RefPtr<MediaStream> stream) mutable {
-        wrapper.resolve(stream.get());
+    auto resolveCallback = [wrapper](MediaStream& stream) mutable {
+        wrapper.resolve(&stream);
     };
-    auto rejectCallback = [wrapper](RefPtr<NavigatorUserMediaError> error) mutable {
-        wrapper.reject(error.get());
+    auto rejectCallback = [wrapper](NavigatorUserMediaError& error) mutable {
+        wrapper.reject(&error);
     };
 
     ExceptionCode ec = 0;
     impl().getUserMedia(options, WTF::move(resolveCallback), WTF::move(rejectCallback), ec);
-    if (ec) {
-        setDOMException(exec, ec);
-        return jsUndefined();
-    }
+    if (ec)
+        wrapper.reject(ec);
 
     return wrapper.promise();
 }