[webkit-reviews] review granted: [Bug 121967] [MediaStream] make MediaStream testable : [Attachment 212753] Proposed patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 121967: [MediaStream] make MediaStream testable
https://bugs.webkit.org/show_bug.cgi?id=121967

Attachment 212753: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=212753&action=review

------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=212753&action=review


> Source/WebCore/platform/mock/MockMediaStreamCenter.cpp:54
> +}
> +
> +
> +static String verifyConstraints(MediaConstraints* constraints)

Extra blank line

> Source/WebCore/platform/mock/MockMediaStreamCenter.cpp:122
> +    if (audioConstraints) {
> +	   
> +	   String invalidQuery = verifyConstraints(audioConstraints.get());

Nit: another blank line.

> Source/WebCore/platform/mock/MockMediaStreamCenter.cpp:131
> +    if (videoConstraints) {
> +	   
> +	   String invalidQuery = verifyConstraints(videoConstraints.get());

And one more.

> Source/WebCore/platform/mock/MockMediaStreamCenter.cpp:151
> +    if (audioConstraints) {
> +	   RefPtr<MediaStreamSource> audioSource =
MediaStreamSource::create(emptyString(), MediaStreamSource::Audio, "Mock audio
device");
> +	   audioSource->setReadyState(MediaStreamSource::Live);
> +	   audioSources.append(audioSource.release());
> +    }
> +    
> +    if (videoConstraints) {
> +	   RefPtr<MediaStreamSource> videoSource =
MediaStreamSource::create(emptyString(), MediaStreamSource::Video, "Mock video
device");
> +	   videoSource->setReadyState(MediaStreamSource::Live);
> +	   videoSources.append(videoSource.release());
> +    }

Could you move these bits up into the if statements above that do the same
thing? Or is it that if there is a videoConstraint that you fail, you never
want to have created the audio source?