bindings: Throw TypeError when required dictionary member is undefined

bindings: Throw TypeError when required dictionary member is undefined

The spec[1] says that when a required dictionary member is undefined,
a TypeError should be thrown.

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

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

[bashi, 2015-04-01 09:57:37]

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

[bashi, 2015-04-01 09:57:37]

PTAL?

[haraken, 2015-04-01 10:04:43]

Does the behavior align with Firefox and IE?

[Jens Widell, 2015-04-01 10:06:20]

LGTM

https://codereview.chromium.org/1057453002/diff/1/Source/bindings/tests/resul...
File Source/bindings/tests/results/core/TestDictionaryDerivedImplementedAs.h
(right):

https://codereview.chromium.org/1057453002/diff/1/Source/bindings/tests/resul...
Source/bindings/tests/results/core/TestDictionaryDerivedImplementedAs.h:30: bool
hasRequiredLongMember() const { return !m_requiredLongMember.isNull(); }
Slightly weird to have this for required members, since it must really return
true. OTOH, it's a bit tricky to actually make the member required in the C++
code without a default value, so...

[bashi, 2015-04-01 10:42:15]

> Does the behavior align with Firefox and IE?

WebCrypto defines Algorithm dictionary with required keyword, and [1] defines
how to convert an object to an instance of Algorithm dictionary. I examined what
happens when required member 'name' is missing.

crypto.subtle.digest({}, new ArrayBuffer(10))

- Firefox throws SyntaxError.
- The current chrome implementation throws TypeError.
- I can't try it on IE (I don't have Windows machine now).

Since the spec says that TypeError should be thrown, I think this behavior makes
sense.

[1]
https://dvcs.w3.org/hg/webcrypto-api/raw-file/tip/spec/Overview.html#dfn-norm...

https://codereview.chromium.org/1057453002/diff/1/Source/bindings/tests/resul...
File Source/bindings/tests/results/core/TestDictionaryDerivedImplementedAs.h
(right):

https://codereview.chromium.org/1057453002/diff/1/Source/bindings/tests/resul...
Source/bindings/tests/results/core/TestDictionaryDerivedImplementedAs.h:30: bool
hasRequiredLongMember() const { return !m_requiredLongMember.isNull(); }
On 2015/04/01 10:06:20, Jens Widell wrote:
> Slightly weird to have this for required members, since it must really return
> true. OTOH, it's a bit tricky to actually make the member required in the C++
> code without a default value, so...

Yeah, and dictionaries can be used as a return value of a method, so we (the
binding layer) can't make sure that a required member is always set. I added
ASSERT_NOT_REACHED() in toV8() when a dictionary doesn't have required members.

[haraken, 2015-04-01 12:04:36]

> > Does the behavior align with Firefox and IE?
> 
> WebCrypto defines Algorithm dictionary with required keyword, and [1] defines
> how to convert an object to an instance of Algorithm dictionary. I examined
what
> happens when required member 'name' is missing.
> 
> crypto.subtle.digest({}, new ArrayBuffer(10))
> 
> - Firefox throws SyntaxError.
> - The current chrome implementation throws TypeError.

Given this doesn't change existing behavior, it won't be problematic. LGTM.

[bashi, 2015-04-02 01:26:58]

The CQ bit was checked by bashi@chromium.org

[bashi, 2015-04-02 01:26:58]

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/1057453002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2015-04-02 01:27:39]

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

[commit-bot: I haz the power, 2015-04-02 05:24:54]

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