[webkit-reviews] review granted: [Bug 131830] media foundation video element skeleton : [Attachment 229602] Patch

[bugzilla-daemon at webkit.org]

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.