Bug 94653 - IndexedDB: tests for injection/extraction of idb keys
Summary: IndexedDB: tests for injection/extraction of idb keys
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alec Flett
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-21 17:21 PDT by Alec Flett
Modified: 2012-08-22 18:49 PDT (History)
4 users (show)

See Also:

Attachments
Patch (5.56 KB, patch)
2012-08-21 17:23 PDT, Alec Flett 		no flags Details | Formatted Diff | Diff
Patch (5.62 KB, patch)
2012-08-22 13:25 PDT, Alec Flett 		no flags Details | Formatted Diff | Diff
Patch for landing (5.61 KB, patch)
2012-08-22 16:55 PDT, Alec Flett 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.