Bug 97375 – IndexedDB: Remove IDBKey from WebKitIDL

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Joshua Bell]

The WebKit IDB implementation creates a special type, IDBKey, and exposes it in the WebKitIDL files for IDL types, e.g.:

        [CallWith=ScriptExecutionContext] IDBRequest put(in any value, in [Optional] IDBKey key)
            raises (IDBDatabaseException);

This requires logic in the code generator and custom binding code (V8 and JSC) to have special cases for IDBKey.

Alternately, we could match the WebIDL and just have this be an "any", and then expose functions (in the binding layer, I guess) which the implementation of put() (for example) would call to map a DOMObject (usually a ScriptObject) into an IDBKey (or fail).

That would also be a good time to make IDBKey a value type instead of a refcounted class.

[Joshua Bell]

I'm going to do this in two phases:

(1) Drop dependencies on IDBKey in other IDB IDLs - functions will take "any" (i.e. ScriptValue) instead.
(2) Clean up the binding code

Note that #1 will probably look like a bad refactor since it will be inlining a bunch of code that was hidden by the binding layer to do the ScriptValue->IDBKey conversion.

[Joshua Bell]

Created attachment 184044 [details]
Patch

[Joshua Bell]

Comment on attachment 184044 [details]
Patch

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

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:72
> +static JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, IDBKey* key)

Should I give this a more meaningful name, e.g. idbKeyToJSValue?

> Source/WebCore/bindings/v8/IDBBindingUtilities.cpp:42
> +static v8::Handle<v8::Value> toV8(IDBKey* key, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)

Should I give this a more meaningful name, e.g. idbKeyToV8Value?

[Joshua Bell]

haraken@ - r?

[Kentaro Hara]

Comment on attachment 184044 [details]
Patch

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

>> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:72
>> +static JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, IDBKey* key)
> 
> Should I give this a more meaningful name, e.g. idbKeyToJSValue?

Sounds nicer. This method is not "toV8()" in our binding terminology (toV8() is a method that wraps a DOM object).

>> Source/WebCore/bindings/v8/IDBBindingUtilities.cpp:42
>> +static v8::Handle<v8::Value> toV8(IDBKey* key, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
> 
> Should I give this a more meaningful name, e.g. idbKeyToV8Value?

Ditto. Let's rename.

[Joshua Bell]

Created attachment 184078 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 184078 [details]
Patch for landing

Clearing flags on attachment: 184078

Committed r140488: <http://trac.webkit.org/changeset/140488>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.