IDL: DOM impl class code generation for IDL dictionaries

IDL: DOM impl class code generation for IDL dictionaries

After this CL, CodeGeneratorV8 generates not only V8 bindings, but also
DOM implementation classes for IDL dictionaries.

- For an IDL file which defines a dictionary, the code generator
  generates bindings code (V8Foo.{cpp,h}) and DOM impl code
  (Foo.{cpp,h}).

- Added Jinja templates for DOM impl generation. The context for these
  templates is created by v8_dictionary.dictionary_impl_context().

- v8_types.cpp_types() takes a new argument called
  |used_as_return_type|. This flags is used by member_impl_context().
  Also, |used_in_cpp_sequence| argument is renamed to |used_as_member|.

IDL dictionaries are still not be able to use in core/modules. We need
to generate impl classes in the right place.
(e.g. core/dom/Foo.idl -> gen/blink/core/dom/Foo.{cpp,h})

Also, following types are not supported yet as dictionary members:
- union types.
- enumeration types.
- composite types (arrays, sequences and dictionaries) of which element
  type is an interface.

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

[bashi, 2014-07-25 10:05:41]

Haraken-san, PTAL?

https://codereview.chromium.org/420763002/diff/1/Source/bindings/scripts/v8_d...
File Source/bindings/scripts/v8_dictionary.py (right):

https://codereview.chromium.org/420763002/diff/1/Source/bindings/scripts/v8_d...
Source/bindings/scripts/v8_dictionary.py:110: def collect_includes(idl_type):
Takes idl_type as argument as this function is recursively called.

https://codereview.chromium.org/420763002/diff/1/Source/bindings/templates/di...
File Source/bindings/templates/dictionary_impl.cpp (right):

https://codereview.chromium.org/420763002/diff/1/Source/bindings/templates/di...
Source/bindings/templates/dictionary_impl.cpp:32:
visitor->trace(m_{{member.name}});
If I understand correctly, Visitor::trace(Nullable<T>) will work as expected
(via TraceIfNeeded<T>::trace(visitor, &m_value)).

https://codereview.chromium.org/420763002/diff/1/Source/bindings/tests/idls/T...
File Source/bindings/tests/idls/TestDictionary.idl (right):

https://codereview.chromium.org/420763002/diff/1/Source/bindings/tests/idls/T...
Source/bindings/tests/idls/TestDictionary.idl:15: sequence<DOMString>
stringSequenceMember;
DictionaryHelper has a specialized get() for Vector<String> so this should work.

[jsbell, 2014-07-25 23:11:41]

What's the status of enumerations as member types in dictionaries? (I didn't see
any mention in the bug or an example in the patch)

* The steps in 4.2.20 and 4.2.21 indicate that passing an invalid enum value as
the value of a dictionary member should throw, due to the conversion.

