⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 285988 in webkit


Ignore:
Timestamp:
Nov 18, 2021, 1:26:01 AM (5 years ago)
Author:
Carlos Garcia Campos
Message:

[GLIB] jsc_value_object_define_property_accessor() throws an exception when called on a value without a wrapper instance
​https://bugs.webkit.org/show_bug.cgi?id=233253

Reviewed by Michael Catanzaro.

Source/JavaScriptCore:

We assumed that getter and setter were always methods, so we always try to set the initial parameter as the
instance. When called with a value not having an instance we get an exception because the expected instance is
nullptr. This patch changes the behavior of jsc_value_object_define_property_accessor() to call the getter and
setter as functions, but keeping the behavior of jsc_class_add_property() in which case they are still called as
methods.

  • API/glib/JSCClass.cpp:

(jsc_class_add_property): Use jscValueAddPropertyAccessor().

  • API/glib/JSCValue.cpp:

(jsObjectCall): Remove useless break after return.
(jscValueObjectDefinePropertyAccessor): Helper to define the property accessor using the given function type for
the getter and setter.
(jsc_value_object_define_property_accessor): Call jscValueObjectDefinePropertyAccessor() with function as
function type.
(jscValueAddPropertyAccessor): Call jscValueObjectDefinePropertyAccessor() with method as function type.

  • API/glib/JSCValuePrivate.h:

Tools:

Add unit tests to check jsc_value_object_define_property_accessor().

  • TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp:

