[IDL] Support record<DOMString, *Callback> in bindings
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Sep 2019 00:26:13 +0000 (00:26 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Sep 2019 00:26:13 +0000 (00:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=202326

Reviewed by Sam Weinig.

Currently, IDLRecord's converter assumes that the value type (template argument V) can be converted by passing
in only an ExecState and the JSValue, since it calls `auto typedValue = Converter<V>::convert(state, subValue)`.
However, IDLCallbackFunctions additionally require the JSDOMGlobalObject (see JSDOMConverterCallbacks.h). This
results in a compilation error in generated code, when attempting to convert the record.

To fix this, teach Converter<IDLRecord<K, V>> to accept three arguments (the ExecState, value, and global
object) in the case where V requires the global object. Additionally, let the bindings generator know that
JSValue to native object conversion requires the global object, by returning whether or not the value type of
the IDL record requires the global object, in the case where the given type is a record.

* bindings/js/JSDOMConvertRecord.h:
* bindings/scripts/CodeGeneratorJS.pm:
(JSValueToNativeDOMConvertNeedsGlobalObject):
* bindings/scripts/test/JS/JSTestObj.cpp:

Test this scenario by augmenting TestObj.idl with record<DOMString, VoidCallback>.

(WebCore::jsTestObjStringVoidCallbackRecordAttrGetter):
(WebCore::jsTestObjStringVoidCallbackRecordAttr):
(WebCore::setJSTestObjStringVoidCallbackRecordAttrSetter):
(WebCore::setJSTestObjStringVoidCallbackRecordAttr):
* bindings/scripts/test/TestObj.idl:

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

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMConvertRecord.h
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
Source/WebCore/bindings/scripts/test/TestObj.idl

index f2ffca6..c98f438 100644 (file)
@@ -1,3 +1,33 @@
+2019-09-28  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [IDL] Support record<DOMString, *Callback> in bindings
+        https://bugs.webkit.org/show_bug.cgi?id=202326
+
+        Reviewed by Sam Weinig.
+
+        Currently, IDLRecord's converter assumes that the value type (template argument V) can be converted by passing
+        in only an ExecState and the JSValue, since it calls `auto typedValue = Converter<V>::convert(state, subValue)`.
+        However, IDLCallbackFunctions additionally require the JSDOMGlobalObject (see JSDOMConverterCallbacks.h). This
+        results in a compilation error in generated code, when attempting to convert the record.
+
+        To fix this, teach Converter<IDLRecord<K, V>> to accept three arguments (the ExecState, value, and global
+        object) in the case where V requires the global object. Additionally, let the bindings generator know that
+        JSValue to native object conversion requires the global object, by returning whether or not the value type of
+        the IDL record requires the global object, in the case where the given type is a record.
+
+        * bindings/js/JSDOMConvertRecord.h:
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (JSValueToNativeDOMConvertNeedsGlobalObject):
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+
+        Test this scenario by augmenting TestObj.idl with record<DOMString, VoidCallback>.
+
+        (WebCore::jsTestObjStringVoidCallbackRecordAttrGetter):
+        (WebCore::jsTestObjStringVoidCallbackRecordAttr):
+        (WebCore::setJSTestObjStringVoidCallbackRecordAttrSetter):
+        (WebCore::setJSTestObjStringVoidCallbackRecordAttr):
+        * bindings/scripts/test/TestObj.idl:
+
 2019-09-28  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][IFC] Line::InitialConstraints's heightAndBaseline should be optional
index dd1e6c3..86d32ee 100644 (file)
@@ -64,8 +64,20 @@ template<typename K, typename V> struct Converter<IDLRecord<K, V>> : DefaultConv
     using KeyType = typename K::ImplementationType;
     using ValueType = typename V::ImplementationType;
 
+    static ReturnType convert(JSC::ExecState& state, JSC::JSValue value, JSDOMGlobalObject& globalObject)
+    {
+        return convertRecord<JSDOMGlobalObject&>(state, value, globalObject);
+    }
+
     static ReturnType convert(JSC::ExecState& state, JSC::JSValue value)
     {
+        return convertRecord(state, value);
+    }
+
+private:
+    template<class...Args>
+    static ReturnType convertRecord(JSC::ExecState& state, JSC::JSValue value, Args ... args)
+    {
         auto& vm = state.vm();
         auto scope = DECLARE_THROW_SCOPE(vm);
 
@@ -111,7 +123,7 @@ template<typename K, typename V> struct Converter<IDLRecord<K, V>> : DefaultConv
                 RETURN_IF_EXCEPTION(scope, { });
 
                 // 3. Let typedValue be value converted to an IDL value of type V.
-                auto typedValue = Converter<V>::convert(state, subValue);
+                auto typedValue = Converter<V>::convert(state, subValue, args...);
                 RETURN_IF_EXCEPTION(scope, { });
                 
                 // 4. If typedKey is already a key in result, set its value to typedValue.
index 8eb1dce..cc028c2 100644 (file)
@@ -6616,6 +6616,7 @@ sub JSValueToNativeDOMConvertNeedsGlobalObject
 
     return 1 if $codeGenerator->IsCallbackInterface($type);
     return 1 if $codeGenerator->IsCallbackFunction($type);
+    return JSValueToNativeDOMConvertNeedsGlobalObject(@{$type->subtypes}[1]) if $codeGenerator->IsRecordType($type);
     return 1 if $type->name eq "ScheduledAction";
     return 0;
 }
index d4205b8..d80969e 100644 (file)
@@ -74,6 +74,7 @@
 #include "JSTestStandaloneDictionary.h"
 #include "JSTestStandaloneEnumeration.h"
 #include "JSTestSubObj.h"
+#include "JSVoidCallback.h"
 #include "JSWindowProxy.h"
 #include "JSXPathNSResolver.h"
 #include "RuntimeEnabledFeatures.h"
@@ -1651,6 +1652,8 @@ JSC::EncodedJSValue jsTestObjStringObjRecordAttr(JSC::ExecState*, JSC::EncodedJS
 bool setJSTestObjStringObjRecordAttr(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
 JSC::EncodedJSValue jsTestObjStringNullableObjRecordAttr(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
 bool setJSTestObjStringNullableObjRecordAttr(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
+JSC::EncodedJSValue jsTestObjStringVoidCallbackRecordAttr(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
+bool setJSTestObjStringVoidCallbackRecordAttr(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
 JSC::EncodedJSValue jsTestObjDictionaryAttr(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
 bool setJSTestObjDictionaryAttr(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
 JSC::EncodedJSValue jsTestObjNullableDictionaryAttr(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
@@ -2020,6 +2023,7 @@ static const HashTableValue JSTestObjPrototypeTableValues[] =
     { "usvstringLongRecordAttr", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjUsvstringLongRecordAttr), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjUsvstringLongRecordAttr) } },
     { "stringObjRecordAttr", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjStringObjRecordAttr), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjStringObjRecordAttr) } },
     { "stringNullableObjRecordAttr", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjStringNullableObjRecordAttr), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjStringNullableObjRecordAttr) } },
+    { "stringVoidCallbackRecordAttr", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjStringVoidCallbackRecordAttr), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjStringVoidCallbackRecordAttr) } },
     { "dictionaryAttr", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjDictionaryAttr), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjDictionaryAttr) } },
     { "nullableDictionaryAttr", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjNullableDictionaryAttr), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjNullableDictionaryAttr) } },
     { "annotatedTypeInUnionAttr", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjAnnotatedTypeInUnionAttr), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjAnnotatedTypeInUnionAttr) } },
