This looks broken from code inspection. I haven't tested yet. Frame* frame = V8Proxy::retrieveFrame(); [...] callback = V8CustomSQLStatementCallback::create(args[2], frame); [...] v8::Handle<v8::Context> context = V8Proxy::context(m_frame.get()); The callbacks should just hold onto a v8::Context. Right now, they should get the v8::Context::GetCurrent() context. (This is also wrong, but that's another bug.)
I'd like the OwnHandle class from Bug 27533 before fixing this code.
Bug 34726 shows how to do a this right.
Created attachment 52810 [details] patch Apologies for uploading a huge patch: it's really just 1 change applied to 5 different locations. Adam, please review the changes to the V8 bindings. Gavin, please review the changes to the JSC bindings + ScriptController.{h|cpp}.
Attachment 52810 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/bindings/js/ScriptController.cpp:172: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] WebCore/bindings/js/JSCustomSQLStatementErrorCallback.cpp:70: One space before end of line comments [whitespace/comments] [5] WebCore/bindings/js/ScriptController.h:43: Code inside a namespace should not be indented. [whitespace/indent] [4] WebCore/storage/SQLTransactionErrorCallback.h:38: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 4 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 52810 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/1694006
Created attachment 52913 [details] patch
Attachment 52913 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/1609345
Attachment 52913 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/1671181
So much copy/paste code. :( + RefPtr<Frame> m_frame I though we decided we wanted to hold a ScriptExecutionContext instead of a m_frame.
(In reply to comment #9) > So much copy/paste code. :( > > + RefPtr<Frame> m_frame > > I though we decided we wanted to hold a ScriptExecutionContext instead of a > m_frame. I was the one talking about eliminating it, but it's fairly easy for Dumi to fix now. Dumi: Take a look at IDBCallbacks.h + V8CustomIDBCallbacks.h It's really not much harder to use the ActiveDOMObject stuff rather than storing a ref to the Frame (which is incorrect for several reasons) and it'll allow you to get notifications when the frame is going away (so you shouldn't do the callback...and should probably cancel the transaction). Whether or not you do that, I'll be trying to unify our callback code at some point.
JSC looks good to me.
Comment on attachment 52913 [details] patch Looks like this breaks the mac and gtk builders.
Created attachment 53030 [details] patch Same patch. Changed some #includes to make them similar in all callbacks. Should build on all ports now.
Comment on attachment 53030 [details] patch I feel like this patch highlights a ton of redundant code that might be profitably refactored or autogenerated. Thanks for updating the patch.
Landed as r57530.