[webkit-reviews] review denied: [Bug 44275] Hooks IDBKeyPath with IDBObjectStorage::put. : [Attachment 64873] Patch

[bugzilla-daemon at webkit.org]

Jeremy Orlow <jorlow at chromium.org> has denied  review:
Bug 44275: Hooks IDBKeyPath with IDBObjectStorage::put.
https://bugs.webkit.org/show_bug.cgi?id=44275

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

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
WebCore/bindings/v8/IDBBindingUtilities.h:42
 +  void createIDBKeysFromSerializedValuesAndKeyPath(const
Vector<PassRefPtr<SerializedScriptValue>, 0> values,  const String& keyPath,
Vector<PassRefPtr<IDBKey>, 0>& keys);
Why a Vector<, 0>?

WebCore/bindings/v8/IDBBindingUtilities.h:42
 +  void createIDBKeysFromSerializedValuesAndKeyPath(const
Vector<PassRefPtr<SerializedScriptValue>, 0> values,  const String& keyPath,
Vector<PassRefPtr<IDBKey>, 0>& keys);
remove the double space before the second const

WebCore/storage/IDBKeyPathBackendImpl.cpp:35
 +	// FIXME: Implement this method once JSC support WireFormat for
SerializedScriptValue.
supports (with an s)

WebCore/storage/IDBObjectStoreBackendImpl.cpp:83
 +	    RefPtr<SerializedScriptValue> originalValue = value;
Change value to prpValue and at the beginning of the function do RefPtr<> value
= prpValue;  Then you don't need any of this stuff.

WebCore/ChangeLog:5
 +	    Hooks IDBKeyPath with IDBObjectStorage::put.
This is not terribly descriptive.  :-)
You forgot the URL
Please add more detail

WebKit/chromium/public/WebIDBKey.h:42
 +	WebIDBKey() { }
Hmm...can we avoid this?  The idea is that you'd use the proper create
constructor depending on what you wanted to do.

WebCore/storage/IDBObjectStoreBackendImpl.h:38
 +  struct IDBKeyPathElement;
Is this still needed?


Looks pretty good...