@@ -3464,6 +3468,38 @@ bool setJSTestObjStringNullableObjRecordAttr(ExecState* state, EncodedJSValue th
     return IDLAttribute<JSTestObj>::set<setJSTestObjStringNullableObjRecordAttrSetter>(*state, thisValue, encodedValue, "stringNullableObjRecordAttr");
 }
 
+static inline JSValue jsTestObjStringVoidCallbackRecordAttrGetter(ExecState& state, JSTestObj& thisObject, ThrowScope& throwScope)
+{
+    UNUSED_PARAM(throwScope);
+    UNUSED_PARAM(state);
+    auto& impl = thisObject.wrapped();
+    JSValue result = toJS<IDLRecord<IDLDOMString, IDLCallbackFunction<JSVoidCallback>>>(state, *thisObject.globalObject(), throwScope, impl.stringVoidCallbackRecordAttr());
+    return result;
+}
+
+EncodedJSValue jsTestObjStringVoidCallbackRecordAttr(ExecState* state, EncodedJSValue thisValue, PropertyName)
+{
+    return IDLAttribute<JSTestObj>::get<jsTestObjStringVoidCallbackRecordAttrGetter, CastedThisErrorBehavior::Assert>(*state, thisValue, "stringVoidCallbackRecordAttr");
+}
+
+static inline bool setJSTestObjStringVoidCallbackRecordAttrSetter(ExecState& state, JSTestObj& thisObject, JSValue value, ThrowScope& throwScope)
+{
+    UNUSED_PARAM(state);
+    UNUSED_PARAM(throwScope);
+    auto& impl = thisObject.wrapped();
+    auto nativeValue = convert<IDLRecord<IDLDOMString, IDLCallbackFunction<JSVoidCallback>>>(state, value, *thisObject.globalObject());
+    RETURN_IF_EXCEPTION(throwScope, false);
+    AttributeSetter::call(state, throwScope, [&] {
+        return impl.setStringVoidCallbackRecordAttr(WTFMove(nativeValue));
+    });
+    return true;
+}
+
+bool setJSTestObjStringVoidCallbackRecordAttr(ExecState* state, EncodedJSValue thisValue, EncodedJSValue encodedValue)
+{
+    return IDLAttribute<JSTestObj>::set<setJSTestObjStringVoidCallbackRecordAttrSetter>(*state, thisValue, encodedValue, "stringVoidCallbackRecordAttr");
+}
+
 static inline JSValue jsTestObjDictionaryAttrGetter(ExecState& state, JSTestObj& thisObject, ThrowScope& throwScope)
 {
     UNUSED_PARAM(throwScope);
index 1e98bcd..7ebc0a8 100644 (file)
@@ -83,6 +83,7 @@ enum TestConfidence { "high", "kinda-low" };
     attribute record<ByteString, long> usvstringLongRecordAttr;
     attribute record<DOMString, TestObj> stringObjRecordAttr;
     attribute record<DOMString, TestObj?> stringNullableObjRecordAttr;
+    attribute record<DOMString, VoidCallback> stringVoidCallbackRecordAttr;
     attribute TestDictionary dictionaryAttr;
     attribute TestDictionary? nullableDictionaryAttr;
     attribute (DOMString or [Clamp] long) annotatedTypeInUnionAttr;