[webkit-reviews] review granted: [Bug 190043] [MediaStream] Use display-specific capture factories : [Attachment 351050] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 190043: [MediaStream] Use display-specific capture factories
https://bugs.webkit.org/show_bug.cgi?id=190043

Attachment 351050: Patch

https://bugs.webkit.org/attachment.cgi?id=351050&action=review




--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 351050
  --> https://bugs.webkit.org/attachment.cgi?id=351050
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351050&action=review

> Source/WebCore/platform/mediastream/RealtimeMediaSource.h:137
> +    class DisplayCaptureFactory {

We should maybe move DisplayCaptureFactory, VideoCaptureFactory et al to their
own file.
We do not gain much in having them inside RealtimeMediaSource.

> Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp:106
> +	   else

We could have a specific RealtimeMediaSourceCenter::createDisplayMediaStream
that would be called from UserMediaRequest::allow based on the
UserMediaRequest::type.

> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:53
> +	   if (device.type() == CaptureDevice::DeviceType::Camera)

Maybe an ASSERT() is all that is needed there.
If the type is not good, the id is probably not good and capture will fail,
right?