[webkit-reviews] review granted: [Bug 90180] MediaStream API: Update MediaStreamTrack to match the specification : [Attachment 153453] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 20 09:14:25 PDT 2012


Adam Barth <abarth at webkit.org> has granted Tommy Widenflycht
<tommyw at google.com>'s request for review:
Bug 90180: MediaStream API: Update MediaStreamTrack to match the specification
https://bugs.webkit.org/show_bug.cgi?id=90180

Attachment 153453: Patch
https://bugs.webkit.org/attachment.cgi?id=153453&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=153453&action=review


> Source/Platform/chromium/public/WebMediaStreamSource.h:56
> +	   ReadyStateLive = 0,
> +	   ReadyStateMuted = 1,
> +	   ReadyStateEnded = 2

Should we COMPILE_ASSERT that these enums match the WebCore versions?

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:112
> +    return LIVE;

SHould we return MUTED or ENDED here?  That seems like it would be safer.

> Tools/DumpRenderTree/chromium/MockWebKitPlatformSupport.cpp:53
> -WebMediaStreamCenter*
MockWebKitPlatformSupport::createMediaStreamCenter(WebMediaStreamCenterClient*)

> +WebMediaStreamCenter*
MockWebKitPlatformSupport::createMediaStreamCenter(WebMediaStreamCenterClient*
client)

Should this return PassOwnPtr?

> Tools/DumpRenderTree/chromium/MockWebMediaStreamCenter.h:39
> +class MockWebMediaStreamCenter : public WebMediaStreamCenter {

This declaration should not be in the WebKit namespace.


More information about the webkit-reviews mailing list