IDL: object type support for IDL dictionary

IDL: object type support for IDL dictionary

We need to support object type for IDL dictionary members because
registerElement() takes a dictionary which has object? member[1].
In dictionary impl classes, dictionary members of which type object
are represented using ScriptValue.

[1] http://w3c.github.io/webcomponents/spec/custom/#dfn-document-registerElement

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

[bashi, 2014-09-24 03:03:06]

Patchset #1 (id:1) has been deleted

[bashi, 2014-09-24 03:42:10]

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

[bashi, 2014-09-24 03:42:11]

Haraken-san, Jens, Could you take a look?

[haraken, 2014-09-24 04:08:57]

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

https://codereview.chromium.org/590443002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:117: return '!(m_{0}.isEmpty() ||
m_{0}.isNull() || m_{0}.isUndefined())'.format(member.name)

Just help me understand: Why do we need to check all of isEmpty(), isNull() and
isUndefined() ?

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

https://codereview.chromium.org/590443002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:405: return
not(idl_type.cpp_type_has_null_value)

Can't we make idl_type.cpp_type_has_null_value return true when idl_type.name ==
'Object' (and avoid adding the above if branch)?

[Jens Widell, 2014-09-24 05:23:06]

https://codereview.chromium.org/590443002/diff/20001/Source/bindings/template...
File Source/bindings/templates/dictionary_v8.cpp (right):

https://codereview.chromium.org/590443002/diff/20001/Source/bindings/template...
Source/bindings/templates/dictionary_v8.cpp:63: }
Replace this and following 3 lines with

{% endif %}
}

[bashi, 2014-09-24 05:28:55]

Thank you for review!

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

https://codereview.chromium.org/590443002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:117: return '!(m_{0}.isEmpty() ||
m_{0}.isNull() || m_{0}.isUndefined())'.format(member.name)
On 2014/09/24 04:08:56, haraken wrote:
> 
> Just help me understand: Why do we need to check all of isEmpty(), isNull()
and
> isUndefined() ?

- Need to check isEmpty() before calling isNull() and isUndefined(), as
ScriptValue requires
- Currently, we treat both null and undefined as 'not present'

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

https://codereview.chromium.org/590443002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:405: return
not(idl_type.cpp_type_has_null_value)
On 2014/09/24 04:08:56, haraken wrote:
> 
> Can't we make idl_type.cpp_type_has_null_value return true when idl_type.name
==
> 'Object' (and avoid adding the above if branch)?

Done.

https://codereview.chromium.org/590443002/diff/20001/Source/bindings/template...
File Source/bindings/templates/dictionary_v8.cpp (right):

https://codereview.chromium.org/590443002/diff/20001/Source/bindings/template...
Source/bindings/templates/dictionary_v8.cpp:63: }
On 2014/09/24 05:23:05, Jens Widell wrote:
> Replace this and following 3 lines with
> 
> {% endif %}
> }

Done.

[haraken, 2014-09-24 05:35:44]

LGTM

[Jens Widell, 2014-09-24 05:48:58]

https://codereview.chromium.org/590443002/diff/40001/Source/bindings/scripts/...
File Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/590443002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:830: idl_type.is_enum or
idl_type.is_dictionary or idl_type.name == 'Object')
For a nullable type, |idl_type.name| has a "OrNull" suffix, so "object?" has the
name "ObjectOrNull" and thus doesn't match this check. All the other checks here
"look through" the nullable type.

Maybe check "idl_type.base_type == 'object'" instead?

[bashi, 2014-09-24 07:19:30]

https://codereview.chromium.org/590443002/diff/40001/Source/bindings/scripts/...
File Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/590443002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:830: idl_type.is_enum or
idl_type.is_dictionary or idl_type.name == 'Object')
On 2014/09/24 05:48:58, Jens Widell wrote:
> For a nullable type, |idl_type.name| has a "OrNull" suffix, so "object?" has
the
> name "ObjectOrNull" and thus doesn't match this check. All the other checks
here
> "look through" the nullable type.
> 
> Maybe check "idl_type.base_type == 'object'" instead?

Done. Thank you for pointing this out!

[bashi, 2014-09-25 23:30:49]

The CQ bit was checked by bashi@chromium.org

[commit-bot: I haz the power, 2014-09-25 23:31:25]

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

[commit-bot: I haz the power, 2014-09-25 23:41:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-25 23:41:53]

Try jobs failed on following builders:
  android_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_db...)
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[bashi, 2014-09-25 23:47:46]

The CQ bit was checked by bashi@chromium.org

[commit-bot: I haz the power, 2014-09-25 23:48:21]

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

[commit-bot: I haz the power, 2014-09-26 03:20:04]

Committed patchset #5 (id:100001) as 182725