Bug 121967

Summary: [MediaStream] make MediaStream testable
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: WebCore Misc.Assignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cdumez, commit-queue, dino, esprehn+autocc, glenn, gtk-ews, gyuyoung.kim, hta, jer.noble, kangil.han, kondapallykalyan, rakuco, rniwa, thiago.lacerda, tommyw, xan.lopez
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 124288    
Attachments:
Description Flags
Proposed patch
none
Updated patch none

Description Eric Carlson 2013-09-26 11:22:15 PDT 
Add necessary infrastructure to make MediaStream layout tests possible.
Comment 1 Eric Carlson 2013-09-26 14:47:31 PDT 
Created attachment 212753 [details]
Proposed patch
Comment 2 WebKit Commit Bot 2013-09-26 14:49:55 PDT 
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.
Comment 3 Dean Jackson 2013-09-26 14:52:16 PDT 
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?
Comment 4 Eric Carlson 2013-09-26 14:58:45 PDT 
(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!
Comment 5 Build Bot 2013-09-26 15:08:56 PDT 
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
Comment 6 kov's GTK+ EWS bot 2013-09-26 15:14:39 PDT 
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
Comment 7 Build Bot 2013-09-26 15:31:33 PDT 
Comment on attachment 212753 [details]
Proposed patch

Attachment 212753 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/2614015
Comment 8 Eric Carlson 2013-09-26 17:24:18 PDT 
Created attachment 212770 [details]
Updated patch

Add a missing build flag
Comment 9 WebKit Commit Bot 2013-09-26 17:25:36 PDT 
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.
Comment 10 Eric Carlson 2013-09-26 17:51:45 PDT 
(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?
Comment 11 Eric Carlson 2013-09-26 18:08:28 PDT 
Committed r156522: https://trac.webkit.org/r156522