Bug 85114 - IndexedDB: Handle generated keys up to 2^53
Summary: IndexedDB: Handle generated keys up to 2^53
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: Joshua Bell
URL:
Keywords:
Depends on: 85441
Blocks:
  Show dependency tree / graph
 
Reported: 2012-04-27 17:49 PDT by Joshua Bell
Modified: 2012-05-03 12:32 PDT (History)
5 users (show)

See Also:

Attachments
Patch (10.98 KB, patch)
2012-04-27 17:52 PDT, Joshua Bell 		no flags Details | Formatted Diff | Diff
Patch for landing (10.98 KB, patch)
2012-05-03 10:41 PDT, Joshua Bell 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 Joshua Bell 2012-04-27 17:49:41 PDT 
IndexedDB: Handle generated keys up to 2^53
Comment 1 Joshua Bell 2012-04-27 17:52:09 PDT 
Created attachment 139311 [details]
Patch
Comment 2 Joshua Bell 2012-04-27 17:54:19 PDT 
I noticed this edge case when perusing the spec, and we had many casts to int that prevented us from getting anywhere close to 2^53.

dgrogan@, alecflett@ - sanity check?
Comment 3 David Grogan 2012-04-30 14:02:14 PDT 
Comment on attachment 139311 [details]
Patch

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

LGTM

> Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp:-605
> -double IDBLevelDBBackingStore::nextAutoIncrementNumber(int64_t databaseId, int64_t objectStoreId)

Seems weird that this was ever a double.  Do you know why it was?
Comment 4 Joshua Bell 2012-04-30 14:13:10 PDT 
(In reply to comment #3)
> > Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp:-605
> > -double IDBLevelDBBackingStore::nextAutoIncrementNumber(int64_t databaseId, int64_t objectStoreId)
> 
> Seems weird that this was ever a double.  Do you know why it was?

Since it's a number-type key it would be stored as a double (equivalent to JS number). The double->int64 conversion could reasonably be done anywhere along the call stack, but this seemed like the best place.
Comment 5 Joshua Bell 2012-04-30 14:13:38 PDT 
tony@ - r?
Comment 6 WebKit Review Bot 2012-05-02 16:02:58 PDT 
Comment on attachment 139311 [details]
Patch

Clearing flags on attachment: 139311

Committed r115902: <http://trac.webkit.org/changeset/115902>
Comment 7 WebKit Review Bot 2012-05-02 16:03:06 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Zhenyao Mo 2012-05-02 18:05:53 PDT 
cc1plus: warnings being treated as errors
third_party/WebKit/Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:686: error: integer constant is too large for 'long' type
make: *** [out/Release/obj.target/webcore_remaining/third_party/WebKit/Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.o] Error 1

Compie failure on Linux 32 bot.  I couldn't get hold of you in time, so I am reverting.  Apparently the constant is too large for 32 bit.
Comment 9 Joshua Bell 2012-05-02 18:16:20 PDT 
Sorry. Probably would compile with L or .0 suffix.
Comment 10 Joshua Bell 2012-05-03 10:36:50 PDT 
(In reply to comment #9)
> Sorry. Probably would compile with L or .0 suffix.

... or more correctly, LL. Verified this with a test app and compiling with gcc -m32. Will re-land shortly.
Comment 11 Joshua Bell 2012-05-03 10:41:18 PDT 
Created attachment 140045 [details]
Patch for landing
Comment 12 WebKit Review Bot 2012-05-03 12:32:21 PDT 
Comment on attachment 140045 [details]
Patch for landing

Clearing flags on attachment: 140045

Committed r115997: <http://trac.webkit.org/changeset/115997>
Comment 13 WebKit Review Bot 2012-05-03 12:32:27 PDT 
All reviewed patches have been landed.  Closing bug.