[webkit-reviews] review denied: [Bug 41295] [Chromium] Add chromium WebMediaPlayer to PlatformMedia : [Attachment 61257] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 12 13:07:42 PDT 2010


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Bo Liu
<boliu at chromium.org>'s request for review:
Bug 41295: [Chromium] Add chromium WebMediaPlayer to PlatformMedia
https://bugs.webkit.org/show_bug.cgi?id=41295

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebCore/platform/graphics/MediaPlayer.h:78
 +	    // Using void* instead of WebKit::WebMediaPlayer
this comment introduces WebKit::WebMediaPlayer into WebCore in a
way that the compiler won't even help us notice if we ever rename
WebKit::WebMediaPlayer.  that seems bad.

HTMLMediaElement has a reference to a MediaPlayer object.  A
MediaPlayer has a MediaPlayerPrivateInterface member as well as
a MediaPlayerClient member, and WebMediaPlayerClientImpl implements
both of those interfaces.

You should be able to static_cast from one of those interfaces to
WebMediaPlayerClientImpl and gets the WebMediaPlayer from there.
This casting can be done at the WebKit/chromium/src level.

BTW, I think you need to invent a WebMediaElement subclass of
WebElement instead of adding a method on WebNode.


More information about the webkit-reviews mailing list