Bug 86123 - IndexedDB: Implement spec behavior for multiEntry indexes with invalid/duplicate subkeys
Summary: IndexedDB: Implement spec behavior for multiEntry indexes with invalid/duplic...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alec Flett
URL:
Keywords:
Depends on: 86122
Blocks:
  Show dependency tree / graph
 
Reported: 2012-05-10 11:55 PDT by Joshua Bell
Modified: 2012-06-21 15:29 PDT (History)
11 users (show)

See Also:

Attachments
Patch (25.62 KB, patch)
2012-06-19 12:55 PDT, Alec Flett 		no flags Details | Formatted Diff | Diff
Patch (25.98 KB, patch)
2012-06-19 13:59 PDT, Alec Flett 		no flags Details | Formatted Diff | Diff
Patch (26.20 KB, patch)
2012-06-19 14:30 PDT, Alec Flett 		no flags Details | Formatted Diff | Diff
Patch (23.08 KB, patch)
2012-06-21 11:35 PDT, Alec Flett 		no flags Details | Formatted Diff | Diff
Patch for landing (23.02 KB, patch)
2012-06-21 11:46 PDT, Joshua Bell 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) 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-05-10 11:55:55 PDT 
The spec details the handling of multiEntry key paths where the key path yields an array and the array includes values that are duplicates or invalid sub-keys. WebKit's implementation (WebCore/binding/v8/IDBBindingUtilities.cpp) yields an overall InvalidType key if any of the subkeys are invalid.

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

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.

For example, the following value of index key [10, 20, null, 30, 20] is converted to [10, 20, 30].
Comment 1 Alec Flett 2012-06-19 12:55:00 PDT 
Created attachment 148395 [details]
Patch
Comment 2 Alec Flett 2012-06-19 12:55:50 PDT 
jsbell@ or dgrogan@ - mind a quick look?
Comment 3 WebKit Review Bot 2012-06-19 12:57:35 PDT 
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 4 Alec Flett 2012-06-19 13:59:26 PDT 
Created attachment 148414 [details]
Patch
Comment 5 Joshua Bell 2012-06-19 14:22:25 PDT 
Comment on attachment 148414 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBLevelDBCoding.cpp:393
> +        ASSERT_NOT_REACHED();

Redundant with fall through.

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:451
> +                if (!m_index->addingKeyAllowed(indexKey->array()[i].get()))

Ouch, good catch.

> LayoutTests/storage/indexeddb/resources/index-multientry.js:124
> +                                         };

Either put this callback all on one line, or correct the indentation.
Comment 6 Alec Flett 2012-06-19 14:30:25 PDT 
Created attachment 148425 [details]
Patch
Comment 7 Alec Flett 2012-06-19 14:31:27 PDT 
Comment on attachment 148425 [details]
Patch

tkent@ - r? cq?
Comment 8 Alec Flett 2012-06-21 11:35:19 PDT 
Created attachment 148848 [details]
Patch
Comment 9 Joshua Bell 2012-06-21 11:46:17 PDT 
Created attachment 148851 [details]
Patch for landing
Comment 10 WebKit Review Bot 2012-06-21 15:28:57 PDT 
Comment on attachment 148851 [details]
Patch for landing

Clearing flags on attachment: 148851

Committed r120967: <http://trac.webkit.org/changeset/120967>
Comment 11 WebKit Review Bot 2012-06-21 15:29:02 PDT 
All reviewed patches have been landed.  Closing bug.