Bug 148434 – Distinguish Web IDL callback interfaces from Web IDL callback functions

• Component: Bindings
• Status: RESOLVED FIXED

[Chris Dumez]

Distinguish Web IDL callback interfaces [2] from Web IDL callback functions [2].

One Web Exposed difference is that when using a callback interface, the user can pass either a function / callable object or a non-callable object that has a method with the same name as the callback interface operation:
https://heycam.github.io/webidl/#es-user-objects

When using a callback function, the user needs to pass a function / callable object:
https://heycam.github.io/webidl/#es-callback-function

[1] https://heycam.github.io/webidl/#dfn-callback-interface
[2] https://heycam.github.io/webidl/#idl-callback-functions

[Chris Dumez]

Currently, it seems our bindings code enforces that callback arguments are Functions:
EncodedJSValue JSC_HOST_CALL jsDataTransferItemPrototypeFunctionGetAsString(ExecState* exec)
{
    JSValue thisValue = exec->thisValue();
    JSDataTransferItem* castedThis = jsDynamicCast<JSDataTransferItem*>(thisValue);
    if (UNLIKELY(!castedThis))
        return throwThisTypeError(*exec, "DataTransferItem", "getAsString");
    ASSERT_GC_OBJECT_INHERITS(castedThis, JSDataTransferItem::info());
    auto& impl = castedThis->impl();
    RefPtr<StringCallback> callback;
    if (!exec->argument(0).isUndefinedOrNull()) {
        if (!exec->uncheckedArgument(0).isFunction())
            return throwArgumentMustBeFunctionError(*exec, 0, "callback", "DataTransferItem", "getAsString");
        callback = JSStringCallback::create(asObject(exec->uncheckedArgument(0)), castedThis->globalObject());
    }
    impl.getAsString(callback);
    return JSValue::encode(jsUndefined());
}

Corresponding spec: https://html.spec.whatwg.org/multipage/interaction.html#the-datatransferitem-interface

We have no distinction between callback functions and callback interfaces. We basically only support callback functions even though they are specified as callback interfaces in our IDL.

[Chris Dumez]

Created attachment 259881 [details]
Patch

[WebKit Commit Bot]

Attachment 259881 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.cpp:25:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.cpp:83:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/bindings/scripts/test/ObjC/DOMTestCallbackFunction.mm:33:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.h:46:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 4 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 259881 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=259881&action=review

> Source/WebCore/bindings/js/JSCallbackData.h:69
> +    enum class InvokeMethod { FunctionOnly, NonCallableObjectOnly, FunctionAndNonCallableObject };

enum class CallbackType { Function, Object, FunctionOrObject };

[Chris Dumez]

Created attachment 259883 [details]
Patch

[WebKit Commit Bot]

Attachment 259883 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.cpp:25:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.cpp:83:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/bindings/scripts/test/ObjC/DOMTestCallbackFunction.mm:33:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.h:46:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 4 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 259883 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 259883 [details]
Patch

Clearing flags on attachment: 259883

Committed r188994: <http://trac.webkit.org/changeset/188994>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.