(getIntProperty):
(setIntProperty):
(testJSCObject):

Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/API/glib/JSCClass.cpp

    r278253 r285988  
    860860    auto context = jscContextGetOrCreate(priv->context);
    861861    GRefPtr<JSCValue> prototype = jscContextGetOrCreateValue(context.get(), toRef(priv->prototype.get()));
    862     jsc_value_object_define_property_accessor(prototype.get(), name, JSC_VALUE_PROPERTY_CONFIGURABLE, propertyType, getter, setter, userData, destroyNotify);
    863 }
     862    jscValueAddPropertyAccessor(prototype.get(), name, propertyType, getter, setter, userData, destroyNotify);
     863}

  • trunk/Source/JavaScriptCore/API/glib/JSCValue.cpp

    r278253 r285988  
    868868    case JSC::JSCCallbackFunction::Type::Constructor:
    869869        return JSObjectCallAsConstructor(jsContext, function, arguments.size(), arguments.data(), exception);
    870         break;
    871870    case JSC::JSCCallbackFunction::Type::Method:
    872871        ASSERT(thisObject);
    … …  
    874873    case JSC::JSCCallbackFunction::Type::Function:
    875874        return JSObjectCallAsFunction(jsContext, function, thisObject, arguments.size(), arguments.data(), exception);
    876         break;
    877875    }
    878876    RELEASE_ASSERT_NOT_REACHED();
    … …  
    10751073}
    10761074
     1075static void jscValueObjectDefinePropertyAccessor(JSCValue* value, const char* propertyName, JSCValuePropertyFlags flags, GType propertyType, JSC::JSCCallbackFunction::Type functionType, GCallback getter, GCallback setter, gpointer userData, GDestroyNotify destroyNotify)
     1076{
     1077    JSCValuePrivate* priv = value->priv;
     1078    auto* jsContext = jscContextGetJSContext(priv->context.get());
     1079    JSC::JSGlobalObject* globalObject = toJS(jsContext);
     1080    JSC::VM& vm = globalObject->vm();
     1081    JSC::JSLockHolder locker(vm);
     1082    auto scope = DECLARE_CATCH_SCOPE(vm);
     1083
     1084    JSC::JSValue jsValue = toJS(globalObject, priv->jsValue);
     1085    JSC::JSObject* object = jsValue.toObject(globalObject);
     1086    JSValueRef exception = nullptr;
     1087    if (handleExceptionIfNeeded(scope, jsContext, &exception) == ExceptionStatus::DidThrow) {
     1088        jscContextHandleExceptionIfNeeded(priv->context.get(), exception);
     1089        return;
     1090    }
     1091
     1092    auto name = OpaqueJSString::tryCreate(String::fromUTF8(propertyName));
     1093    if (!name)
     1094        return;
     1095
     1096    JSC::PropertyDescriptor descriptor;
     1097    descriptor.setEnumerable(flags & JSC_VALUE_PROPERTY_ENUMERABLE);
     1098    descriptor.setConfigurable(flags & JSC_VALUE_PROPERTY_CONFIGURABLE);
     1099    if (getter) {
     1100        GRefPtr<GClosure> closure;
     1101        if (functionType == JSC::JSCCallbackFunction::Type::Function && userData)
     1102            closure = adoptGRef(g_cclosure_new_swap(getter, userData, reinterpret_cast<GClosureNotify>(reinterpret_cast<GCallback>(destroyNotify))));
     1103        else
     1104            closure = adoptGRef(g_cclosure_new(getter, userData, reinterpret_cast<GClosureNotify>(reinterpret_cast<GCallback>(destroyNotify))));
     1105        auto function = JSC::JSCCallbackFunction::create(vm, globalObject, "get"_s, functionType, nullptr, WTFMove(closure), propertyType, Vector<GType> { });
     1106        descriptor.setGetter(function);
     1107    }
     1108    if (setter) {
     1109        GRefPtr<GClosure> closure = adoptGRef(g_cclosure_new(setter, userData, getter ? nullptr : reinterpret_cast<GClosureNotify>(reinterpret_cast<GCallback>(destroyNotify))));
     1110        auto function = JSC::JSCCallbackFunction::create(vm, globalObject, "set"_s, functionType, nullptr, WTFMove(closure), G_TYPE_NONE, Vector<GType> { propertyType });
     1111        descriptor.setSetter(function);
     1112    }
     1113    object->methodTable(vm)->defineOwnProperty(object, globalObject, name->identifier(&vm), descriptor, true);
     1114    if (handleExceptionIfNeeded(scope, jsContext, &exception) == ExceptionStatus::DidThrow) {
     1115        jscContextHandleExceptionIfNeeded(priv->context.get(), exception);
     1116        return;
     1117    }
     1118}
     1119
    10771120/**
    10781121 * jsc_value_object_define_property_accessor:
    … …  
    10961139 * If you really want to return a new copy of the boxed type, use #JSC_TYPE_VALUE and return a #JSCValue created
    10971140 * with jsc_value_new_object() that receives the copy as instance parameter.
     1141 *
     1142 * Note that @getter and @setter are called as functions and not methods, so they don't receive an instance as
     1143 * first parameter. Use jsc_class_add_property() if you want to add property accessor invoked as a method.
    10981144 */
    10991145void jsc_value_object_define_property_accessor(JSCValue* value, const char* propertyName, JSCValuePropertyFlags flags, GType propertyType, GCallback getter, GCallback setter, gpointer userData, GDestroyNotify destroyNotify)
    … …  
    11041150    g_return_if_fail(getter || setter);
    11051151
    1106     JSCValuePrivate* priv = value->priv;
    1107     auto* jsContext = jscContextGetJSContext(priv->context.get());
    1108     JSC::JSGlobalObject* globalObject = toJS(jsContext);
    1109     JSC::VM& vm = globalObject->vm();
    1110     JSC::JSLockHolder locker(vm);
    1111     auto scope = DECLARE_CATCH_SCOPE(vm);
    1112 
    1113     JSC::JSValue jsValue = toJS(globalObject, priv->jsValue);
    1114     JSC::JSObject* object = jsValue.toObject(globalObject);
    1115     JSValueRef exception = nullptr;
    1116     if (handleExceptionIfNeeded(scope, jsContext, &exception) == ExceptionStatus::DidThrow) {
    1117         jscContextHandleExceptionIfNeeded(priv->context.get(), exception);
    1118         return;
    1119     }
    1120 
    1121     auto name = OpaqueJSString::tryCreate(String::fromUTF8(propertyName));
    1122     if (!name)
    1123         return;
    1124 
    1125     JSC::PropertyDescriptor descriptor;
    1126     descriptor.setEnumerable(flags & JSC_VALUE_PROPERTY_ENUMERABLE);
    1127     descriptor.setConfigurable(flags & JSC_VALUE_PROPERTY_CONFIGURABLE);
    1128     if (getter) {
    1129         GRefPtr<GClosure> closure = adoptGRef(g_cclosure_new(getter, userData, reinterpret_cast<GClosureNotify>(reinterpret_cast<GCallback>(destroyNotify))));
    1130         auto function = JSC::JSCCallbackFunction::create(vm, globalObject, "get"_s,
    1131             JSC::JSCCallbackFunction::Type::Method, nullptr, WTFMove(closure), propertyType, Vector<GType> { });
    1132         descriptor.setGetter(function);
    1133     }
    1134     if (setter) {
    1135         GRefPtr<GClosure> closure = adoptGRef(g_cclosure_new(setter, userData, getter ? nullptr : reinterpret_cast<GClosureNotify>(reinterpret_cast<GCallback>(destroyNotify))));
    1136         auto function = JSC::JSCCallbackFunction::create(vm, globalObject, "set"_s,
    1137             JSC::JSCCallbackFunction::Type::Method, nullptr, WTFMove(closure), G_TYPE_NONE, Vector<GType> { propertyType });
    1138         descriptor.setSetter(function);
    1139     }
    1140     object->methodTable(vm)->defineOwnProperty(object, globalObject, name->identifier(&vm), descriptor, true);
    1141     if (handleExceptionIfNeeded(scope, jsContext, &exception) == ExceptionStatus::DidThrow) {
    1142         jscContextHandleExceptionIfNeeded(priv->context.get(), exception);
    1143         return;
    1144     }
     1152    jscValueObjectDefinePropertyAccessor(value, propertyName, flags, propertyType, JSC::JSCCallbackFunction::Type::Function, getter, setter, userData, destroyNotify);
     1153}
     1154
     1155void jscValueAddPropertyAccessor(JSCValue* value, const char* propertyName, GType propertyType, GCallback getter, GCallback setter, gpointer userData, GDestroyNotify destroyNotify)
     1156{
     1157    jscValueObjectDefinePropertyAccessor(value, propertyName, JSC_VALUE_PROPERTY_CONFIGURABLE, propertyType, JSC::JSCCallbackFunction::Type::Method, getter, setter, userData, destroyNotify);
    11451158}
    11461159

  • trunk/Source/JavaScriptCore/API/glib/JSCValuePrivate.h

    r273836 r285988  
    2424JS_EXPORT_PRIVATE JSValueRef jscValueGetJSValue(JSCValue*);
    2525JSCValue* jscValueCreate(JSCContext*, JSValueRef);
     26void jscValueAddPropertyAccessor(JSCValue*, const char*, GType, GCallback, GCallback, gpointer, GDestroyNotify);

  • trunk/Source/JavaScriptCore/ChangeLog

    r285978 r285988  
     12021-11-18  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GLIB] jsc_value_object_define_property_accessor() throws an exception when called on a value without a wrapper instance
     4        https://bugs.webkit.org/show_bug.cgi?id=233253
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        We assumed that getter and setter were always methods, so we always try to set the initial parameter as the
     9        instance. When called with a value not having an instance we get an exception because the expected instance is
     10        nullptr. This patch changes the behavior of jsc_value_object_define_property_accessor() to call the getter and
     11        setter as functions, but keeping the behavior of jsc_class_add_property() in which case they are still called as
     12        methods.
     13
     14        * API/glib/JSCClass.cpp:
     15        (jsc_class_add_property): Use jscValueAddPropertyAccessor().
     16        * API/glib/JSCValue.cpp:
     17        (jsObjectCall): Remove useless break after return.
     18        (jscValueObjectDefinePropertyAccessor): Helper to define the property accessor using the given function type for
     19        the getter and setter.
     20        (jsc_value_object_define_property_accessor): Call jscValueObjectDefinePropertyAccessor() with function as
     21        function type.
     22        (jscValueAddPropertyAccessor): Call jscValueObjectDefinePropertyAccessor() with method as function type.
     23        * API/glib/JSCValuePrivate.h:
     24
    1252021-11-17  Yusuke Suzuki  <ysuzuki@apple.com>
    226

  • trunk/Tools/ChangeLog

    r285980 r285988  
     12021-11-18  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GLIB] jsc_value_object_define_property_accessor() throws an exception when called on a value without a wrapper instance
     4        https://bugs.webkit.org/show_bug.cgi?id=233253
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        Add unit tests to check jsc_value_object_define_property_accessor().
     9
     10        * TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp:
     11        (getIntProperty):
     12        (setIntProperty):
     13        (testJSCObject):
     14
    1152021-11-17  Alex Christensen  <achristensen@webkit.org>
    216

  • trunk/Tools/TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp

    r283606 r285988  
    11651165}
    11661166
     1167static int getIntProperty(int* property)
     1168{
     1169    return *property;
     1170}
     1171
     1172static void setIntProperty(int value, int* property)
     1173{
     1174    *property = value;
     1175}
     1176
    11671177static void testJSCObject()
    11681178{
    … …  
    14111421        g_assert_true(jsc_value_is_undefined(result.get()));
    14121422        g_assert_did_throw(exceptionHandler, didThrow);
     1423    }
     1424
     1425    {
     1426        LeakChecker checker;
     1427        GRefPtr<JSCContext> context = adoptGRef(jsc_context_new());
     1428        checker.watch(context.get());
     1429        ExceptionHandler exceptionHandler(context.get());
     1430
     1431        GRefPtr<JSCValue> object = adoptGRef(jsc_value_new_object(context.get(), nullptr, nullptr));
     1432        checker.watch(object.get());
     1433        g_assert_true(jsc_value_is_object(object.get()));
     1434        g_assert_true(jsc_value_object_is_instance_of(object.get(), "Object"));
     1435
     1436        GUniquePtr<char*> properties(jsc_value_object_enumerate_properties(object.get()));
     1437        g_assert_null(properties.get());
     1438
     1439        int property = 25;
     1440        g_assert_false(jsc_value_object_has_property(object.get(), "val"));
     1441        jsc_value_object_define_property_accessor(object.get(), "val", static_cast<JSCValuePropertyFlags>(0), G_TYPE_INT, G_CALLBACK(getIntProperty), nullptr, &property, nullptr);
     1442        g_assert_true(jsc_value_object_has_property(object.get(), "val"));
     1443        properties.reset(jsc_value_object_enumerate_properties(object.get()));
     1444        g_assert_null(properties.get());
     1445        jsc_context_set_value(context.get(), "f", object.get());
     1446
     1447        GRefPtr<JSCValue> value = adoptGRef(jsc_context_evaluate(context.get(), "f.val;", -1));
     1448        checker.watch(value.get());
     1449        g_assert_true(jsc_value_is_number(value.get()));
     1450        g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 25);
     1451
     1452        bool didThrow = false;
     1453        g_assert_throw_begin(exceptionHandler, didThrow);
     1454        value = adoptGRef(jsc_context_evaluate(context.get(), "'use strict'; f.val = 32;", -1));
     1455        checker.watch(value.get());
     1456        g_assert_true(jsc_value_is_undefined(value.get()));
     1457        g_assert_did_throw(exceptionHandler, didThrow);
     1458
     1459        value = adoptGRef(jsc_context_evaluate(context.get(), "f.propertyIsEnumerable('val');", -1));
     1460        checker.watch(value.get());
     1461        g_assert_true(jsc_value_is_boolean(value.get()));
     1462        g_assert_true(jsc_value_to_boolean(value.get()) == FALSE);
     1463
     1464        value = adoptGRef(jsc_context_evaluate(context.get(), "delete f.val;", -1));
     1465        checker.watch(value.get());
     1466        g_assert_true(jsc_value_object_has_property(object.get(), "val"));
     1467        value = adoptGRef(jsc_context_evaluate(context.get(), "f.val;", -1));
     1468        checker.watch(value.get());
     1469        g_assert_true(jsc_value_is_number(value.get()));
     1470        g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 25);
     1471
     1472        g_assert_false(jsc_value_object_delete_property(object.get(), "val"));
     1473        g_assert_true(jsc_value_object_has_property(object.get(), "val"));
     1474
     1475        g_assert_throw_begin(exceptionHandler, didThrow);
     1476        jsc_value_object_define_property_accessor(object.get(), "val", static_cast<JSCValuePropertyFlags>(0), G_TYPE_INT, G_CALLBACK(getIntProperty), nullptr, &property, nullptr);
     1477        g_assert_did_throw(exceptionHandler, didThrow);
     1478
     1479        property = 32;
     1480        g_assert_false(jsc_value_object_has_property(object.get(), "val2"));
     1481        jsc_value_object_define_property_accessor(object.get(), "val2", JSC_VALUE_PROPERTY_ENUMERABLE, G_TYPE_INT, G_CALLBACK(getIntProperty), G_CALLBACK(setIntProperty), &property, nullptr);
     1482        g_assert_true(jsc_value_object_has_property(object.get(), "val2"));
     1483        value = adoptGRef(jsc_context_evaluate(context.get(), "f.val2;", -1));
     1484        checker.watch(value.get());
     1485        g_assert_true(jsc_value_is_number(value.get()));
     1486        g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 32);
     1487
     1488        properties.reset(jsc_value_object_enumerate_properties(object.get()));
     1489        g_assert_cmpuint(g_strv_length(properties.get()), ==, 1);
     1490        g_assert_cmpstr(properties.get()[0], ==, "val2");
     1491        g_assert_null(properties.get()[1]);
     1492
     1493        value = adoptGRef(jsc_context_evaluate(context.get(), "'use strict'; f.val2 = 45;", -1));
     1494        checker.watch(value.get());
     1495        value = adoptGRef(jsc_context_evaluate(context.get(), "f.val2;", -1));
     1496        checker.watch(value.get());
     1497        g_assert_true(jsc_value_is_number(value.get()));
     1498        g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 45);
     1499
     1500        value = adoptGRef(jsc_context_evaluate(context.get(), "f.propertyIsEnumerable('val2');", -1));
     1501        checker.watch(value.get());
     1502        g_assert_true(jsc_value_is_boolean(value.get()));
     1503        g_assert_true(jsc_value_to_boolean(value.get()) == TRUE);
     1504
     1505        g_assert_false(jsc_value_object_delete_property(object.get(), "val2"));
     1506        g_assert_true(jsc_value_object_has_property(object.get(), "val2"));
     1507
     1508        property = 125;
     1509        g_assert_false(jsc_value_object_has_property(object.get(), "val3"));
     1510        jsc_value_object_define_property_accessor(object.get(), "val3", JSC_VALUE_PROPERTY_CONFIGURABLE, G_TYPE_INT, G_CALLBACK(getIntProperty), G_CALLBACK(setIntProperty), &property, nullptr);
     1511        g_assert_true(jsc_value_object_has_property(object.get(), "val3"));
     1512        value = adoptGRef(jsc_context_evaluate(context.get(), "f.val3;", -1));
     1513        checker.watch(value.get());
     1514        g_assert_true(jsc_value_is_number(value.get()));
     1515        g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 125);
     1516
     1517        properties.reset(jsc_value_object_enumerate_properties(object.get()));
     1518        g_assert_cmpuint(g_strv_length(properties.get()), ==, 1);
     1519        g_assert_cmpstr(properties.get()[0], ==, "val2");
     1520        g_assert_null(properties.get()[1]);
     1521
     1522        property = 150;
     1523        jsc_value_object_define_property_accessor(object.get(), "val3", JSC_VALUE_PROPERTY_CONFIGURABLE, G_TYPE_INT, G_CALLBACK(getIntProperty), nullptr, &property, nullptr);
     1524        g_assert_true(jsc_value_object_has_property(object.get(), "val3"));
     1525        value = adoptGRef(jsc_context_evaluate(context.get(), "f.val3;", -1));
     1526        checker.watch(value.get());
     1527        g_assert_true(jsc_value_is_number(value.get()));
     1528        g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 150);
     1529
     1530        properties.reset(jsc_value_object_enumerate_properties(object.get()));
     1531        g_assert_cmpuint(g_strv_length(properties.get()), ==, 1);
     1532        g_assert_cmpstr(properties.get()[0], ==, "val2");
     1533        g_assert_null(properties.get()[1]);
     1534
     1535        value = adoptGRef(jsc_context_evaluate(context.get(), "delete f.val3;", -1));
     1536        checker.watch(value.get());
     1537        g_assert_false(jsc_value_object_has_property(object.get(), "val3"));
     1538        value = adoptGRef(jsc_context_evaluate(context.get(), "f.val3;", -1));
     1539        checker.watch(value.get());
     1540        g_assert_true(jsc_value_is_undefined(value.get()));
     1541
     1542        property = 250;
     1543        g_assert_false(jsc_value_object_has_property(object.get(), "val4"));
     1544        jsc_value_object_define_property_accessor(object.get(), "val4", static_cast<JSCValuePropertyFlags>(JSC_VALUE_PROPERTY_CONFIGURABLE | JSC_VALUE_PROPERTY_ENUMERABLE),
     1545            G_TYPE_INT, G_CALLBACK(getIntProperty), nullptr, &property, nullptr);
     1546        g_assert_true(jsc_value_object_has_property(object.get(), "val4"));
     1547        value = adoptGRef(jsc_context_evaluate(context.get(), "f.val4;", -1));
     1548        checker.watch(value.get());
     1549        g_assert_true(jsc_value_is_number(value.get()));
     1550        g_assert_cmpint(jsc_value_to_int32(value.get()), ==, 250);
     1551
     1552        properties.reset(jsc_value_object_enumerate_properties(object.get()));
     1553        g_assert_cmpuint(g_strv_length(properties.get()), ==, 2);
     1554        g_assert_cmpstr(properties.get()[0], ==, "val2");
     1555        g_assert_cmpstr(properties.get()[1], ==, "val4");
     1556        g_assert_null(properties.get()[2]);
     1557
     1558        g_assert_true(jsc_value_object_delete_property(object.get(), "val4"));
     1559        g_assert_false(jsc_value_object_has_property(object.get(), "val4"));
     1560
     1561        properties.reset(jsc_value_object_enumerate_properties(object.get()));
     1562        g_assert_cmpuint(g_strv_length(properties.get()), ==, 1);
     1563        g_assert_cmpstr(properties.get()[0], ==, "val2");
     1564        g_assert_null(properties.get()[1]);
    14131565    }
    14141566
Note: See TracChangeset for help on using the changeset viewer.