Bug 128316 – IDB: storage/indexeddb/create-index-with-integer-keys.html fails

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brady Eidson]

IDB: storage/indexeddb/create-index-with-integer-keys.html fails

The test creates an object store, puts a record in the object store, creates an index, then opens an index cursor.

The index cursor is supposed to represent that record in the object store, but comes up undefined.

We don't write index records for indexes that are created *after* a record is put in its referenced object store.  Doh!

[Radar WebKit Bug Importer]

<rdar://problem/16002857>

[Brady Eidson]

Populating an index if it is created after an object store already has records is what "IndexPopulator" is about.

That mechanism is still nuts, though.  We're going to do this way better.

[Brady Eidson]

Have this working in a sane manner with a little JSC in the DatabaseProcess.

Working out a kink in the patch (the test still has one wrong result) then I'll post for review.

[Brady Eidson]

Created attachment 223428 [details]
Patch v1 - Figure out the index keys to write in the DatabaseProcess as part of the createIndex operation itself.

[Brady Eidson]

Created attachment 223429 [details]
Patch v2 - Fix style

[Geoffrey Garen]

Comment on attachment 223429 [details]
Patch v2 - Fix style  

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

r=me

> Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp:208
> +        for (auto i : keyDatas)

This should be "auto&" to avoid an apparent copy.

> Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp:209
> +            keys.append(i.maybeCreateIDBKey());

Let's null check before append to avoid a nullptr dereference later.

> Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp:329
> +                indexKeys.append(i.maybeCreateIDBKey());

Let's null check before append to avoid a nullptr dereference later.

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:355
> +        if (!indexKey->isValid())
> +            return;

Maybe we should check isValid in both cases -- maybe up at the top, by the nullptr check? It seems that an array key can be invalid, so I guess we should check it.

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:363
> +        for (auto i : indexKey->array())

auto&?

> Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp:646
> +        for (auto indexKey : indexKeys) {

auto&?

> Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.h:117
> +    OwnPtr<JSC::JSGlobalObject> m_globalObject;

Please use JSC::Strong. It's like OwnPtr, but for GC instead of malloc. Otherwise, crash-o.

[Brady Eidson]

http://trac.webkit.org/changeset/163627