Bug 121967 – [MediaStream] make MediaStream testable

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Eric Carlson]

Add necessary infrastructure to make MediaStream layout tests possible.

[Eric Carlson]

Created attachment 212753 [details]
Proposed patch

[WebKit Commit Bot]

Attachment 212753 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/mediastream/MediaStream-add-remove-tracks-expected.txt', u'LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html', u'LayoutTests/fast/mediastream/MediaStream-onended-expected.txt', u'LayoutTests/fast/mediastream/MediaStream-onended.html', u'LayoutTests/fast/mediastream/MediaStreamConstructor-expected.txt', u'LayoutTests/fast/mediastream/MediaStreamConstructor.html', u'LayoutTests/fast/mediastream/MediaStreamTrackEvent-constructor-expected.txt', u'LayoutTests/fast/mediastream/MediaStreamTrackEvent-constructor.html', u'LayoutTests/fast/mediastream/argument-types-expected.txt', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Modules/mediastream/MediaStreamTrack.idl', u'Source/WebCore/Modules/mediastream/NavigatorUserMediaError.idl', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/dom/DOMError.h', u'Source/WebCore/dom/DOMError.idl', u'Source/WebCore/platform/mock/MockMediaStreamCenter.cpp', u'Source/WebCore/platform/mock/MockMediaStreamCenter.h', u'Source/WebCore/testing/Internals.cpp', u'Source/WebKit/ChangeLog', u'Source/WebKit/WebKit.xcodeproj/project.pbxproj', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.h', u'Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm', u'Source/WebKit/mac/WebView/WebUIDelegatePrivate.h', u'Source/WebKit/mac/WebView/WebView.mm', u'Source/WebKit/mac/WebView/WebViewData.h', u'Source/WebKit/mac/WebView/WebViewPrivate.h', u'Tools/ChangeLog', u'Tools/DumpRenderTree/mac/UIDelegate.mm']" exit_code: 1
Source/WebKit/mac/WebView/WebViewPrivate.h:720:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 33 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dean Jackson]

Comment on attachment 212753 [details]
Proposed patch

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?

[Eric Carlson]

(In reply to comment #3)
> (From update of attachment 212753 [details])
> > 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?

Good point, I will change this (and remove the nasty blank lines).

Thanks!

[Build Bot]

Comment on attachment 212753 [details]
Proposed patch

Attachment 212753 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/2613012

[kov's GTK+ EWS bot]

Comment on attachment 212753 [details]
Proposed patch

Attachment 212753 [details] did not pass gtk-wk2-ews (gtk-wk2):
Output: http://webkit-queues.appspot.com/results/2597022

[Build Bot]

Comment on attachment 212753 [details]
Proposed patch

Attachment 212753 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/2614015

[Eric Carlson]

Created attachment 212770 [details]
Updated patch

Add a missing build flag

[WebKit Commit Bot]

Attachment 212770 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/mediastream/MediaStream-add-remove-tracks-expected.txt', u'LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html', u'LayoutTests/fast/mediastream/MediaStream-onended-expected.txt', u'LayoutTests/fast/mediastream/MediaStream-onended.html', u'LayoutTests/fast/mediastream/MediaStreamConstructor-expected.txt', u'LayoutTests/fast/mediastream/MediaStreamConstructor.html', u'LayoutTests/fast/mediastream/MediaStreamTrackEvent-constructor-expected.txt', u'LayoutTests/fast/mediastream/MediaStreamTrackEvent-constructor.html', u'LayoutTests/fast/mediastream/argument-types-expected.txt', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Modules/mediastream/MediaStreamTrack.idl', u'Source/WebCore/Modules/mediastream/NavigatorUserMediaError.idl', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/dom/DOMError.h', u'Source/WebCore/dom/DOMError.idl', u'Source/WebCore/platform/mock/MockMediaStreamCenter.cpp', u'Source/WebCore/platform/mock/MockMediaStreamCenter.h', u'Source/WebCore/testing/Internals.cpp', u'Source/WebKit/ChangeLog', u'Source/WebKit/WebKit.xcodeproj/project.pbxproj', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.h', u'Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm', u'Source/WebKit/mac/WebView/WebUIDelegatePrivate.h', u'Source/WebKit/mac/WebView/WebView.mm', u'Source/WebKit/mac/WebView/WebViewData.h', u'Source/WebKit/mac/WebView/WebViewPrivate.h', u'Tools/ChangeLog', u'Tools/DumpRenderTree/mac/UIDelegate.mm']" exit_code: 1
Source/WebKit/mac/WebView/WebViewPrivate.h:720:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 33 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

(In reply to comment #9)
> Attachment 212770 [details] did not pass style-queue:
> 
> Source/WebKit/mac/WebView/WebViewPrivate.h:720:  Extra space before ( in function call  [whitespace/parens] [4]

 "@interface WebView (WebViewUserMedia)" is a function call?

[Eric Carlson]

Committed r156522: https://trac.webkit.org/r156522