Bug 94329

Summary: [BlackBerry] Adapt to changes in the platform media player API
Product: WebKit Reporter: Robin Cao <robin.webkit>
Component: WebKit BlackBerryAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, feature-media-reviews, mifenton, rwlbuis, staikos, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Other   
Attachments:
Description Flags
patch none

Description Robin Cao 2012-08-17 03:52:13 PDT 
The platform now provides an abtract interface PlatformPlayer which roughly corresponds to WebKit's MediaPlayerPrivate.

We need to adapt to these changes.
Comment 1 Robin Cao 2012-08-17 05:11:42 PDT 
Created attachment 159094 [details]
patch
Comment 2 Antonio Gomes 2012-08-17 07:14:58 PDT 
Comment on attachment 159094 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=159094&action=review

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:175
> -    m_platformPlayer->cancelLoad();
> +    if (m_platformPlayer)
> +        m_platformPlayer->cancelLoad();

I think this and all the other null checks are changes in the behavior. Why is that needed now?
Comment 3 Eric Carlson 2012-08-17 09:41:34 PDT 
Comment on attachment 159094 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=159094&action=review

This looks OK to me, but I know absolutely nothing about either the old or new platform player so I will let someone else give it a real review.

>> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:175
>> +        m_platformPlayer->cancelLoad();
> 
> I think this and all the other null checks are changes in the behavior. Why is that needed now?

Because m_platformPlayer is allocated in load rather than in the constructor?
Comment 4 Antonio Gomes 2012-08-17 10:08:10 PDT 
Comment on attachment 159094 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=159094&action=review

>>> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:175
>>> +        m_platformPlayer->cancelLoad();
>> 
>> I think this and all the other null checks are changes in the behavior. Why is that needed now?
> 
> Because m_platformPlayer is allocated in load rather than in the constructor?

Very reasonable.
Comment 5 WebKit Review Bot 2012-08-20 01:47:20 PDT 
Comment on attachment 159094 [details]
patch

Clearing flags on attachment: 159094

Committed r126008: <http://trac.webkit.org/changeset/126008>
Comment 6 WebKit Review Bot 2012-08-20 01:47:23 PDT 
All reviewed patches have been landed.  Closing bug.