Changeset 126146 in webkit


Ignore:
Timestamp:
Aug 21, 2012 1:54:02 AM (12 years ago)
Author:
Simon Hausmann
Message:

[Qt] REGRESSION(r125428): fast/profiler/nested-start-and-stop-profiler.html fails
​https://bugs.webkit.org/show_bug.cgi?id=93897

Reviewed by Kenneth Rohde Christiansen.

Source/WebCore:

Before r125428 run-time methods (wrapped signals, slots or invokable functions) were subclasses of
JSInternalFunction and therefore real function objects in the JavaScript sense. r125428 changed them
to be just callable objects, but they did not have Function.prototype as prototype anymore for example
nor was their name correct (resulting in a layout test failure).

This patch changes run-time methods back to being real function objects that have a correct name and
have Function.prototype in their prototype change

The objects returned by JSObjectMakeFunctionWithCallbackInjected are light-weight internal function objects
that do not support JSObject{Set/Get}Private. Therefore we inject our own prototype right before the
Function.prototype prototype, which uses private data to store a pointer to our C++ QtRuntimeMethod object.
This complicates the retrieval of the pointer to that instance slightly, which is why this patch introduces
the toRuntimeMethod convenience function that looks up our prototype first and does a check for type-safety.

At the same time the patch removes the length properties from the run-time method itself as well as connect/disconnect.
The length property on a function signifies the number of arguments, but in all three cases that number is
actually variable, because of overloading. That is why we choose not to expose it in the first place.

  • bridge/qt/qt_runtime.cpp:

(JSC::Bindings::prototypeForSignalsAndSlots):
(JSC::Bindings::QtRuntimeMethod::call):
(JSC::Bindings::QtRuntimeMethod::jsObjectRef):
(JSC::Bindings::QtRuntimeMethod::toRuntimeMethod):
(Bindings):
(JSC::Bindings::QtRuntimeMethod::connectOrDisconnect):

  • bridge/qt/qt_runtime.h:

(QtRuntimeMethod): Remove unused member variables.

Source/WebKit/qt:

Fixed some test expectations.

  • tests/qobjectbridge/tst_qobjectbridge.cpp:

(tst_QObjectBridge::objectDeleted): Since runtime methods are real function objects again, we
can go back to testing Function.prototype.call, as it was done before r125428.
(tst_QObjectBridge::introspectQtMethods_data): Removed tests for the length property.
(tst_QObjectBridge::introspectQtMethods): Changed test expectation of the properties of
run-time methods back to being non-configurable, as before r125428.

