Bug 202326 - [IDL] Support record<DOMString, *Callback> in bindings
Summary: [IDL] Support record<DOMString, *Callback> in bindings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-09-27 12:01 PDT by Wenson Hsieh
Modified: 2019-09-29 13:46 PDT (History)
9 users (show)

See Also:


Attachments
WIP (10.08 KB, patch)
2019-09-27 12:27 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (11.00 KB, patch)
2019-09-27 12:45 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2019-09-27 12:01:09 PDT
This currently results in a compilation error.
Comment 2 Wenson Hsieh 2019-09-27 12:16:23 PDT
(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.
Comment 3 Wenson Hsieh 2019-09-27 12:27:53 PDT Comment hidden (obsolete)
Comment 4 Wenson Hsieh 2019-09-27 12:45:04 PDT
Created attachment 379755 [details]
Patch
Comment 5 Sam Weinig 2019-09-28 16:32:19 PDT
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.)
Comment 6 Wenson Hsieh 2019-09-28 17:02:49 PDT
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.
Comment 7 WebKit Commit Bot 2019-09-28 17:26:20 PDT
Comment on attachment 379755 [details]
Patch

Clearing flags on attachment: 379755

Committed r250485: <https://trac.webkit.org/changeset/250485>
Comment 8 WebKit Commit Bot 2019-09-28 17:26:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2019-09-28 17:27:17 PDT
<rdar://problem/55812214>
Comment 10 Sam Weinig 2019-09-29 12:07:55 PDT
(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.
Comment 11 Wenson Hsieh 2019-09-29 13:46:14 PDT
(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.