[webkit-reviews] review denied: [Bug 34539] Implement NPN_GetValueForURL and NPN_SetValueForURL and provide a stub for NPN_GetAuthenticationInfo : [Attachment 70346] Implement NPN_GetValueForURL and NPN_SetValueForURL and NPN_GetAuthenticationInfo [Final #7]...

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has denied Dawit A. <adawit at kde.org>'s request
for review:
Bug 34539: Implement NPN_GetValueForURL and NPN_SetValueForURL and provide a
stub for NPN_GetAuthenticationInfo
https://bugs.webkit.org/show_bug.cgi?id=34539

Attachment 70346: Implement NPN_GetValueForURL and NPN_SetValueForURL and
NPN_GetAuthenticationInfo [Final #7]...
https://bugs.webkit.org/attachment.cgi?id=70346&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=70346&action=review

Looks good, but please fix the following things.

> WebCore/platform/network/ProxyServer.h:71
> +Vector<ProxyServer> proxyServersForURL(const KURL&, const NetworkingContext*
context);

There's no need for the NetworkingContext* to have a variable name here.

> WebCore/platform/network/qt/ProxyServerQt.cpp:44
> +    if (context) {

This function nests unnecessarily, you should use early return style, i.e:
if (!context)
    return servers;
And the same for !accessManager and !proxyFactory

> WebKit2/WebProcess/Plugins/PluginView.cpp:847
> +    const NetworkingContext* netContext = frameLoader ?
frameLoader->networkingContext() : 0;
> +    Vector<ProxyServer> proxyServers = proxyServersForURL(KURL(KURL(),
urlString), netContext);

s/netContext/context/