[webkit-reviews] review granted: [Bug 189930] [MediaStream] Update constraints supported by getDisplayMedia : [Attachment 350686] 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 189930: [MediaStream] Update constraints supported by getDisplayMedia
https://bugs.webkit.org/show_bug.cgi?id=189930

Attachment 350686: Patch

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




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

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

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:163
> +    // 2. For each member present in constraints whose value, value, is a
dictionary, run the following steps:

s/, value,//

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:175
> +

Remove this line?

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:213
> +	   case MediaConstraintType::EchoCancellation:

>From your quote of the spec, we should also reject if any of these value has a
min/exact value.
Maybe we should add some tests for these values.

> Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp:245
> +	       if (request.type != MediaStreamRequest::Type::UserMedia ||
!request.audioConstraints.isValid)

To improve readability, we could try to split more getDisplayDevice vs.
getUserMedia code path.
For instance, have one top level request.type check in this function and
introduce getCaptureDevices and getScreenDevices to iterate over relevant
devices according request.type.