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 treegraph
 
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

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.