* TestDictionary.idl should have cases for a nullable enum type ("If the Type is
an identifier followed by ?, then the identifier must identify an interface,
enumeration ...." and a non-nullable, defaulted enum type ("If the type of the
dictionary member is an enumeration, then its default value if specified must be
one of the enumeration’s values.")

[bashi, 2014-07-26 12:57:36]

On 2014/07/25 23:11:41, jsbell wrote:
> What's the status of enumerations as member types in dictionaries? (I didn't
see
> any mention in the bug or an example in the patch)
> 
> * The steps in 4.2.20 and 4.2.21 indicate that passing an invalid enum value
as
> the value of a dictionary member should throw, due to the conversion.
> 
> * TestDictionary.idl should have cases for a nullable enum type ("If the Type
is
> an identifier followed by ?, then the identifier must identify an interface,
> enumeration ...." and a non-nullable, defaulted enum type ("If the type of the
> dictionary member is an enumeration, then its default value if specified must
be
> one of the enumeration’s values.")

Thank you for review! Yes, I should make the CL work with enumerations. Also, I
should have explained what we are going to support at this point(e.g. union
types are not supported). I'll do it in the next week.

[haraken, 2014-07-27 07:46:36]

Mostly looks good.

Let's support enumerations in a follow-up CL.

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

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:82: 'has_name':
has_name_for_dictionary_member(member),

has_name => has_method_name
has_name_for_dictionary_member => has_method_name_for_dictionary_member

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:107: def
should_use_nullable_container():

Can we use is_implicit_nullable/is_explicit_nullable in v8_types.py?

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:110: def collect_includes(idl_type):

Can we move this method to v8_types.py?

Similarly to v8_types.includes_for_type, let's name this method
v8_types.impl_includes_for_type. You can also define IMPL_INCLUDES_FOR_TYPE if
necessary (just like INCLUDES_FOR_TYPE).

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:148: return '!m_%s.isNull()' %
member.name

I think we should share this code with property_getter in v8_interface.py.

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:155: return
v8_types.cpp_template_type('Nullable', member_cpp_type)

Ditto. We can share this code with v8_method.py.

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Other than this, we have several code in bindings/scripts/ to do special
conversion for nullable types. Probably we want to introduce good helper methods
to v8_types.py and get rid of the duplicated code for the nullable types.
Specifically, we want to get rid of getter_cpp_type, getter_method_expression,
has_method_expression and method_cpp_type from this file.

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

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:124: def cpp_type(idl_type,
extended_attributes=None, raw_type=False, used_as_argument=False,
used_as_variadic_argument=False, used_as_member=False,
used_as_return_type=False):

What's a difference between used_as_argument and used_as_return_type?

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:138: used_as_member:

I'd prefer used_in_cpp_sequence, since "member" is confusing with Member of
oilpan.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/template...
File Source/bindings/templates/dictionary_impl.h (right):

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/template...
Source/bindings/templates/dictionary_impl.h:24: bool {{member.has_name}}() const
{ return {{member.has_method_expression}}; }

has_name => has_method_name (for consistency with has_method_expression)

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/template...
Source/bindings/templates/dictionary_impl.h:25: {{member.getter_cpp_type}}
{{member.name}}() const { return {{member.getter_method_expression}}; }

getter_method_expression => getter_expression (for consistency with
getter_cpp_type)

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/id...
File Source/bindings/tests/idls/TestDictionary.idl (right):

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/id...
Source/bindings/tests/idls/TestDictionary.idl:13: TestInterface?
testInterfaceOrNullMember;

Would you add test cases for:

TestInterfaceGarbageCollected testInterfaceGarbageCollectedMember;
TestInterfaceGarbageCollected? testInterfaceGarbageCollectedOrNullMember;
TestInterfaceWillBeGarbageCollected testInterfaceWillBeGarbageCollectedMember;
TestInterfaceWillBeGarbageCollected?
testInterfaceWillBeGarbageCollectedOrNullMember;

These members needs to be traced in the trace() method.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/id...
Source/bindings/tests/idls/TestDictionary.idl:15: sequence<DOMString>
stringSequenceMember;

Just help me understand: What's the difference between DOMString[] and
sequence<DOMString>?

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/re...
File Source/bindings/tests/results/TestDictionary.cpp (right):

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/re...
Source/bindings/tests/results/TestDictionary.cpp:19: m_booleanMember = value;

If the setter is just setting a value, you can inline the setter.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/re...
File Source/bindings/tests/results/TestDictionary.h (right):

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/re...
Source/bindings/tests/results/TestDictionary.h:18: class TestDictionary : public
GarbageCollectedFinalized<TestDictionary> {

Add FINAL.

[bashi, 2014-07-29 03:52:52]

I see. I'll add enumeration support in follow-up CLs.

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

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:82: 'has_name':
has_name_for_dictionary_member(member),
On 2014/07/27 07:46:35, haraken wrote:
> 
> has_name => has_method_name
> has_name_for_dictionary_member => has_method_name_for_dictionary_member

Done.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:107: def
should_use_nullable_container():
On 2014/07/27 07:46:35, haraken wrote:
> 
> Can we use is_implicit_nullable/is_explicit_nullable in v8_types.py?

It seems that we can't use is_implicit_nullable/is_explicit_nullable unless we
change the definitions of those method. They check |idl_type.is_nullable| first,
but we don't want to check it here. For example, for both DOMString and
DOMString? types, we want to use String. Same for interface types.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:110: def collect_includes(idl_type):
On 2014/07/27 07:46:35, haraken wrote:
> 
> Can we move this method to v8_types.py?
> 
> Similarly to v8_types.includes_for_type, let's name this method
> v8_types.impl_includes_for_type. You can also define IMPL_INCLUDES_FOR_TYPE if
> necessary (just like INCLUDES_FOR_TYPE).

Done.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:148: return '!m_%s.isNull()' %
member.name
On 2014/07/27 07:46:35, haraken wrote:
> 
> I think we should share this code with property_getter in v8_interface.py.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Hmm, the semantics of Nullable usage are different between here (dictionary
impl) and others. For dictionaries, we use Nullable to implement the 'present'
notion. Others use Nullable to represent IDL's nullable. This means that we will
have different rules for code generation.

For example, suppose we are going to share the implementation of
|is_null_expression|. In dictionary case, 'int' type should be
"m_intMember.isNull()". In other cases, 'int' type should be ''. To make this
workable, we need an extra flag for dictionaries. I think this is just adding
complexity.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:155: return
v8_types.cpp_template_type('Nullable', member_cpp_type)
On 2014/07/27 07:46:35, haraken wrote:
> 
> Ditto. We can share this code with v8_method.py.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> Other than this, we have several code in bindings/scripts/ to do special
> conversion for nullable types. Probably we want to introduce good helper
methods
> to v8_types.py and get rid of the duplicated code for the nullable types.
> Specifically, we want to get rid of getter_cpp_type, getter_method_expression,
> has_method_expression and method_cpp_type from this file.

I put these functions in this file because these are dictionary specific(as
described above). We can put them into v8_types, but they're unlikely to be able
to share with other (IDL's) nullable logic.

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

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:124: def cpp_type(idl_type,
extended_attributes=None, raw_type=False, used_as_argument=False,
used_as_variadic_argument=False, used_as_member=False,
used_as_return_type=False):
On 2014/07/27 07:46:36, haraken wrote:
> 
> What's a difference between used_as_argument and used_as_return_type?

Removed and renamed used_as_argument to used_as_rvalue_type.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:138: used_as_member:
On 2014/07/27 07:46:36, haraken wrote:
> 
> I'd prefer used_in_cpp_sequence, since "member" is confusing with Member of
> oilpan.

Reverted the rename. But in fact, it seems that we use this flag to use Member
in terms of oilpan(Line 191 in this CL)...

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/template...
File Source/bindings/templates/dictionary_impl.h (right):

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/template...
Source/bindings/templates/dictionary_impl.h:24: bool {{member.has_name}}() const
{ return {{member.has_method_expression}}; }
On 2014/07/27 07:46:36, haraken wrote:
> 
> has_name => has_method_name (for consistency with has_method_expression)

Done.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/template...
Source/bindings/templates/dictionary_impl.h:25: {{member.getter_cpp_type}}
{{member.name}}() const { return {{member.getter_method_expression}}; }
On 2014/07/27 07:46:36, haraken wrote:
> 
> getter_method_expression => getter_expression (for consistency with
> getter_cpp_type)

Done.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/id...
File Source/bindings/tests/idls/TestDictionary.idl (right):

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/id...
Source/bindings/tests/idls/TestDictionary.idl:13: TestInterface?
testInterfaceOrNullMember;
On 2014/07/27 07:46:36, haraken wrote:
> 
> Would you add test cases for:
> 
> TestInterfaceGarbageCollected testInterfaceGarbageCollectedMember;
> TestInterfaceGarbageCollected? testInterfaceGarbageCollectedOrNullMember;
> TestInterfaceWillBeGarbageCollected testInterfaceWillBeGarbageCollectedMember;
> TestInterfaceWillBeGarbageCollected?
> testInterfaceWillBeGarbageCollectedOrNullMember;
> 
> These members needs to be traced in the trace() method.

Done.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/id...
Source/bindings/tests/idls/TestDictionary.idl:15: sequence<DOMString>
stringSequenceMember;
On 2014/07/27 07:46:36, haraken wrote:
> 
> Just help me understand: What's the difference between DOMString[] and
> sequence<DOMString>?

In terms of the spec, sequence<T> is passed by value, and T[] is passed by
reference. However, it seems that we don't much care about the difference in the
code generator. We may need to investigate if this is a problem.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/re...
File Source/bindings/tests/results/TestDictionary.cpp (right):

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/re...
Source/bindings/tests/results/TestDictionary.cpp:19: m_booleanMember = value;
On 2014/07/27 07:46:36, haraken wrote:
> 
> If the setter is just setting a value, you can inline the setter.

Done.

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/re...
File Source/bindings/tests/results/TestDictionary.h (right):

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/tests/re...
Source/bindings/tests/results/TestDictionary.h:18: class TestDictionary : public
GarbageCollectedFinalized<TestDictionary> {
On 2014/07/27 07:46:36, haraken wrote:
> 
> Add FINAL.

Done.

[haraken, 2014-07-29 11:09:19]

LGTM

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

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:148: return '!m_%s.isNull()' %
member.name
On 2014/07/29 03:52:51, bashi1 wrote:
> On 2014/07/27 07:46:35, haraken wrote:
> > 
> > I think we should share this code with property_getter in v8_interface.py.
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> Hmm, the semantics of Nullable usage are different between here (dictionary
> impl) and others. For dictionaries, we use Nullable to implement the 'present'
> notion. Others use Nullable to represent IDL's nullable. This means that we
will
> have different rules for code generation.
> 
> For example, suppose we are going to share the implementation of
> |is_null_expression|. In dictionary case, 'int' type should be
> "m_intMember.isNull()". In other cases, 'int' type should be ''. To make this
> workable, we need an extra flag for dictionaries. I think this is just adding
> complexity.

Thanks for the clarification, makes sense!

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

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:138: used_as_member:
On 2014/07/29 03:52:51, bashi1 wrote:
> On 2014/07/27 07:46:36, haraken wrote:
> > 
> > I'd prefer used_in_cpp_sequence, since "member" is confusing with Member of
> > oilpan.
> 
> Reverted the rename. But in fact, it seems that we use this flag to use Member
> in terms of oilpan(Line 191 in this CL)...

It's true that oilpan's member is used in the container but it's also true that
oilpan's member is used in other places. (So used_in_cpp_sequence captures the
reality better.)

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

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:110: return 'const %s&' %
cpp_rvalue_type

I'm not quite sure why we need this special-casing. We might want to remove it
in a follow-up if possible.

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

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_methods.py:254: used_as_rvalue_type=True),
argument.name)

Nit: 4-space indent

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/tests/re...
File Source/bindings/tests/results/TestDictionary.h (right):

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/tests/re...
Source/bindings/tests/results/TestDictionary.h:73: Nullable<int> m_longMember;

Just help me understand: Where does the difference appear between a 'long'
member and a 'long?' member? (I understand we're using Nullable<> to represent
IDL's nullable or a missing value. Then it seems to me that there is no
difference in implementation between nullable types and not-nullable types.)

[bashi, 2014-07-29 11:51:21]

Thank you for review! I'll upload a new patch set and land it tomorrow.

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

https://codereview.chromium.org/420763002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_types.py:138: used_as_member:
On 2014/07/29 11:09:19, haraken wrote:
> On 2014/07/29 03:52:51, bashi1 wrote:
> > On 2014/07/27 07:46:36, haraken wrote:
> > > 
> > > I'd prefer used_in_cpp_sequence, since "member" is confusing with Member
of
> > > oilpan.
> > 
> > Reverted the rename. But in fact, it seems that we use this flag to use
Member
> > in terms of oilpan(Line 191 in this CL)...
> 
> It's true that oilpan's member is used in the container but it's also true
that
> oilpan's member is used in other places. (So used_in_cpp_sequence captures the
> reality better.)

I see. Thank you for clarification.

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

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:110: return 'const %s&' %
cpp_rvalue_type
On 2014/07/29 11:09:19, haraken wrote:
> 
> I'm not quite sure why we need this special-casing. We might want to remove it
> in a follow-up if possible.

Sorry, this should be in cpp_type(); This is a kind of leftover when I tried to
sequence<sequence<T>> workable. I got Vector<const Vector&<T> >. Since we don't
support sequence<sequence<T>>, this special casing should be gone. Will do
before landing.

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/tests/re...
File Source/bindings/tests/results/TestDictionary.h (right):

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/tests/re...
Source/bindings/tests/results/TestDictionary.h:73: Nullable<int> m_longMember;
On 2014/07/29 11:09:19, haraken wrote:
> 
> Just help me understand: Where does the difference appear between a 'long'
> member and a 'long?' member? (I understand we're using Nullable<> to represent
> IDL's nullable or a missing value. Then it seems to me that there is no
> difference in implementation between nullable types and not-nullable types.)

From the spec's viewpoint or the implementation's viewpoint?

Speaking of the spec, I can't figure out why nullable is needed, given that 'not
specified' and 'null' would mean the same thing in dictionaries, IIUC.

In the terms of our implementation, there should be no difference, as you
described. Nullable is supported because the spec says it is allowed (and easy
to implement).

