[webkit-reviews] review granted: [Bug 38414] Auto-generate the callbacks code : [Attachment 54908] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon May 3 07:58:11 PDT 2010
Adam Barth <abarth at webkit.org> has granted Dumitru Daniliuc
<dumi at chromium.org>'s request for review:
Bug 38414: Auto-generate the callbacks code
https://bugs.webkit.org/show_bug.cgi?id=38414
Attachment 54908: patch
https://bugs.webkit.org/attachment.cgi?id=54908&action=review
------- Additional Comments from Adam Barth <abarth at webkit.org>
Thanks so much for writing this patch. I'm sure we'll have to iterate on this
code as we port more of the callbacks to autogen, but this looks like a great
start.
WebCore/bindings/scripts/CodeGeneratorV8.pm:2266
+ pop(@implContent); # remove the last comma and newline
Isn't there some sort of join function that can do this more cleanly?
WebCore/bindings/scripts/CodeGeneratorV8.pm:2761
+ sub GetNativeTypeForCallbacks
It's strange that we have this for V8 but not for jsc.
WebCore/bindings/scripts/test/JS/JSTestCallback.h:44
+ COMPILE_ASSERT(false) virtual int
callbackWithNonBoolReturnType(ScriptExecutionContext*, Class3* class3Param);
I don't think this is quite right, but it will have the desired effect of not
compiling.
WebCore/bindings/scripts/test/V8/V8TestCallback.cpp:37
+ , m_frame(frame)
I don't think holding the frame is right, as we discussed earlier. However I
think that's ok for now given that this is what the custom code does.
WebCore/bindings/scripts/test/V8/V8TestCallback.cpp:66
+ return !invokeCallback(m_callback, 1, argv, callbackReturnValue);
It's strange to see this constant here, but I guess it makes sense.
More information about the webkit-reviews
mailing list