[webkit-reviews] review denied: [Bug 84214] [BlackBerry] Loading media data with http authentication : [Attachment 138047] Patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Jonathan Dong
<jonathan.dong at torchmobile.com.cn>'s request for review:
Bug 84214: [BlackBerry] Loading media data with http authentication
https://bugs.webkit.org/show_bug.cgi?id=84214

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

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=138047&action=review


Still some stuff to clean up.

>
Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:66
3
> +ProtectionSpace generateProtectionSpaceFromMMRAuthChallenge(const
MMRAuthChallenge& authChallenge)

Should be static.

>
Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:66
7
> +	   return ProtectionSpace();

Better maybe ASSERT(url.isValid()); since the call sites already check it, so
it should not happen.

>
Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.cpp:69
1
> +    }

How about this? :

if (credential.isEmpty()) {
 if (frameView() && frameView()->hostWindow())
     isConfirmed =
frameView()->hostWindow()->platformPageClient()->authenticationChallenge(url,
protectionSpace, credential);
} else
    isConfirmed = true;

if (isConfirmed)
    authChallenge.setCredential(credential.user().utf8().data(),
credential.password().utf8().data(),
static_cast<MMRAuthChallenge::CredentialPersistence>(credential.persistence()))
;

>
Source/WebCore/platform/graphics/blackberry/MediaPlayerPrivateBlackBerry.h:134
> +    virtual void onAuthenticationAccepted(const
BlackBerry::Platform::MMRAuthChallenge&);

Can these be const?