Bug 202326 – [IDL] Support record<DOMString, *Callback> in bindings

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Wenson Hsieh]

This currently results in a compilation error.

[Wenson Hsieh]

Example: https://www.w3.org/TR/clipboard-apis/#dom-clipboarditem-createdelayed

[Wenson Hsieh]

(In reply to Wenson Hsieh from comment #0)
> This currently results in a compilation error.

(Here’s the error, as observed on my machine:)

In file included from /Volumes/main/Users/whsieh/Build/Release/DerivedSources/WebCore/unified-sources/UnifiedSource15.cpp:1:
In file included from /Volumes/main/Users/whsieh/Build/Release/DerivedSources/WebCore/JSClipboardItem.cpp:24:
In file included from /Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/DOMPromiseProxy.h:30:
In file included from /Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h:29:
In file included from /Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/JSDOMConvert.h:43:
/Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/JSDOMConvertRecord.h:126:35: error: no matching function for call to 'convert'
                auto typedValue = Converter<V>::convert(state, subValue, args...);
/Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/JSDOMConvertRecord.h:74:16: note: in instantiation of function template specialization 'WebCore::Converter<WebCore::IDLRecord<WebCore::IDLDOMString, WebCore::IDLCallbackFunction<WebCore::JSClipboardItemDelayedCallback> > >::convertRecord<>' requested here
        return convertRecord(state, value);
In file included from /Volumes/main/Users/whsieh/Build/Release/DerivedSources/WebCore/unified-sources/UnifiedSource15.cpp:1:
In file included from /Volumes/main/Users/whsieh/Build/Release/DerivedSources/WebCore/JSClipboardItem.cpp:22:
In file included from /Volumes/main/Users/whsieh/Build/Release/DerivedSources/WebCore/JSClipboardItem.h:24:
In file included from /Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/JSDOMConvertDictionary.h:29:
/Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/JSDOMConvertBase.h:61:26: note: in instantiation of member function 'WebCore::Converter<WebCore::IDLRecord<WebCore::IDLDOMString, WebCore::IDLCallbackFunction<WebCore::JSClipboardItemDelayedCallback> > >::convert' requested here
    return Converter<T>::convert(state, value);
In file included from /Volumes/main/Users/whsieh/Build/Release/DerivedSources/WebCore/unified-sources/UnifiedSource15.cpp:1:
/Volumes/main/Users/whsieh/Build/Release/DerivedSources/WebCore/JSClipboardItem.cpp:338:18: note: in instantiation of function template specialization 'WebCore::convert<WebCore::IDLRecord<WebCore::IDLDOMString, WebCore::IDLCallbackFunction<WebCore::JSClipboardItemDelayedCallback> > >' requested here
    auto items = convert<IDLRecord<IDLDOMString, IDLCallbackFunction<JSClipboardItemDelayedCallback>>>(*state, state->uncheckedArgument(0));
In file included from /Volumes/main/Users/whsieh/Build/Release/DerivedSources/WebCore/unified-sources/UnifiedSource15.cpp:1:
In file included from /Volumes/main/Users/whsieh/Build/Release/DerivedSources/WebCore/JSClipboardItem.cpp:24:
In file included from /Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/DOMPromiseProxy.h:30:
In file included from /Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h:29:
In file included from /Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/JSDOMConvert.h:31:
/Users/whsieh/work/OpenSource/Source/WebCore/bindings/js/JSDOMConvertCallbacks.h:38:22: note: candidate function template not viable: requires at least 3 arguments, but 2 were provided
    static RefPtr<T> convert(JSC::ExecState& state, JSC::JSValue value, JSDOMGlobalObject& globalObject, ExceptionThrower&& exceptionThrower = ExceptionThrower())
1 error generated.

[Wenson Hsieh]

Created attachment 379753 [details]
WIP

[Wenson Hsieh]

Created attachment 379755 [details]
Patch

[Sam Weinig]

Comment on attachment 379755 [details]
Patch

Looks good. Out of curiosity, do we handle *Callback (or any other JSDOMGlobalObject requiring type) in other aggregate and / or parametrized types (e.g. sequence<>, Promise<>, FrozenArray<>, union, etc.)

[Wenson Hsieh]

Thanks for the review!

(In reply to Sam Weinig from comment #5)
> Comment on attachment 379755 [details]
> Patch
> 
> Looks good. Out of curiosity, do we handle *Callback (or any other
> JSDOMGlobalObject requiring type) in other aggregate and / or parametrized
> types (e.g. sequence<>, Promise<>, FrozenArray<>, union, etc.)

From what I could tell, we don’t have any precedent for “*Callback” being used as the type parameter for sequence, Promise, FrozenArray, or union. The quick test of introducing this in an IDL somewhere:

    `void doThing(sequence<VoidCallback> callbacks);`

…results in pretty much the same error as I encountered here, where compilation fails when attempting to convert the callback object without the global object.

The implementation of JSValueToNativeDOMConvertNeedsGlobalObject seems to suggest that callbacks are also unique, w.r.t. requiring the global object when converting from JS object to native object.

[WebKit Commit Bot]

Comment on attachment 379755 [details]
Patch

Clearing flags on attachment: 379755

Committed r250485: <https://trac.webkit.org/changeset/250485>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55812214>

[Sam Weinig]

(In reply to Wenson Hsieh from comment #6)
> Thanks for the review!
> 
> (In reply to Sam Weinig from comment #5)
> > Comment on attachment 379755 [details]
> > Patch
> > 
> > Looks good. Out of curiosity, do we handle *Callback (or any other
> > JSDOMGlobalObject requiring type) in other aggregate and / or parametrized
> > types (e.g. sequence<>, Promise<>, FrozenArray<>, union, etc.)
> 
> From what I could tell, we don’t have any precedent for “*Callback” being
> used as the type parameter for sequence, Promise, FrozenArray, or union. The
> quick test of introducing this in an IDL somewhere:
> 
>     `void doThing(sequence<VoidCallback> callbacks);`
> 
> …results in pretty much the same error as I encountered here, where
> compilation fails when attempting to convert the callback object without the
> global object.
> 

Probably worth either fixing them now, so if they are ever needed, the person who needs it won't have to figure this out again.

[Wenson Hsieh]

(In reply to Sam Weinig from comment #10)
> (In reply to Wenson Hsieh from comment #6)
> > Thanks for the review!
> > 
> > (In reply to Sam Weinig from comment #5)
> > > Comment on attachment 379755 [details]
> > > Patch
> > > 
> > > Looks good. Out of curiosity, do we handle *Callback (or any other
> > > JSDOMGlobalObject requiring type) in other aggregate and / or parametrized
> > > types (e.g. sequence<>, Promise<>, FrozenArray<>, union, etc.)
> > 
> > From what I could tell, we don’t have any precedent for “*Callback” being
> > used as the type parameter for sequence, Promise, FrozenArray, or union. The
> > quick test of introducing this in an IDL somewhere:
> > 
> >     `void doThing(sequence<VoidCallback> callbacks);`
> > 
> > …results in pretty much the same error as I encountered here, where
> > compilation fails when attempting to convert the callback object without the
> > global object.
> > 
> 
> Probably worth either fixing them now, so if they are ever needed, the
> person who needs it won't have to figure this out again.

I filed: https://bugs.webkit.org/show_bug.cgi?id=202354.