WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
153625
Modern IDB: SQLite backend mismanages key generator values
https://bugs.webkit.org/show_bug.cgi?id=153625
Summary
Modern IDB: SQLite backend mismanages key generator values
Brady Eidson
Reported
2016-01-28 17:20:03 PST
Modern IDB: SQLite backend mismanages key generator values There's mixed assumptions about whether the value stored is the current value or the next value. Fixing those fixes tests.
Attachments
Patch v1
(5.41 KB, patch)
2016-01-28 17:23 PST
,
Brady Eidson
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Brady Eidson
Comment 1
2016-01-28 17:23:19 PST
This likely won't apply because it's waiting on
https://bugs.webkit.org/show_bug.cgi?id=153623
I'll mark it as a patch and put up for review once the other bug is resolved.
Brady Eidson
Comment 2
2016-01-28 17:23:31 PST
Created
attachment 270166
[details]
Patch v1
Andy Estes
Comment 3
2016-01-28 22:03:19 PST
Comment on
attachment 270166
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=270166&action=review
r=me. My comments are mostly high-level, and could be addressed in a follow-up if you wish.
> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:1362 > - if (currentValue > maxGeneratorValue) > + if (currentValue + 1 > maxGeneratorValue) > return { IDBDatabaseException::ConstraintError, "Cannot generate new key value over 2^53 for object store operation" }; > > generatedKey = currentValue + 1;
It might be a somewhat more clear to create a new local called nextGeneratedKey instead of incrementing currentValue in two places. I also don't love that we alternatively name a key generator value with terms containing "value", "key", and "number". Seems like a little more consistency would make this read easier.
> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:1418 > - return uncheckedSetKeyGeneratorValue(objectStoreID, newKeyInteger); > + return uncheckedSetKeyGeneratorValue(objectStoreID, newKeyInteger - 1);
This -1 is mysterious to me, even more so because the equivalent code in the memory backing store doesn't do this. I'm sure it's right given the test progressions, but I feel like this could use some clarity.
WebKit Commit Bot
Comment 4
2016-01-28 22:50:48 PST
Comment on
attachment 270166
[details]
Patch v1 Clearing flags on attachment: 270166 Committed
r195801
: <
http://trac.webkit.org/changeset/195801
>
WebKit Commit Bot
Comment 5
2016-01-28 22:50:53 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug