bindings: Add support for the record<K,V> WebIDL type.

bindings: Add support for the record<K,V> WebIDL type.

Records (https://heycam.github.io/webidl/#idl-record) are ordered
associative arrays mapping instances of K to instances of V, where K must be
a ByteString, a DOMString or an USVString.

In C++, we represent records as Vector<std::pair<k,v>> (or HeapVector),
which are converted from V8 objects via

    NativeValueTraits<IDLRecord<K, V>>::nativeValue()

To convert the C++ Vectors back to V8 objects, we piggyback on already
existing ToV8() overloads for Vector<std::pair<>> and HeapVector.

The majority of the required work to support this new IDL type has already
landed and concerned making it possible to convert JS values to arbitrary
C++ types without knowing much about them (there is a single JS->C++
conversion function that needs to work for pretty much any JS data type).

This CL mainly makes the Python bindings code aware of this new type and
allows it to generate the appropriate JS->C++ and C++->V8 calls, and adds
the IDLRecord NativeValueTraits template specialization mentioned above. All
the rest is new tests and updates to existing expectations.

It is also important to note that the JS->C++ conversion code relies on a
NativeValueTraits specialization existing for the record value types, and
they are always used regardless of whether they are the fastest way to
convert data or not -- for example, it always creates and uses an
ExceptionState even if the actual conversion functions do not use it (as is
the case for toDOMWindow() or ScriptValue()).

Finally, while here rename IDLSequence::MaybeWrappedCppType to IDLSequence::MaybeMemberCppType to avoid confusion (and do the same for IDLRecord), as "wrapper" in a bindings context tends to refer to the V8 wrapper for DOM objects, which is not the case here.

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org

Review-Url: https://codereview.chromium.org/2732093003
Cr-Commit-Position: refs/heads/master@{#456048}
Committed: https://chromium.googlesource.com/chromium/src/+/90659e8a6bbc7e40ea3145c19af3138f484beb52

[Raphael Kubo da Costa (rakuco), 2017-03-07 13:54:21]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 13:54:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-07 14:08:05]

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

[commit-bot: I haz the power, 2017-03-07 14:08:06]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Raphael Kubo da Costa (rakuco), 2017-03-07 14:33:11]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 14:34:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-07 15:57:17]

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

[commit-bot: I haz the power, 2017-03-07 15:57:18]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Raphael Kubo da Costa (rakuco), 2017-03-07 17:08:21]

PTAL. This is the last part of the record code that needs to make it to the
bindings.

The only problem is that it's... making media_blink_unittests fail to link on
Windows debug builds?! It's not clear why, or why those new files are even part
of media_blink_unittests, so help here is much appreciated.

[Yuki, 2017-03-08 13:37:52]

I think you need to use one of
  CORE_TEMPLATE_CLASS_EXPORT
  CORE_EXTERN_TEMPLATE_EXPORT
  CORE_TEMPALTE_EXPORT
