Bug 56587 - Implement Layout Tests for the Media Stream API
Summary: Implement Layout Tests for the Media Stream API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Tommy Widenflycht
URL:
Keywords:
Depends on: 58550 61068 74882 79114 82870 82881 85883
Blocks: 56459
  Show dependency tree / graph
 
Reported: 2011-03-17 13:48 PDT by Leandro Graciá Gil
Modified: 2012-07-24 05:58 PDT (History)
6 users (show)

See Also:

Attachments
Patch (89.21 KB, patch)
2012-02-15 06:41 PST, Tommy Widenflycht 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Leandro Graciá Gil 2011-03-17 13:48:51 PDT 
Implement the needed code in the LayoutTestController and provide a StreamClientMock to enable layout testing for the Media Stream API.
Comment 1 Leandro Graciá Gil 2011-03-23 13:19:36 PDT 
Updating the bug platform settings (had the default ones).
Comment 2 Tommy Widenflycht 2012-02-15 06:41:34 PST 
Created attachment 127175 [details]
Patch
Comment 3 Tommy Widenflycht 2012-02-15 06:44:10 PST 
I have uploaded a complete implementation for discussion purposes only.
Comment 4 Adam Barth 2012-02-15 15:00:00 PST 
Comment on attachment 127175 [details]
Patch

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

The overall structure of this patch looks great.

> Source/WebCore/mediastream/UserMediaClientMock.cpp:47
> +    : m_allowAudio(false), m_allowVideo(false)

These should be on separate lines.

> Source/WebCore/mediastream/UserMediaClientMock.cpp:71
> +        String id("mock_audio_device");
> +        String name("AudioCapture");

We usually just use the assignment form of the constructor.

> Source/WebCore/platform/mediastream/PeerConnectionHandlerMock.cpp:50
> +PeerConnectionHandlerMock::PeerConnectionHandlerMock(PeerConnectionHandlerClient* client, const String& serverConfiguration, PassRefPtr<SecurityOrigin> securityOrigin)

You'll probably need ASSERT_UNUSED for serverConfiguration and securityOrigin to avoid breaking the compile for some ports that don't like unused parameters.  (There are a bunch more examples of this comment in this file.)
Comment 5 Adam Barth 2012-02-15 15:01:53 PST 
Maybe @fishd would like to take a look at the uber patch?

There's a bunch of machinery in the patch that's not needed for the getusermedia-not-allowed.html test.  Maybe a good starting point would be to break off whatever chunk is needed for getusermedia-not-allowed.html into a first patch,.
Comment 6 Tommy Widenflycht 2012-02-16 07:00:33 PST 
(In reply to comment #4)
> (From update of attachment 127175 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127175&action=review
> 
> The overall structure of this patch looks great.
> 
> > Source/WebCore/mediastream/UserMediaClientMock.cpp:47
> > +    : m_allowAudio(false), m_allowVideo(false)
> 
> These should be on separate lines.
> 
> > Source/WebCore/mediastream/UserMediaClientMock.cpp:71
> > +        String id("mock_audio_device");
> > +        String name("AudioCapture");
> 
> We usually just use the assignment form of the constructor.

Thanks for having a first look!

I fixed your comments and uploaded just the UserMediaClientMock related code to 
https://bugs.webkit.org/show_bug.cgi?id=61068
Comment 7 Tommy Widenflycht 2012-07-24 05:58:43 PDT 
We now have reasonable layout tests and therefore doesn't need this tracking bug anymore.