Bug 86122 – IndexedDB: Index key paths that yield invalid keys should not fail an add/put

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Joshua Bell]

WebKit's IDB implementation enforces an older spec restriction, namely that during the call to IDBOBjectStore.add/put(), if evaluating the key path of any index yields an invalid key the add/put should - instead, they should be ignored.

The spec was updated to drop this restriction (indexes key paths are not checked at all during the add/put call). The steps for storing a record now say:

http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#dfn-steps-for-storing-a-record-into-an-object-store

7.2 Evaluate index's key path on value. If this does not yield a value, take no further actions for this index. Otherwise set the result to index key.
7.3 If index's multiEntry flag is false or if index key is not an Array, and if index key is not a valid key, take no further actions for this index.
7.4 If index's multiEntry flag is true, and index key is an Array, remove any elements from index key that are not valid keys and remove any duplicate elements from index key such that only one instance of the duplicate value remains.

[Joshua Bell]

Created attachment 142814 [details]
Patch

[Joshua Bell]

tony@ - r?

[Tony Chang]

Comment on attachment 142814 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:289
> +            indexKeys.append(PassRefPtr<IDBKey>());

Nit: Should this be using a IDBKey::create function?  Creating a PassRefPtr explicitly feels weird.

[Joshua Bell]

Comment on attachment 142814 [details]
Patch

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

>> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:289
>> +            indexKeys.append(PassRefPtr<IDBKey>());
> 
> Nit: Should this be using a IDBKey::create function?  Creating a PassRefPtr explicitly feels weird.

So does calling release() on a known-empty RefPtr (the previous code). I'll see if I can come up with something cleaner without a big restructure.

[Joshua Bell]

Created attachment 143065 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 143065 [details]
Patch for landing

Clearing flags on attachment: 143065

Committed r117808: <http://trac.webkit.org/changeset/117808>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.