binding: Makes Window/Location's attributes accessor-type properties.

third_party/WebKit/Source/bindings/scripts/v8_attributes.py

Index: third_party/WebKit/Source/bindings/scripts/v8_attributes.py
diff --git a/third_party/WebKit/Source/bindings/scripts/v8_attributes.py b/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
index 93fe014b355f82be68139f8cc1f39a113b34afc5..c6adc2982f7b94325d56540719b436655eb44450 100644
--- a/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
+++ b/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
@@ -57,7 +57,7 @@ def attribute_context(interface, attribute):
     is_do_not_check_security = 'DoNotCheckSecurity' in extended_attributes
     is_check_security_for_receiver = (
         has_extended_attribute_value(interface, 'CheckSecurity', 'Receiver') and
-        not is_do_not_check_security)
+        is_do_not_check_security)
     is_check_security_for_return_value = (
         has_extended_attribute_value(attribute, 'CheckSecurity', 'ReturnValue'))
     if is_check_security_for_receiver or is_check_security_for_return_value:
@@ -85,8 +85,8 @@ def attribute_context(interface, attribute):
         assert idl_type.is_wrapper_type or 'LogActivity' in extended_attributes, '[PerWorldBindings] should only be used with wrapper types: %s.%s' % (interface.name, attribute.name)
 
     if (base_idl_type == 'EventHandler' and
-        interface.name in ['Window', 'WorkerGlobalScope'] and
-        attribute.name == 'onerror'):
+            interface.name in ['Window', 'WorkerGlobalScope'] and
+            attribute.name == 'onerror'):
         includes.add('bindings/core/v8/V8ErrorHandler.h')
 
     cached_attribute_validation_method = extended_attributes.get('CachedAttribute')
@@ -120,9 +120,9 @@ def attribute_context(interface, attribute):
         'is_custom_element_callbacks': is_custom_element_callbacks,
         # TODO(yukishiino): Make all DOM attributes accessor-type properties.
         'is_data_type_property': is_data_type_property(interface, attribute),
-        'is_getter_raises_exception':  # [RaisesException]
+        'is_getter_raises_exception': (  # [RaisesException]
             'RaisesException' in extended_attributes and
-            extended_attributes['RaisesException'] in (None, 'Getter'),
+            extended_attributes['RaisesException'] in (None, 'Getter')),
         'is_implemented_in_private_script': is_implemented_in_private_script,
         'is_keep_alive_for_gc': keep_alive_for_gc,
         'is_lenient_this': 'LenientThis' in extended_attributes,
@@ -155,9 +155,9 @@ def attribute_context(interface, attribute):
         'reflect_only': extended_attribute_value_as_list(attribute, 'ReflectOnly'),
         'runtime_enabled_function': v8_utilities.runtime_enabled_function_name(attribute),  # [RuntimeEnabled]
         'should_be_exposed_to_script': not (is_implemented_in_private_script and is_only_exposed_to_private_script),
-        'world_suffixes': ['', 'ForMainWorld']
-                          if 'PerWorldBindings' in extended_attributes
-                          else [''],  # [PerWorldBindings]
+        'world_suffixes': (['', 'ForMainWorld']
+                           if 'PerWorldBindings' in extended_attributes
+                           else ['']),  # [PerWorldBindings]
     }
 
     if is_constructor_attribute(attribute):
@@ -188,18 +188,18 @@ def getter_context(interface, attribute, context):
     # always use a local variable (for readability and CG simplicity).
     if 'ImplementedInPrivateScript' in extended_attributes:
         if (not idl_type.is_wrapper_type and
-            not idl_type.is_basic_type and
-            not idl_type.is_enum):
+                not idl_type.is_basic_type and
+                not idl_type.is_enum):
             raise Exception('Private scripts supports only primitive types and DOM wrappers.')
 
         context['cpp_value_original'] = cpp_value
         cpp_value = 'result'
     elif (idl_type.is_explicit_nullable or
-        base_idl_type == 'EventHandler' or
-        'CachedAttribute' in extended_attributes or
-        'ReflectOnly' in extended_attributes or
-        context['is_keep_alive_for_gc'] or
-        context['is_getter_raises_exception']):
+            base_idl_type == 'EventHandler' or
+            'CachedAttribute' in extended_attributes or
+            'ReflectOnly' in extended_attributes or
+            context['is_keep_alive_for_gc'] or
+            context['is_getter_raises_exception']):
         context['cpp_value_original'] = cpp_value
         cpp_value = 'cppValue'
 
@@ -210,6 +210,21 @@ def getter_context(interface, attribute, context):
             cpp_value, extended_attributes=extended_attributes, script_wrappable='impl',
             for_main_world=for_main_world, is_static=attribute.is_static)
 