[haraken, 2014-07-29 12:25:49]

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/tests/re...
File Source/bindings/tests/results/TestDictionary.h (right):

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/tests/re...
Source/bindings/tests/results/TestDictionary.h:73: Nullable<int> m_longMember;
On 2014/07/29 11:51:21, bashi1 wrote:
> On 2014/07/29 11:09:19, haraken wrote:
> > 
> > Just help me understand: Where does the difference appear between a 'long'
> > member and a 'long?' member? (I understand we're using Nullable<> to
represent
> > IDL's nullable or a missing value. Then it seems to me that there is no
> > difference in implementation between nullable types and not-nullable types.)
> 
> From the spec's viewpoint or the implementation's viewpoint?
> 
> Speaking of the spec, I can't figure out why nullable is needed, given that
'not
> specified' and 'null' would mean the same thing in dictionaries, IIUC.
> 
> In the terms of our implementation, there should be no difference, as you
> described. Nullable is supported because the spec says it is allowed (and easy
> to implement).

Makes sense, thanks for the clarification!

[bashi, 2014-07-29 23:59:49]

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

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:110: return 'const %s&' %
cpp_rvalue_type
On 2014/07/29 11:51:21, bashi1 wrote:
> On 2014/07/29 11:09:19, haraken wrote:
> > 
> > I'm not quite sure why we need this special-casing. We might want to remove
it
> > in a follow-up if possible.
> 
> Sorry, this should be in cpp_type(); This is a kind of leftover when I tried
to
> sequence<sequence<T>> workable. I got Vector<const Vector&<T> >. Since we
don't
> support sequence<sequence<T>>, this special casing should be gone. Will do
> before landing.

