Bug 153410

Summary: Modern IDB: Support IDBObjectStore.createIndex in the SQLite backing store
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Severity: Normal CC: achristensen, alecflett, commit-queue, jsbell
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149117, 153021    
Description Flags
Patch v1
darin: review+
Patch for landing none

Description Brady Eidson 2016-01-24 12:28:35 PST
Modern IDB: Support IDBObjectStore.createIndex in the SQLite backing store
Comment 1 Brady Eidson 2016-01-24 12:32:24 PST
Created attachment 269703 [details]
Patch v1
Comment 2 Darin Adler 2016-01-24 14:16:15 PST
Comment on attachment 269703 [details]
Patch v1

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

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:737
> +        ASSERT(m_vm);

Not sure this assertion is valuable given that the code to create m_vm is just above.

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:742
> +        const IDBKeyData& key = cursor->currentKey();


> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:743
> +        const Vector<uint8_t>& valueBuffer = cursor->currentValueBuffer();


> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:745
> +        Deprecated::ScriptValue value = deserializeIDBValueBuffer(m_globalObject->globalExec(), Vector<uint8_t>(valueBuffer), true);


> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:835
> +    }
> +    {

I think there should be a blank line between these two.

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp:56
> +    auto cursor = std::unique_ptr<SQLiteIDBCursor>(new SQLiteIDBCursor(transaction, objectStoreIdentifier));

Can this use make_unique instead? Problem with private? Long term we would love to just be able to "grep for new without adopt" and it’s not so easy with unique_ptr used like this.

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp:82
> +    , m_indexID(IDBIndexMetadata::InvalidId)

Could we initialize this in the class definition?

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp:83
> +    , m_cursorDirection(IndexedDB::CursorDirection::Next)

Could we initialize this in the class definition?
Comment 3 Brady Eidson 2016-01-24 14:53:23 PST
Created attachment 269704 [details]
Patch for landing
Comment 4 WebKit Commit Bot 2016-01-24 15:42:21 PST
Comment on attachment 269704 [details]
Patch for landing

Clearing flags on attachment: 269704

Committed r195517: <http://trac.webkit.org/changeset/195517>
Comment 5 WebKit Commit Bot 2016-01-24 15:42:24 PST
All reviewed patches have been landed.  Closing bug.