The spec has changed the constraints that getDisplayMedia supports.
Created attachment 350686 [details] Patch
<rdar://problem/44740305>
Comment on attachment 350686 [details] 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.
Comment on attachment 350686 [details] 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,// This is verbatim from the spec. >> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:175 >> + > > Remove this line? OK >> 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. These are audio-only constraints, and we don't support audio with getDisplayMedia. Good idea about the test, I will add an invalid audio constraint to the test. >> 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. Good idea, I will do that.
Created attachment 350756 [details] Patch for landing
Comment on attachment 350756 [details] Patch for landing Clearing flags on attachment: 350756 Committed r236465: <https://trac.webkit.org/changeset/236465>
All reviewed patches have been landed. Closing bug.
Reopening to attach new patch.
Created attachment 350787 [details] Fix broken API tests
Comment on attachment 350787 [details] Fix broken API tests Clearing flags on attachment: 350787 Committed r236476: <https://trac.webkit.org/changeset/236476>