Implement cross-origin attributes using access check interceptors.

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 d893f28546176010d951c5ce5b3c82b18c87f60b..09cb8d81f6563a04be634537af95927f64ed3d63 100644
--- a/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
+++ b/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
@@ -488,6 +488,9 @@ void namedPropertyEnumeratorCallback(const v8::PropertyCallbackInfo<v8::Array>&
 
 
 {##############################################################################}
+{# TODO(dcheng): This shouldn't be necessary with cross-origin interceptors, but
+   v8 doesn't support querying the incumbent context. For now, always

[Yuki, 2016/12/07 12:03:40]

"incumbent realm" is an idea of HTML, hence it should be implemented in Blink, I
think.  It's not V8's fault.

[dcheng, 2016/12/08 09:04:10]

It's awkward to implement in Blink though: according to domenic@, we should not
be creating per-realm function objects except for cross-origin accesses.

If we're OK with keeping this behavior in the long-term, I will just change the
comments to explain the implementation strategy. WDYT?

[Yuki, 2016/12/09 07:02:49]

I'm not saying that it's okay to create per-realm function objects
(originSafeMethodGetter), but I'm saying that Blink may/should be responsible to
support a way to control "incumbent realm" because it's out of scope of
ECMAScript.  "incumbent realm" is defined in HTML spec.

I'm saying that Blink should support "incumbent realm" on Blink side and remove
the ugly hack of originSafeMethodGetter.  It's possible that Blink needs some
help from V8, but in general, HTML things should be handled in Blink, not in V8,
I think.

I think/suppose that, once Blink supports "incumbent realm", we can implement
postMessage in a correct way using incumbent realm and we no longer need
originSafeMethodGetter.

[dcheng, 2016/12/09 07:35:20]

Ah, I understand. I've updated the comment to talk specifically about supporting
the incumbent realm. WDYT?

[Yuki, 2016/12/09 08:40:23]

I like the new comment.  Thanks.  :)

+   incorrectly create per-realm representations. #}
 {% block origin_safe_method_setter %}
 {% if has_origin_safe_method_setter %}
 static void {{cpp_class}}OriginSafeMethodSetter(v8::Local<v8::Name> name, v8::Local<v8::Value> v8Value, const v8::PropertyCallbackInfo<void>& info) {
@@ -656,12 +659,11 @@ void {{v8_class}}::constructorCallback(const v8::FunctionCallbackInfo<v8::Value>
 
 
 {##############################################################################}
-{% macro install_do_not_check_security_method(method, world_suffix, instance_template, prototype_template) %}
+{% macro install_origin_safe_method(method, world_suffix, instance_template, prototype_template) %}
 {% from 'utilities.cpp.tmpl' import property_location %}
-{# Methods that are [DoNotCheckSecurity] are always readable, but if they are
-   changed and then accessed from a different origin, we do not return the
-   underlying value, but instead return a new copy of the original function.
-   This is achieved by storing the changed value as a hidden property. #}
+{# TODO(dcheng): This shouldn't be necessary with cross-origin interceptors, but
+   v8 doesn't support querying the incumbent context. For now, always

[Yuki, 2016/12/07 12:03:40]

ditto.

+   incorrectly create per-realm representations. #}
 {% set getter_callback =
        '%sV8Internal::%sOriginSafeMethodGetterCallback%s' %
        (cpp_class, method.name, world_suffix) %}
@@ -682,7 +684,7 @@ void {{v8_class}}::constructorCallback(const v8::FunctionCallbackInfo<v8::Value>
 {% set only_exposed_to_private_script = 'V8DOMConfiguration::OnlyExposedToPrivateScript' if method.only_exposed_to_private_script else 'V8DOMConfiguration::ExposedToAllScripts' %}
 {% set holder_check = 'V8DOMConfiguration::CheckHolder' %}
 const V8DOMConfiguration::AttributeConfiguration {{method.name}}OriginSafeAttributeConfiguration = {
-    "{{method.name}}", {{getter_callback}}, {{setter_callback}}, {{getter_callback_for_main_world}}, {{setter_callback_for_main_world}}, nullptr, &{{v8_class}}::wrapperTypeInfo, v8::ALL_CAN_READ, {{property_attribute}}, {{only_exposed_to_private_script}}, {{property_location(method)}}, {{holder_check}},
+    "{{method.name}}", {{getter_callback}}, {{setter_callback}}, {{getter_callback_for_main_world}}, {{setter_callback_for_main_world}}, nullptr, &{{v8_class}}::wrapperTypeInfo, v8::DEFAULT, {{property_attribute}}, {{only_exposed_to_private_script}}, {{property_location(method)}}, {{holder_check}},
 };
 V8DOMConfiguration::installAttribute(isolate, world, {{instance_template}}, {{prototype_template}}, {{method.name}}OriginSafeAttributeConfiguration);
 {%- endmacro %}
@@ -703,9 +705,7 @@ V8DOMConfiguration::installAttribute(isolate, world, {{instance_template}}, {{pr
        'indexedPropertyEnumerator<%s>' % cpp_class
        if indexed_property_getter.is_enumerable else '0' %}
 {% set property_handler_flags =
-       'v8::PropertyHandlerFlags::kAllCanRead'
-       if indexed_property_getter.do_not_check_security
-       else 'v8::PropertyHandlerFlags::kNone' %}
+       '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}});
 {{target}}->SetHandler(indexedPropertyHandlerConfig);
 {%- endmacro %}
@@ -729,10 +729,6 @@ v8::IndexedPropertyHandlerConfiguration indexedPropertyHandlerConfig({{indexed_p
        if named_property_getter.is_enumerable else '0' %}
 {% set property_handler_flags_list =
        ['int(v8::PropertyHandlerFlags::kOnlyInterceptStrings)'] %}
-{% if named_property_getter.do_not_check_security %}
-{% set property_handler_flags_list =
-       property_handler_flags_list + ['int(v8::PropertyHandlerFlags::kAllCanRead)'] %}
-{% endif %}
 {% if not is_override_builtins %}
 {% set property_handler_flags_list =
        property_handler_flags_list + ['int(v8::PropertyHandlerFlags::kNonMasking)'] %}