IDL: Values for dictionaries should be a object, null or undefined

IDL: Values for dictionaries should be a object, null or undefined

To follow the spec[1]. We should throw an type error when
a V8 value for IDL dictionary is neither an object, null, nor
undefined. The current implementation returns an
impl instance with default values.

[1] http://heycam.github.io/webidl/#es-dictionary

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

[bashi, 2014-09-12 09:00:48]

bashi@chromium.org changed reviewers:
+ haraken@chromium.org

[bashi, 2014-09-12 09:00:48]

Haraken-san, Could you take a look?

[Jens Widell, 2014-09-12 09:13:53]

jl@opera.com changed reviewers:
+ jl@opera.com

[Jens Widell, 2014-09-12 09:13:53]

https://codereview.chromium.org/568703002/diff/40001/Source/bindings/template...
File Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/568703002/diff/40001/Source/bindings/template...
Source/bindings/templates/methods.cpp:200: {% elif argument.is_dictionary or
argument.idl_type == 'Dictionary' %}
It would make sense to have this check before we call the dictionary's toImpl()
rather than after, since the check only looks at the raw argument value
(info[N]). As it is now, toImpl() needs to support being called with a bogus
value and return something that will never be used.

[bashi, 2014-09-12 10:00:49]

Thank you for review!

https://codereview.chromium.org/568703002/diff/40001/Source/bindings/template...
File Source/bindings/templates/methods.cpp (right):

https://codereview.chromium.org/568703002/diff/40001/Source/bindings/template...
Source/bindings/templates/methods.cpp:200: {% elif argument.is_dictionary or
argument.idl_type == 'Dictionary' %}
On 2014/09/12 09:13:53, Jens Widell wrote:
> It would make sense to have this check before we call the dictionary's
toImpl()
> rather than after, since the check only looks at the raw argument value
> (info[N]). As it is now, toImpl() needs to support being called with a bogus
> value and return something that will never be used.

Done, and now I can change if -> ASSERT() in toImpl() :)

[haraken, 2014-09-12 10:07:39]

LGTM

[Jens Widell, 2014-09-12 10:22:31]

LGTM

[bashi, 2014-09-12 11:16:09]

The CQ bit was checked by bashi@chromium.org

[commit-bot: I haz the power, 2014-09-12 11:16:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/568703002/80001

[commit-bot: I haz the power, 2014-09-12 11:22:06]

Committed patchset #5 (id:80001) as 181891