Adding CallWith=ScriptState to a function causes the V8 binding generator to invoke the function with an EmptyScriptState rather than with ScriptState::current(). Since the EmptyScriptState has a null v8::Context, any clients which depend upon a valid v8::Context will fail.
Created attachment 175112 [details] Patch This change causes CodeGeneratorV8.pm to pass ScriptState::current() rather than an EmptyScriptState to functions marked with CallWith=ScriptState. With this patch I was able to update IndexedDB to use the ScriptState provided by the binding generator for serialization.
Comment on attachment 175112 [details] Patch Great. Do you want to prepare a proper patch for review (e.g., with a ChangeLog)
Comment on attachment 175112 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175112&action=review The change makes sense. > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3529 > - my @callWithArgs = GenerateCallWith($callWith, \@callWithOutput, $indent, 0, 1, $function); > + my @callWithArgs = GenerateCallWith($callWith, \@callWithOutput, $indent, 0, 0, $function); I think that now no one uses 1. Maybe you can kill the $emptyContext argument from GenerateCallWith().
Comment on attachment 175112 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175112&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1633 > } else { Then you can remove this if-else branch.
Created attachment 175115 [details] Patch
Comment on attachment 175115 [details] Patch LGTM. If you set r?, I can r+ it.
Do you know if there is a reason why EmptyScriptState was used in the past in preference to ScriptState::current()?
(In reply to comment #7) > Do you know if there is a reason why EmptyScriptState was used in the past in preference to ScriptState::current()? Don't know. Would you git blame its history and cc folks?
Passing EmptyScriptState to functions marked with CallWith=ScriptState seems to have been introduced to CodeGeneratorV8.pm in revision 58298 on 2010-04-27 by Adam Barth. There was no previous use of either EmptyScriptState or ScriptState::current().
I have no idea why I used EmptyScriptState.
Either way I think this change looks reasonable.
Created attachment 175158 [details] Patch I have updated the ChangeLog to note that this patch has been reviewed by Kentaro Hara.
Comment on attachment 175158 [details] Patch From next time, instead of writing my name to the ChangeLog, please set r?. Then a reviewer can set r+ or r-.
Comment on attachment 175158 [details] Patch And if you want to commit it, please set cq?. (You can find these forms at the right bottom of https://bugs.webkit.org/attachment.cgi?id=175158&action=review)
Comment on attachment 175158 [details] Patch Attachment 175158 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14912534 New failing tests: traversal/exception-forwarding.html
Created attachment 175245 [details] Patch Previously any exception thrown while in EmptyScriptState was cleared when EmptyScriptState went out of scope. I've updated the patch to explicitly clear an exception before throwing it.
Comment on attachment 175245 [details] Patch Attachment 175245 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14894690 New failing tests: fast/dom/shadow/shadow-dom-event-dispatching.html
Comment on attachment 175245 [details] Patch It looks like the failure is not related to the patch. Let's try again.
Comment on attachment 175245 [details] Patch Clearing flags on attachment: 175245 Committed r135325: <http://trac.webkit.org/changeset/135325>
All reviewed patches have been landed. Closing bug.