IDL: Clean up argument conversion error handling

IDL: Clean up argument conversion error handling

Remove all handling of the conditional (now never generated) v8::TryCatch
local from the code generator.

Also restructure v8_value_to_local_cpp_value() which now has two major
code paths: conversions that need error handling (strings and conversions
that use ExceptionState,) and conversions that don't need error handling.
The unused code path that generated code using TONATIVE_VOID() is removed.

This patch does not change any generated code, aside from in the bindings
tests.

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

[Jens Widell, 2014-09-26 09:45:56]

jl@opera.com changed reviewers:
+ haraken@chromium.org

[Jens Widell, 2014-09-26 09:45:57]

PTAL

TONATIVE_VOID() is now only used in custom code. Typically this is custom code
that no longer matches what the code generator would generate in areas where it
should, which I intend to go through and fix up next.

https://codereview.chromium.org/611453002/diff/1/Source/bindings/scripts/v8_t...
File Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/611453002/diff/1/Source/bindings/scripts/v8_t...
Source/bindings/scripts/v8_types.py:573: if idl_type.base_type in ('void',
'object', 'EventHandler', 'EventListener'):
These are IDL types where we don't have meaningful conversion support, and
previously returned some bogus code, and that doesn't (currently) matter because
whatever we return from here won't be used.

All methods with |object| arguments are [Custom], |EventHandler|/|EventListener|
have special handling in the templates, and |void| obviously doesn't really make
sense to convert.

Returning this C++ comment is just meant to aid debugging if this return value
is ever actually used (which will be a compilation error too, since this is not
a valid C++ expression.)

[haraken, 2014-09-26 09:49:30]

LGTM

https://codereview.chromium.org/611453002/diff/1/Source/bindings/scripts/v8_t...
File Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/611453002/diff/1/Source/bindings/scripts/v8_t...
Source/bindings/scripts/v8_types.py:568: return '/* no V8 -> C++ conversion for
IDL union type: %s */' % idl_type.name

Can we raise an exception instead?

[Jens Widell, 2014-09-26 09:57:05]

On 2014/09/26 09:49:30, haraken wrote:
> LGTM
> 
>
https://codereview.chromium.org/611453002/diff/1/Source/bindings/scripts/v8_t...
> File Source/bindings/scripts/v8_types.py (right):
> 
>
https://codereview.chromium.org/611453002/diff/1/Source/bindings/scripts/v8_t...
> Source/bindings/scripts/v8_types.py:568: return '/* no V8 -> C++ conversion
for
> IDL union type: %s */' % idl_type.name
> 
> Can we raise an exception instead?

Not straight away, no. This function is called for each and every method
argument. We'd have to avoid calling it for precisely these types one level up,
pretty much.

We could (and it might make some sense to) avoid calling it at all for [Custom]
methods, since their declared arguments in the IDL has no effect whatsoever on
generated code. That would take care of the |object| case. For |EventTarget| and
|EventListener|, we'd need to avoid calling it because we know the template code
won't use the returned value for those. But that would just be exactly the same
logic somewhere else, and then this check here but raising an exception.

[haraken, 2014-09-26 10:04:07]

On 2014/09/26 09:57:05, Jens Widell wrote:
> On 2014/09/26 09:49:30, haraken wrote:
> > LGTM
> > 
> >
>
https://codereview.chromium.org/611453002/diff/1/Source/bindings/scripts/v8_t...
> > File Source/bindings/scripts/v8_types.py (right):
> > 
> >
>
https://codereview.chromium.org/611453002/diff/1/Source/bindings/scripts/v8_t...
> > Source/bindings/scripts/v8_types.py:568: return '/* no V8 -> C++ conversion
> for
> > IDL union type: %s */' % idl_type.name
> > 
> > Can we raise an exception instead?
> 
> Not straight away, no. This function is called for each and every method
> argument. We'd have to avoid calling it for precisely these types one level
up,
> pretty much.
> 
> We could (and it might make some sense to) avoid calling it at all for
[Custom]
> methods, since their declared arguments in the IDL has no effect whatsoever on
> generated code. That would take care of the |object| case. For |EventTarget|
and
> |EventListener|, we'd need to avoid calling it because we know the template
code
> won't use the returned value for those. But that would just be exactly the
same
> logic somewhere else, and then this check here but raising an exception.

Makes sense.

[Jens Widell, 2014-09-26 10:07:06]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-09-26 10:08:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/611453002/1

[commit-bot: I haz the power, 2014-09-26 11:02:12]

Committed patchset #1 (id:1) as 182754