Blink bindings: use v8 to enforce method call access checks

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

Index: third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
diff --git a/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl b/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
index 004c06040ca379329ea3ad058bb8a086f7ca9965..a795d9730fccc3648cd10f7ac01da569c1c78a54 100644
--- a/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
+++ b/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
@@ -29,47 +29,16 @@ static void {{method.name}}{{method.overload_index}}Method{{world_suffix}}(const
     return;
   }
   {% endif %}
-  {% set local_dom_window_only = interface_name == 'Window' and not method.is_cross_origin %}
-  {% if local_dom_window_only %}
-  {% if method.is_check_security_for_receiver %}
-  {{cpp_class}}* uncheckedImpl = {{v8_class}}::toImpl(info.Holder());
-  {% else %}
+  {% if interface_name == 'Window' and not method.is_cross_origin %}
   // Same-origin methods are never exposed via the cross-origin interceptors.
   // Since same-origin access requires a LocalDOMWindow, it is safe to downcast
   // here.
   LocalDOMWindow* impl = toLocalDOMWindow({{v8_class}}::toImpl(info.Holder()));
-  {% endif %}{# method.is_check_security_for_receiver #}
   {% else %}
   {{cpp_class}}* impl = {{v8_class}}::toImpl(info.Holder());
-  {% endif %}{# local_dom_window_only #}
+  {% endif %}{# interface_name == 'Window' and not method.is_cross_origin #}
   {% endif %}{# not method.is_static #}
 
-  {# Security checks #}
-  {% if method.is_check_security_for_receiver %}
-  {{define_exception_state}}
-  {% if interface_name == 'EventTarget' %}
-  // Performance hack for EventTarget.  Checking whether it's a Window or not

[dcheng, 2017/02/27 05:34:19]

I think this shouldn't be needed anymore, as the common fast path for security
token checks is quick (it's just a pointer comparison). But if there are some
steps of how to confirm this, that would be helpful.

-  // prior to the call to BindingSecurity::shouldAllowAccessTo increases 30%
-  // of speed performance on Android Nexus 7 as of Dec 2015.  ALWAYS_INLINE
-  // didn't work in this case.
-  if (const DOMWindow* window = impl->toDOMWindow()) {
-    if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(info.GetIsolate()), window, exceptionState)) {
-      return;
-    }
-  }
-  {% else %}{# interface_name == 'EventTarget' #}
-  {% if local_dom_window_only %}
-  if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(info.GetIsolate()), uncheckedImpl, exceptionState)) {
-  {% else %}
-  if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(info.GetIsolate()), impl, exceptionState)) {
-  {% endif %}{# local_dom_window_only #}
-    return;
-  }
-  {% if local_dom_window_only %}
-  LocalDOMWindow* impl = toLocalDOMWindow(uncheckedImpl);
-  {% endif %}{# local_dom_window_only #}
-  {% endif %}{# interface_name == 'EventTarget' #}
-  {% endif %}{# method.is_check_security_for_receiver #}
   {% if method.is_check_security_for_return_value %}
   {{define_exception_state}}
   if (!BindingSecurity::shouldAllowAccessTo(currentDOMWindow(info.GetIsolate()), {{method.cpp_value}}, exceptionState)) {
@@ -78,8 +47,7 @@ static void {{method.name}}{{method.overload_index}}Method{{world_suffix}}(const
   }
   {% endif %}
 
-  {% set log_activity = world_suffix in method.activity_logging_world_list %}
-  {% if 'scriptState' in function_call or log_activity %}
+  {% if 'scriptState' in function_call %}
   {% if method.is_static %}
   ScriptState* scriptState = ScriptState::forFunctionObject(info);
   {% else %}
@@ -87,15 +55,6 @@ static void {{method.name}}{{method.overload_index}}Method{{world_suffix}}(const
   {% endif %}
   {% endif %}
 
-  {% if log_activity %}
-  V8PerContextData* contextData = scriptState->perContextData();
-  if (contextData && contextData->activityLogger()) {
-    ExceptionState exceptionState(info.GetIsolate(), ExceptionState::ExecutionContext, "{{interface_name}}", "{{method.name}}");
-    Vector<v8::Local<v8::Value>> loggerArgs = toImplArguments<Vector<v8::Local<v8::Value>>>(info, 0, exceptionState);
-    contextData->activityLogger()->logMethod("{{interface_name}}.{{method.name}}", info.Length(), loggerArgs.data());
-  }
-  {% endif %}
-
   {% if method.is_custom_element_callbacks %}
   V0CustomElementProcessingStack::CallbackDeliveryScope deliveryScope;
   {% endif %}
@@ -521,6 +480,19 @@ void {{v8_class_or_partial}}::{{method.name}}MethodCallback{{world_suffix}}(cons
   Deprecation::countDeprecation(currentExecutionContext(info.GetIsolate()), UseCounter::{{method.deprecate_as}});
   {% endif %}
   {% endif %}{# not method.overloads #}
+  {% if world_suffix in method.activity_logging_world_list %}

[dcheng, 2017/02/27 05:34:19]

Restoring the activity logger back to the original location.

+  {% if method.is_static %}
+  ScriptState* scriptState = ScriptState::forFunctionObject(info);
+  {% else %}
+  ScriptState* scriptState = ScriptState::forReceiverObject(info);
+  {% endif %}
+  V8PerContextData* contextData = scriptState->perContextData();
+  if (contextData && contextData->activityLogger()) {
+    ExceptionState exceptionState(info.GetIsolate(), ExceptionState::ExecutionContext, "{{interface_name}}", "{{method.name}}");
+    Vector<v8::Local<v8::Value>> loggerArgs = toImplArguments<Vector<v8::Local<v8::Value>>>(info, 0, exceptionState);
+    contextData->activityLogger()->logMethod("{{interface_name}}.{{method.name}}", info.Length(), loggerArgs.data());
+  }
+  {% endif %}
   {% if method.is_ce_reactions %}
   CEReactionsScope ceReactionsScope;
   {% endif %}
@@ -626,7 +598,9 @@ v8SetReturnValue(info, wrapper);
        if method.property_attributes else 'v8::None' %}
 {% set holder_check = 'V8DOMConfiguration::DoNotCheckHolder'
        if method.returns_promise else 'V8DOMConfiguration::CheckHolder' %}
-{"{{method.name}}", {{method_callback}}, {{method_callback_for_main_world}}, {{method.length}}, {{property_attribute}}, {{property_location(method)}}, {{holder_check}}}
+{% set access_check = 'V8DOMConfiguration::CheckAccess'
+       if method.is_check_security_for_receiver else 'V8DOMConfiguration::DoNotCheckAccess' %}
+{"{{method.name}}", {{method_callback}}, {{method_callback_for_main_world}}, {{method.length}}, {{property_attribute}}, {{property_location(method)}}, {{holder_check}}, {{access_check}}}
 {%- endmacro %}