From f562073fe2ebf14cc0d9b5c8f5c93f617ccea91e Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Thu, 19 Feb 2015 23:09:51 -0800 Subject: [PATCH] Make it possible to inherit from any constructable host object This CL makes it possible for authors to extend any host object (e.g., DOM objects) and to use those objects in all the usual places they can be used in the API. R=esprehn@chromium.org Review URL: https://codereview.chromium.org/936193005 --- engine/bindings/scripts/dart_methods.py | 2 +- .../scripts/templates/interface_dart.template | 11 ++++--- .../scripts/templates/methods_cpp.template | 8 +++-- engine/tonic/dart_wrappable.cc | 23 ++++++++++++++ engine/tonic/dart_wrappable.h | 3 +- tests/dom/inherit-from-text-expected.txt | 6 ++++ tests/dom/inherit-from-text.sky | 31 +++++++++++++++++++ 7 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 tests/dom/inherit-from-text-expected.txt create mode 100644 tests/dom/inherit-from-text.sky diff --git a/engine/bindings/scripts/dart_methods.py b/engine/bindings/scripts/dart_methods.py index 014f28b7b49..3ce01527852 100644 --- a/engine/bindings/scripts/dart_methods.py +++ b/engine/bindings/scripts/dart_methods.py @@ -106,7 +106,7 @@ def argument_context(interface, method, argument, index): idl_type = argument.idl_type this_cpp_value = cpp_value(interface, method, index) auto_scope = not 'DartNoAutoScope' in extended_attributes - arg_index = index + 1 if not (method.is_static or method.is_constructor) else index + arg_index = index + 1 if not method.is_static else index preprocessed_type = str(idl_type.preprocessed_type) local_cpp_type = idl_type.cpp_type_args(argument.extended_attributes, raw_type=True) default_value = argument.default_cpp_value diff --git a/engine/bindings/scripts/templates/interface_dart.template b/engine/bindings/scripts/templates/interface_dart.template index 8c20f30844e..b4c66a245f2 100644 --- a/engine/bindings/scripts/templates/interface_dart.template +++ b/engine/bindings/scripts/templates/interface_dart.template @@ -19,16 +19,17 @@ part of sky.core; {%- endfor -%} {%- endmacro -%} -abstract class {{interface_name}} extends {{ parent_interface if parent_interface else 'NativeFieldWrapperClass2' }} { +{% if not constructors %}abstract {% endif -%} +class {{interface_name}} extends {{ parent_interface if parent_interface else 'NativeFieldWrapperClass2' }} { // Constructors {# TODO(eseidel): We only ever have one constructor. #} -{%- for constructor in constructors -%} - static {{interface_name}} _constructor({{ args_macro(constructor.arguments) }}) native "{{interface_name}}_constructorCallback"; - factory {{interface_name}}({{ args_macro(constructor.arguments) }}) => _constructor( +{%- for constructor in constructors %} + void _constructor({{ args_macro(constructor.arguments) }}) native "{{interface_name}}_constructorCallback"; + {{interface_name}}({{ args_macro(constructor.arguments) }}) { _constructor( {%- for arg in constructor.arguments -%} {{ arg.name }}{% if not loop.last %}, {% endif %} {%- endfor -%} - ); + ); } {% endfor %} // Attributes diff --git a/engine/bindings/scripts/templates/methods_cpp.template b/engine/bindings/scripts/templates/methods_cpp.template index aac59cd3b93..aa32f392b27 100644 --- a/engine/bindings/scripts/templates/methods_cpp.template +++ b/engine/bindings/scripts/templates/methods_cpp.template @@ -335,7 +335,7 @@ static void {{static_method_name(constructor.name, overload_index)}}(Dart_Native ExceptionState es; {% endif %} {{generate_arguments(constructor) | indent(8)}} - {{callback_return(constructor, constructor.dart_set_return_value, constructor.cpp_value) | indent(8)}} + {{constructor.cpp_value}}->AssociateWithDartWrapper(args); {% if constructor.has_exception_state %} if (es.had_exception()) { exception = es.GetDartException(args, {{constructor.auto_scope}}); @@ -368,6 +368,8 @@ static void eventConstructorCallback(Dart_NativeArguments args) {% macro generate_resolver_constructor(dart_class, class_name, constructor) %} {% for native_entry in constructor.native_entries %} {% set resolver_string = native_entry.resolver_string %} +{% set args_one_based = constructor.number_of_arguments + 1 %} +{% set args_required_one_based = constructor.number_of_required_arguments + 1 %} {% if constructor.overload_index %} {% set constructor_name = static_method_name(constructor.name) + "Dispatcher" %} {% else %} @@ -376,9 +378,9 @@ static void eventConstructorCallback(Dart_NativeArguments args) {% if has_custom_constructor %} if (name == "{{resolver_string}}") { {% elif constructor.number_of_arguments == constructor.number_of_required_arguments %} -if (argumentCount == {{constructor.number_of_arguments}} && name == "{{resolver_string}}") { +if (argumentCount == {{args_one_based}} && name == "{{resolver_string}}") { {% else %} -if (argumentCount >= {{constructor.number_of_required_arguments}} && argumentCount <= {{constructor.number_of_arguments}} && name == "{{resolver_string}}") { +if (argumentCount >= {{args_required_one_based}} && argumentCount <= {{args_one_based}} && name == "{{resolver_string}}") { {% endif %} *autoSetupScope = {{constructor.auto_scope}}; return {{dart_class}}Internal::{{constructor_name}}; diff --git a/engine/tonic/dart_wrappable.cc b/engine/tonic/dart_wrappable.cc index c52a27ac80b..c04bc1ea2f0 100644 --- a/engine/tonic/dart_wrappable.cc +++ b/engine/tonic/dart_wrappable.cc @@ -41,6 +41,29 @@ Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { return wrapper; } +void DartWrappable::AssociateWithDartWrapper(Dart_NativeArguments args) { + DCHECK(!dart_wrapper_); + + Dart_Handle wrapper = Dart_GetNativeArgument(args, 0); + CHECK(!LogIfError(wrapper)); + + intptr_t native_fields[kNumberOfNativeFields]; + CHECK(!LogIfError(Dart_GetNativeFieldsOfArgument( + args, 0, kNumberOfNativeFields, native_fields))); + CHECK(!native_fields[kPeerIndex]); + CHECK(!native_fields[kWrapperInfoIndex]); + + const DartWrapperInfo& info = GetDartWrapperInfo(); + CHECK(!LogIfError(Dart_SetNativeInstanceField( + wrapper, kPeerIndex, reinterpret_cast(this)))); + CHECK(!LogIfError(Dart_SetNativeInstanceField( + wrapper, kWrapperInfoIndex, reinterpret_cast(&info)))); + + info.ref_object(this); // Balanced in FinalizeDartWrapper. + dart_wrapper_ = Dart_NewPrologueWeakPersistentHandle( + wrapper, this, info.size_in_bytes, &FinalizeDartWrapper); +} + void DartWrappable::FinalizeDartWrapper(void* isolate_callback_data, Dart_WeakPersistentHandle wrapper, void* peer) { diff --git a/engine/tonic/dart_wrappable.h b/engine/tonic/dart_wrappable.h index f070d7f5057..687a07ceb2e 100644 --- a/engine/tonic/dart_wrappable.h +++ b/engine/tonic/dart_wrappable.h @@ -40,6 +40,7 @@ class DartWrappable { virtual void AcceptDartGCVisitor(DartGCVisitor& visitor) const; Dart_Handle CreateDartWrapper(DartState* dart_state); + void AssociateWithDartWrapper(Dart_NativeArguments args); Dart_WeakPersistentHandle dart_wrapper() const { return dart_wrapper_; } protected: @@ -133,7 +134,7 @@ struct DartConverter> { }; template -static T* GetReceiver(Dart_NativeArguments args) { +inline T* GetReceiver(Dart_NativeArguments args) { intptr_t receiver; Dart_Handle result = Dart_GetNativeReceiver(args, &receiver); DCHECK(!Dart_IsError(result)); diff --git a/tests/dom/inherit-from-text-expected.txt b/tests/dom/inherit-from-text-expected.txt new file mode 100644 index 00000000000..879dff8b29a --- /dev/null +++ b/tests/dom/inherit-from-text-expected.txt @@ -0,0 +1,6 @@ +CONSOLE: unittest-suite-wait-for-done +CONSOLE: PASS: should be able to insert in DOM +CONSOLE: +CONSOLE: All 1 tests passed. +CONSOLE: unittest-suite-success +DONE diff --git a/tests/dom/inherit-from-text.sky b/tests/dom/inherit-from-text.sky new file mode 100644 index 00000000000..ffede0634a2 --- /dev/null +++ b/tests/dom/inherit-from-text.sky @@ -0,0 +1,31 @@ + + +