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.
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.
Created attachment 270166 [details] Patch v1
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.
Comment on attachment 270166 [details] Patch v1 Clearing flags on attachment: 270166 Committed r195801: <http://trac.webkit.org/changeset/195801>
All reviewed patches have been landed. Closing bug.