Bug 94653

Summary: IndexedDB: tests for injection/extraction of idb keys
Product: WebKit Reporter: Alec Flett <alecflett>
Component: New BugsAssignee: Alec Flett <alecflett>
Status: RESOLVED FIXED    
Severity: Normal CC: dgrogan, jsbell, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Alec Flett 2012-08-21 17:21:57 PDT
IndexedDB: tests for injection/extraction of idb keys
Comment 1 Alec Flett 2012-08-21 17:23:21 PDT
Created attachment 159813 [details]
Patch
Comment 2 Alec Flett 2012-08-21 17:23:48 PDT
jsbell/dgrogan- these are ported over from chromium, so we can kill the chromium tests.
Comment 3 Joshua Bell 2012-08-22 11:53:54 PDT
Comment on attachment 159813 [details]
Patch

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

lgtm

> Source/WebKit/chromium/ChangeLog:8
> +        Added unit tests for key injection/extraction using 

Might want to mention that this is moving a Chromium test here now that the functionality is constrained to WebKit. Otherwise, the test is a little weird since half of it is still just testing the SSV serialization format.

> Source/WebKit/chromium/tests/IDBKeyPathTest.cpp:130
> +#define arraysize(x) (sizeof((x)) / sizeof((x[0])))

Use WTF_ARRAY_LENGTH macro instead?

> Source/WebKit/chromium/tests/IDBKeyPathTest.cpp:171
> +    ssv = SerializedScriptValue::createFromWire(String(data));

This isn't passing the length into String() - what's different here?
Comment 4 Alec Flett 2012-08-22 13:25:31 PDT
Comment on attachment 159813 [details]
Patch

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

>> Source/WebKit/chromium/tests/IDBKeyPathTest.cpp:130
>> +#define arraysize(x) (sizeof((x)) / sizeof((x[0])))
> 
> Use WTF_ARRAY_LENGTH macro instead?

ah! Knew there must have been something equivalent.

>> Source/WebKit/chromium/tests/IDBKeyPathTest.cpp:171
>> +    ssv = SerializedScriptValue::createFromWire(String(data));
> 
> This isn't passing the length into String() - what's different here?

a bug..fixed.
Comment 5 Alec Flett 2012-08-22 13:25:39 PDT
Created attachment 159999 [details]
Patch
Comment 6 Alec Flett 2012-08-22 13:26:25 PDT
tony@ - this is actually migration of code (chromium->webkit) for the chromium review I just asked about..
Comment 7 Tony Chang 2012-08-22 15:38:20 PDT
Comment on attachment 159999 [details]
Patch

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

> Source/WebKit/chromium/tests/IDBKeyPathTest.cpp:190
> +    const UChar expectedData[] = {0x01ff, 0x003f, 0x3f6f, 0x5301, 0x6603,
> +                                   0x6f6f, 0x013f, 0x0353, 0x6f7a, 0x3f6f,

Nit: Indent is weird.

> Source/WebKit/chromium/tests/IDBKeyPathTest.cpp:205
> +    const UChar expectedData2[] = {0x01ff, 0x003f, 0x3f6f, 0x5301, 0x6603,
> +                                    0x6f6f, 0x013f, 0x0353, 0x6f7a, 0x3f6f,

Nit: ditto
Comment 8 Alec Flett 2012-08-22 16:55:51 PDT
Created attachment 160037 [details]
Patch for landing
Comment 9 WebKit Review Bot 2012-08-22 18:49:34 PDT
Comment on attachment 160037 [details]
Patch for landing

Clearing flags on attachment: 160037

Committed r126381: <http://trac.webkit.org/changeset/126381>
Comment 10 WebKit Review Bot 2012-08-22 18:49:37 PDT
All reviewed patches have been landed.  Closing bug.