[Webkit-unassigned] [Bug 41295] [Chromium] Add chromium WebMediaPlayer to PlatformMedia

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=41295


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #61276|review?                     |review-
               Flag|                            |




--- Comment #12 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-07-14 00:03:37 PST ---
(From update of attachment 61276)
WebKit/chromium/public/WebMediaElement.h:46
 +      WEBKIT_API WebMediaPlayer* getWebMediaPlayer() const;
webkit style is to avoid the "get" prefix for simple getter functions.
how about just calling this method mediaPlayer()?  we drop the "web"
prefix for method names that return a Web* type.

WebKit/chromium/src/WebMediaElement.cpp:46
 +      PlatformMedia pm = constUnwrap<HTMLMediaElement>()->platformMedia();
I think it would be better to create a method named fromMediaElement on
WebMediaPlayerClientImpl.  That hides the details of how a WebMediaPlayerClientImpl
can be extracted from a HTMLMediaElement in the WebMediaPlayerClientImpl class.

You should also create a public accessor on WebMediaPlayerClientImpl named mediaPlayer()
that returns m_webMediaPlayer.get().  That way you do not need to use friend to access
private data.

So, for this method you should end up with:

WebMediaPlayer* WebMediaElement::getWebMediaPlayer() const
{
    return WebMediaPlayerClientImpl::fromMediaElement(this)->mediaPlayer();
}

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.