Bug 93324 – [BlackBerry] HTML5 media does not handle SSL certificate failures

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Jonathan Dong]

RIM PR: 116205

We should pass stream client Id to platform player when loading a media url, which is needed by BlackBerry::Platform::MediaSSLHandlerStream for handling the certificate failure if media url is a https url and has a certificate failure reported by mm-renderer.

[Jonathan Dong]

Created attachment 156840 [details]
Patch

[Antonio Gomes]

Comment on attachment 156840 [details]
Patch

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

td;dr

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:160
> +    HTMLMediaElement* element = static_cast<HTMLMediaElement*>(m_webCorePlayer->mediaPlayerClient());

that is a layer violation

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:166
> +    int playerId = static_cast<FrameLoaderClientBlackBerry*>(topdoc->frame()->loader()->client())->playerId();

that is a BIG layer violation

[Joe Mason]

MediaPlayerPrivateBlackBerry already does both of those in other functions.  Can we clean up all the layering violations in a followup patch?

[Eric Seidel (no email)]

Comment on attachment 156840 [details]
Patch

Per above.

[Eric Carlson]

(In reply to comment #3)
> MediaPlayerPrivateBlackBerry already does both of those in other functions.  Can we clean up all the layering violations in a followup patch?

I r+'d earlier several bugs with layering violations with the agreement that the violations would be cleaned up, eg. https://bugs.webkit.org/show_bug.cgi?id=82930 from mid April. Max filed bug 84291 about one part of the cleanup, but so far it seems that nothing more has been done.

[Jonathan Dong]

Created attachment 164831 [details]
Patch

[Eric Carlson]

Comment on attachment 164831 [details]
Patch

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

> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:169
> -        m_platformPlayer->load(modifiedUrl.utf8().data(), m_webCorePlayer->userAgent().utf8().data(), cookiePairs.utf8().data());
> +        m_platformPlayer->load(playerID, modifiedUrl.utf8().data(), m_webCorePlayer->userAgent().utf8().data(), cookiePairs.utf8().data());
>      else
> -        m_platformPlayer->load(modifiedUrl.utf8().data(), m_webCorePlayer->userAgent().utf8().data(), 0);
> +        m_platformPlayer->load(playerID, modifiedUrl.utf8().data(), m_webCorePlayer->userAgent().utf8().data(), 0);

Why did this compile if the number of parameters was incorrect?

[Jonathan Dong]

Comment on attachment 164831 [details]
Patch

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

>> Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:169
>> +        m_platformPlayer->load(playerID, modifiedUrl.utf8().data(), m_webCorePlayer->userAgent().utf8().data(), 0);
> 
> Why did this compile if the number of parameters was incorrect?

Because our porting haven't got the buildbot for now :(, and we work on our private webkit repo firstly, then sync with upstream.
This patch intends to pass the playerID into m_platformPlayer in order to create a SSLHandlingStream to perform SSL verification for the media engine.

[Joe Mason]

(In reply to comment #7)
> (From update of attachment 164831 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164831&action=review
> 
> > Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:169
> > -        m_platformPlayer->load(modifiedUrl.utf8().data(), m_webCorePlayer->userAgent().utf8().data(), cookiePairs.utf8().data());
> > +        m_platformPlayer->load(playerID, modifiedUrl.utf8().data(), m_webCorePlayer->userAgent().utf8().data(), cookiePairs.utf8().data());
> >      else
> > -        m_platformPlayer->load(modifiedUrl.utf8().data(), m_webCorePlayer->userAgent().utf8().data(), 0);
> > +        m_platformPlayer->load(playerID, modifiedUrl.utf8().data(), m_webCorePlayer->userAgent().utf8().data(), 0);
> 
> Why did this compile if the number of parameters was incorrect?

The number of parameters used to be correct. Then we updated the BlackBerry::Platform::PlatformPlayer class in the blackberry "platform" library (an OS service), so now webkit won't compile with the most recent version of the platform lib. This patch updates webkit to compile with the latest version of "platform".

[Eric Carlson]

Comment on attachment 164831 [details]
Patch

rs=me given Joe's informal review.

[WebKit Review Bot]

Comment on attachment 164831 [details]
Patch

Clearing flags on attachment: 164831

Committed r129244: <http://trac.webkit.org/changeset/129244>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.