LayoutTests:

  • platform/qt/Skipped: Unskip test that is now passing.
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r126145 r126146  
     12012-08-17  Simon Hausmann  <simon.hausmann@nokia.com>
     2
     3        [Qt] REGRESSION(r125428): fast/profiler/nested-start-and-stop-profiler.html fails
     4        https://bugs.webkit.org/show_bug.cgi?id=93897
     5
     6        Reviewed by Kenneth Rohde Christiansen.
     7
     8        * platform/qt/Skipped: Unskip test that is now passing.
     9
    1102012-08-21  Zan Dobersek  <zandobersek@gmail.com>
    211

  • trunk/LayoutTests/platform/qt/Skipped

    r126087 r126146  
    27242724svg/custom/use-instanceRoot-as-event-target.xhtml
    27252725
    2726 # https://bugs.webkit.org/show_bug.cgi?id=93897
    2727 fast/profiler/nested-start-and-stop-profiler.html
    2728 
    27292726# New test introduced in r125648 fast/events/autoscroll-in-textarea.html fails
    27302727# https://bugs.webkit.org/show_bug.cgi?id=94076

  • trunk/Source/WebCore/ChangeLog

    r126142 r126146  
     12012-08-17  Simon Hausmann  <simon.hausmann@nokia.com>
     2
     3        [Qt] REGRESSION(r125428): fast/profiler/nested-start-and-stop-profiler.html fails
     4        https://bugs.webkit.org/show_bug.cgi?id=93897
     5
     6        Reviewed by Kenneth Rohde Christiansen.
     7
     8        Before r125428 run-time methods (wrapped signals, slots or invokable functions) were subclasses of
     9        JSInternalFunction and therefore real function objects in the JavaScript sense. r125428 changed them
     10        to be just callable objects, but they did not have Function.prototype as prototype anymore for example
     11        nor was their name correct (resulting in a layout test failure).
     12
     13        This patch changes run-time methods back to being real function objects that have a correct name and
     14        have Function.prototype in their prototype change
     15
     16        The objects returned by JSObjectMakeFunctionWithCallbackInjected are light-weight internal function objects
     17        that do not support JSObject{Set/Get}Private. Therefore we inject our own prototype right before the
     18        Function.prototype prototype, which uses private data to store a pointer to our C++ QtRuntimeMethod object.
     19        This complicates the retrieval of the pointer to that instance slightly, which is why this patch introduces
     20        the toRuntimeMethod convenience function that looks up our prototype first and does a check for type-safety.
     21
     22        At the same time the patch removes the length properties from the run-time method itself as well as connect/disconnect.
     23        The length property on a function signifies the number of arguments, but in all three cases that number is
     24        actually variable, because of overloading. That is why we choose not to expose it in the first place.
     25
     26        * bridge/qt/qt_runtime.cpp:
     27        (JSC::Bindings::prototypeForSignalsAndSlots):
     28        (JSC::Bindings::QtRuntimeMethod::call):
     29        (JSC::Bindings::QtRuntimeMethod::jsObjectRef):
     30        (JSC::Bindings::QtRuntimeMethod::toRuntimeMethod):
     31        (Bindings):
     32        (JSC::Bindings::QtRuntimeMethod::connectOrDisconnect):
     33        * bridge/qt/qt_runtime.h:
     34        (QtRuntimeMethod): Remove unused member variables.
     35
    1362012-08-21  Simon Hausmann  <simon.hausmann@nokia.com>
    237

  • trunk/Source/WebCore/bridge/qt/qt_runtime.cpp

    r126143 r126146  
    12831283{
    12841284    static JSClassDefinition classDef = {
    1285         0, 0, 0, 0, 0, 0,
    1286         0, 0, 0, 0, 0, 0, 0, QtRuntimeMethod::call, 0, 0, 0
     1285        0, kJSClassAttributeNoAutomaticPrototype, 0, 0, 0, 0,
     1286        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
    12871287    };
    12881288    static JSClassRef cls = JSClassCreate(&classDef);
    … …  
    13081308JSValueRef QtRuntimeMethod::call(JSContextRef context, JSObjectRef function, JSObjectRef /*thisObject*/, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
    13091309{
    1310     QtRuntimeMethod* d = reinterpret_cast<QtRuntimeMethod*>(JSObjectGetPrivate(function));
     1310    QtRuntimeMethod* d = toRuntimeMethod(context, function);
    13111311    if (!d) {
    13121312        setException(context, exception, QStringLiteral("cannot call function of deleted runtime method"));
    … …  
    13541354        return cachedWrapper;
    13551355
    1356     static const JSClassDefinition classDefForConnect = {
    1357         0, 0, "connect", 0, 0, 0,
    1358         0, 0, 0, 0, 0, 0, 0, connect, 0, 0, 0
    1359     };
    1360 
    1361     static const JSClassDefinition classDefForDisconnect = {
    1362         0, 0, "disconnect", 0, 0, 0,
    1363         0, 0, 0, 0, 0, 0, 0, disconnect, 0, 0, 0
    1364     };
    1365 
    1366     static JSClassRef classRefConnect = JSClassCreate(&classDefForConnect);
    1367     static JSClassRef classRefDisconnect = JSClassCreate(&classDefForDisconnect);
    1368     bool isSignal = m_flags & MethodIsSignal;
    1369     JSObjectRef object = JSObjectMake(context, prototypeForSignalsAndSlots(), this);
    1370     JSObjectRef connectFunction = JSObjectMake(context, classRefConnect, this);
    1371     JSObjectRef disconnectFunction = JSObjectMake(context, classRefDisconnect, this);
    1372     JSPropertyAttributes attributes = kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete;
    1373 
    13741356    static JSStringRef connectStr = JSStringCreateWithUTF8CString("connect");
    13751357    static JSStringRef disconnectStr = JSStringCreateWithUTF8CString("disconnect");
    1376     static JSStringRef lengthStr = JSStringCreateWithUTF8CString("length");
    1377     static JSStringRef nameStr = JSStringCreateWithUTF8CString("name");
    13781358    JSRetainPtr<JSStringRef> actualNameStr(Adopt, JSStringCreateWithUTF8CString(m_identifier.constData()));
    13791359
    1380     JSObjectSetProperty(context, connectFunction, lengthStr, JSValueMakeNumber(context, isSignal ? 1 : 0), attributes, exception);
    1381     JSObjectSetProperty(context, connectFunction, nameStr, JSValueMakeString(context, connectStr), attributes, exception);
    1382     JSObjectSetProperty(context, disconnectFunction, lengthStr, JSValueMakeNumber(context, isSignal ? 1 : 0), attributes, exception);
    1383     JSObjectSetProperty(context, disconnectFunction, nameStr, JSValueMakeString(context, disconnectStr), attributes, exception);
    1384 
     1360    JSObjectRef object = JSObjectMakeFunctionWithCallback(context, actualNameStr.get(), call);
     1361
     1362    JSObjectRef generalFunctionProto = JSValueToObject(context, JSObjectGetPrototype(context, object), 0);
     1363    JSObjectRef runtimeMethodProto = JSObjectMake(context, prototypeForSignalsAndSlots(), this);
     1364    JSObjectSetPrototype(context, runtimeMethodProto, generalFunctionProto);
     1365
     1366    JSObjectSetPrototype(context, object, runtimeMethodProto);
     1367
     1368    JSObjectRef connectFunction = JSObjectMakeFunctionWithCallback(context, connectStr, connect);
     1369    JSObjectSetPrototype(context, connectFunction, runtimeMethodProto);
     1370
     1371    JSObjectRef disconnectFunction = JSObjectMakeFunctionWithCallback(context, disconnectStr, disconnect);
     1372    JSObjectSetPrototype(context, disconnectFunction, runtimeMethodProto);
     1373
     1374    const JSPropertyAttributes attributes = kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete;
    13851375    JSObjectSetProperty(context, object, connectStr, connectFunction, attributes, exception);
    13861376    JSObjectSetProperty(context, object, disconnectStr, disconnectFunction, attributes, exception);
    1387     JSObjectSetProperty(context, object, lengthStr, JSValueMakeNumber(context, 0), attributes, exception);
    1388     JSObjectSetProperty(context, object, nameStr, JSValueMakeString(context, actualNameStr.get()), attributes, exception);
    13891377
    13901378    m_instance->m_cachedMethods.set(context, this, object);
    … …  
    13931381}
    13941382
     1383QtRuntimeMethod* QtRuntimeMethod::toRuntimeMethod(JSContextRef context, JSObjectRef object)
     1384{
     1385    JSObjectRef proto = JSValueToObject(context, JSObjectGetPrototype(context, object), 0);
     1386    if (!proto)
     1387        return 0;
     1388    if (!JSValueIsObjectOfClass(context, proto, prototypeForSignalsAndSlots()))
     1389        return 0;
     1390    return static_cast<QtRuntimeMethod*>(JSObjectGetPrivate(proto));
     1391}
     1392
    13951393JSValueRef QtRuntimeMethod::connectOrDisconnect(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception, bool connect)
    13961394{
    1397     QtRuntimeMethod* d = static_cast<QtRuntimeMethod*>(JSObjectGetPrivate(thisObject));
     1395    QtRuntimeMethod* d = toRuntimeMethod(context, thisObject);
    13981396    if (!d)
    1399         d = static_cast<QtRuntimeMethod*>(JSObjectGetPrivate(function));
     1397        d = toRuntimeMethod(context, function);
     1398    if (!d) {
     1399        QString errorStr = QStringLiteral("QtMetaMethod.%1: Cannot connect to/from deleted QObject").arg(connect ?  QStringLiteral("connect") : QStringLiteral("disconnect"));
     1400        setException(context, exception, errorStr);
     1401        return JSValueMakeUndefined(context);
     1402    }
    14001403
    14011404    QString functionName = connect ? QStringLiteral("connect") : QStringLiteral("disconnect");
    … …  
    14331436        // object.signal.connect(someFunction);
    14341437        if (JSObjectIsFunction(context, targetFunction)) {
    1435             if (JSValueIsObjectOfClass(context, targetFunction, prototypeForSignalsAndSlots())) {
    1436                 // object.signal.connect(otherObject.slot);
    1437                 if (QtRuntimeMethod* targetMethod = static_cast<QtRuntimeMethod*>(JSObjectGetPrivate(targetFunction)))
    1438                     targetObject = toRef(QtInstance::getQtInstance(targetMethod->m_object.data(), d->m_instance->rootObject(), QtInstance::QtOwnership)->createRuntimeObject(toJS(context)));
    1439             }
     1438            // object.signal.connect(otherObject.slot);
     1439            if (QtRuntimeMethod* targetMethod = toRuntimeMethod(context, targetFunction))
     1440                targetObject = toRef(QtInstance::getQtInstance(targetMethod->m_object.data(), d->m_instance->rootObject(), QtInstance::QtOwnership)->createRuntimeObject(toJS(context)));
    14401441        } else
    14411442            targetFunction = 0;

  • trunk/Source/WebCore/bridge/qt/qt_runtime.h

    r125873 r126146  
    115115
    116116private:
    117     static const JSStaticFunction connectFunction;
    118     static const JSStaticFunction disconnectFunction;
     117    static QtRuntimeMethod* toRuntimeMethod(JSContextRef, JSObjectRef);
    119118
    120119    static JSValueRef connectOrDisconnect(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception, bool connect);

  • trunk/Source/WebKit/qt/ChangeLog

    r125738 r126146  
     12012-08-17  Simon Hausmann  <simon.hausmann@nokia.com>
     2
     3        [Qt] REGRESSION(r125428): fast/profiler/nested-start-and-stop-profiler.html fails
     4        https://bugs.webkit.org/show_bug.cgi?id=93897
     5
     6        Reviewed by Kenneth Rohde Christiansen.
     7
     8        Fixed some test expectations.
     9
     10        * tests/qobjectbridge/tst_qobjectbridge.cpp:
     11        (tst_QObjectBridge::objectDeleted): Since runtime methods are real function objects again, we
     12        can go back to testing Function.prototype.call, as it was done before r125428.
     13        (tst_QObjectBridge::introspectQtMethods_data): Removed tests for the length property.
     14        (tst_QObjectBridge::introspectQtMethods): Changed test expectation of the properties of
     15        run-time methods back to being non-configurable, as before r125428.
     16
    1172012-08-15  Ryosuke Niwa  <rniwa@webkit.org>
    218

  • trunk/Source/WebKit/qt/tests/qobjectbridge/tst_qobjectbridge.cpp

    r125603 r126146  
    18801880    QCOMPARE(qobj->intProperty(), 123);
    18811881    qobj->resetQtFunctionInvoked();
    1882     evalJS("bar.myInvokable(bar);");
     1882    evalJS("bar.myInvokable.call(bar);");
    18831883    QCOMPARE(qobj->qtFunctionInvoked(), 0);
    18841884
    … …  
    21492149
    21502150    QTest::newRow("myObject.mySignal")
    2151         << "myObject" << "mySignal" << (QStringList() << "connect" << "disconnect" << "length" << "name");
     2151        << "myObject" << "mySignal" << (QStringList() << "connect" << "disconnect" << "name");
    21522152    QTest::newRow("myObject.mySlot")
    2153         << "myObject" << "mySlot" << (QStringList() << "connect" << "disconnect" << "length" << "name");
     2153        << "myObject" << "mySlot" << (QStringList() << "connect" << "disconnect" << "name");
    21542154    QTest::newRow("myObject.myInvokable")
    2155         << "myObject" << "myInvokable" << (QStringList() << "connect" << "disconnect" << "length" << "name");
     2155        << "myObject" << "myInvokable" << (QStringList() << "connect" << "disconnect" << "name");
    21562156    QTest::newRow("myObject.mySignal.connect")
    2157         << "myObject.mySignal" << "connect" << (QStringList() << "length" << "name");
     2157        << "myObject.mySignal" << "connect" << (QStringList() << "name");
    21582158    QTest::newRow("myObject.mySignal.disconnect")
    2159         << "myObject.mySignal" << "disconnect" << (QStringList() << "length" << "name");
     2159        << "myObject.mySignal" << "disconnect" << (QStringList() << "name");
    21602160}
    21612161
    … …  
    21782178        QCOMPARE(evalJS(QString::fromLatin1("descriptor.value === %0['%1']").arg(methodLookup).arg(name)), sTrue);
    21792179        QCOMPARE(evalJS(QString::fromLatin1("descriptor.enumerable")), sFalse);
    2180         QCOMPARE(evalJS(QString::fromLatin1("descriptor.configurable")), sTrue);
     2180        QCOMPARE(evalJS(QString::fromLatin1("descriptor.configurable")), sFalse);
    21812181    }
    21822182
Note: See TracChangeset for help on using the changeset viewer.