Bug 73280 – Upstream credential storage files of BlackBerry porting

• Component: New Bugs
• Status: RESOLVED FIXED

[Jonathan Dong]

files: WebCore/platform/network/blackberry/CredentialStorageBlackBerry.(cpp|h),  WebCore/platform/network/blackberry/CredentialBackingStore.(cpp|h)

[Jonathan Dong]

WebCore/platform/network/blackberry/CredentialStorageBlackBerry.cpp,  WebCore/platform/network/blackberry/CredentialBackingStore.(cpp|h)

sorry.

[Jonathan Dong]

Created attachment 116891 [details]
Patch

[Leo Yang]

Comment on attachment 116891 [details]
Patch

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

This is an informal review. LGTM, but some minor style issues.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:35
> +CredentialBackingStore* CredentialBackingStore::instance()
> +{
> +    static CredentialBackingStore *backingStore = 0;

The star in the above line should be near the type.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:47
> +CredentialBackingStore::CredentialBackingStore()
> +    : m_addStatement(0),
> +    m_updateStatement(0),
> +    m_hasStatement(0),
> +    m_getStatement(0),
> +    m_removeStatement(0)
> +{

comma should be at the beginning of each member variable.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:103
> +String CredentialBackingStore::encryptedString(const String& plainText) const
> +{
> +    // FIXME: need encrypt plain_text here

Nit: need --> Need

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:110
> +String CredentialBackingStore::decryptedString(const String& cipherText) const
> +{
> +    // FIXME: need decrypt cipher_text here

Ditto.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.h:36
> +    static CredentialBackingStore* instance();
> +    ~CredentialBackingStore();
> +

Blank line is not necessary.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.h:39
> +    bool open(const String& dbPath);
> +    void close();
> +

Ditto

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.h:45
> +    Credential getLogin(const ProtectionSpace&);
> +    bool removeLogin(const ProtectionSpace&);
> +

Ditto

[Jonathan Dong]

Created attachment 117315 [details]
Patch

[Antonio Gomes]

Comment on attachment 117315 [details]
Patch

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

it looks good.

r- because we need another round to fix the nits.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:53
> +bool CredentialBackingStore::open(const String& dbPath)

UNUSED_PARAM(dbPath) missing.

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:68
> +bool CredentialBackingStore::addLogin(const KURL& url, const ProtectionSpace& protectionSpace, const Credential& credential)
> +{
> +    notImplemented();
> +    return false;
> +}

ditto

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:74
> +bool CredentialBackingStore::updateLogin(const KURL& url, const ProtectionSpace& protectionSpace, const Credential& credential)
> +{
> +    notImplemented();
> +    return false;
> +}

ditto

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:86
> +Credential CredentialBackingStore::getLogin(const ProtectionSpace& protectionSpace)
> +{
> +    notImplemented();
> +    return Credential();
> +}

ditto

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.cpp:92
> +{
> +    notImplemented();
> +    return false;
> +}

ditto

> Source/WebCore/platform/network/blackberry/CredentialBackingStore.h:55
> +    SQLiteStatement *m_addStatement;
> +    SQLiteStatement *m_updateStatement;
> +    SQLiteStatement *m_hasStatement;
> +    SQLiteStatement *m_getStatement;
> +    SQLiteStatement *m_removeStatement;

* position on the left

> Source/WebCore/platform/network/blackberry/CredentialStorageBlackBerry.cpp:35
> +#else
> +    return Credential();
> +#endif

UNUSED_PARAM needed here too.

[Jonathan Dong]

Created attachment 117334 [details]
Patch

[Rob Buis]

Comment on attachment 117334 [details]
Patch

Antonio's nits are fixed, so r+.

[WebKit Review Bot]

Comment on attachment 117334 [details]
Patch

Clearing flags on attachment: 117334

Committed r101668: <http://trac.webkit.org/changeset/101668>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.