[Webkit-unassigned] [Bug 44275] Hooks IDBKeyPath with IDBObjectStorage::put.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=44275





--- Comment #4 from Marcus Bulach <bulach at chromium.org>  2010-08-20 06:38:45 PST ---
thanks Jeremy! all comments addressed, another look please?

(In reply to comment #2)
> (From update of attachment 64873 [details])
> 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>?

sorry, this file shouldn't be here, removed.
(but to answer your question: Forward.h doesn't declare the default value for inlineCapacity, so we need to provide here, or include the real header).

> 
> 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
file removed (but fixed somewhere else).

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

> 
> 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.
A-ha, I knew there was a simpler way. thanks!

> 
> WebCore/ChangeLog:5
>  +          Hooks IDBKeyPath with IDBObjectStorage::put.
> This is not terribly descriptive.  :-)
> You forgot the URL
> Please add more detail
I tried to add more details, let me know if it's any clearer.


> 
> 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.
WebVector :(
it requires a public trivial constructor even for declaring an empty vector, not sure how to avoid this..


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

> 
> 
> Looks pretty good...
thanks! another look please?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.