Decouple arity errors creation from throwing.

Decouple arity errors creation from throwing.

In order to reduce duplicated functions between V8Binding.h and
ScriptPromise.h, this CL adds arity error creation functions to V8Binding.h
and deletes arity error throwing functions from V8Binding.h and
ScriptPromise.h.

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

[yhirano, 2014-08-12 06:53:04]

This is a followup CL for https://codereview.chromium.org/232563003/ . This
change increases the binary size less than 5kb on x64-linux.

[Jens Widell, 2014-08-12 07:18:04]

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

https://codereview.chromium.org/460923002/diff/20001/Source/bindings/template...
Source/bindings/templates/methods.cpp:325: {% if method.idl_type == 'Promise' %}
This if/else block is repeated twice in this macro and once in
throw_minimum_arity_type_error(). Could we add a utility macro for it that takes
the method and a "create exception" expression as arguments?

https://codereview.chromium.org/460923002/diff/20001/Source/bindings/template...
Source/bindings/templates/methods.cpp:417: {# FIXME: We should return a rejected
Promise here and the
This FIXME applies to the code immediately following the if-statement too, so
technically it belongs above. Directly below {# Report full list ... #} would be
suitable, I think.

[yhirano, 2014-08-12 07:43:19]

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

https://codereview.chromium.org/460923002/diff/20001/Source/bindings/template...
Source/bindings/templates/methods.cpp:325: {% if method.idl_type == 'Promise' %}
On 2014/08/12 07:18:03, Jens Lindström wrote:
> This if/else block is repeated twice in this macro and once in
> throw_minimum_arity_type_error(). Could we add a utility macro for it that
takes
> the method and a "create exception" expression as arguments?

Done.

https://codereview.chromium.org/460923002/diff/20001/Source/bindings/template...
Source/bindings/templates/methods.cpp:417: {# FIXME: We should return a rejected
Promise here and the
On 2014/08/12 07:18:03, Jens Lindström wrote:
> This FIXME applies to the code immediately following the if-statement too, so
> technically it belongs above. Directly below {# Report full list ... #} would
be
> suitable, I think.
This function has three blocks and all should be addressed, I think. I moved the
comment to the top of this macro.

[haraken, 2014-08-12 08:15:05]

LGTM, but let's wait for an approval from Jens.

[Jens Widell, 2014-08-12 08:42:27]

LGTM

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

https://codereview.chromium.org/460923002/diff/20001/Source/bindings/template...
Source/bindings/templates/methods.cpp:325: {% if method.idl_type == 'Promise' %}
On 2014/08/12 07:43:19, yhirano wrote:
> On 2014/08/12 07:18:03, Jens Lindström wrote:
> > This if/else block is repeated twice in this macro and once in
> > throw_minimum_arity_type_error(). Could we add a utility macro for it that
> takes
> > the method and a "create exception" expression as arguments?
> 
> Done.

I meant the "if promise, set return value, else throw" block. But this was also
a nice  improvement. And what I was going for isn't complete straight-forward.

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

https://codereview.chromium.org/460923002/diff/60001/Source/bindings/template...
Source/bindings/templates/methods.cpp:320: {% macro type_error_message(method,
error_message) %}
Nit: nbarth@ once told me "Order here is top-down" meaning that macros should be
defined following the macros in which they are used, rather than the other way
around.

https://codereview.chromium.org/460923002/diff/60001/Source/bindings/template...
Source/bindings/templates/methods.cpp:367: {%- else %}
Nit: Could use elif here, I suppose.

[yhirano, 2014-08-12 09:36:10]

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

https://codereview.chromium.org/460923002/diff/60001/Source/bindings/template...
Source/bindings/templates/methods.cpp:320: {% macro type_error_message(method,
error_message) %}
On 2014/08/12 08:42:26, Jens Lindström wrote:
> Nit: nbarth@ once told me "Order here is top-down" meaning that macros should
be
> defined following the macros in which they are used, rather than the other way
> around.

Done.

https://codereview.chromium.org/460923002/diff/60001/Source/bindings/template...
Source/bindings/templates/methods.cpp:367: {%- else %}
On 2014/08/12 08:42:26, Jens Lindström wrote:
> Nit: Could use elif here, I suppose.

Done.

[Jens Widell, 2014-08-12 09:39:55]

On 2014/08/12 09:36:10, yhirano wrote:
>
https://codereview.chromium.org/460923002/diff/60001/Source/bindings/template...
> File Source/bindings/templates/methods.cpp (right):
> 
>
https://codereview.chromium.org/460923002/diff/60001/Source/bindings/template...
> Source/bindings/templates/methods.cpp:320: {% macro type_error_message(method,
> error_message) %}
> On 2014/08/12 08:42:26, Jens Lindström wrote:
> > Nit: nbarth@ once told me "Order here is top-down" meaning that macros
should
> be
> > defined following the macros in which they are used, rather than the other
way
> > around.
> 
> Done.
> 
>
https://codereview.chromium.org/460923002/diff/60001/Source/bindings/template...
> Source/bindings/templates/methods.cpp:367: {%- else %}
> On 2014/08/12 08:42:26, Jens Lindström wrote:
> > Nit: Could use elif here, I suppose.
> 
> Done.

Thanks x 2. :-)

[bashi, 2014-08-12 23:41:49]

LGTM

[yhirano, 2014-08-13 03:38:33]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2014-08-13 03:39:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yhirano@chromium.org/460923002/80001

[commit-bot: I haz the power, 2014-08-13 04:49:31]

Change committed as 180137