Changeset 185191 in webkit

Timestamp:

Jun 3, 2015 11:06:50 PM (9 years ago)

Author:

youenn.fablet@crf.canon.fr

Message:

MediaDevices.getUserMedia should put promises in resolve/reject state synchronously
​https://bugs.webkit.org/show_bug.cgi?id=145308

Reviewed by Darin Adler.

Removed the calls to callToMainThread in which were resolved and rejected promises.
Cleaned up the code to remove unneeded callback copies.

Covered by existing tests.

• Modules/mediastream/UserMediaRequest.cpp:

(WebCore::UserMediaRequest::create):
(WebCore::UserMediaRequest::UserMediaRequest):
(WebCore::UserMediaRequest::didCreateStream):
(WebCore::UserMediaRequest::failedToCreateStreamWithConstraintsError):
(WebCore::UserMediaRequest::failedToCreateStreamWithPermissionError):

• Modules/mediastream/UserMediaRequest.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/mediastream/UserMediaRequest.cpp (modified) (6 diffs)
• Modules/mediastream/UserMediaRequest.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-06-03  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        MediaDevices.getUserMedia should put promises in resolve/reject state synchronously
+        https://bugs.webkit.org/show_bug.cgi?id=145308
+
+        Reviewed by Darin Adler.
+
+        Removed the calls to callToMainThread in which were resolved and rejected promises.
+        Cleaned up the code to remove unneeded callback copies.
+
+        Covered by existing tests.
+
+        * Modules/mediastream/UserMediaRequest.cpp:
+        (WebCore::UserMediaRequest::create):
+        (WebCore::UserMediaRequest::UserMediaRequest):
+        (WebCore::UserMediaRequest::didCreateStream):
+        (WebCore::UserMediaRequest::failedToCreateStreamWithConstraintsError):
+        (WebCore::UserMediaRequest::failedToCreateStreamWithPermissionError):
+        * Modules/mediastream/UserMediaRequest.h:
+
 2015-06-03  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp

@@ -76 +76 @@
     ASSERT(successCallback);
 
-    auto resolveCallback = [successCallback](MediaStream& stream) mutable {
-        successCallback->handleEvent(&stream);
+    auto resolveCallback = [successCallback, context](MediaStream& stream) mutable {
+        ASSERT(context);
+
+        RefPtr<MediaStream> mediaStream = &stream;
+        context->postTask([successCallback, mediaStream](ScriptExecutionContext&) {
+            successCallback->handleEvent(mediaStream.get());
+        });
     };
-    auto rejectCallback = [errorCallback](NavigatorUserMediaError& error) mutable {
-        if (errorCallback)
-            errorCallback->handleEvent(&error);
+    auto rejectCallback = [errorCallback, context](NavigatorUserMediaError& error) mutable {
+        ASSERT(context);
+
+        if (errorCallback) {
+            RefPtr<NavigatorUserMediaError> eventError = &error;
+            context->postTask([errorCallback, eventError](ScriptExecutionContext&) {
+                errorCallback->handleEvent(eventError.get());
+            });
+        }
     };
 
@@ -87 +98 @@
 }
 
-RefPtr<UserMediaRequest> UserMediaRequest::create(ScriptExecutionContext* context, UserMediaController* controller, const Dictionary& options, MediaDevices::ResolveCallback resolveCallback, MediaDevices::RejectCallback rejectCallback, ExceptionCode& ec)
+RefPtr<UserMediaRequest> UserMediaRequest::create(ScriptExecutionContext* context, UserMediaController* controller, const Dictionary& options, MediaDevices::ResolveCallback&& resolveCallback, MediaDevices::RejectCallback&& rejectCallback, ExceptionCode& ec)
 {
     ASSERT(resolveCallback && rejectCallback);
@@ -105 +116 @@
 }
 
-UserMediaRequest::UserMediaRequest(ScriptExecutionContext* context, UserMediaController* controller, PassRefPtr<MediaConstraints> audioConstraints, PassRefPtr<MediaConstraints> videoConstraints, MediaDevices::ResolveCallback resolveCallback, MediaDevices::RejectCallback rejectCallback)
+UserMediaRequest::UserMediaRequest(ScriptExecutionContext* context, UserMediaController* controller, PassRefPtr<MediaConstraints> audioConstraints, PassRefPtr<MediaConstraints> videoConstraints, MediaDevices::ResolveCallback&& resolveCallback, MediaDevices::RejectCallback&& rejectCallback)
     : ContextDestructionObserver(context)
     , m_audioConstraints(audioConstraints)
     , m_videoConstraints(videoConstraints)
     , m_controller(controller)
-    , m_resolveCallback(resolveCallback)
-    , m_rejectCallback(rejectCallback)
+    , m_resolveCallback(WTF::move(resolveCallback))
+    , m_rejectCallback(WTF::move(rejectCallback))
 {
 }
@@ -169 +180 @@
         return;
 
-    RefPtr<UserMediaRequest> protectedThis(this);
-    callOnMainThread([protectedThis, privateStream] {
-        // 4 - Create the MediaStream and pass it to the success callback.
-        RefPtr<MediaStream> stream = MediaStream::create(*protectedThis->m_scriptExecutionContext, privateStream);
-        for (auto& track : stream->getAudioTracks())
-            track->applyConstraints(protectedThis->m_audioConstraints);
-        for (auto& track : stream->getVideoTracks())
-            track->applyConstraints(protectedThis->m_videoConstraints);
-
-        protectedThis->m_resolveCallback(*stream);
-    });
+    // 4 - Create the MediaStream and pass it to the success callback.
+    RefPtr<MediaStream> stream = MediaStream::create(*m_scriptExecutionContext, privateStream);
+    for (auto& track : stream->getAudioTracks())
+        track->applyConstraints(m_audioConstraints);
+    for (auto& track : stream->getVideoTracks())
+        track->applyConstraints(m_videoConstraints);
+
+    m_resolveCallback(*stream);
 }
 