Done. As a side effect, setter functions in impl classes takes "String", not
"const String&". I think this is ok because copying String isn't so costly and
we are already using it in argument_declarations_for_private_script(). We can
make "String" to "const String&", if we want to.

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

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_methods.py:254: used_as_rvalue_type=True),
argument.name)
On 2014/07/29 11:09:19, haraken wrote:
> 
> Nit: 4-space indent

Done.

https://codereview.chromium.org/420763002/diff/60001/Source/bindings/template...
File Source/bindings/templates/dictionary_impl.h (right):

https://codereview.chromium.org/420763002/diff/60001/Source/bindings/template...
Source/bindings/templates/dictionary_impl.h:26: void
{{member.setter_name}}({{member.rvalue_cpp_type}} value) { m_{{member.name}} =
value;}
Forgot to add this...

[haraken, 2014-07-30 00:00:56]

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

https://codereview.chromium.org/420763002/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_dictionary.py:110: return 'const %s&' %
cpp_rvalue_type
On 2014/07/29 23:59:49, bashi1 wrote:
> On 2014/07/29 11:51:21, bashi1 wrote:
> > On 2014/07/29 11:09:19, haraken wrote:
> > > 
> > > I'm not quite sure why we need this special-casing. We might want to
remove
> it
> > > in a follow-up if possible.
> > 
> > Sorry, this should be in cpp_type(); This is a kind of leftover when I tried
> to
> > sequence<sequence<T>> workable. I got Vector<const Vector&<T> >. Since we
> don't
> > support sequence<sequence<T>>, this special casing should be gone. Will do
> > before landing.
> 
> Done. As a side effect, setter functions in impl classes takes "String", not
> "const String&". I think this is ok because copying String isn't so costly and
> we are already using it in argument_declarations_for_private_script(). We can
> make "String" to "const String&", if we want to.

This would be fine.

[bashi, 2014-07-30 00:02:18]

The CQ bit was checked by bashi@chromium.org

[commit-bot: I haz the power, 2014-07-30 00:02:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bashi@chromium.org/420763002/60001

[commit-bot: I haz the power, 2014-07-30 01:02:20]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/19566)

[commit-bot: I haz the power, 2014-07-30 02:45:38]

Change committed as 179190