Bug 82831

Summary: [BlackBerry] Upstream BlackBerry change to MediaPlayer.h
Product: WebKit Reporter: Charles Wei <charles.wei>
Component: WebKit BlackBerryAssignee: Charles Wei <charles.wei>
Status: RESOLVED INVALID    
Severity: Normal CC: andersca, eric.carlson, leo.yang, rwlbuis, staikos, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 73144    
Attachments:
Description Flags
Patch eric.carlson: review-

Charles Wei
Reported 2012-03-31 05:30:23 PDT
Need to upstream BlackBerry-specific change to MediaPlayer.h, MediaPlayerPrivateBlackBerry.h(cpp) are already upstreamed and need this patch to build upstreaming code.
Attachments
Patch (1.88 KB, patch)
2012-03-31 05:40 PDT, Charles Wei 		eric.carlson: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Charles Wei
Comment 1 2012-03-31 05:40:38 PDT
Created attachment 134954 [details] Patch
Rob Buis
Comment 2 2012-03-31 05:55:20 PDT
Comment on attachment 134954 [details] Patch 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?
Charles Wei
Comment 3 2012-03-31 06:07:04 PDT
(In reply to comment #2) > (From update of attachment 134954 [details]) > 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
Antonio Gomes
Comment 4 2012-04-03 08:34:05 PDT
Comment on attachment 134954 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134954&action=review > Source/WebCore/platform/graphics/MediaPlayer.h:89 > + MediaPlayerPrivateInterface* qnxMediaPlayer; "qnx" does not seem too good here. >>> 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?
Eric Carlson
Comment 5 2012-04-03 09:31:59 PDT
Comment on attachment 134954 [details] Patch 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?
Note You need to log in before you can comment on or make changes to this bug.