@@ -188 +196 @@
         return;
 
-    RefPtr<UserMediaRequest> protectedThis(this);
-    RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(NavigatorUserMediaError::constraintNotSatisfiedErrorName(), constraintName);
-    callOnMainThread([protectedThis, error] {
-        protectedThis->m_rejectCallback(*error);
-    });
+    m_rejectCallback(NavigatorUserMediaError::create(NavigatorUserMediaError::constraintNotSatisfiedErrorName(), constraintName).get());
 }
 
@@ -200 +204 @@
         return;
 
-    RefPtr<UserMediaRequest> protectedThis(this);
     // FIXME: Replace NavigatorUserMediaError with MediaStreamError (see bug 143335)
-    RefPtr<NavigatorUserMediaError> error = NavigatorUserMediaError::create(NavigatorUserMediaError::permissionDeniedErrorName(), emptyString());
-    callOnMainThread([protectedThis, error] {
-        protectedThis->m_rejectCallback(*error);
-    });
+    m_rejectCallback(NavigatorUserMediaError::create(NavigatorUserMediaError::permissionDeniedErrorName(), emptyString()));
 }
 

trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.h

@@ -60 +60 @@
 public:
     static RefPtr<UserMediaRequest> create(ScriptExecutionContext*, UserMediaController*, const Dictionary& options, PassRefPtr<NavigatorUserMediaSuccessCallback>, PassRefPtr<NavigatorUserMediaErrorCallback>, ExceptionCode&);
-    static RefPtr<UserMediaRequest> create(ScriptExecutionContext*, UserMediaController*, const Dictionary& options, MediaDevices::ResolveCallback, MediaDevices::RejectCallback, ExceptionCode&);
+    static RefPtr<UserMediaRequest> create(ScriptExecutionContext*, UserMediaController*, const Dictionary& options, MediaDevices::ResolveCallback&&, MediaDevices::RejectCallback&&, ExceptionCode&);
     ~UserMediaRequest();
 
@@ -73 +73 @@
 
 private:
-    UserMediaRequest(ScriptExecutionContext*, UserMediaController*, PassRefPtr<MediaConstraints> audioConstraints, PassRefPtr<MediaConstraints> videoConstraints, MediaDevices::ResolveCallback, MediaDevices::RejectCallback);
+    UserMediaRequest(ScriptExecutionContext*, UserMediaController*, PassRefPtr<MediaConstraints> audioConstraints, PassRefPtr<MediaConstraints> videoConstraints, MediaDevices::ResolveCallback&&, MediaDevices::RejectCallback&&);
 
     // MediaStreamCreationClient