v8binding: Don't allow author script to define indexed accessor prop.

third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl

 {% extends 'interface_base.cpp.tmpl' %}
 
 {##############################################################################}
 {% block indexed_property_getter %}
 {% if indexed_property_getter and not indexed_property_getter.is_custom %}
 {% set getter = indexed_property_getter %}
 static void indexedPropertyGetter(uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& info) {
   {% if getter.is_raises_exception %}
   ExceptionState exceptionState(info.GetIsolate(), ExceptionState::kIndexedGetterContext, "{{interface_name}}");
   {% endif %}
@@ skipping 93 matching lines @@
     return;
   V8SetReturnValue(info, v8Value);
 }
 
 {% endif %}
 {% endblock %}
 
 
 {##############################################################################}
 {% block indexed_property_setter_callback %}
-{% if indexed_property_setter or named_property_setter %}
+{% if indexed_property_getter or named_property_setter %}
 {% set setter = indexed_property_setter or named_property_setter %}
 void {{v8_class_or_partial}}::indexedPropertySetterCallback(uint32_t index, v8::Local<v8::Value> v8Value, const v8::PropertyCallbackInfo<v8::Value>& info) {
   {% if setter.is_ce_reactions %}
   CEReactionsScope ceReactionsScope;
   {% endif %}
 
   {% if indexed_property_setter %}
 
   {% if setter.is_custom %}
   {{v8_class}}::indexedPropertySetterCustom(index, v8Value, info);
   {% else %}
   {{cpp_class}}V8Internal::indexedPropertySetter(index, v8Value, info);
   {% endif %}
 
-  {% else %}{# otherwise, named property #}
+  {% elif named_property_setter %}
 
   const AtomicString& propertyName = AtomicString::Number(index);
 
   {% if setter.is_custom %}
   {{v8_class}}::namedPropertySetterCustom(propertyName, v8Value, info);
   {% else %}
   {{cpp_class}}V8Internal::namedPropertySetter(propertyName, v8Value, info);
   {% endif %}
 
-  {% endif %}{# indexed_property_setter #}
+  {% else %}{# neither of indexed_property_setter nor named_property_setter #}
+
+  // No indexed property setter defined.  Do not fall back to the default
+  // setter.
+  V8SetReturnValue(info, v8::Null(info.GetIsolate()));
+  if (info.ShouldThrowOnError()) {

[haraken, 2017/07/05 11:51:13]

What is info.ShouldThrowOnError()? I saw this the first time. When should we
call it?

[Yuki, 2017/07/05 12:37:42]

It corresponds to "use strict"; in ES.
  window[0] = true;
should be simple noop (just ignore the assignment), but
  "use strict";
  window[0] = true;
should throw a TypeError.

info.ShouldThrowOnError() tells us whether we should report an error as an
exception or not.
I don't have a simple answer.  It depends on API specifications.  There are
cases that we should throw even if no "use strict" specified.

+    ExceptionState exceptionState(info.GetIsolate(),
+                                  ExceptionState::kIndexedSetterContext,
+                                  "{{interface_name}}");
+    exceptionState.ThrowTypeError("Index property setter is not supported.");
+  }
+
+  {% endif %}{# indexed/named_property_setter #}
 }
 
 {% endif %}
 {% endblock %}
 
 
 {##############################################################################}
 {% block indexed_property_deleter %}
 {% if indexed_property_deleter and not indexed_property_deleter.is_custom %}
 {% set deleter = indexed_property_deleter %}
@@ skipping 55 matching lines @@
   {% endif %}
 
   {% endif %}{# indexed_property_deleter #}
 }
 
 {% endif %}
 {% endblock %}
 
 
 {##############################################################################}
+{% block indexed_property_definer_callback %}
+{% if indexed_property_getter %}
+void {{v8_class_or_partial}}::indexedPropertyDefinerCallback(
+    uint32_t index,
+    const v8::PropertyDescriptor& desc,
+    const v8::PropertyCallbackInfo<v8::Value>& info) {
+{% if indexed_property_setter %}
+  // https://heycam.github.io/webidl/#legacy-platform-object-defineownproperty
+  // 3.9.3. [[DefineOwnProperty]]
+  // step 1.1. If the result of calling IsDataDescriptor(Desc) is false, then
+  //   return false.
+  if (desc.has_get() || desc.has_set()) {
+    V8SetReturnValue(info, v8::Null(info.GetIsolate()));

[haraken, 2017/07/05 11:51:13]

Does this mean we "return false"?

[Yuki, 2017/07/05 12:37:42]

This V8 callback API doesn't correspond to the spec here.  The return value has
nothing to do with "return false" in the spec.

V8 falls back to OrdinaryDefineOwnProperty when the return value is the empty
handle (not even undefined), and stops further processing when the return value
is a non-empty handle (whatever the value is).  Only empty or non-empty matters.

Could be a little confusing, but it's documented in v8.h.
https://cs.chromium.org/chromium/src/v8/include/v8.h?q=GenericNamedPropertyDe...

+    if (info.ShouldThrowOnError()) {
+      ExceptionState exceptionState(info.GetIsolate(),
+                                    ExceptionState::kIndexedSetterContext,
+                                    "{{interface_name}}");
+      exceptionState.ThrowTypeError("Accessor properties are not allowed.");
+    }
+    return;
+  }
+
+  // Retutn nothing and fall back to indexedPropertySetterCallback.

[haraken, 2017/07/05 11:51:13]

Return

[Yuki, 2017/07/05 12:37:42]

Done.

+{% else %}{# indexed_property_setter #}
+  // https://heycam.github.io/webidl/#legacy-platform-object-defineownproperty
+  // 3.9.3. [[DefineOwnProperty]]
+  // step 1.2. If O does not implement an interface with an indexed property
+  //   setter, then return false.
+  //
+  // https://html.spec.whatwg.org/C/window-object.html#windowproxy-defineownproperty
+  // 7.4.6 [[DefineOwnProperty]] (P, Desc)
+  // step 2.1. If P is an array index property name, return false.
+  V8SetReturnValue(info, v8::Null(info.GetIsolate()));
+  if (info.ShouldThrowOnError()) {
+    ExceptionState exceptionState(info.GetIsolate(),
+                                  ExceptionState::kIndexedSetterContext,
+                                  "{{interface_name}}");
+    exceptionState.ThrowTypeError("Index property setter is not supported.");
+  }
+{% endif %}{# indexed_property_setter #}
+}
+
+{% endif %}{# indexed_property_getter #}
+{% endblock %}
+
+
+{##############################################################################}
 {% block named_property_getter %}
 {% if named_property_getter and not named_property_getter.is_custom %}
 {% set getter = named_property_getter %}
 static void namedPropertyGetter(const AtomicString& name, const v8::PropertyCallbackInfo<v8::Value>& info) {
   {% if getter.is_raises_exception %}
   const CString& nameInUtf8 = name.Utf8();
   ExceptionState exceptionState(info.GetIsolate(), ExceptionState::kGetterContext, "{{interface_name}}", nameInUtf8.data());
   {% endif %}
   {% if getter.is_call_with_script_state %}
   ScriptState* scriptState = ScriptState::ForReceiverObject(info);
@@ skipping 475 matching lines @@
   V8DOMConfiguration::InstallAttribute(isolate, world, {{instance_template}}, {{prototype_template}}, attributeConfig);
 {%- endmacro %}
 
 
 {##############################################################################}
 {% macro install_indexed_property_handler(target) %}
 {% set indexed_property_getter_callback =
        '%s::indexedPropertyGetterCallback' % v8_class_or_partial %}
 {% set indexed_property_setter_callback =
        '%s::indexedPropertySetterCallback' % v8_class_or_partial
-       if indexed_property_setter or named_property_setter else 'nullptr' %}
-{% set indexed_property_query_callback = 'nullptr' %}{# Unused #}
+       if indexed_property_getter or named_property_setter else 'nullptr' %}
+{% set indexed_property_descriptor_callback = 'nullptr' %}
 {% set indexed_property_deleter_callback =
        '%s::indexedPropertyDeleterCallback' % v8_class_or_partial
        if indexed_property_deleter or named_property_deleter else 'nullptr' %}
 {% set indexed_property_enumerator_callback =
        'IndexedPropertyEnumerator<%s>' % cpp_class
        if indexed_property_getter.is_enumerable else 'nullptr' %}
+{% set indexed_property_definer_callback =
+       '%s::indexedPropertyDefinerCallback' % v8_class_or_partial
+       if indexed_property_getter else 'nullptr' %}
 {% set property_handler_flags =
        'v8::PropertyHandlerFlags::kNone' %}
-v8::IndexedPropertyHandlerConfiguration indexedPropertyHandlerConfig({{indexed_property_getter_callback}}, {{indexed_property_setter_callback}}, {{indexed_property_query_callback}}, {{indexed_property_deleter_callback}}, {{indexed_property_enumerator_callback}}, v8::Local<v8::Value>(), {{property_handler_flags}});
+v8::IndexedPropertyHandlerConfiguration indexedPropertyHandlerConfig(
+    {{indexed_property_getter_callback}},
+    {{indexed_property_setter_callback}},
+    {{indexed_property_descriptor_callback}},
+    {{indexed_property_deleter_callback}},
+    {{indexed_property_enumerator_callback}},
+    {{indexed_property_definer_callback}},
+    v8::Local<v8::Value>(),
+    {{property_handler_flags}});
 {{target}}->SetHandler(indexedPropertyHandlerConfig);
 {%- endmacro %}
 
 
 {##############################################################################}
 {% macro install_named_property_handler(target) %}
 {% set named_property_getter_callback =
        '%s::namedPropertyGetterCallback' % v8_class_or_partial %}
 {% set named_property_setter_callback =
        '%s::namedPropertySetterCallback' % v8_class_or_partial
@@ skipping 228 matching lines @@
 }
 
 {% for method in methods if method.overloads and method.overloads.has_partial_overloads %}
 void {{v8_class}}::register{{method.name | blink_capitalize}}MethodForPartialInterface(void (*method)(const v8::FunctionCallbackInfo<v8::Value>&)) {
   {{cpp_class}}V8Internal::{{method.name}}MethodForPartialInterface = method;
 }
 
 {% endfor %}
 {% endif %}{# has_partial_interface #}
 {% endblock %}