Currently when serializing a ScriptValue in IDBObjectStore::put(), the ScriptValue is serialized using the ScriptState provided by ScriptState::current(). In JSC there is no ScriptState::current(), and it will be necessary to obtain an ExecState from DOMRequestState to serialize the ScriptValue.
Created attachment 174760 [details] Patch
Comment on attachment 174760 [details] Patch This looks ok to me. I'd be curious what abarth thinks. I've always found it a little odd that SerializedScriptValue::deserialize takes a ScriptState and not something better. (In fact I wonder now if there's a way we can fix SerializedScriptValue rather than fix this here)
Another possible approach would be to update DOMRequestState to add a method to return the ScriptState* (presumably with ScriptState::forContext()); we could then use requestState.scriptState() for both V8 and JSC and skip adding the new serializeIDBValue() function.
Can't you just add a CallWith=ScriptState to the IDL to get the script state from the bindings layer?
(In reply to comment #4) > Can't you just add a CallWith=ScriptState to the IDL to get the script state from the bindings layer? That solution would be ideal. However it seems that adding CallWith=ScriptState to a function causes the V8 binding generator to invoke the function with an EmptyScriptState rather than with ScriptState::current(). And at the beginning of ScriptValue::serialize(), a ScriptScope is created with this EmptyScriptState. Since the EmptyScriptState has a null v8::Context, the v8::Context::Scope in ScriptScope enters the null context, triggering an assertion failure in V8.
> However it seems that adding CallWith=ScriptState to a function causes the V8 binding generator to invoke the function with an EmptyScriptState rather than with ScriptState::current(). That sounds like a bug in the code generator that we should fix.
Created attachment 175501 [details] Patch
Now that the V8 binding generator has been updated in <https://bugs.webkit.org/show_bug.cgi?id=102739>, I've created a new patch which obtains the ScriptState from the IDL binding generator. Creating a new serializeIDBValue() utility function is no longer necessary.
Comment on attachment 175501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175501&action=review IDB changes LGTM but would be nice to start off with consistent naming, i.e.... > Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp:136 > + return put(IDBObjectStoreBackendInterface::AddOrUpdate, IDBAny::create(this), scriptState, value, key, ec); ^^^ "scriptState" vs. > Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp:139 > +PassRefPtr<IDBRequest> IDBObjectStore::put(IDBObjectStoreBackendInterface::PutMode putMode, PassRefPtr<IDBAny> source, ScriptState* state, ScriptValue& value, PassRefPtr<IDBKey> prpKey, ExceptionCode& ec) ^^^ "state"
Comment on attachment 175501 [details] Patch LGTM. Please address jsbell's comments before landing.
Created attachment 175550 [details] Patch I've changed variables previously named 'scriptState' to 'state' for the sake of consistency.
Comment on attachment 175550 [details] Patch Clearing flags on attachment: 175550 Committed r135471: <http://trac.webkit.org/changeset/135471>
All reviewed patches have been landed. Closing bug.
Comment on attachment 175550 [details] Patch Thanks for improving the code generator. I think this patch came out very nicely this way.