Title: [289934] branches/safari-613-branch
Revision
289934
Author
repst...@apple.com
Date
2022-02-16 12:16:11 -0800 (Wed, 16 Feb 2022)

Log Message

Cherry-pick r288795. rdar://problem/88696673

    [WebIDL] Add a sensible error message for a primitive non-null EventListener
    https://bugs.webkit.org/show_bug.cgi?id=235863

    Patch by Alexey Shvayka <ashva...@apple.com> on 2022-01-29
    Reviewed by Darin Adler.

    Source/WebCore:

    This patch moves isObject() check to JSEventListener converter, aligning it with
    callback interface converter and removing now unused create() overload, and makes
    code generator pass an exception thrower with a user-friendly TypeError message.

    Since JSEventListener is quite different from other callback interfaces, we can't
    make IsCallbackInterface() recognize it just yet, thus the type name has to be
    hard-coded if we want the error message to be consistent with other generated ones.

    Test: fast/events/event-listener-not-an-object.html

    * bindings/js/JSDOMConvertEventListener.h:
    (WebCore::Converter<IDLEventListener<T>>::convert):
    * bindings/js/JSEventListener.cpp:
    * bindings/js/JSEventListener.h:
    * bindings/scripts/CodeGeneratorJS.pm:
    (GetArgumentExceptionFunction):
    * bindings/scripts/test/JS/*: Updated.
    * dom/EventTarget.idl:
    Rename EventListener parameter to "listener" to match MDN / Chrome DevTools / matchMedia().

    LayoutTests:

    * fast/events/event-listener-not-an-object-expected.txt: Added.
    * fast/events/event-listener-not-an-object.html: Added.

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@288795 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Added: branches/safari-613-branch/LayoutTests/fast/events/event-listener-not-an-object-expected.txt (0 => 289934)


--- branches/safari-613-branch/LayoutTests/fast/events/event-listener-not-an-object-expected.txt	                        (rev 0)
+++ branches/safari-613-branch/LayoutTests/fast/events/event-listener-not-an-object-expected.txt	2022-02-16 20:16:11 UTC (rev 289934)
@@ -0,0 +1,15 @@
+A TypeError with sensible message should be thrown for a primitive EventListener that is neither null nor undefined
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS () => { (new EventTarget).addEventListener("foo", null); } did not throw exception.
+PASS () => { matchMedia("").removeListener(undefined); } did not throw exception.
+PASS () => { (new EventTarget).addEventListener("foo", 1); } threw exception TypeError: Argument 2 ('listener') to EventTarget.addEventListener must be an object.
+PASS () => { (new EventTarget).removeEventListener("foo", false); } threw exception TypeError: Argument 2 ('listener') to EventTarget.removeEventListener must be an object.
+PASS () => { matchMedia("").addListener(Symbol()); } threw exception TypeError: Argument 1 ('listener') to MediaQueryList.addListener must be an object.
+PASS () => { matchMedia("").removeListener("bar"); } threw exception TypeError: Argument 1 ('listener') to MediaQueryList.removeListener must be an object.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: branches/safari-613-branch/LayoutTests/fast/events/event-listener-not-an-object.html (0 => 289934)


--- branches/safari-613-branch/LayoutTests/fast/events/event-listener-not-an-object.html	                        (rev 0)
+++ branches/safari-613-branch/LayoutTests/fast/events/event-listener-not-an-object.html	2022-02-16 20:16:11 UTC (rev 289934)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<body>
+<script src=""
+<script>
+description("A TypeError with sensible message should be thrown for a primitive EventListener that is neither null nor undefined");
+
+shouldNotThrow(() => { (new EventTarget).addEventListener("foo", null); });
+shouldNotThrow(() => { matchMedia("").removeListener(undefined); });
+
+shouldThrowErrorName(() => { (new EventTarget).addEventListener("foo", 1); }, "TypeError");
+shouldThrowErrorName(() => { (new EventTarget).removeEventListener("foo", false); }, "TypeError");
+shouldThrowErrorName(() => { matchMedia("").addListener(Symbol()); }, "TypeError");
+shouldThrowErrorName(() => { matchMedia("").removeListener("bar"); }, "TypeError");
+</script>
+<script src=""
+</body>

Modified: branches/safari-613-branch/Source/WebCore/bindings/js/JSDOMConvertEventListener.h (289933 => 289934)


--- branches/safari-613-branch/Source/WebCore/bindings/js/JSDOMConvertEventListener.h	2022-02-16 20:00:25 UTC (rev 289933)
+++ branches/safari-613-branch/Source/WebCore/bindings/js/JSDOMConvertEventListener.h	2022-02-16 20:16:11 UTC (rev 289934)
@@ -33,15 +33,18 @@
 template<typename T> struct Converter<IDLEventListener<T>> : DefaultConverter<IDLEventListener<T>> {
     using ReturnType = RefPtr<T>;
 
-    static ReturnType convert(JSC::JSGlobalObject& lexicalGlobalObject, JSC::JSValue value, JSC::JSObject& thisObject)
+    template<typename ExceptionThrower = DefaultExceptionThrower>
+    static ReturnType convert(JSC::JSGlobalObject& lexicalGlobalObject, JSC::JSValue value, JSC::JSObject& thisObject, ExceptionThrower&& exceptionThrower = ExceptionThrower())
     {
         auto scope = DECLARE_THROW_SCOPE(JSC::getVM(&lexicalGlobalObject));
 
-        auto listener = T::create(value, thisObject, false, currentWorld(lexicalGlobalObject));
-        if (!listener)
-            throwTypeError(&lexicalGlobalObject, scope);
-    
-        return listener;
+        if (UNLIKELY(!value.isObject())) {
+            exceptionThrower(lexicalGlobalObject, scope);
+            return nullptr;
+        }
+
+        bool isAttribute = false;
+        return T::create(*asObject(value), thisObject, isAttribute, currentWorld(lexicalGlobalObject));
     }
 };
 

Modified: branches/safari-613-branch/Source/WebCore/bindings/js/JSEventListener.cpp (289933 => 289934)


--- branches/safari-613-branch/Source/WebCore/bindings/js/JSEventListener.cpp	2022-02-16 20:00:25 UTC (rev 289933)
+++ branches/safari-613-branch/Source/WebCore/bindings/js/JSEventListener.cpp	2022-02-16 20:16:11 UTC (rev 289934)
@@ -66,14 +66,6 @@
     return adoptRef(*new JSEventListener(&listener, &wrapper, isAttribute, world));
 }
 
-RefPtr<JSEventListener> JSEventListener::create(JSC::JSValue listener, JSC::JSObject& wrapper, bool isAttribute, DOMWrapperWorld& world)
-{
-    if (UNLIKELY(!listener.isObject()))
-        return nullptr;
-
-    return adoptRef(*new JSEventListener(asObject(listener), &wrapper, isAttribute, world));
-}
-
 JSObject* JSEventListener::initializeJSFunction(ScriptExecutionContext&) const
 {
     return nullptr;

Modified: branches/safari-613-branch/Source/WebCore/bindings/js/JSEventListener.h (289933 => 289934)


--- branches/safari-613-branch/Source/WebCore/bindings/js/JSEventListener.h	2022-02-16 20:00:25 UTC (rev 289933)
+++ branches/safari-613-branch/Source/WebCore/bindings/js/JSEventListener.h	2022-02-16 20:16:11 UTC (rev 289934)
@@ -37,7 +37,6 @@
 class JSEventListener : public EventListener {
 public:
     WEBCORE_EXPORT static Ref<JSEventListener> create(JSC::JSObject& listener, JSC::JSObject& wrapper, bool isAttribute, DOMWrapperWorld&);
-    WEBCORE_EXPORT static RefPtr<JSEventListener> create(JSC::JSValue listener, JSC::JSObject& wrapper, bool isAttribute, DOMWrapperWorld&);
 
     virtual ~JSEventListener();
 

Modified: branches/safari-613-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (289933 => 289934)


--- branches/safari-613-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2022-02-16 20:00:25 UTC (rev 289933)
+++ branches/safari-613-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2022-02-16 20:16:11 UTC (rev 289934)
@@ -1731,7 +1731,7 @@
     my $visibleInterfaceName = $codeGenerator->GetVisibleInterfaceName($interface);
     my $typeName = GetTypeNameForDisplayInException($argument->type);
 
-    if ($codeGenerator->IsCallbackInterface($argument->type)) {
+    if ($codeGenerator->IsCallbackInterface($argument->type) || $argument->type->name eq "EventListener") {
         return "throwArgumentMustBeObjectError(lexicalGlobalObject, scope, ${argumentIndex}, \"${name}\", \"${visibleInterfaceName}\", ${quotedFunctionName});";
     }
 

Modified: branches/safari-613-branch/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp (289933 => 289934)


--- branches/safari-613-branch/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2022-02-16 20:00:25 UTC (rev 289933)
+++ branches/safari-613-branch/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2022-02-16 20:16:11 UTC (rev 289934)
@@ -6168,7 +6168,7 @@
     auto type = convert<IDLDOMString>(*lexicalGlobalObject, argument0.value());
     RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
     EnsureStillAliveScope argument1 = callFrame->uncheckedArgument(1);
-    auto listener = convert<IDLEventListener<JSEventListener>>(*lexicalGlobalObject, argument1.value(), *castedThis);
+    auto listener = convert<IDLEventListener<JSEventListener>>(*lexicalGlobalObject, argument1.value(), *castedThis, [](JSC::JSGlobalObject& lexicalGlobalObject, JSC::ThrowScope& scope) { throwArgumentMustBeObjectError(lexicalGlobalObject, scope, 1, "listener", "TestObject", "addEventListener"); });
     RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
     EnsureStillAliveScope argument2 = callFrame->argument(2);
     auto useCapture = convert<IDLBoolean>(*lexicalGlobalObject, argument2.value());
@@ -6197,7 +6197,7 @@
     auto type = convert<IDLDOMString>(*lexicalGlobalObject, argument0.value());
     RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
     EnsureStillAliveScope argument1 = callFrame->uncheckedArgument(1);
-    auto listener = convert<IDLEventListener<JSEventListener>>(*lexicalGlobalObject, argument1.value(), *castedThis);
+    auto listener = convert<IDLEventListener<JSEventListener>>(*lexicalGlobalObject, argument1.value(), *castedThis, [](JSC::JSGlobalObject& lexicalGlobalObject, JSC::ThrowScope& scope) { throwArgumentMustBeObjectError(lexicalGlobalObject, scope, 1, "listener", "TestObject", "removeEventListener"); });
     RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
     EnsureStillAliveScope argument2 = callFrame->argument(2);
     auto useCapture = convert<IDLBoolean>(*lexicalGlobalObject, argument2.value());

Modified: branches/safari-613-branch/Source/WebCore/dom/EventTarget.idl (289933 => 289934)


--- branches/safari-613-branch/Source/WebCore/dom/EventTarget.idl	2022-02-16 20:00:25 UTC (rev 289933)
+++ branches/safari-613-branch/Source/WebCore/dom/EventTarget.idl	2022-02-16 20:16:11 UTC (rev 289934)
@@ -28,7 +28,7 @@
 ] interface EventTarget {
     [CallWith=ScriptExecutionContext] constructor();
 
-    [ImplementedAs=addEventListenerForBindings] undefined addEventListener([AtomString] DOMString type, EventListener? callback, optional (AddEventListenerOptions or boolean) options = false);
-    [ImplementedAs=removeEventListenerForBindings] undefined removeEventListener([AtomString] DOMString type, EventListener? callback, optional (EventListenerOptions or boolean) options = false);
+    [ImplementedAs=addEventListenerForBindings] undefined addEventListener([AtomString] DOMString type, EventListener? listener, optional (AddEventListenerOptions or boolean) options = false);
+    [ImplementedAs=removeEventListenerForBindings] undefined removeEventListener([AtomString] DOMString type, EventListener? listener, optional (EventListenerOptions or boolean) options = false);
     [ImplementedAs=dispatchEventForBindings] boolean dispatchEvent(Event event);
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to