[Webkit-unassigned] [Bug 91813] IndexedDB: Size the Vector in encodeInt/encodeVarInt/encodeString

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=91813





--- Comment #2 from Joshua Bell <jsbell at chromium.org>  2012-07-20 09:41:01 PST ---
(From update of attachment 153401)
View in context: https://bugs.webkit.org/attachment.cgi?id=153401&action=review

LGTM with test nits.

I see three other places where a zero-length Vector<char> is allocated and appended to; would it be worth speculatively giving these a small inline buffer (e.g. 16 bytes) since most IDBKeys will be short?

Also, there is one case (encodeBool) where a vector is sized via a constructor param rather than template param. Can you change it to use the template param style, for consistency? (I was unaware of that style when I wrote it.)


Thanks for going after these FIXMEs!

> Source/WebCore/ChangeLog:11
> +        No new tests - Low level functions covered by existing tests.

You could also mention this is covered by Chromium webkit_unit_tests IDBLevelDBCodingTest.* - this validates the sizes of buffers returned by encodeVarInt.

> Source/WebCore/Modules/indexeddb/IDBLevelDBCoding.cpp:245
> +    Vector<char, 9> ret;

I don't see a test case in Source/WebKit/chromium/tests/IDBLevelDBCodingTest.cpp for a value that returns a 9-byte buffer. Can you add one?

The test in TEST(IDBLevelDBCodingTest, EncodeVarInt) that encodes -100 should generate this, but it mistakenly calls encodeInt rather than encodeVarInt - can you fix it as part of this patch?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.