LibWeb/Bindings: Implement callback interface object bindings

Generate correct bindings for callback interfaces: only create an
interface object when the interface declares constants, and set up
the prototype correctly.

This also lets us tidy up some IDL for these callback interfaces.

Libraries/LibWeb/CSS/MediaQueryList.idl

@@ -1,5 +1,6 @@
 #import <DOM/EventTarget.idl>
 #import <DOM/EventHandler.idl>
+#import <DOM/EventListener.idl>
 
 // https://drafts.csswg.org/cssom-view/#the-mediaquerylist-interface
 [Exposed=Window]

Libraries/LibWeb/DOM/Document.idl

@@ -31,6 +31,7 @@
 #import <ViewTransition/ViewTransition.idl>
 #import <XPath/XPathResult.idl>
 #import <XPath/XPathExpression.idl>
+#import <XPath/XPathNSResolver.idl>
 
 // https://dom.spec.whatwg.org/#document
 // https://html.spec.whatwg.org/multipage/dom.html#the-document-object

Libraries/LibWeb/DOM/EventListener.idl

@@ -0,0 +1,7 @@
+#import <DOM/Event.idl>
+
+// https://dom.spec.whatwg.org/#callbackdef-eventlistener
+[Exposed=*, ImplementedAs=IDLEventListener]
+callback interface EventListener {
+    undefined handleEvent(Event event);
+};

Libraries/LibWeb/DOM/EventTarget.idl

@@ -1,4 +1,5 @@
 #import <DOM/AbortSignal.idl>
+#import <DOM/EventListener.idl>
 
 // https://dom.spec.whatwg.org/#eventtarget
 [Exposed=*]
@@ -10,11 +11,6 @@ interface EventTarget {
     [ImplementedAs=dispatch_event_binding] boolean dispatchEvent(Event event);
 };
 
-// FIXME: support callback interface
-//callback interface EventListener {
-//    undefined handleEvent(Event event);
-//};
-
 dictionary EventListenerOptions {
     boolean capture = false;
 };

Libraries/LibWeb/DOM/NodeFilter.cpp

