Simplify invokeCallback() and support void return values for IDL callbacks

Simplify invokeCallback() and support void return values for IDL callbacks

Now that all callbacks are forced to be v8::Functions, invokeCallback() can be
a lot simpler: no more need to look for a handleEvent method on a v8::Object.

Only a single caller made use of the |callbackReturnValue|, so it's been removed
and that caller now takes care of calling the callback.

invokeCallback() is now just a TryCatch around ScriptController::callFunction.
I also inverted the return value of invokeCallback: true means "everything went fine",
false means "threw an exception".

In making that change, it became clear that most callbacks don't actually have
a return value (and their callers don't care what the called function returns),
and were only using "boolean" in the IDL files because that was all the code
generator supported. I've added support for "void", which is used everywhere
except for a few cases in WebSQL.

R=haraken@chromium.org, nbarth@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163665

[haraken, 2013-12-11 00:54:29]

LGTM

https://codereview.chromium.org/111603006/diff/1/Source/bindings/v8/custom/V8...
File Source/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp (right):

https://codereview.chromium.org/111603006/diff/1/Source/bindings/v8/custom/V8...
Source/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:72:
v8::Handle<v8::Value> result =
ScriptController::callFunction(executionContext(), m_callback.newLocal(isolate),
isolate->GetCurrentContext()->Global(), 2, argv, isolate);

I'm curious why you changed invokeCallback() to callFunction().

https://codereview.chromium.org/111603006/diff/1/Source/bindings/v8/custom/V8...
Source/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:74: // FIXME:
This comment doesn't make much sense given what the code is actually doing.

Yeah, probably we can remove the comment.

[adamk, 2013-12-11 01:05:48]

https://codereview.chromium.org/111603006/diff/1/Source/bindings/v8/custom/V8...
File Source/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp (right):

https://codereview.chromium.org/111603006/diff/1/Source/bindings/v8/custom/V8...
Source/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:72:
v8::Handle<v8::Value> result =
ScriptController::callFunction(executionContext(), m_callback.newLocal(isolate),
isolate->GetCurrentContext()->Global(), 2, argv, isolate);
On 2013/12/11 00:54:30, haraken wrote:
> 
> I'm curious why you changed invokeCallback() to callFunction().

This allowed me to get rid of the callbackReturnValue argument to
invokeCallback(), as none of the other callers looked at it.

https://codereview.chromium.org/111603006/diff/1/Source/bindings/v8/custom/V8...
Source/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:74: // FIXME:
This comment doesn't make much sense given what the code is actually doing.
On 2013/12/11 00:54:30, haraken wrote:
> 
> Yeah, probably we can remove the comment.

I'm going to leave it for posterity, I think; there's lots of callback stuff
that just seems wrong in WebSQL, and I think this comment might be part of that
trail of breadcrumbs.

[commit-bot: I haz the power, 2013-12-11 01:12:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adamk@chromium.org/111603006/1

[commit-bot: I haz the power, 2013-12-11 02:25:22]

Change committed as 163665