[webkit-reviews] review denied: [Bug 85575] [BlackBerry] Autofill backing store implementation upstream : [Attachment 140169] 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 85575: [BlackBerry] Autofill backing store implementation upstream
https://bugs.webkit.org/show_bug.cgi?id=85575

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

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


Can be cleaned up some more.

> Source/WebCore/platform/network/blackberry/AutofillBackingStore.cpp:24
> +#include "NotImplemented.h"

Can be removed?

> Source/WebCore/platform/network/blackberry/AutofillBackingStore.cpp:163
> +    int numOfRow = m_hasStatement->getColumnInt(0);

Do you mean numberOfRows here or rowNumber? 
Better not abbreviate.

> Source/WebCore/platform/network/blackberry/AutofillBackingStore.cpp:170
> +    return false;

You can combine these into one return statement.

> Source/WebCore/platform/network/blackberry/AutofillBackingStore.h:23
> +#include <wtf/Vector.h>

Does not seem like we need this include here.

> Source/WebCore/platform/network/blackberry/AutofillBackingStore.h:40
> +    bool has(const String& name, const String& value);

has is an unclear name for a function. How about contains?

> Source/WebCore/platform/network/blackberry/AutofillBackingStore.h:41
> +

Please check if any of the functions are const.