@@ -13,12 +13,12 @@ namespace Web::DOM {
 
 GC_DEFINE_ALLOCATOR(NodeFilter);
 
-GC::Ref<NodeFilter> NodeFilter::create(JS::Realm& realm, WebIDL::CallbackType& callback)
+GC::Ref<NodeFilter> NodeFilter::create(JS::Realm& realm, GC::Ref<WebIDL::CallbackType> callback)
 {
     return realm.create<NodeFilter>(realm, callback);
 }
 
-NodeFilter::NodeFilter(JS::Realm& realm, WebIDL::CallbackType& callback)
+NodeFilter::NodeFilter(JS::Realm& realm, GC::Ref<WebIDL::CallbackType> callback)
     : PlatformObject(realm.intrinsics().object_prototype())
     , m_callback(callback)
 {

Libraries/LibWeb/DOM/NodeFilter.h

@@ -16,7 +16,7 @@ class NodeFilter final : public Bindings::PlatformObject {
     GC_DECLARE_ALLOCATOR(NodeFilter);
 
 public:
-    [[nodiscard]] static GC::Ref<NodeFilter> create(JS::Realm&, WebIDL::CallbackType&);
+    [[nodiscard]] static GC::Ref<NodeFilter> create(JS::Realm&, GC::Ref<WebIDL::CallbackType>);
 
     virtual ~NodeFilter() = default;
 
@@ -43,7 +43,7 @@ public:
     };
 
 private:
-    NodeFilter(JS::Realm&, WebIDL::CallbackType&);
+    NodeFilter(JS::Realm&, GC::Ref<WebIDL::CallbackType>);
 
     virtual void visit_edges(Cell::Visitor&) override;
 

Libraries/LibWeb/DOM/NodeFilter.idl

@@ -2,10 +2,7 @@
 
 // https://dom.spec.whatwg.org/#callbackdef-nodefilter
 [Exposed=Window]
-interface NodeFilter {
-
-    // FIXME: This should be a callback interface.
-
+callback interface NodeFilter {
     // Constants for acceptNode()
     const unsigned short FILTER_ACCEPT = 1;
     const unsigned short FILTER_REJECT = 2;
@@ -26,7 +23,5 @@ interface NodeFilter {
     const unsigned long SHOW_DOCUMENT_FRAGMENT = 0x400;
     const unsigned long SHOW_NOTATION = 0x800; // legacy
 
-    // FIXME: Uncomment this once NodeFilter is a callback interface.
-    // unsigned short acceptNode(Node node);
-
+    unsigned short acceptNode(Node node);
 };

Libraries/LibWeb/DOM/NodeIterator.cpp

@@ -138,12 +138,9 @@ JS::ThrowCompletionOr<GC::Ptr<Node>> NodeIterator::traverse(Direction direction)
 }
 
 // https://dom.spec.whatwg.org/#concept-traversal-filter
-JS::Object* NodeIterator::filter() const
+GC::Ptr<NodeFilter> NodeIterator::filter() const
 {
-    if (!m_filter)
-        return nullptr;
-
-    return m_filter->callback().callback;
+    return m_filter;
 }
 
 // https://dom.spec.whatwg.org/#concept-node-filter

Libraries/LibWeb/DOM/NodeIterator.h

@@ -28,7 +28,7 @@ public:
     bool pointer_before_reference_node() const { return m_reference.is_before_node; }
     unsigned what_to_show() const { return m_what_to_show; }
 
-    JS::Object* filter() const;
+    GC::Ptr<NodeFilter> filter() const;
 
     JS::ThrowCompletionOr<GC::Ptr<Node>> next_node();
     JS::ThrowCompletionOr<GC::Ptr<Node>> previous_node();

Libraries/LibWeb/DOM/NodeIterator.idl

@@ -9,8 +9,7 @@ interface NodeIterator {
     readonly attribute boolean pointerBeforeReferenceNode;
     readonly attribute unsigned long whatToShow;
 
-    // FIXME: `object?` should be `NodeFilter?`, but we don't yet properly support callback interfaces.
-    readonly attribute object? filter;
+    readonly attribute NodeFilter? filter;
 
     Node? nextNode();
     Node? previousNode();

Libraries/LibWeb/DOM/TreeWalker.cpp

@@ -243,12 +243,9 @@ JS::ThrowCompletionOr<GC::Ptr<Node>> TreeWalker::next_node()
 }
 
 // https://dom.spec.whatwg.org/#concept-traversal-filter
-JS::Object* TreeWalker::filter() const
+GC::Ptr<NodeFilter> TreeWalker::filter() const
 {
-    if (!m_filter)
-        return nullptr;
-
-    return m_filter->callback().callback;
+    return m_filter;
 }
 
 // https://dom.spec.whatwg.org/#concept-node-filter

Libraries/LibWeb/DOM/TreeWalker.h

@@ -33,7 +33,7 @@ public:
 
     GC::Ref<Node> root() { return m_root; }
 
-    JS::Object* filter() const;
+    GC::Ptr<NodeFilter> filter() const;
 
     unsigned what_to_show() const { return m_what_to_show; }
 

Libraries/LibWeb/DOM/TreeWalker.idl

@@ -7,8 +7,7 @@ interface TreeWalker {
     [SameObject] readonly attribute Node root;
     readonly attribute unsigned long whatToShow;
 
-    // FIXME: `object?` should be `NodeFilter?`, but we don't yet properly support callback interfaces.
-    readonly attribute object? filter;
+    readonly attribute NodeFilter? filter;
 
     attribute Node currentNode;
 

Libraries/LibWeb/XPath/XPathEvaluator.idl

@@ -1,9 +1,5 @@
 #import <DOM/Node.idl>
-
-// FIXME: callback interfaces are not currently supported
-// callback interface XPathNSResolver {
-//   DOMString? lookupNamespaceURI(DOMString? prefix);
-// };
+#import <XPath/XPathNSResolver.idl>
 
 // https://dom.spec.whatwg.org/#mixin-xpathevaluatorbase
 interface mixin XPathEvaluatorBase {
@@ -21,4 +17,3 @@ interface XPathEvaluator {
 };
 
 XPathEvaluator includes XPathEvaluatorBase;
-

Libraries/LibWeb/XPath/XPathNSResolver.h

@@ -23,6 +23,7 @@ public:
 
     virtual ~XPathNSResolver() = default;
     virtual void visit_edges(Cell::Visitor&) override;
+    WebIDL::CallbackType& callback() { return *m_callback; }
 
 private:
     GC::Ref<WebIDL::CallbackType> m_callback;

Libraries/LibWeb/XPath/XPathNSResolver.idl

@@ -0,0 +1,5 @@
+// https://dom.spec.whatwg.org/#callbackdef-xpathnsresolver
+[Exposed=Window]
+callback interface XPathNSResolver {
+    DOMString? lookupNamespaceURI(DOMString? prefix);
+};

Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp

@@ -206,6 +206,14 @@ static bool is_javascript_builtin(Type const& type)
     return types.span().contains_slow(type.name());
 }
 
+static Interface const* callback_interface_for_type(Interface const& interface, Type const& type)
+{
+    auto const* referenced_interface = interface.referenced_interface(type.name());
+    if (referenced_interface && referenced_interface->is_callback_interface)
+        return referenced_interface;
+    return nullptr;
+}
+
 static StringView sequence_storage_type_to_cpp_storage_type_name(SequenceStorageType sequence_storage_type)
 {
     switch (sequence_storage_type) {
@@ -261,6 +269,9 @@ CppType idl_type_name_to_cpp_type(Type const& type, Interface const& interface)
     if (is_javascript_builtin(type))
         return { .name = ByteString::formatted("GC::Root<JS::{}>", type.name()), .sequence_storage_type = SequenceStorageType::RootVector };
 
+    if (auto const* callback_interface = callback_interface_for_type(interface, type))
+        return { .name = ByteString::formatted("GC::Root<{}>", callback_interface->implemented_name), .sequence_storage_type = SequenceStorageType::RootVector };
+
     if (interface.callback_functions.contains(type.name()))
         return { .name = "GC::Root<WebIDL::CallbackType>", .sequence_storage_type = SequenceStorageType::RootVector };
 
@@ -699,13 +710,8 @@ static void generate_to_cpp(SourceGenerator& generator, ParameterType& parameter
         generate_to_string(scoped_generator, parameter, variadic, optional, optional_default_value);
     } else if (parameter.type->is_boolean() || parameter.type->is_integer()) {
         generate_to_integral(scoped_generator, parameter, optional, optional_default_value);
-    } else if (parameter.type->name().is_one_of("EventListener", "NodeFilter", "XPathNSResolver")) {
-        // FIXME: Replace this with support for callback interfaces. https://webidl.spec.whatwg.org/#idl-callback-interface
-
-        if (parameter.type->name() == "EventListener")
-            scoped_generator.set("cpp_type", "IDLEventListener");
-        else
-            scoped_generator.set("cpp_type", parameter.type->name());
+    } else if (auto const* callback_interface = callback_interface_for_type(interface, parameter.type)) {
+        scoped_generator.set("cpp_type", callback_interface->implemented_name);
 
         if (parameter.type->is_nullable()) {
             scoped_generator.append(R"~~~(
@@ -715,7 +721,7 @@ static void generate_to_cpp(SourceGenerator& generator, ParameterType& parameter
             return vm.throw_completion<JS::TypeError>(JS::ErrorType::NotAnObject, @js_name@@js_suffix@);
 
         auto callback_type = vm.heap().allocate<WebIDL::CallbackType>(@js_name@@js_suffix@.as_object(), HTML::incumbent_realm());
-        @cpp_name@ = TRY(throw_dom_exception_if_needed(vm, [&] { return @cpp_type@::create(realm, *callback_type); }));
+        @cpp_name@ = TRY(throw_dom_exception_if_needed(vm, [&] { return @cpp_type@::create(realm, callback_type); }));
     }
 )~~~");
         } else {
@@ -724,7 +730,7 @@ static void generate_to_cpp(SourceGenerator& generator, ParameterType& parameter
         return vm.throw_completion<JS::TypeError>(JS::ErrorType::NotAnObject, @js_name@@js_suffix@);
 
     auto callback_type = vm.heap().allocate<WebIDL::CallbackType>(@js_name@@js_suffix@.as_object(), HTML::incumbent_realm());
-    auto @cpp_name@ = adopt_ref(*new @cpp_type@(callback_type));
+    auto @cpp_name@ = TRY(throw_dom_exception_if_needed(vm, [&] { return @cpp_type@::create(realm, callback_type); }));
 )~~~");
         }
     } else if (IDL::is_platform_object(*parameter.type)) {
@@ -1563,7 +1569,19 @@ static void generate_to_cpp(SourceGenerator& generator, ParameterType& parameter
 )~~~");
         }
 
-        // FIXME: 5. If types includes a callback interface type, then return the result of converting V to that callback interface type.
+        // 5. If types includes a callback interface type, then return the result of converting V to that callback interface type.
+        for (auto& type : types) {
+            if (!callback_interface_for_type(interface, type))
+                continue;
+
+            IDL::Parameter callback_interface_parameter { .type = *type, .name = acceptable_cpp_name, .optional_default_value = {}, .extended_attributes = {} };
+            generate_to_cpp(union_generator, callback_interface_parameter, js_name, js_suffix, "callback_interface_union_type"sv, interface, false, false, {}, false, recursion_depth + 1);
+
+            union_generator.append(R"~~~(
+        return callback_interface_union_type;
+)~~~");
+            break;
+        }
 
         // 6. If types includes object, then return the IDL value that is a reference to the object V.
         if (includes_object) {
@@ -2216,6 +2234,10 @@ static void generate_wrap_statement(SourceGenerator& generator, ByteString const
   @result_expression@ @value_non_optional@->callback;
 )~~~");
         }
+    } else if (callback_interface_for_type(interface, type)) {
+        scoped_generator.append(R"~~~(
+  @result_expression@ @value@->callback().callback;
+)~~~");
     } else if (interface.dictionaries.contains(type.name())) {
         // https://webidl.spec.whatwg.org/#es-dictionary
         auto dictionary_generator = scoped_generator.fork();
@@ -3090,7 +3112,6 @@ JS::ThrowCompletionOr<GC::Ref<JS::Object>> @constructor_class@::construct([[mayb
 {
     WebIDL::log_trace(vm(), "@constructor_class@::construct");
 )~~~");
-        generator.set("constructor.length", "0");
         generator.append(R"~~~(
     return vm().throw_completion<JS::TypeError>(JS::ErrorType::NotAConstructor, "@namespaced_name@");
 }
@@ -4108,6 +4129,9 @@ static void generate_prototype_or_global_mixin_definitions(IDL::Interface const&
         generator.set("iterator_name", ByteString::formatted("{}Iterator", interface.name));
     }
 
+    if (interface.is_callback_interface)
+        return;
+
     if (!interface.attributes.is_empty() || !interface.functions.is_empty() || interface.has_stringifier || interface.set_entry_type.has_value() || interface.map_key_type.has_value()) {
         generator.append(R"~~~(
 [[maybe_unused]] static JS::ThrowCompletionOr<@fully_qualified_name@*> impl_from(JS::VM& vm)
@@ -5580,11 +5604,16 @@ public:
     virtual ~@constructor_class@() override;
 
     virtual JS::ThrowCompletionOr<JS::Value> call() override;
+)~~~");
+
+    if (!interface.is_callback_interface) {
+        generator.append(R"~~~(
     virtual JS::ThrowCompletionOr<GC::Ref<JS::Object>> construct(JS::FunctionObject& new_target) override;
 
 private:
     virtual bool has_constructor() const override { return true; }
 )~~~");
+    }
 
     for (auto& attribute : interface.static_attributes) {
         auto attribute_generator = generator.fork();
@@ -5646,6 +5675,7 @@ void generate_constructor_implementation(IDL::Interface const& interface, String
     generator.set("fully_qualified_name", interface.fully_qualified_name);
     generator.set("parent_name", interface.parent_name);
     generator.set("prototype_base_class", interface.prototype_base_class);
+    generator.set("constructor.length", "0");
 
     generator.append(R"~~~(
 #include <LibIDL/Types.h>
@@ -5715,7 +5745,8 @@ JS::ThrowCompletionOr<JS::Value> @constructor_class@::call()
 
 )~~~");
 
-    generate_constructors(generator, interface);
+    if (!interface.is_callback_interface)
+        generate_constructors(generator, interface);
 
     generator.append(R"~~~(
 
@@ -5727,7 +5758,7 @@ void @constructor_class@::initialize(JS::Realm& realm)
     Base::initialize(realm);
 )~~~");
 
-    if (interface.prototype_base_class != "ObjectPrototype") {
+    if (!interface.is_callback_interface && interface.prototype_base_class != "ObjectPrototype") {
         generator.append(R"~~~(
     set_prototype(&ensure_web_constructor<@prototype_base_class@>(realm, "@parent_name@"_fly_string));
 )~~~");
@@ -5736,10 +5767,14 @@ void @constructor_class@::initialize(JS::Realm& realm)
     generator.append(R"~~~(
     define_direct_property(vm.names.length, JS::Value(@constructor.length@), JS::Attribute::Configurable);
     define_direct_property(vm.names.name, JS::PrimitiveString::create(vm, "@name@"_string), JS::Attribute::Configurable);
-    define_direct_property(vm.names.prototype, &ensure_web_prototype<@prototype_class@>(realm, "@namespaced_name@"_fly_string), 0);
-
 )~~~");
 
+    if (!interface.is_callback_interface) {
+        generator.append(R"~~~(
+    define_direct_property(vm.names.prototype, &ensure_web_prototype<@prototype_class@>(realm, "@namespaced_name@"_fly_string), 0);
+)~~~");
+    }
+
     for (auto& constant : interface.constants) {
         auto constant_generator = generator.fork();
         constant_generator.set("constant.name", constant.name);
@@ -6008,6 +6043,14 @@ void @prototype_class@::initialize(JS::Realm& realm)
 )~~~");
         if (interface.supports_named_properties())
             generate_named_properties_object_definitions(interface, builder);
+    } else if (interface.is_callback_interface) {
+        generator.append(R"~~~(
+void @prototype_class@::initialize(JS::Realm& realm)
+{
+    Base::initialize(realm);
+    set_prototype(realm.intrinsics().object_prototype());
+}
+)~~~");
     } else {
         generate_prototype_or_global_mixin_initialization(interface, builder, GenerateUnforgeables::No);
         generate_prototype_or_global_mixin_initialization(interface, builder, GenerateUnforgeables::Yes);

Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateWindowOrWorkerInterfaces.cpp

@@ -69,6 +69,13 @@ static Optional<LegacyConstructor> const& lookup_legacy_constructor(IDL::Interfa
     return s_legacy_constructors.get(interface.name).value();
 }
 
+static bool should_have_interface_object(IDL::Interface const& interface)
+{
+    if (interface.is_callback_interface)
+        return !interface.constants.is_empty();
+    return true;
+}
+
 static ErrorOr<void> generate_intrinsic_definitions_header(StringView output_path, InterfaceSets const& interface_sets)
 {
     StringBuilder builder;
@@ -437,6 +444,9 @@ void add_@global_object_snake_name@_exposed_interfaces(JS::Object& global)
 )~~~");
 
     auto add_interface = [class_name](SourceGenerator& gen, IDL::Interface const& interface) {
+        if (!should_have_interface_object(interface))
+            return;
+
         auto legacy_constructor = lookup_legacy_constructor(interface);
         Optional<ByteString const&> legacy_alias_name;
         if (class_name == "Window"sv)

Tests/LibWeb/Text/expected/all-window-properties.txt

@@ -316,7 +316,6 @@ NavigationHistoryEntry
 NavigationTransition
 Navigator
 Node
-NodeFilter
 NodeIterator
 NodeList
 Notification

Tests/LibWeb/Text/expected/wpt-import/webidl/ecmascript-binding/legacy-callback-interface-object.txt

@@ -2,13 +2,12 @@ Harness status: OK
 
 Found 8 tests
 
-6 Pass
-2 Fail
+8 Pass
 Pass	Must be a function according to typeof
 Pass	Must have the correct [[Prototype]]
 Pass	Must have the correct property descriptor
 Pass	Must throw a TypeError when called or constructed
-Fail	Must not have a .prototype property
+Pass	Must not have a .prototype property
 Pass	Must have an own name property equal to the interface name and with the correct descriptors
 Pass	Must have an own length property with value zero and the correct descriptors
-Fail	instanceof must throw but only when we don't bail out early
+Pass	instanceof must throw but only when we don't bail out early