+    def v8_set_return_value_statement_for_security_failure():
+        # In the case that the security check fails, there is no common rule for
+        # the return value.  The return value depends on each attribute.
+        # However, we shouldn't return null or undefiend for attributes of type
+        # boolean or DOMString at least.  We're making the best guess here.

[haraken, 2016/04/27 11:08:54]

Hmm, this logic looks pretty random. What does Firefox do?

Does this CL keep the existing behavior?

[Yuki, 2016/04/27 15:09:56]

Probably Firefox does what the spec specifies.  If the spec requires "attribute
A must return 0", then FF may do so.  If the spec requires "attribute B must
return an empty string", then FF may do so.
AFAIK, yes, this should keep the existing behavior.  The reason why I needed to
introduce this hack is to keep the existing behavior and not to break the
existing layout tests.

Having said that, this CL introduces more chances to fail the access check than
before, and in those cases, there is improvement.

For example, (un)fortunately, there was no chance (or no layout test) that an
attribute of type boolean returns a value without throwing a SecurityError. 
This CL makes it possible.  In that case, the old implementation returns null
despite the type of the attribute is (non-nullable) boolean.  That's totally
incorrect.  So we need to return false in that case.


In short, the old implementation always returns null regardless of the type of
the attribute.  It's wrong.  (Un)luckily we didn't hit such case in layout
tests.  The new implementation returns a value of the type of the attribute,
it's more correct than before, the type at least matches.


You wrote "this logic looks pretty random", but this function just returns "zero
value in the given type".  I don't think this is that random.  I think "always
null regardless of the given type" is much much more random or broken.

+        if idl_type.is_explicit_nullable:
+            return 'v8SetReturnValueNull(info)'
+        if idl_type.is_boolean_type:
+            return 'v8SetReturnValueBool(info, false)'
+        if idl_type.is_numeric_type:
+            return 'v8SetReturnValueInt(info, 0)'
+        if idl_type.is_string_type:
+            return 'v8SetReturnValueEmptyString(info)'
+        return 'v8SetReturnValueNull(info)'
+
     context.update({
         'cpp_value': cpp_value,
         'cpp_value_to_v8_value': idl_type.cpp_value_to_v8_value(
@@ -217,8 +232,11 @@ def getter_context(interface, attribute, context):
             extended_attributes=extended_attributes),
         'v8_set_return_value_for_main_world': v8_set_return_value_statement(for_main_world=True),
         'v8_set_return_value': v8_set_return_value_statement(),
+        'v8_set_return_value_for_security_failure':
+            v8_set_return_value_statement_for_security_failure(),
     })
 
+
 def getter_expression(interface, attribute, context):
     arguments = []
     this_getter_base_name = getter_base_name(interface, attribute, arguments)
@@ -234,8 +252,8 @@ def getter_expression(interface, attribute, context):
     # static member functions, which for instance members (non-static members)
     # take *impl as their first argument
     if ('PartialInterfaceImplementedAs' in attribute.extended_attributes and
-        not 'ImplementedInPrivateScript' in attribute.extended_attributes and
-        not attribute.is_static):
+            'ImplementedInPrivateScript' not in attribute.extended_attributes and
+            not attribute.is_static):
         arguments.append('*impl')
     if attribute.idl_type.is_explicit_nullable:
         arguments.append('isNull')
@@ -373,8 +391,8 @@ def setter_expression(interface, attribute, context):
     # static member functions, which for instance members (non-static members)
     # take *impl as their first argument
     if ('PartialInterfaceImplementedAs' in extended_attributes and
-        not 'ImplementedInPrivateScript' in extended_attributes and
-        not attribute.is_static):
+            'ImplementedInPrivateScript' not in extended_attributes and
+            not attribute.is_static):
         arguments.append('*impl')
     idl_type = attribute.idl_type
     if 'ImplementedInPrivateScript' in extended_attributes:
@@ -386,7 +404,7 @@ def setter_expression(interface, attribute, context):
         context['event_handler_getter_expression'] = '%s(%s)' % (
             getter_name, ', '.join(arguments))
         if (interface.name in ['Window', 'WorkerGlobalScope'] and
-            attribute.name == 'onerror'):
+                attribute.name == 'onerror'):
             includes.add('bindings/core/v8/V8ErrorHandler.h')
             arguments.append('V8EventListenerList::findOrCreateWrapper<V8ErrorHandler>(v8Value, true, ScriptState::current(info.GetIsolate()))')
         else:
@@ -443,8 +461,7 @@ def is_writable(attribute):
 
 def is_data_type_property(interface, attribute):
     return (is_constructor_attribute(attribute) or
-            interface.name == 'Window' or
-            interface.name == 'Location')
+            'DoNotCheckSecurity' in attribute.extended_attributes)
 
 
 # [PutForwards], [Replaceable]