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

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

Index: third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
diff --git a/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl b/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
index ce5780e5de6fbd952b8f1a1c8716f428c4b84b86..14822406f2a0df309beb5b52b5657c1a8fa13900 100644
--- a/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
+++ b/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
@@ -111,7 +111,7 @@ static void indexedPropertySetter(uint32_t index, v8::Local<v8::Value> v8Value,
 
 {##############################################################################}
 {% 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 %}
@@ -126,7 +126,7 @@ void {{v8_class_or_partial}}::indexedPropertySetterCallback(uint32_t index, v8::
   {{cpp_class}}V8Internal::indexedPropertySetter(index, v8Value, info);
   {% endif %}
 
-  {% else %}{# otherwise, named property #}
+  {% elif named_property_setter %}
 
   const AtomicString& propertyName = AtomicString::Number(index);
 
@@ -136,7 +136,19 @@ void {{v8_class_or_partial}}::indexedPropertySetterCallback(uint32_t index, v8::
   {{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 %}
@@ -211,6 +223,53 @@ void {{v8_class_or_partial}}::indexedPropertyDeleterCallback(uint32_t index, con
 {% 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 %}
@@ -707,17 +766,28 @@ for (const auto& attributeConfig : {{method.name}}OriginSafeAttributeConfigurati
        '%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 %}