[webkit-reviews] review granted: [Bug 128316] IDB: storage/indexeddb/create-index-with-integer-keys.html fails : [Attachment 223429] Patch v2 - Fix style
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Feb 7 10:17:12 PST 2014
Geoffrey Garen <ggaren at apple.com> has granted Brady Eidson
<beidson at apple.com>'s request for review:
Bug 128316: IDB: storage/indexeddb/create-index-with-integer-keys.html fails
https://bugs.webkit.org/show_bug.cgi?id=128316
Attachment 223429: Patch v2 - Fix style
https://bugs.webkit.org/attachment.cgi?id=223429&action=review
------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
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/UniqueIDBDatabaseBackingStoreSQ
Lite.cpp:646
> + for (auto indexKey : indexKeys) {
auto&?
>
Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQ
Lite.h:117
> + OwnPtr<JSC::JSGlobalObject> m_globalObject;
Please use JSC::Strong. It's like OwnPtr, but for GC instead of malloc.
Otherwise, crash-o.
More information about the webkit-reviews
mailing list