[webkit-reviews] review denied: [Bug 82831] [BlackBerry] Upstream BlackBerry change to MediaPlayer.h : [Attachment 134954] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has denied Charles Wei
<charles.wei at torchmobile.com.cn>'s request for review:
Bug 82831: [BlackBerry] Upstream BlackBerry change to MediaPlayer.h
https://bugs.webkit.org/show_bug.cgi?id=82831

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

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=134954&action=review


>>>> Source/WebCore/platform/graphics/MediaPlayer.h:312
>>>> +	  MediaPlayerPrivateInterface* implementation() { return
m_private.get(); }
>>> 
>>> Why is this needed?
>> 
>> BlackBerry porting need to access the implementation which stores
implementation-specific information, like volume-control and volume-status.
>> 
>> Look at html/HTMLMediaElement.cpp
>>		 html/shadow/MediaControlElements.cpp
>>		 rendering/MediaControlElements.cpp
> 
> Does not it make more sense to add methods to MediaPlayer itself, and hide
the "implementation" for the callee?

The "PrivateInterface" part of the class name should be a strong hint that the
interface is really private. Will it be a burden to add MediaPlayer methods to
access the information you need?