[webkit-reviews] review denied: [Bug 34539] Implement NPN_GetValueForURL and NPN_SetValueForURL and provide a stub for NPN_GetAuthenticationInfo : [Attachment 65021] Implement NPN_GetValueForURL and NPN_SetValueForURL and NPN_GetAuthenticationInfo [Update V]...

[bugzilla-daemon at webkit.org]

Andreas Kling <andreas.kling at nokia.com> has denied 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 65021: Implement NPN_GetValueForURL and NPN_SetValueForURL and
NPN_GetAuthenticationInfo [Update V]...
https://bugs.webkit.org/attachment.cgi?id=65021&action=review

------- Additional Comments from Andreas Kling <andreas.kling at nokia.com>
>WebCore/plugins/PluginView.cpp:1387
> +	 if (platformGetValueForURL(variable, url, value, len, &result))
> +	     return result;

platformGetValueForURL() doesn't actually modify 'result' so you're returning
uninitialized data here.

>WebCore/plugins/PluginView.cpp:1393
> +	     if (u.isValid()) {

If !isValid(), result should be set to an error code.

>WebCore/plugins/PluginView.cpp:1398
> +		     strncpy(*value, cookieStr.utf8().data(), cookieStrLen);

cookieStrLen should come from cookieStr.utf8().length() here.

>WebCore/plugins/PluginView.cpp:1431
> +	     if (u.isValid()) {

If !isValid(), result should be set to an error code.

>WebCore/plugins/qt/PluginViewQt.cpp:71
> +  #include <QString>

Unnecessary include.

>WebCore/plugins/qt/PluginViewQt.cpp:725
> +	 qDebug() << "PluginView::platformGetValueForURL:" << variable << ","
<< url;

Please put this inside a preprocessor conditional of some sort.

>WebCore/plugins/qt/PluginViewQt.cpp:758
> +	     strncpy(*value, proxyStr.toUtf8().constData(), proxyStrLen);

Though safe in this particular case, proxyStrLen should come from
proxyStr.toUtf8().length()

>WebCore/plugins/PluginView.cpp:1455
> +	 LOG(Plugins, "PluginView::getAuthenticationInfo: protocol=%s, host=%d,
port=%d", protocol, host, port);

"host" is const char*, so it should be %s, not %d.

Furthermore there are some coding style issues:
- Tabs should always be 4 spaces
- "type *foo" should be "type* foo"