[webkit-reviews] review granted: [Bug 131830] media foundation video element skeleton : [Attachment 229602] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 22 11:13:45 PDT 2014
Brent Fulgham <bfulgham at webkit.org> has granted Alex Christensen
<alex.christensen at flexsim.com>'s request for review:
Bug 131830: media foundation video element skeleton
https://bugs.webkit.org/show_bug.cgi?id=131830
Attachment 229602: Patch
https://bugs.webkit.org/attachment.cgi?id=229602&action=review
------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=229602&action=review
Looks good. I had a few minor suggestions (I thin you should mark the methods
in MediaPlayerPrivateMediaFoundation as 'override' where appropriate.
I assume the new USE(MEDIA_FOUNDATION) stuff will be added in a future patch to
FeatureDefines.props?
>
Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:160
> + || !m_player->visible())
I think this could be one line.
> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.h:49
> + virtual bool hasVideo() const;
Should these be marked 'override'?
> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.h:70
> +
Please get rid of this extra whitespace.
More information about the webkit-reviews
mailing list