Bug 73280

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

Created attachment 116891 [details] Patch

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

Created attachment 117315 [details] Patch

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.

Created attachment 117334 [details] Patch

Comment on attachment 117334 [details] Patch Antonio's nits are fixed, so r+.

Comment on attachment 117334 [details] Patch Clearing flags on attachment: 117334 Committed r101668: <http://trac.webkit.org/changeset/101668>

All reviewed patches have been landed. Closing bug.