[webkit-reviews] review granted: [Bug 89607] IndexedDB: refactor index-writing to be more self-contained : [Attachment 149100] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Alec Flett
<alecflett at chromium.org>'s request for review:
Bug 89607: IndexedDB: refactor index-writing to be more self-contained
https://bugs.webkit.org/show_bug.cgi?id=89607

Attachment 149100: Patch
https://bugs.webkit.org/attachment.cgi?id=149100&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=149100&action=review


> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:222
> +    IndexWriter(PassRefPtr<IDBIndexBackendImpl> index)

Nit: explicit

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:223
> +	       : m_index(index)

Nit: Too much indent here (should be 8 spaces total).

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:229
> +    bool loadIndexKeysForValue(SerializedScriptValue* objectValue,
> +				  const IDBKey* primaryKey = 0,
> +				  String* errorMessage = 0)

Nit: I think it's rare in WebKit style to wrap function declarations. If you do
want to wrap it, only indent one level (8 spaces total).

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:249
> +	       m_indexKeys.append(indexKey);
> +	   } else {

Nit: You could early return true here and remove the else block.

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:269
> +    bool writeIndexKeys(const IDBBackingStore::ObjectStoreRecordIdentifier*
recordIdentifier,
> +			   IDBBackingStore& backingStore, int64_t databaseId,
int64_t objectStoreId,
> +			   String* errorMessage = 0)

Nit: Weird indent like above.

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:271
> +	   for (size_t i = 0; i < m_indexKeys.size(); i++) {

Nit: ++i