[bindings] v8_interface.py should support 'any' or 'object' in getter/setter

[bindings] v8_interface.py should support 'any' or 'object' in getter/setter

Certain idl specifications such as [1], defines a 'getter' with 'any' as return type.
In order to support this, v8_interface.py should add conditional to allow either 'any' or 'object'.

R=jl@opera.com, haraken@chromium.org

[1] http://www.w3.org/TR/webdatabase/#sqlresultsetrowlist

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

[vivekg, 2015-03-03 12:36:33]

vivekg@chromium.org changed reviewers:
+ haraken@chromium.org, vivekg@chromium.org

[vivekg, 2015-03-03 12:36:34]

This is also required in https://codereview.chromium.org/972153002

PTAL and let me know your feedback.

https://codereview.chromium.org/975673002/diff/1/Source/bindings/scripts/v8_i...
File Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/975673002/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:1233: return 'result.isNull()'
Without the above condition, the code is generated as
http://www.mergely.com/MUMV9p1k/

[Jens Widell, 2015-03-03 12:50:19]

https://codereview.chromium.org/975673002/diff/1/Source/bindings/scripts/v8_i...
File Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/975673002/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:1233: return 'result.isNull()'
On 2015/03/03 12:36:33, vivekg_ wrote:
> Without the above condition, the code is generated as
> http://www.mergely.com/MUMV9p1k/

I wonder if 'result.isEmpty()' is a better condition than 'result.isNull()'.

It seems the purpose is to convey "not handled", and for ScriptValue it seems
better to use an empty value rather than a null one for that, since a null
ScriptValue is more "work" to construct, and also, since interpreting null as
"not handled" makes it impossible to actually return null.

[vivekg, 2015-03-03 12:56:02]

PTAL, thanks.

https://codereview.chromium.org/975673002/diff/1/Source/bindings/scripts/v8_i...
File Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/975673002/diff/1/Source/bindings/scripts/v8_i...
Source/bindings/scripts/v8_interface.py:1233: return 'result.isNull()'
On 2015/03/03 at 12:50:19, Jens Widell wrote:
> On 2015/03/03 12:36:33, vivekg_ wrote:
> > Without the above condition, the code is generated as
> > http://www.mergely.com/MUMV9p1k/
> 
> I wonder if 'result.isEmpty()' is a better condition than 'result.isNull()'.
> 
> It seems the purpose is to convey "not handled", and for ScriptValue it seems
better to use an empty value rather than a null one for that, since a null
ScriptValue is more "work" to construct, and also, since interpreting null as
"not handled" makes it impossible to actually return null.

Yeah, makes perfect sense. I will upload a new patch. Thanks :)

[Jens Widell, 2015-03-03 13:02:39]

LGTM

https://codereview.chromium.org/975673002/diff/20001/Source/bindings/scripts/...
File Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/975673002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_interface.py:1232: if idl_type.name == 'Any' or
idl_type.name == 'Object':
Nit: "idl_type.base_type in ('any', 'object')"

Using 'idl_type.name' has the disadvantage that nullable types have different
names; i.e. you'd need to handle ObjectOrNull as well, for completeness.

[vivekg, 2015-03-03 13:05:04]

Done.

https://codereview.chromium.org/975673002/diff/20001/Source/bindings/scripts/...
File Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/975673002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_interface.py:1232: if idl_type.name == 'Any' or
idl_type.name == 'Object':
On 2015/03/03 at 13:02:39, Jens Widell wrote:
> Nit: "idl_type.base_type in ('any', 'object')"
> 
> Using 'idl_type.name' has the disadvantage that nullable types have different
names; i.e. you'd need to handle ObjectOrNull as well, for completeness.

Done.

[vivekg, 2015-03-03 13:09:02]

The CQ bit was checked by vivekg@chromium.org

[vivekg, 2015-03-03 13:09:02]

The patchset sent to the CQ was uploaded after l-g-t-m from jl@opera.com
Link to the patchset: https://codereview.chromium.org/975673002/#ps40001 (title:
"Patch for landing.")

[commit-bot: I haz the power, 2015-03-03 13:09:13]

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

[vivekg, 2015-03-03 13:42:57]

vivekg@chromium.org changed reviewers:
+ jsbell@chromium.org

[vivekg, 2015-03-03 13:43:18]

vivekg@chromium.org changed reviewers:
- jsbell@chromium.org

[commit-bot: I haz the power, 2015-03-03 14:27:48]

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

[haraken, 2015-03-03 14:53:27]

LGTM