[webkit-reviews] review granted: [Bug 24042] MediaPlayer should cache plug-in proxy : [Attachment 27817] proposed patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 24042: MediaPlayer should cache plug-in proxy
https://bugs.webkit.org/show_bug.cgi?id=24042

Attachment 27817: proposed patch
https://bugs.webkit.org/attachment.cgi?id=27817&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>

> Index: WebCore/platform/graphics/MediaPlayer.cpp
> ===================================================================

> +#if ENABLE(PLUGIN_PROXY_FOR_VIDEO)
> +    virtual void setPoster(const String&) { }

setPoster(const String& /*url*/) would be slightly clearer.


> Index: WebCore/platform/graphics/MediaPlayer.h
> ===================================================================
> --- WebCore/platform/graphics/MediaPlayer.h	(revision 41083)
> +++ WebCore/platform/graphics/MediaPlayer.h	(working copy)
> @@ -149,6 +149,9 @@ private:
>      bool m_visible;
>      float m_rate;
>      float m_volume;
> +#if ENABLE(PLUGIN_PROXY_FOR_VIDEO)
> +    WebMediaPlayerProxy* m_playerProxy;
> +#endif

Maybe add a comment to say that m_playerProxy is not owned and where
it comes from?