Skip to content
Permalink
Browse files
[Qt] REGRESSION(r125428): fast/profiler/nested-start-and-stop-profile…
…r.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.

Canonical link: https://commits.webkit.org/112383@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@126146 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
tronical committed Aug 21, 2012
1 parent 2dc906e commit d383fd1e1d715c7875bb1708ab99ba5744e90f99
Showing 7 changed files with 105 additions and 48 deletions.
@@ -1,3 +1,12 @@
2012-08-17 Simon Hausmann <simon.hausmann@nokia.com>

[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.

* platform/qt/Skipped: Unskip test that is now passing.

2012-08-21 Zan Dobersek <zandobersek@gmail.com>

Unreviewed GTK gardening.
@@ -2723,9 +2723,6 @@ fast/js/dfg-compare-final-object-to-final-object-or-other-when-both-proven-final
# https://bugs.webkit.org/show_bug.cgi?id=93812
svg/custom/use-instanceRoot-as-event-target.xhtml

# https://bugs.webkit.org/show_bug.cgi?id=93897
fast/profiler/nested-start-and-stop-profiler.html

# New test introduced in r125648 fast/events/autoscroll-in-textarea.html fails
# https://bugs.webkit.org/show_bug.cgi?id=94076
fast/events/autoscroll-in-textarea.html
@@ -1,3 +1,38 @@
2012-08-17 Simon Hausmann <simon.hausmann@nokia.com>

[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.

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.

2012-08-21 Simon Hausmann <simon.hausmann@nokia.com>

Unreviewed build fix for newer Qt 5 versions: QVariant::WidgetStar has been removed,
@@ -1282,8 +1282,8 @@ static int findSignalIndex(const QMetaObject* meta, int initialIndex, QByteArray
static JSClassRef prototypeForSignalsAndSlots()
{
static JSClassDefinition classDef = {
0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, QtRuntimeMethod::call, 0, 0, 0
0, kJSClassAttributeNoAutomaticPrototype, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
};
static JSClassRef cls = JSClassCreate(&classDef);
return cls;
@@ -1307,7 +1307,7 @@ QtRuntimeMethod::~QtRuntimeMethod()

JSValueRef QtRuntimeMethod::call(JSContextRef context, JSObjectRef function, JSObjectRef /*thisObject*/, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
{
QtRuntimeMethod* d = reinterpret_cast<QtRuntimeMethod*>(JSObjectGetPrivate(function));
QtRuntimeMethod* d = toRuntimeMethod(context, function);
if (!d) {
setException(context, exception, QStringLiteral("cannot call function of deleted runtime method"));
return JSValueMakeUndefined(context);
@@ -1353,50 +1353,53 @@ JSObjectRef QtRuntimeMethod::jsObjectRef(JSContextRef context, JSValueRef* excep
if (JSObjectRef cachedWrapper = m_instance->m_cachedMethods.get(this))
return cachedWrapper;

static const JSClassDefinition classDefForConnect = {
0, 0, "connect", 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, connect, 0, 0, 0
};

static const JSClassDefinition classDefForDisconnect = {
0, 0, "disconnect", 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, disconnect, 0, 0, 0
};

static JSClassRef classRefConnect = JSClassCreate(&classDefForConnect);
static JSClassRef classRefDisconnect = JSClassCreate(&classDefForDisconnect);
bool isSignal = m_flags & MethodIsSignal;
JSObjectRef object = JSObjectMake(context, prototypeForSignalsAndSlots(), this);
JSObjectRef connectFunction = JSObjectMake(context, classRefConnect, this);
JSObjectRef disconnectFunction = JSObjectMake(context, classRefDisconnect, this);
JSPropertyAttributes attributes = kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete;

static JSStringRef connectStr = JSStringCreateWithUTF8CString("connect");
static JSStringRef disconnectStr = JSStringCreateWithUTF8CString("disconnect");
static JSStringRef lengthStr = JSStringCreateWithUTF8CString("length");
static JSStringRef nameStr = JSStringCreateWithUTF8CString("name");
JSRetainPtr<JSStringRef> actualNameStr(Adopt, JSStringCreateWithUTF8CString(m_identifier.constData()));

JSObjectSetProperty(context, connectFunction, lengthStr, JSValueMakeNumber(context, isSignal ? 1 : 0), attributes, exception);
JSObjectSetProperty(context, connectFunction, nameStr, JSValueMakeString(context, connectStr), attributes, exception);
JSObjectSetProperty(context, disconnectFunction, lengthStr, JSValueMakeNumber(context, isSignal ? 1 : 0), attributes, exception);
JSObjectSetProperty(context, disconnectFunction, nameStr, JSValueMakeString(context, disconnectStr), attributes, exception);
JSObjectRef object = JSObjectMakeFunctionWithCallback(context, actualNameStr.get(), call);

JSObjectRef generalFunctionProto = JSValueToObject(context, JSObjectGetPrototype(context, object), 0);
JSObjectRef runtimeMethodProto = JSObjectMake(context, prototypeForSignalsAndSlots(), this);
JSObjectSetPrototype(context, runtimeMethodProto, generalFunctionProto);

JSObjectSetPrototype(context, object, runtimeMethodProto);

JSObjectRef connectFunction = JSObjectMakeFunctionWithCallback(context, connectStr, connect);
JSObjectSetPrototype(context, connectFunction, runtimeMethodProto);

JSObjectRef disconnectFunction = JSObjectMakeFunctionWithCallback(context, disconnectStr, disconnect);
JSObjectSetPrototype(context, disconnectFunction, runtimeMethodProto);

const JSPropertyAttributes attributes = kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete;
JSObjectSetProperty(context, object, connectStr, connectFunction, attributes, exception);
JSObjectSetProperty(context, object, disconnectStr, disconnectFunction, attributes, exception);
JSObjectSetProperty(context, object, lengthStr, JSValueMakeNumber(context, 0), attributes, exception);
JSObjectSetProperty(context, object, nameStr, JSValueMakeString(context, actualNameStr.get()), attributes, exception);

m_instance->m_cachedMethods.set(context, this, object);

return object;
}

QtRuntimeMethod* QtRuntimeMethod::toRuntimeMethod(JSContextRef context, JSObjectRef object)
{
JSObjectRef proto = JSValueToObject(context, JSObjectGetPrototype(context, object), 0);
if (!proto)
return 0;
if (!JSValueIsObjectOfClass(context, proto, prototypeForSignalsAndSlots()))
return 0;
return static_cast<QtRuntimeMethod*>(JSObjectGetPrivate(proto));
}

JSValueRef QtRuntimeMethod::connectOrDisconnect(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception, bool connect)
{
QtRuntimeMethod* d = static_cast<QtRuntimeMethod*>(JSObjectGetPrivate(thisObject));
QtRuntimeMethod* d = toRuntimeMethod(context, thisObject);
if (!d)
d = static_cast<QtRuntimeMethod*>(JSObjectGetPrivate(function));
d = toRuntimeMethod(context, function);
if (!d) {
QString errorStr = QStringLiteral("QtMetaMethod.%1: Cannot connect to/from deleted QObject").arg(connect ? QStringLiteral("connect") : QStringLiteral("disconnect"));
setException(context, exception, errorStr);
return JSValueMakeUndefined(context);
}

QString functionName = connect ? QStringLiteral("connect") : QStringLiteral("disconnect");

@@ -1432,11 +1435,9 @@ JSValueRef QtRuntimeMethod::connectOrDisconnect(JSContextRef context, JSObjectRe

// object.signal.connect(someFunction);
if (JSObjectIsFunction(context, targetFunction)) {
if (JSValueIsObjectOfClass(context, targetFunction, prototypeForSignalsAndSlots())) {
// object.signal.connect(otherObject.slot);
if (QtRuntimeMethod* targetMethod = static_cast<QtRuntimeMethod*>(JSObjectGetPrivate(targetFunction)))
targetObject = toRef(QtInstance::getQtInstance(targetMethod->m_object.data(), d->m_instance->rootObject(), QtInstance::QtOwnership)->createRuntimeObject(toJS(context)));
}
// object.signal.connect(otherObject.slot);
if (QtRuntimeMethod* targetMethod = toRuntimeMethod(context, targetFunction))
targetObject = toRef(QtInstance::getQtInstance(targetMethod->m_object.data(), d->m_instance->rootObject(), QtInstance::QtOwnership)->createRuntimeObject(toJS(context)));
} else
targetFunction = 0;
} else {
@@ -114,8 +114,7 @@ class QtRuntimeMethod {
const QByteArray& name() { return m_identifier; }

private:
static const JSStaticFunction connectFunction;
static const JSStaticFunction disconnectFunction;
static QtRuntimeMethod* toRuntimeMethod(JSContextRef, JSObjectRef);

static JSValueRef connectOrDisconnect(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception, bool connect);
QPointer<QObject> m_object;
@@ -1,3 +1,19 @@
2012-08-17 Simon Hausmann <simon.hausmann@nokia.com>

[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.

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.

2012-08-15 Ryosuke Niwa <rniwa@webkit.org>

Update manual tests and comments to refer to TestRunner instead of LayoutTestController
@@ -1879,7 +1879,7 @@ void tst_QObjectBridge::objectDeleted()
evalJS("bar.intProperty = 123;");
QCOMPARE(qobj->intProperty(), 123);
qobj->resetQtFunctionInvoked();
evalJS("bar.myInvokable(bar);");
evalJS("bar.myInvokable.call(bar);");
QCOMPARE(qobj->qtFunctionInvoked(), 0);

// do this, to ensure that we cache that it implements call
@@ -2148,15 +2148,15 @@ void tst_QObjectBridge::introspectQtMethods_data()
QTest::addColumn<QStringList>("expectedPropertyNames");

QTest::newRow("myObject.mySignal")
<< "myObject" << "mySignal" << (QStringList() << "connect" << "disconnect" << "length" << "name");
<< "myObject" << "mySignal" << (QStringList() << "connect" << "disconnect" << "name");
QTest::newRow("myObject.mySlot")
<< "myObject" << "mySlot" << (QStringList() << "connect" << "disconnect" << "length" << "name");
<< "myObject" << "mySlot" << (QStringList() << "connect" << "disconnect" << "name");
QTest::newRow("myObject.myInvokable")
<< "myObject" << "myInvokable" << (QStringList() << "connect" << "disconnect" << "length" << "name");
<< "myObject" << "myInvokable" << (QStringList() << "connect" << "disconnect" << "name");
QTest::newRow("myObject.mySignal.connect")
<< "myObject.mySignal" << "connect" << (QStringList() << "length" << "name");
<< "myObject.mySignal" << "connect" << (QStringList() << "name");
QTest::newRow("myObject.mySignal.disconnect")
<< "myObject.mySignal" << "disconnect" << (QStringList() << "length" << "name");
<< "myObject.mySignal" << "disconnect" << (QStringList() << "name");
}

void tst_QObjectBridge::introspectQtMethods()
@@ -2177,7 +2177,7 @@ void tst_QObjectBridge::introspectQtMethods()
QCOMPARE(evalJS("descriptor.set"), sUndefined);
QCOMPARE(evalJS(QString::fromLatin1("descriptor.value === %0['%1']").arg(methodLookup).arg(name)), sTrue);
QCOMPARE(evalJS(QString::fromLatin1("descriptor.enumerable")), sFalse);
QCOMPARE(evalJS(QString::fromLatin1("descriptor.configurable")), sTrue);
QCOMPARE(evalJS(QString::fromLatin1("descriptor.configurable")), sFalse);
}

QVERIFY(evalJSV("var props=[]; for (var p in myObject.deleteLater) {props.push(p);}; props.sort()").toStringList().isEmpty());

0 comments on commit d383fd1

Please sign in to comment.