IDL: Make idl_type.is_wrapper_type false for callback interfaces

IDL: Make idl_type.is_wrapper_type false for callback interfaces

Callback interface types (when used as method arguments) have special
pre-conversion error handling, and special conversion code, and making
idl_type.is_wrapper_type false disables the normal interface type
error handling that is generated after the conversion (by checking for
null) when [TypeChecking=Interface].

Other effects of making idl_type.is_wrapper_type false for callback
interfaces are:

- Callback interface types can no longer be used as arguments to methods
  implemented in private script. Reverting this effect would be trivial.

- If a callback interface type argument is the distinguishing argument
  for an overloaded method, we stop generating code that doesn't compile
  and instead generate no code at all for that method overload.

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

[Jens Widell, 2015-05-11 13:18:58]

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

[Jens Widell, 2015-05-11 13:18:59]

PTAL

The reason why I ended up with this is that I tried adding
[TypeChecking=Interface] to Window, and it caused a bunch of type checks for
callback interface arguments to be added. When I investigated the generated
code, I realized all the added checks were redundant.

The change in v8_value_to_local_cpp_value() was necessarily as a side-effect,
since callback interface types were previously handled, wrongly, by the generic
wrapper type code path. It doesn't really matter what we do, since callback
interfaces have special handling in the Jinja templates, as long as we don't
raise Python exceptions.

[haraken, 2015-05-11 13:28:38]

LGTM.

It sounds a bit strange that callback interfaces are not wrapper types, but from
your descriptions it makes more sense to treat them as not wrapper types.

[Jens Widell, 2015-05-11 14:16:59]

On 2015/05/11 13:28:38, haraken wrote:
> LGTM.
> 
> It sounds a bit strange that callback interfaces are not wrapper types, but
from
> your descriptions it makes more sense to treat them as not wrapper types.

I'm leaning towards them being more different from "regular" wrapper types than
similar to them.

There's no ScriptWrappable sub-type involved, there's no implementation object
(no toImpl()), and the V8MyCallback class is instantiated referencing a JS
object but isn't referenced from the JS object or stored in a wrapper map.

Also, WebIDL has the concept "platform object" that maps reasonably well to what
we internally call "wrapper types" and that specifically doesn't include
callback interfaces: http://heycam.github.io/webidl/#dfn-platform-object

[Jens Widell, 2015-05-11 14:17:02]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2015-05-11 14:17:19]

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

[haraken, 2015-05-11 15:09:55]

On 2015/05/11 14:16:59, Jens Widell wrote:
> On 2015/05/11 13:28:38, haraken wrote:
> > LGTM.
> > 
> > It sounds a bit strange that callback interfaces are not wrapper types, but
> from
> > your descriptions it makes more sense to treat them as not wrapper types.
> 
> I'm leaning towards them being more different from "regular" wrapper types
than
> similar to them.
> 
> There's no ScriptWrappable sub-type involved, there's no implementation object
> (no toImpl()), and the V8MyCallback class is instantiated referencing a JS
> object but isn't referenced from the JS object or stored in a wrapper map.
> 
> Also, WebIDL has the concept "platform object" that maps reasonably well to
what
> we internally call "wrapper types" and that specifically doesn't include
> callback interfaces: http://heycam.github.io/webidl/#dfn-platform-object

Makes sense.

[commit-bot: I haz the power, 2015-05-11 15:33:28]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195181