Extended implementation to use interface as arguments

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

 # Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Generate template values for a callback function.
 
 Design doc: http://www.chromium.org/developers/design-documents/idl-compiler
 """
 
 from v8_globals import includes  # pylint: disable=W0403
 import v8_utilities  # pylint: disable=W0403
 
 CALLBACK_FUNCTION_H_INCLUDES = frozenset([
     'bindings/core/v8/ScopedPersistent.h',
     'platform/heap/Handle.h',
     'wtf/text/WTFString.h',
 ])
 CALLBACK_FUNCTION_CPP_INCLUDES = frozenset([
     'bindings/core/v8/ScriptState.h',
     'bindings/core/v8/V8Binding.h',
     'wtf/Assertions.h',
 ])
 
 
 def callback_function_context(callback_function):
     includes.clear()
     includes.update(CALLBACK_FUNCTION_CPP_INCLUDES)
     idl_type = callback_function.idl_type
     idl_type_str = str(idl_type)
+    cpp_includes = set(CALLBACK_FUNCTION_CPP_INCLUDES)

[peria, 2016/09/23 02:50:05]

you can use |includes| instead of this?

[lkawai, 2016/09/23 05:35:44]

Done.

+    forward_declarations = []
+    for argument in callback_function.arguments:
+        for argument_type in list(argument.idl_type.idl_types()):
+            if argument_type.is_interface_type:

[bashi, 2016/09/23 02:16:05]

why simply do following won't work?

if argument.idl_type.is_interface_type:
  ...

if argument.idl_type could be None, you will need to check it first.

[lkawai, 2016/09/23 02:43:42]

As I defined callback function using array of interface, argument type is
IdlArrayType, not IdlType. Therefore, it needs these writings. Do you have any
other ideas to address this point?

[peria, 2016/09/23 02:50:05]

I recommend to make a utility function which takes an idl_type, and returns a
set of includes.
Then you can check if the idl_type is a container and call it recursively to
work for nested containers.

[bashi, 2016/09/23 02:57:56]

Hmm, I'm not sure why you need to use IdlArrayType here. My understanding is:
- callback_function -> an object of IdlCallbackFunction
- callback_function.arguments -> a list of IdlArgument
- IdlArgument has |idl_type|

If above is correct, you can just do:

for argument in callback_function.arguments:
  if argument.idl_type.is_interface:
    forward_declarations.append(...)

[lkawai, 2016/09/23 05:35:44]

Acknowledged.

[lkawai, 2016/09/23 05:35:44]

Done.

[lkawai, 2016/09/23 05:35:45]

Done.

+                forward_declarations.append('%s' % argument_type.name)

[peria, 2016/09/23 02:50:05]

do we need "'%s' %"?

[lkawai, 2016/09/23 05:35:44]

Done.

+        cpp_includes.update(argument.idl_type.includes_for_type(callback_function.extended_attributes))

[bashi, 2016/09/23 02:16:05]

Could you use idl_type.add_includes_for_type instead of using cpp_includes?

for argument in callback_function.arguments:
  argument.idl_type.add_includes_for_type(callback_function.extended_attributes)

[lkawai, 2016/09/23 02:43:42]

Done.

[peria, 2016/09/23 02:50:05]

Not related to this CL, but I feel the name idl_type.add_includes_for_type() is
confusing, and it effects on v8_global.includes.  I think we need to fix them.

[bashi, 2016/09/23 03:04:05]

Yeah, I totally agree we should avoid using global variables. v8_globals.py
exists for historical reason (it was enough at the beginning of the code
generator and we haven't been encouraged to keep the code generator "healthy").

[lkawai, 2016/09/23 05:35:44]

Acknowledged.

     context = {
         'cpp_class': callback_function.name,
-        'cpp_includes': sorted(CALLBACK_FUNCTION_CPP_INCLUDES),
+        'cpp_includes': sorted(cpp_includes),

[bashi, 2016/09/23 02:16:05]

cpp_includes -> includes

[lkawai, 2016/09/23 02:43:42]

Done.

+        'forward_declarations': forward_declarations,
         'header_includes': sorted(CALLBACK_FUNCTION_H_INCLUDES),
         'idl_type': idl_type_str,
         'return_cpp_type': (idl_type.cpp_type + '&') if idl_type.cpp_type != 'void' else None,
         'return_value': idl_type.v8_value_to_local_cpp_value(
             callback_function.extended_attributes, 'v8ReturnValue', 'cppValue',
             bailout_return_value="false") if idl_type.cpp_type != 'void' else None,
         'v8_class': v8_utilities.v8_class_name(callback_function),
     }
     context.update(arguments_context(callback_function.arguments, context['return_cpp_type']))
     return context
@@ skipping 11 matching lines @@
     argument_declarations = ['ScriptState* scriptState', 'ScriptWrappable* scriptWrappable']
     argument_declarations.extend(
         '%s %s' % (argument.idl_type.callback_cpp_type, argument.name)
         for argument in arguments)
     if return_cpp_type:
         argument_declarations.append('%s returnValue' % return_cpp_type)
     return {
         'argument_declarations': argument_declarations,
         'arguments': [argument_context(argument) for argument in arguments],
     }