Bug 94329 - [BlackBerry] Adapt to changes in the platform media player API
Summary: [BlackBerry] Adapt to changes in the platform media player API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-17 03:52 PDT by Robin Cao
Modified: 2012-08-20 01:47 PDT (History)
7 users (show)

See Also:


Attachments
patch (19.47 KB, patch)
2012-08-17 05:11 PDT, Robin Cao
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.