Decouple arity errors creation from throwing.

Source/bindings/templates/methods.cpp

 {##############################################################################}
 {% macro generate_method(method, world_suffix) %}
 {% filter conditional(method.conditional_string) %}
 static void {{method.name}}{{method.overload_index}}Method{{world_suffix}}(const v8::FunctionCallbackInfo<v8::Value>& info)
 {
     {# Local variables #}
     {% if method.has_exception_state %}
     ExceptionState exceptionState(ExceptionState::ExecutionContext, "{{method.name}}", "{{interface_name}}", info.Holder(), info.GetIsolate());
     {% endif %}
     {# Overloaded methods have length checked during overload resolution #}
     {% if method.number_of_required_arguments and not method.overload_index %}
     if (UNLIKELY(info.Length() < {{method.number_of_required_arguments}})) {
-        {{throw_minimum_arity_type_error(method, method.number_of_required_arguments)}};
+        {{throw_minimum_arity_type_error(method, method.number_of_required_arguments) | indent(8)}}
         return;
     }
     {% endif %}
     {% if not method.is_static %}
     {{cpp_class}}* impl = {{v8_class}}::toNative(info.Holder());
     {% endif %}
     {% if method.is_custom_element_callbacks %}
     CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
     {% endif %}
     {# Security checks #}
@@ skipping 291 matching lines @@
 v8SetReturnValueNull(info);
 {% endmacro %}
 
 
 {######################################}
 {% macro throw_type_error(method, error_message) %}
 {% if method.has_exception_state %}
 exceptionState.throwTypeError({{error_message}});
 {{throw_from_exception_state(method)}};
 {% elif method.is_constructor %}
 {% if method.idl_type == 'Promise' %}

[Jens Widell, 2014/08/12 07:18:03]

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?

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

Done.

[Jens Widell, 2014/08/12 08:42:26]

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.

 {# FIXME: reduce code duplication between sync / async exception handling. #}
-v8SetReturnValue(info, ScriptPromise::rejectWithTypeError(ScriptState::current(info.GetIsolate()), ExceptionMessages::failedToConstruct("{{interface_name}}", {{error_message}})).v8Value());
+v8SetReturnValue(info, ScriptPromise::rejecRaw(info.GetIsolate(), V8ThrowException::createTypeError(ExceptionMessages::failedToConstruct("{{interface_name}}", {{error_message}}), info.GetIsolate())));
 {% else %}
 V8ThrowException::throwTypeError(ExceptionMessages::failedToConstruct("{{interface_name}}", {{error_message}}), info.GetIsolate());
 {% endif %}
 {% else %}{# method.has_exception_state #}
 {% if method.idl_type == 'Promise' %}
-v8SetReturnValue(info, ScriptPromise::rejectWithTypeError(ScriptState::current(info.GetIsolate()), ExceptionMessages::failedToExecute("{{method.name}}", "{{interface_name}}", {{error_message}})).v8Value());
+v8SetReturnValue(info, ScriptPromise::rejectRaw(info.GetIsolate(), V8ThrowException::createTypeError(ExceptionMessages::failedToExecute("{{method.name}}", "{{interface_name}}", {{error_message}}), info.GetIsolate())));
 {% else %}
 V8ThrowException::throwTypeError(ExceptionMessages::failedToExecute("{{method.name}}", "{{interface_name}}", {{error_message}}), info.GetIsolate());
 {% endif %}
 {% endif %}{# method.has_exception_state #}
 {% endmacro %}
 
 
 {######################################}
 {% macro throw_from_exception_state(method) %}
 {% if method.idl_type == 'Promise' %}
 v8SetReturnValue(info, exceptionState.reject(ScriptState::current(info.GetIsolate())).v8Value())
 {%- else %}
 exceptionState.throwIfNeeded()
 {%- endif %}
 {%- endmacro %}
 
 
 {######################################}
-{% macro throw_arity_type_error(method, valid_arities) %}
-{% if method.idl_type == 'Promise' %}
-{% if method.has_exception_state %}
-v8SetReturnValue(info, ScriptPromise::rejectWithArityTypeError(ScriptState::current(info.GetIsolate()), exceptionState, {{valid_arities}}, info.Length()).v8Value())
-{%- elif method.is_constructor %}
-v8SetReturnValue(info, ScriptPromise::rejectWithArityTypeErrorForConstructor(ScriptState::current(info.GetIsolate()), "{{interface_name}}", {{valid_arities}}, info.Length()).v8Value())
+{% macro create_minimum_arity_type_error_without_exception_state(method, number_of_required_arguments) %}
+{% if method.is_constructor %}
+createMinimumArityTypeErrorForConstructor("{{interface_name}}", {{number_of_required_arguments}}, info.Length(), info.GetIsolate())
 {%- else %}
-v8SetReturnValue(info, ScriptPromise::rejectWithArityTypeErrorForMethod(ScriptState::current(info.GetIsolate()), "{{method.name}}", "{{interface_name}}", {{valid_arities}}, info.Length()).v8Value())
+createMinimumArityTypeErrorForMethod("{{method.name}}", "{{interface_name}}", {{number_of_required_arguments}}, info.Length(), info.GetIsolate())
 {%- endif %}
-{%- else %}{# methods.idl_type == 'Promise' #}
-{% if method.has_exception_state %}
-throwArityTypeError(exceptionState, {{valid_arities}}, info.Length())
-{%- elif method.is_constructor %}
-throwArityTypeErrorForConstructor("{{interface_name}}", {{valid_arities}}, info.Length(), info.GetIsolate())
-{%- else %}
-throwArityTypeErrorForMethod("{{method.name}}", "{{interface_name}}", {{valid_arities}}, info.Length(), info.GetIsolate())
-{%- endif %}
-{%- endif %}{# methods.idl_type == 'Promise' #}
-{% endmacro %}
+{%- endmacro %}
 
 
 {######################################}
 {% macro throw_minimum_arity_type_error(method, number_of_required_arguments) %}
+{% if method.has_exception_state %}
+setMinimumArityTypeError(exceptionState, {{number_of_required_arguments}}, info.Length());
+{{throw_from_exception_state(method)}};
+{%- else %}
 {% if method.idl_type == 'Promise' %}
-{% if method.has_exception_state %}
-v8SetReturnValue(info, ScriptPromise::rejectWithMinimumArityTypeError(ScriptState::current(info.GetIsolate()), exceptionState, {{number_of_required_arguments}}, info.Length()).v8Value())
-{%- elif method.is_constructor %}
-v8SetReturnValue(info, ScriptPromise::rejectWithMinimumArityTypeErrorForConstructor(ScriptState::current(info.GetIsolate()), "{{interface_name}}", {{number_of_required_arguments}}, info.Length()).v8Value())
+v8SetReturnValue(info, ScriptPromise::rejectRaw(info.GetIsolate(), {{create_minimum_arity_type_error_without_exception_state(method, number_of_required_arguments)}}));
 {%- else %}
-v8SetReturnValue(info, ScriptPromise::rejectWithMinimumArityTypeErrorForMethod(ScriptState::current(info.GetIsolate()), "{{method.name}}", "{{interface_name}}", {{number_of_required_arguments}}, info.Length()).v8Value())
-{%- endif %}
-{%- else %}{# methods.idl_type == 'Promise' #}
-{% if method.has_exception_state %}
-throwMinimumArityTypeError(exceptionState, {{number_of_required_arguments}}, info.Length())
-{%- elif method.is_constructor %}
-throwMinimumArityTypeErrorForConstructor("{{interface_name}}", {{number_of_required_arguments}}, info.Length(), info.GetIsolate())
-{%- else %}
-throwMinimumArityTypeErrorForMethod("{{method.name}}", "{{interface_name}}", {{number_of_required_arguments}}, info.Length(), info.GetIsolate())
-{%- endif %}
-{%- endif %}{# methods.idl_type == 'Promise' #}
-{% endmacro %}
+V8ThrowException::throwException({{create_minimum_arity_type_error_without_exception_state(method, number_of_required_arguments)}}, info.GetIsolate());
+{%- endif %}{# method.idl_type == 'Promise' #}
+{%- endif %}{# method.has_exception_state #}
+{%- endmacro %}
 
 
 {##############################################################################}
 {% macro overload_resolution_method(overloads, world_suffix) %}
 static void {{overloads.name}}Method{{world_suffix}}(const v8::FunctionCallbackInfo<v8::Value>& info)
 {
     ExceptionState exceptionState(ExceptionState::ExecutionContext, "{{overloads.name}}", "{{interface_name}}", info.Holder(), info.GetIsolate());
     {% if overloads.measure_all_as %}
     UseCounter::count(callingExecutionContext(info.GetIsolate()), UseCounter::{{overloads.measure_all_as}});
     {% endif %}
@@ skipping 23 matching lines @@
         }
         {% endfilter %}
         {% endfor %}
         break;
     {% endfor %}
     default:
         {# Invalid arity, throw error #}
         {# Report full list of valid arities if gaps and above minimum #}
         {% if overloads.valid_arities %}
         if (info.Length() >= {{overloads.minarg}}) {
-            throwArityTypeError(exceptionState, "{{overloads.valid_arities}}", info.Length());
+            {# FIXME: We should return a rejected Promise here and the

[Jens Widell, 2014/08/12 07:18:03]

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]

This function has three blocks and all should be addressed, I think. I moved the
comment to the top of this macro.

+            the following throwIfNeeded when ALL methods return Promise. In
+            order to do so, we must ensure either ALL or NO methods in this
+            overload return Promise #}
+            setArityTypeError(exceptionState, "{{overloads.valid_arities}}", info.Length());
+            exceptionState.throwIfNeeded();
             return;
         }
         {% endif %}
         {# Otherwise just report "not enough arguments" #}
         exceptionState.throwTypeError(ExceptionMessages::notEnoughArguments({{overloads.minarg}}, info.Length()));
         exceptionState.throwIfNeeded();
         return;
     }
     {# No match, throw error #}
     exceptionState.throwTypeError("No function was found that matched the signature provided.");
@@ skipping 160 matching lines @@
         v8SetReturnValue(info, info.Holder());
         return;
     }
     {% endif %}
     {% if constructor.has_exception_state %}
     ExceptionState exceptionState(ExceptionState::ConstructionContext, "{{interface_name}}", info.Holder(), info.GetIsolate());
     {% endif %}
     {# Overloaded constructors have length checked during overload resolution #}
     {% if constructor.number_of_required_arguments and not constructor.overload_index %}
     if (UNLIKELY(info.Length() < {{constructor.number_of_required_arguments}})) {
-        {{throw_minimum_arity_type_error(constructor, constructor.number_of_required_arguments)}};
+        {{throw_minimum_arity_type_error(constructor, constructor.number_of_required_arguments) | indent(8)}}
         return;
     }
     {% endif %}
     {% if constructor.arguments %}
     {{generate_arguments(constructor) | indent}}
     {% endif %}
     {{cpp_method_call(constructor, constructor.v8_set_return_value, constructor.cpp_value) | indent}}
 }
 {% endmacro %}
 
 
 {##############################################################################}
 {% macro generate_constructor_wrapper(constructor) %}
 {% if has_custom_wrap %}
 v8::Handle<v8::Object> wrapper = wrap(impl.get(), info.Holder(), info.GetIsolate());
 {% else %}
 {% set constructor_class = v8_class + ('Constructor'
                                        if constructor.is_named_constructor else
                                        '') %}
 v8::Handle<v8::Object> wrapper = info.Holder();
 V8DOMWrapper::associateObjectWithWrapper<{{v8_class}}>(impl.release(), &{{constructor_class}}::wrapperTypeInfo, wrapper, info.GetIsolate(), {{wrapper_configuration}});
 {% endif %}
 v8SetReturnValue(info, wrapper);
 {% endmacro %}