for templates.  Of course, you need to choose the appropriate one.  See
core/CoreExport.h and their use cases.

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:314: private:
Sort the sections public: first and private: last unless you cannot do it
syntactically.

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:315: static
inline bool isPropertyEnumerable(v8::Isolate* isolate,
It's |inline| by default if member functions are defined inside a class/struct
definition.  |inline| here is just redundant.

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:318: if
(descriptor->IsUndefined())
We don't need this because we're checking IsObject().  If IsObject(), it's not
Undefined.

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:340:
v8::Local<v8::Value> originalValue,
Can we name this |value| simply?

Since this is a public API's signature, I don't want to call this
|originalValue| where "original" doesn't have a clear meaning for a call site.

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_types.py:550: def
native_value_traits_type_name(idl_type):
I'm on the fence about this name.

Off the top of my head, this is |idl_type_name|.  I understand this is (mainly
or only?) used for NativeValueTraits.  Is there any possibility that this is
used for non-NativeValueTraits things?

(idl_type_name could be confusing if it's already used elsewhere.  I'm not
insisting to this name.)

[Raphael Kubo da Costa (rakuco), 2017-03-08 18:13:19]

Thanks for the review! I've addressed the items in my local checkout, and will
upload a new version after fixing the Windows component build issue (I'll upload
a separate CL for that).

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:314: private:
On 2017/03/08 13:37:52, Yuki(slow_til_mar08) wrote:
> Sort the sections public: first and private: last unless you cannot do it
> syntactically.

Done.

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:315: static
inline bool isPropertyEnumerable(v8::Isolate* isolate,
On 2017/03/08 13:37:52, Yuki(slow_til_mar08) wrote:
> It's |inline| by default if member functions are defined inside a class/struct
> definition.  |inline| here is just redundant.

Done.

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:318: if
(descriptor->IsUndefined())
On 2017/03/08 13:37:52, Yuki(slow_til_mar08) wrote:
> We don't need this because we're checking IsObject().  If IsObject(), it's not
> Undefined.

Done.

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:340:
v8::Local<v8::Value> originalValue,
On 2017/03/08 13:37:52, Yuki(slow_til_mar08) wrote:
> Can we name this |value| simply?
> 
> Since this is a public API's signature, I don't want to call this
> |originalValue| where "original" doesn't have a clear meaning for a call site.

We can, but I thought the preference was to leave names described in the specs
to the actual implementation. In other words,
https://heycam.github.io/webidl/#es-record uses |value| in steps 4.2.2 and
4.2.3, which is why I chose to use |originalValue| here. I'm fine with using
|value| here and something else in the code if that's preferred.

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_types.py:550: def
native_value_traits_type_name(idl_type):
On 2017/03/08 13:37:52, Yuki(slow_til_mar08) wrote:
> I'm on the fence about this name.
> 
> Off the top of my head, this is |idl_type_name|.  I understand this is (mainly
> or only?) used for NativeValueTraits.  Is there any possibility that this is
> used for non-NativeValueTraits things?
> 
> (idl_type_name could be confusing if it's already used elsewhere.  I'm not
> insisting to this name.)

At least for now, this function only makes sense in conjunction with
NativeValueTraits, hence the name -- its purpose is mostly to convert an
existing idl_type.name into its IDL<Foo> version.

[Raphael Kubo da Costa (rakuco), 2017-03-08 19:33:16]

Patch v6 is up. The only thing that I didn't change was |originalValue| ->
|value|, as I'm waiting for feedback to see if this is indeed desirable.

This is supposed to work on top of https://codereview.chromium.org/2742453002/

[Yuki, 2017-03-09 08:32:55]

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:340:
v8::Local<v8::Value> originalValue,
On 2017/03/08 18:13:19, Raphael Kubo da Costa (rakuco) wrote:
> On 2017/03/08 13:37:52, Yuki(slow_til_mar08) wrote:
> > Can we name this |value| simply?
> > 
> > Since this is a public API's signature, I don't want to call this
> > |originalValue| where "original" doesn't have a clear meaning for a call
site.
> 
> We can, but I thought the preference was to leave names described in the specs
> to the actual implementation. In other words,
> https://heycam.github.io/webidl/#es-record uses |value| in steps 4.2.2 and
> 4.2.3, which is why I chose to use |originalValue| here. I'm fine with using
> |value| here and something else in the code if that's preferred.

I didn't notice your intention, and later use of |value|.

Then, what about |v8Value|?

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:318: static
inline ImplType nativeValue(v8::Isolate* isolate,
nit: No need of |inline| here, too.

Optionally, you can remove all |inline| keywords in this file.

[Yuki, 2017-03-09 10:10:08]

LGTM with minor comments.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/bindings/record-type.html (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/bindings/record-type.html:8: function
assert_record_equals(obj, expected, description) {
nit: s/obj/actual/

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/bindings/record-type.html:109: "The same
Oilpan-based value was stored in the record");
nit: s/Oilpan-based value/DOM object/

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/bindings/record-type.html:117: }, "Test
Oilpan-based types");
nit: s/Oilpan-based/DOM Object/

[haraken, 2017-03-09 12:52:56]

LGTM

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:340:
->GetOwnPropertyNames(context,

Can this API fail? If it fails only on OOM scenarios, we can just use
ToLocalChecked().

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:374:
->GetOwnPropertyDescriptor(

Ditto. Can this API fail?

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:417: // we
are always appending.

Move the comment to above the line 416.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437: return
false;

You need to set an ExceptionState here, right? Or you can stop passing an
exception state to isPropertyEnumerable and handle the exception on the caller
side.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/v8_types.py:559: elif
idl_type.is_basic_type or idl_type.name in ('Date', 'Promise'):

I think Date is included in basic types.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/v8_types.py:923: 'Date':
'v8DateOrNaN({isolate}, {cpp_value})',

Not related to this CL, do you think it's a good idea to implement ToV8 for
primitive types so that we can always use ToV8 for C++ => JS conversions?

[Yuki, 2017-03-09 13:00:57]

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/v8_types.py:923: 'Date':
'v8DateOrNaN({isolate}, {cpp_value})',
On 2017/03/09 12:52:55, haraken wrote:
> 
> Not related to this CL, do you think it's a good idea to implement ToV8 for
> primitive types so that we can always use ToV8 for C++ => JS conversions?

We already have support of ToV8 for primitive types.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...

[Raphael Kubo da Costa (rakuco), 2017-03-09 16:08:34]

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/bindings/record-type.html (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/bindings/record-type.html:8: function
assert_record_equals(obj, expected, description) {
On 2017/03/09 10:10:08, Yuki wrote:
> nit: s/obj/actual/

Done.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/bindings/record-type.html:109: "The same
Oilpan-based value was stored in the record");
On 2017/03/09 10:10:08, Yuki wrote:
> nit: s/Oilpan-based value/DOM object/

Done.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/bindings/record-type.html:117: }, "Test
Oilpan-based types");
On 2017/03/09 10:10:08, Yuki wrote:
> nit: s/Oilpan-based/DOM Object/

Done.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:318: static
inline ImplType nativeValue(v8::Isolate* isolate,
On 2017/03/09 08:32:55, Yuki wrote:
> nit: No need of |inline| here, too.
> 
> Optionally, you can remove all |inline| keywords in this file.

OK. I'll upload a separate patch getting rid of inline everywhere.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:340:
->GetOwnPropertyNames(context,
On 2017/03/09 12:52:55, haraken wrote:
> 
> Can this API fail? If it fails only on OOM scenarios, we can just use
> ToLocalChecked().

To be honest, I don't know. The comments in v8.h aren't explicit about this, and
usage in V8 and Chromium varies; most tests use ToLocalChecked(), while "real"
code tends to go for ToLocal().

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:374:
->GetOwnPropertyDescriptor(
On 2017/03/09 12:52:55, haraken wrote:
> 
> Ditto. Can this API fail?

Same thing -- we seem to be one of the very first users of this API outside V8
and there isn't a lot of information on how it can fail, so I opted for what
seemed like the safest approach.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:417: // we
are always appending.
On 2017/03/09 12:52:55, haraken wrote:
> 
> Move the comment to above the line 416.

Done.

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437: return
false;
On 2017/03/09 12:52:55, haraken wrote:
> 
> You need to set an ExceptionState here, right? Or you can stop passing an
> exception state to isPropertyEnumerable and handle the exception on the caller
> side.

isPropertyEnumerable() is only taking an ExceptionState because it calls
toBoolean(), which takes one.

Do you mean a failure in this call here should have a more fatal effect than
just causing the caller of this function to skip an item in the loop?

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/scripts/v8_types.py (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/v8_types.py:559: elif
idl_type.is_basic_type or idl_type.name in ('Date', 'Promise'):
On 2017/03/09 12:52:55, haraken wrote:
> 
> I think Date is included in basic types.

Indeed. I'll fix this.

[haraken, 2017-03-09 16:11:20]

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437: return
false;
On 2017/03/09 16:08:34, Raphael Kubo da Costa (rakuco) wrote:
> On 2017/03/09 12:52:55, haraken wrote:
> > 
> > You need to set an ExceptionState here, right? Or you can stop passing an
> > exception state to isPropertyEnumerable and handle the exception on the
caller
> > side.
> 
> isPropertyEnumerable() is only taking an ExceptionState because it calls
> toBoolean(), which takes one.
> 
> Do you mean a failure in this call here should have a more fatal effect than
> just causing the caller of this function to skip an item in the loop?

I guess the Get may throw an exception if the getter is overridden by user
scripts etc. In that case, you'll need to set the exception to the exception
state.

[Raphael Kubo da Costa (rakuco), 2017-03-09 16:12:59]

Description was changed from

==========
bindings: Add support for the record<K,V> WebIDL type.

Records (https://heycam.github.io/webidl/#idl-record) are ordered
associative arrays mapping instances of K to instances of V, where K must be
a ByteString, a DOMString or an USVString.

In C++, we represent records as Vector<std::pair<k,v>> (or HeapVector),
which are converted from V8 objects via

    NativeValueTraits<IDLRecord<K, V>>::nativeValue()

To convert the C++ Vectors back to V8 objects, we piggyback on already
existing ToV8() overloads for Vector<std::pair<>> and HeapVector.

The majority of the required work to support this new IDL type has already
landed and concerned making it possible to convert JS values to arbitrary
C++ types without knowing much about them (there is a single JS->C++
conversion function that needs to work for pretty much any JS data type).

This CL mainly makes the Python bindings code aware of this new type and
allows it to generate the appropriate JS->C++ and C++->V8 calls, and adds
the IDLRecord NativeValueTraits template specialization mentioned above. All
the rest is new tests and updates to existing expectations.

It is also important to note that the JS->C++ conversion code relies on a
NativeValueTraits specialization existing for the record value types, and
they are always used regardless of whether they are the fastest way to
convert data or not -- for example, it always creates and uses an
ExceptionState even if the actual conversion functions do not use it (as is
the case for toDOMWindow() or ScriptValue()).

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org
==========

to

==========
bindings: Add support for the record<K,V> WebIDL type.

Records (https://heycam.github.io/webidl/#idl-record) are ordered
associative arrays mapping instances of K to instances of V, where K must be
a ByteString, a DOMString or an USVString.

In C++, we represent records as Vector<std::pair<k,v>> (or HeapVector),
which are converted from V8 objects via

    NativeValueTraits<IDLRecord<K, V>>::nativeValue()

To convert the C++ Vectors back to V8 objects, we piggyback on already
existing ToV8() overloads for Vector<std::pair<>> and HeapVector.

The majority of the required work to support this new IDL type has already
landed and concerned making it possible to convert JS values to arbitrary
C++ types without knowing much about them (there is a single JS->C++
conversion function that needs to work for pretty much any JS data type).

This CL mainly makes the Python bindings code aware of this new type and
allows it to generate the appropriate JS->C++ and C++->V8 calls, and adds
the IDLRecord NativeValueTraits template specialization mentioned above. All
the rest is new tests and updates to existing expectations.

It is also important to note that the JS->C++ conversion code relies on a
NativeValueTraits specialization existing for the record value types, and
they are always used regardless of whether they are the fastest way to
convert data or not -- for example, it always creates and uses an
ExceptionState even if the actual conversion functions do not use it (as is
the case for toDOMWindow() or ScriptValue()).

Finally, while here rename IDLSequence::MaybeWrappedCppType to
IDLSequence::MaybeMemberCppType to avoid confusion (and do the same for
IDLRecord), as "wrapper" in a bindings context tends to refer to the V8 wrapper
for DOM objects, which is not the case here.

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org
==========

[Raphael Kubo da Costa (rakuco), 2017-03-09 16:16:31]

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437: return
false;
On 2017/03/09 16:11:20, haraken wrote:
> On 2017/03/09 16:08:34, Raphael Kubo da Costa (rakuco) wrote:
> > On 2017/03/09 12:52:55, haraken wrote:
> > > 
> > > You need to set an ExceptionState here, right? Or you can stop passing an
> > > exception state to isPropertyEnumerable and handle the exception on the
> caller
> > > side.
> > 
> > isPropertyEnumerable() is only taking an ExceptionState because it calls
> > toBoolean(), which takes one.
> > 
> > Do you mean a failure in this call here should have a more fatal effect than
> > just causing the caller of this function to skip an item in the loop?
> 
> I guess the Get may throw an exception if the getter is overridden by user
> scripts etc. In that case, you'll need to set the exception to the exception
> state.
> 

Something like this, then?

  v8::TryCatch block(isolate);
  if (!descriptor->IsObject())
    return false;
  if (!...) {
    exceptionState.rethrowV8Exception(block.Exception());
    return false;
  }
  return toBoolean(...);

[haraken, 2017-03-09 16:18:11]

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437: return
false;
On 2017/03/09 16:16:30, Raphael Kubo da Costa (rakuco) wrote:
> On 2017/03/09 16:11:20, haraken wrote:
> > On 2017/03/09 16:08:34, Raphael Kubo da Costa (rakuco) wrote:
> > > On 2017/03/09 12:52:55, haraken wrote:
> > > > 
> > > > You need to set an ExceptionState here, right? Or you can stop passing
an
> > > > exception state to isPropertyEnumerable and handle the exception on the
> > caller
> > > > side.
> > > 
> > > isPropertyEnumerable() is only taking an ExceptionState because it calls
> > > toBoolean(), which takes one.
> > > 
> > > Do you mean a failure in this call here should have a more fatal effect
than
> > > just causing the caller of this function to skip an item in the loop?
> > 
> > I guess the Get may throw an exception if the getter is overridden by user
> > scripts etc. In that case, you'll need to set the exception to the exception
> > state.
> > 
> 
> Something like this, then?
> 
>   v8::TryCatch block(isolate);
>   if (!descriptor->IsObject())
>     return false;
>   if (!...) {
>     exceptionState.rethrowV8Exception(block.Exception());
>     return false;
>   }
>   return toBoolean(...);

Or you can inline the method to the caller site. This method is used by only one
place. Then you can use the TryCatch block on the caller site.

[Raphael Kubo da Costa (rakuco), 2017-03-09 16:36:06]

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437: return
false;
On 2017/03/09 16:18:11, haraken wrote:
> On 2017/03/09 16:16:30, Raphael Kubo da Costa (rakuco) wrote:
> > On 2017/03/09 16:11:20, haraken wrote:
> > > On 2017/03/09 16:08:34, Raphael Kubo da Costa (rakuco) wrote:
> > > > On 2017/03/09 12:52:55, haraken wrote:
> > > > > 
> > > > > You need to set an ExceptionState here, right? Or you can stop passing
> an
> > > > > exception state to isPropertyEnumerable and handle the exception on
the
> > > caller
> > > > > side.
> > > > 
> > > > isPropertyEnumerable() is only taking an ExceptionState because it calls
> > > > toBoolean(), which takes one.
> > > > 
> > > > Do you mean a failure in this call here should have a more fatal effect
> than
> > > > just causing the caller of this function to skip an item in the loop?
> > > 
> > > I guess the Get may throw an exception if the getter is overridden by user
> > > scripts etc. In that case, you'll need to set the exception to the
exception
> > > state.
> > > 
> > 
> > Something like this, then?
> > 
> >   v8::TryCatch block(isolate);
> >   if (!descriptor->IsObject())
> >     return false;
> >   if (!...) {
> >     exceptionState.rethrowV8Exception(block.Exception());
> >     return false;
> >   }
> >   return toBoolean(...);
> 
> Or you can inline the method to the caller site. This method is used by only
one
> place. Then you can use the TryCatch block on the caller site.

Hmm, I think I'll go with the TryCatch block here, otherwise the code in the
caller will have to look like this:

  if (!desc->IsObject())
    continue;
  v8::Local<v8::Value> enumerable;
  if (!...) {
    exceptionState.rethrowV8Exception(block.Exception());
    return ImplType();
  }
  if (!toBoolean(...)) {
    if (exceptionState.hadException())
      return ImplType();
    continue;
  }

while still legible, I think it looks more confusing to readers who're following
the code to see how the conversion is done.

[haraken, 2017-03-09 17:49:36]

On 2017/03/09 16:36:06, Raphael Kubo da Costa (rakuco) wrote:
>
https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h
(right):
> 
>
https://codereview.chromium.org/2732093003/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437: return
> false;
> On 2017/03/09 16:18:11, haraken wrote:
> > On 2017/03/09 16:16:30, Raphael Kubo da Costa (rakuco) wrote:
> > > On 2017/03/09 16:11:20, haraken wrote:
> > > > On 2017/03/09 16:08:34, Raphael Kubo da Costa (rakuco) wrote:
> > > > > On 2017/03/09 12:52:55, haraken wrote:
> > > > > > 
> > > > > > You need to set an ExceptionState here, right? Or you can stop
passing
> > an
> > > > > > exception state to isPropertyEnumerable and handle the exception on
> the
> > > > caller
> > > > > > side.
> > > > > 
> > > > > isPropertyEnumerable() is only taking an ExceptionState because it
calls
> > > > > toBoolean(), which takes one.
> > > > > 
> > > > > Do you mean a failure in this call here should have a more fatal
effect
> > than
> > > > > just causing the caller of this function to skip an item in the loop?
> > > > 
> > > > I guess the Get may throw an exception if the getter is overridden by
user
> > > > scripts etc. In that case, you'll need to set the exception to the
> exception
> > > > state.
> > > > 
> > > 
> > > Something like this, then?
> > > 
> > >   v8::TryCatch block(isolate);
> > >   if (!descriptor->IsObject())
> > >     return false;
> > >   if (!...) {
> > >     exceptionState.rethrowV8Exception(block.Exception());
> > >     return false;
> > >   }
> > >   return toBoolean(...);
> > 
> > Or you can inline the method to the caller site. This method is used by only
> one
> > place. Then you can use the TryCatch block on the caller site.
> 
> Hmm, I think I'll go with the TryCatch block here, otherwise the code in the
> caller will have to look like this:
> 
>   if (!desc->IsObject())
>     continue;
>   v8::Local<v8::Value> enumerable;
>   if (!...) {
>     exceptionState.rethrowV8Exception(block.Exception());
>     return ImplType();
>   }
>   if (!toBoolean(...)) {
>     if (exceptionState.hadException())
>       return ImplType();
>     continue;
>   }
> 
> while still legible, I think it looks more confusing to readers who're
following
> the code to see how the conversion is done.

I don't think that's too bad, but I'm okay with your approach.

BTW, looking up "enumerable" is a right way to judge if the property is
enumerable? (Maybe should we use GetOwnProperty rather than Get?) Or should we
expose some V8 API to check the enumerability?

[Raphael Kubo da Costa (rakuco), 2017-03-09 21:55:53]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 21:56:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Raphael Kubo da Costa (rakuco), 2017-03-09 22:54:57]

On 2017/03/09 17:49:36, haraken wrote:
> BTW, looking up "enumerable" is a right way to judge if the property is
> enumerable? (Maybe should we use GetOwnProperty rather than Get?) Or should we
> expose some V8 API to check the enumerability?

There's no GetOwnProperty in V8's API, only GetOwnProperties() and we'd have to
loop through all of them every time. This is the approach is similar to what's
specified in ECMA-262 here:
https://tc39.github.io/ecma262/#sec-topropertydescriptor (step 4).

With that said, I agree it'd be a good idea to provide some helpers in V8 (I
think JavaScriptCore has something like this).

[Raphael Kubo da Costa (rakuco), 2017-03-09 22:57:38]

Patch 8 is up: I've made isPropertyEnumerable() rethrow an exception in all
cases, and added some new tests to NativeValueTraitsImplTests for non-enumerable
properties and for exceptions being thrown in
GetOwnPropertyDescriptor+isPropertyEnumerable. In the latter, I could only get
as far as getting an exception in GetOwnPropertyDescriptor, as throwing
something when accessing the "enumerable" property causes V8's implementation of
ToPropertyDescriptor to throw early.

[bashi, 2017-03-10 00:23:17]

lgtm on my side (haraken@ and yukishiino@ may want to have another look)

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/tests/idls/core/TestDictionary.idl
(right):

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/tests/idls/core/TestDictionary.idl:41:
record<USVString, TestObject> oilpanRecordMember;
nit: We usually use 'garbageCollected' for oilpan objects.

[commit-bot: I haz the power, 2017-03-10 01:40:42]

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

[commit-bot: I haz the power, 2017-03-10 01:40:43]

Dry run: This issue passed the CQ dry run.

[Yuki, 2017-03-10 06:54:32]

LGTM.

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437:
.ToLocal(&enumerable)) {
You can safely use ToLocalChecked() here because |descriptor| must be a property
descriptor returned by V8, i.e. |descriptor| never be an author-script-defined
object.

Thus, there must be a property named "enumerable" and whose value must be a
boolean.

Get() could fail if OOM, etc., but in that case, it's okay to crash with
ToLocalChecked.

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:441: return
toBoolean(isolate, enumerable, exceptionState);
This is okay, but you can write
    return enumerable.BooleanValue(context).ToChecked();
given that it's guaranteed to be v8::Boolean.
And in this way, you can get rid of ExceptionState.

[Raphael Kubo da Costa (rakuco), 2017-03-10 08:19:09]

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437:
.ToLocal(&enumerable)) {
On 2017/03/10 06:54:32, Yuki wrote:
> You can safely use ToLocalChecked() here because |descriptor| must be a
property
> descriptor returned by V8, i.e. |descriptor| never be an author-script-defined
> object.
> 
> Thus, there must be a property named "enumerable" and whose value must be a
> boolean.
> 
> Get() could fail if OOM, etc., but in that case, it's okay to crash with
> ToLocalChecked.

Are you sure about this? One of the tests I added to NativeValueTraitsImplTests
uses a Proxy to intercept calls to Object.getOwnPropertyDescriptor and return an
object that throws an exception when its "enumerable" property is accessed (the
same could be done in a layout test).

https://tc39.github.io/ecma262/#sec-topropertydescriptor also does not seem to
make any guarantees that "enumerable" is always present, only that it is
converted to a boolean.

While my test causes the GetOwnPropertyDescriptor() call to fail instead of this
one, it could also be modified to crash here, so using ToLocalChecked() here
would likely crash Chromium as well as my test when authors are doing something
nasty.

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:441: return
toBoolean(isolate, enumerable, exceptionState);
On 2017/03/10 06:54:32, Yuki wrote:
> This is okay, but you can write
>     return enumerable.BooleanValue(context).ToChecked();
> given that it's guaranteed to be v8::Boolean.
> And in this way, you can get rid of ExceptionState.

Same here: while V8 itself performs some boolean conversions in
GetOwnPropertyDescriptor(), wouldn't it still be possible for this to fail given
users can be in control of the object returned by GetOwnPropertyDescriptor()?

[Raphael Kubo da Costa (rakuco), 2017-03-10 08:22:18]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 08:22:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[haraken, 2017-03-10 09:22:38]

LGTM

[Yuki, 2017-03-10 09:52:22]

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:437:
.ToLocal(&enumerable)) {
On 2017/03/10 08:19:09, Raphael Kubo da Costa (rakuco) wrote:
> On 2017/03/10 06:54:32, Yuki wrote:
> > You can safely use ToLocalChecked() here because |descriptor| must be a
> property
> > descriptor returned by V8, i.e. |descriptor| never be an
author-script-defined
> > object.
> > 
> > Thus, there must be a property named "enumerable" and whose value must be a
> > boolean.
> > 
> > Get() could fail if OOM, etc., but in that case, it's okay to crash with
> > ToLocalChecked.
> 
> Are you sure about this? One of the tests I added to
NativeValueTraitsImplTests
> uses a Proxy to intercept calls to Object.getOwnPropertyDescriptor and return
an
> object that throws an exception when its "enumerable" property is accessed
(the
> same could be done in a layout test).
> 
> https://tc39.github.io/ecma262/#sec-topropertydescriptor also does not seem to
> make any guarantees that "enumerable" is always present, only that it is
> converted to a boolean.
> 
> While my test causes the GetOwnPropertyDescriptor() call to fail instead of
this
> one, it could also be modified to crash here, so using ToLocalChecked() here
> would likely crash Chromium as well as my test when authors are doing
something
> nasty.

I didn't test anything for myself, so I can be wrong.  Please correct me
anything, and help me understand things better.

Proxy.getOwnPropertyDescriptor can throw an exception, but it should happen
during we call v8::Object::GetOwnPropertyDescriptor, right?  It's not happening
when we call v8::Object::Get(descriptor, "enumerable").  IIUC, the property
descriptor itself is created by V8 and filled with the result of
Proxy.getOwnPropertyDescriptor.  At least, the ES2017 seems saying so.

https://tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-interna...
where resultDesc is NOT created by author script.

Plus, CompletePropertyDescriptor(resultDesc) fills "enumerable" if it's not yet
set.  So, it must exist.

I didn't investigate V8's implementation, so something could be different from
the spec (though, I think that V8 follows the spec well in general).

Anyway, this is my thought.

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:441: return
toBoolean(isolate, enumerable, exceptionState);
On 2017/03/10 08:19:09, Raphael Kubo da Costa (rakuco) wrote:
> On 2017/03/10 06:54:32, Yuki wrote:
> > This is okay, but you can write
> >     return enumerable.BooleanValue(context).ToChecked();
> > given that it's guaranteed to be v8::Boolean.
> > And in this way, you can get rid of ExceptionState.
> 
> Same here: while V8 itself performs some boolean conversions in
> GetOwnPropertyDescriptor(), wouldn't it still be possible for this to fail
given
> users can be in control of the object returned by GetOwnPropertyDescriptor()?

IIUC, there is no way for author script to install any accessor property into a
property descriptor.

resultDesc is first created as an empty property descriptor, and gets filled
with trapResultObj.  trapResultObj may have accessor properties, but resultDesc
may not, I think.

[commit-bot: I haz the power, 2017-03-10 10:12:29]

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

[commit-bot: I haz the power, 2017-03-10 10:12:30]

Dry run: This issue passed the CQ dry run.

[Raphael Kubo da Costa (rakuco), 2017-03-10 10:42:37]

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:441: return
toBoolean(isolate, enumerable, exceptionState);
On 2017/03/10 09:52:21, Yuki wrote:
> On 2017/03/10 08:19:09, Raphael Kubo da Costa (rakuco) wrote:
> > On 2017/03/10 06:54:32, Yuki wrote:
> > > This is okay, but you can write
> > >     return enumerable.BooleanValue(context).ToChecked();
> > > given that it's guaranteed to be v8::Boolean.
> > > And in this way, you can get rid of ExceptionState.
> > 
> > Same here: while V8 itself performs some boolean conversions in
> > GetOwnPropertyDescriptor(), wouldn't it still be possible for this to fail
> given
> > users can be in control of the object returned by
GetOwnPropertyDescriptor()?
> 
> IIUC, there is no way for author script to install any accessor property into
a
> property descriptor.
> 
> resultDesc is first created as an empty property descriptor, and gets filled
> with trapResultObj.  trapResultObj may have accessor properties, but
resultDesc
> may not, I think.

You're right about those 2 points: while the proxy handler's
getOwnPropertyDescriptor might return a bogus object, the object returned by
Proxy.getOwnPropertyDescriptor is sanitized and not created by a user. I'll
upload a new patch with fewer checks.

[Raphael Kubo da Costa (rakuco), 2017-03-10 10:52:04]

Patch v10 is up: isPropertyEnumerable() is gone since there's no need to handle
ExceptionState in that part of the code. I added a DCHECK for desc->IsObject()
as well as some explanation as to why we can call To{Local}Checked().

[Raphael Kubo da Costa (rakuco), 2017-03-10 10:52:13]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 10:52:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yuki, 2017-03-10 11:06:07]

LGTM.

https://codereview.chromium.org/2732093003/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2732093003/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:389:
->Get(isolate->GetCurrentContext(),
nit: s/isolate->GetCurrentContext()/context/

Maybe, desc.As<v8::Object>() is shorter?  I don't care this point and I'm fine
with either of Cast or As, though.

[Raphael Kubo da Costa (rakuco), 2017-03-10 11:11:22]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 11:11:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 13:02:51]

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

[commit-bot: I haz the power, 2017-03-10 13:02:53]

Dry run: This issue passed the CQ dry run.

[Raphael Kubo da Costa (rakuco), 2017-03-10 13:04:52]

The CQ bit was checked by raphael.kubo.da.costa@intel.com

[Raphael Kubo da Costa (rakuco), 2017-03-10 13:04:52]

The patchset sent to the CQ was uploaded after l-g-t-m from bashi@chromium.org,
haraken@chromium.org, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2732093003/#ps200001
(title: "s/isolate->GetCurrentContext()/context/")

[commit-bot: I haz the power, 2017-03-10 13:05:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 13:09:33]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1489151092531810,
"parent_rev": "af8b67666d6c89aec9abebe7df21a2732d91ea2a", "commit_rev":
"90659e8a6bbc7e40ea3145c19af3138f484beb52"}

[commit-bot: I haz the power, 2017-03-10 13:10:14]

Description was changed from

==========
bindings: Add support for the record<K,V> WebIDL type.

Records (https://heycam.github.io/webidl/#idl-record) are ordered
associative arrays mapping instances of K to instances of V, where K must be
a ByteString, a DOMString or an USVString.

In C++, we represent records as Vector<std::pair<k,v>> (or HeapVector),
which are converted from V8 objects via

    NativeValueTraits<IDLRecord<K, V>>::nativeValue()

To convert the C++ Vectors back to V8 objects, we piggyback on already
existing ToV8() overloads for Vector<std::pair<>> and HeapVector.

The majority of the required work to support this new IDL type has already
landed and concerned making it possible to convert JS values to arbitrary
C++ types without knowing much about them (there is a single JS->C++
conversion function that needs to work for pretty much any JS data type).

This CL mainly makes the Python bindings code aware of this new type and
allows it to generate the appropriate JS->C++ and C++->V8 calls, and adds
the IDLRecord NativeValueTraits template specialization mentioned above. All
the rest is new tests and updates to existing expectations.

It is also important to note that the JS->C++ conversion code relies on a
NativeValueTraits specialization existing for the record value types, and
they are always used regardless of whether they are the fastest way to
convert data or not -- for example, it always creates and uses an
ExceptionState even if the actual conversion functions do not use it (as is
the case for toDOMWindow() or ScriptValue()).

Finally, while here rename IDLSequence::MaybeWrappedCppType to
IDLSequence::MaybeMemberCppType to avoid confusion (and do the same for
IDLRecord), as "wrapper" in a bindings context tends to refer to the V8 wrapper
for DOM objects, which is not the case here.

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org
==========

to

==========
bindings: Add support for the record<K,V> WebIDL type.

Records (https://heycam.github.io/webidl/#idl-record) are ordered
associative arrays mapping instances of K to instances of V, where K must be
a ByteString, a DOMString or an USVString.

In C++, we represent records as Vector<std::pair<k,v>> (or HeapVector),
which are converted from V8 objects via

    NativeValueTraits<IDLRecord<K, V>>::nativeValue()

To convert the C++ Vectors back to V8 objects, we piggyback on already
existing ToV8() overloads for Vector<std::pair<>> and HeapVector.

The majority of the required work to support this new IDL type has already
landed and concerned making it possible to convert JS values to arbitrary
C++ types without knowing much about them (there is a single JS->C++
conversion function that needs to work for pretty much any JS data type).

This CL mainly makes the Python bindings code aware of this new type and
allows it to generate the appropriate JS->C++ and C++->V8 calls, and adds
the IDLRecord NativeValueTraits template specialization mentioned above. All
the rest is new tests and updates to existing expectations.

It is also important to note that the JS->C++ conversion code relies on a
NativeValueTraits specialization existing for the record value types, and
they are always used regardless of whether they are the fastest way to
convert data or not -- for example, it always creates and uses an
ExceptionState even if the actual conversion functions do not use it (as is
the case for toDOMWindow() or ScriptValue()).

Finally, while here rename IDLSequence::MaybeWrappedCppType to
IDLSequence::MaybeMemberCppType to avoid confusion (and do the same for
IDLRecord), as "wrapper" in a bindings context tends to refer to the V8 wrapper
for DOM objects, which is not the case here.

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org

Review-Url: https://codereview.chromium.org/2732093003
Cr-Commit-Position: refs/heads/master@{#456048}
Committed:
https://chromium.googlesource.com/chromium/src/+/90659e8a6bbc7e40ea3145c19af3...
==========

[commit-bot: I haz the power, 2017-03-10 13:10:15]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/90659e8a6bbc7e40ea3145c19af3...