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

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

 # Copyright (C) 2013 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
 # copyright notice, this list of conditions and the following disclaimer
@@ skipping 39 matching lines @@
     extended_attributes = attribute.extended_attributes
 
     idl_type.add_includes_for_type(extended_attributes)
     if idl_type.enum_values:
         includes.add('core/inspector/ConsoleMessage.h')
 
     # [CheckSecurity]
     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:
         includes.add('bindings/core/v8/BindingSecurity.h')
     # [Constructor]
     # TODO(yukishiino): Constructors are much like methods although constructors
     # are not methods.  Constructors must be data-type properties, and we can
     # support them as a kind of methods.
     constructor_type = idl_type.constructor_type_name if is_constructor_attribute(attribute) else None
     # [CustomElementCallbacks], [Reflect]
     is_custom_element_callbacks = 'CustomElementCallbacks' in extended_attributes
     is_reflect = 'Reflect' in extended_attributes
     if is_custom_element_callbacks or is_reflect:
         includes.add('core/dom/custom/V0CustomElementProcessingStack.h')
     # [ImplementedInPrivateScript]
     is_implemented_in_private_script = 'ImplementedInPrivateScript' in extended_attributes
     if is_implemented_in_private_script:
         includes.add('bindings/core/v8/PrivateScriptRunner.h')
         includes.add('core/frame/LocalFrame.h')
         includes.add('platform/ScriptForbiddenScope.h')
     # [OnlyExposedToPrivateScript]
     is_only_exposed_to_private_script = 'OnlyExposedToPrivateScript' in extended_attributes
     # [PerWorldBindings]
     if 'PerWorldBindings' in extended_attributes:
         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')
     keep_alive_for_gc = is_keep_alive_for_gc(interface, attribute)
     if cached_attribute_validation_method or keep_alive_for_gc:
         includes.add('bindings/core/v8/V8HiddenValue.h')
 
     context = {
         'access_control_list': access_control_list(interface, attribute),
         'activity_logging_world_list_for_getter': v8_utilities.activity_logging_world_list(attribute, 'Getter'),  # [ActivityLogging]
@@ skipping 13 matching lines @@
         'has_custom_setter': has_custom_setter(attribute),
         'has_setter': has_setter(interface, attribute),
         'idl_type': str(idl_type),  # need trailing [] on array for Dictionary::ConversionContext::setConversionType
         'is_call_with_execution_context': has_extended_attribute_value(attribute, 'CallWith', 'ExecutionContext'),
         'is_call_with_script_state': has_extended_attribute_value(attribute, 'CallWith', 'ScriptState'),
         'is_check_security_for_receiver': is_check_security_for_receiver,
         'is_check_security_for_return_value': is_check_security_for_return_value,
         '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,
         'is_nullable': idl_type.is_nullable,
         'is_explicit_nullable': idl_type.is_explicit_nullable,
         'is_partial_interface_member':
             'PartialInterfaceImplementedAs' in extended_attributes,
         'is_per_world_bindings': 'PerWorldBindings' in extended_attributes,
         'is_put_forwards': 'PutForwards' in extended_attributes,
         'is_read_only': attribute.is_read_only,
@@ skipping 12 matching lines @@
         'only_exposed_to_private_script': is_only_exposed_to_private_script,
         'private_script_v8_value_to_local_cpp_value': idl_type.v8_value_to_local_cpp_value(
             extended_attributes, 'v8Value', 'cppValue', bailout_return_value='false', isolate='scriptState->isolate()'),
         'property_attributes': property_attributes(interface, attribute),
         'reflect_empty': extended_attributes.get('ReflectEmpty'),
         'reflect_invalid': extended_attributes.get('ReflectInvalid', ''),
         'reflect_missing': extended_attributes.get('ReflectMissing'),
         '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):
         update_constructor_attribute_context(interface, attribute, context)
     if not has_custom_getter(attribute):
         getter_context(interface, attribute, context)
     if not has_custom_setter(attribute) and has_setter(interface, attribute):
         setter_context(interface, attribute, context)
 
     return context
@@ skipping 10 matching lines @@
 
     cpp_value = getter_expression(interface, attribute, context)
     # Normally we can inline the function call into the return statement to
     # avoid the overhead of using a Ref<> temporary, but for some cases
     # (nullable types, EventHandler, [CachedAttribute], or if there are
     # exceptions), we need to use a local variable.
     # FIXME: check if compilers are smart enough to inline this, and if so,
     # 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'
 
     def v8_set_return_value_statement(for_main_world=False):
         if context['is_keep_alive_for_gc'] or 'CachedAttribute' in extended_attributes:
             return 'v8SetReturnValue(info, v8Value)'
         return idl_type.v8_set_return_value(
             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(
             cpp_value=cpp_value, creation_context='holder',
             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)
     getter_name = scoped_name(interface, attribute, this_getter_base_name)
 
     if 'ImplementedInPrivateScript' in attribute.extended_attributes:
         arguments.append('toLocalFrame(toFrameIfNotDetached(info.GetIsolate()->GetCurrentContext()))')
         arguments.append('impl')
         arguments.append('&result')
     arguments.extend(v8_utilities.call_with_arguments(
         attribute.extended_attributes.get('CallWith')))
     # Members of IDL partial interface definitions are implemented in C++ as
     # 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')
     if context['is_getter_raises_exception']:
         arguments.append('exceptionState')
     if attribute.idl_type.use_output_parameter_for_result:
         arguments.append('result')
 
     expression = '%s(%s)' % (getter_name, ', '.join(arguments))
     # Needed to handle getter expressions returning Type& as the
@@ skipping 117 matching lines @@
         extended_attributes.get('SetterCallWith') or
         extended_attributes.get('CallWith'))
 
     this_setter_base_name = setter_base_name(interface, attribute, arguments)
     setter_name = scoped_name(interface, attribute, this_setter_base_name)
 
     # Members of IDL partial interface definitions are implemented in C++ as
     # 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:
         arguments.append('toLocalFrame(toFrameIfNotDetached(info.GetIsolate()->GetCurrentContext()))')
         arguments.append('impl')
         arguments.append('cppValue')
     elif idl_type.base_type == 'EventHandler':
         getter_name = scoped_name(interface, attribute, cpp_name(attribute))
         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:
             arguments.append('V8EventListenerList::getEventListener(ScriptState::current(info.GetIsolate()), v8Value, true, ListenerFindOrCreate)')
     else:
         arguments.append('cppValue')
     if context['is_setter_raises_exception']:
         arguments.append('exceptionState')
 
     return '%s(%s)' % (setter_name, ', '.join(arguments))
@@ skipping 36 matching lines @@
 
 # Property descriptor's {writable: boolean}
 def is_writable(attribute):
     return (not attribute.is_read_only or
             'PutForwards' in attribute.extended_attributes or
             'Replaceable' in attribute.extended_attributes)
 
 
 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]
 def has_setter(interface, attribute):
     if (is_data_type_property(interface, attribute) and
         (is_constructor_attribute(attribute) or
          'Replaceable' in attribute.extended_attributes)):
         return False
 
     return is_writable(attribute)
@@ skipping 54 matching lines @@
     lambda self: strip_suffix(self.base_type, 'Constructor'))
 
 
 def is_constructor_attribute(attribute):
     # FIXME: replace this with [ConstructorAttribute] extended attribute
     return attribute.idl_type.name.endswith('Constructor')
 
 
 def update_constructor_attribute_context(interface, attribute, context):
     context['needs_constructor_getter_callback'] = context['measure_as'] or context['deprecate_as'] or context['origin_trial_enabled_function']  # TODO(chasej): Should/can this be true when OriginTrialEnabled is inherited from containing interface?