[webkit-reviews] review granted: [Bug 34994] Add sync bindings for Worker access to DB : [Attachment 55358] patch #2: IDL files + JSC and V8 bindings

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Dumitru Daniliuc
<dumi at chromium.org>'s request for review:
Bug 34994: Add sync bindings for Worker access to DB
https://bugs.webkit.org/show_bug.cgi?id=34994

Attachment 55358: patch #2: IDL files + JSC and V8 bindings
https://bugs.webkit.org/attachment.cgi?id=55358&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
There are still a few more bugs, but this is vastly better than the rest of the
custom bindings.  Thanks so much for iterating on this patch.  Please address
the comments below before landing.

WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:725
 +	EXCEPTION_BLOCK(String, name, toWebCoreString(args[0]));
Nice.

WebCore/bindings/v8/custom/V8DatabaseSyncCustom.cpp:52
 +	EXCEPTION_BLOCK(String, newVersion, toWebCoreString(args[0]));
Are these both supposed to be args[0]?

WebCore/bindings/v8/custom/V8DatabaseSyncCustom.cpp:65
 +	database->changeVersion(toWebCoreString(args[0]),
toWebCoreString(args[1]), callback.release(), ec);
We probably shouldn't call toString again.  Why not use oldVersion and
newVersion above?

WebCore/bindings/v8/custom/V8SQLTransactionCustom.cpp:72
 +		sqlArgsLength = length->Uint32Value();
This doesn't call valueOf, right?

WebCore/bindings/v8/custom/V8SQLTransactionCustom.cpp:78
 +		    v8::TryCatch block;
Why not use our fancy macro here?