From 87ca0ba70cd9cb4cd33e4c59986ede6b40cfe4be Mon Sep 17 00:00:00 2001 From: Arno Rehn Date: Fri, 16 Apr 2021 22:45:21 +0200 Subject: Use QProperty observation to push property updates to clients If the property is BINDABLE but lacks a NOTIFY signal, the client will have no way to register a callback for change notifications. Document this behavior as such. A future patch could synthesize signals for purely BINDABLE properties on the client side, but this needs some more thought. Change-Id: I5e723e294dc01890956fee179fb3ba30aecf8cc1 Reviewed-by: Fabian Kosmale --- src/webchannel/qmetaobjectpublisher.cpp | 103 +++++++++++++++++++++++--------- src/webchannel/qmetaobjectpublisher_p.h | 63 ++++++++++++++++++- src/webchannel/qwebchannel.cpp | 3 + 3 files changed, 138 insertions(+), 31 deletions(-) (limited to 'src') diff --git a/src/webchannel/qmetaobjectpublisher.cpp b/src/webchannel/qmetaobjectpublisher.cpp index c97f755..b40aeab 100644 --- a/src/webchannel/qmetaobjectpublisher.cpp +++ b/src/webchannel/qmetaobjectpublisher.cpp @@ -183,6 +183,24 @@ const int PROPERTY_UPDATE_INTERVAL = 50; Q_DECLARE_TYPEINFO(OverloadResolutionCandidate, Q_MOVABLE_TYPE); +void QWebChannelPropertyChangeNotifier::notify( + QPropertyObserver *self, QUntypedPropertyData *) +{ + auto This = static_cast(self); + + // Use the indirection with Qt::AutoConnection to ensure invocation + // in the correct thread. + // Explicitly copy the parameters into the lambda so that this instance can be destroyed after posting a queued + // invocation. The current design doesn't allow this anyway, but I don't want bad surprises in a possible future + // commit. + QMetaObject::invokeMethod( + This->publisher, + [publisher=This->publisher, object=This->object, propertyIndex=This->propertyIndex] + { + publisher->propertyValueChanged(object, propertyIndex); + }, Qt::AutoConnection); +} + QMetaObjectPublisher::QMetaObjectPublisher(QWebChannel *webChannel) : QObject(webChannel) , webChannel(webChannel) @@ -247,8 +265,8 @@ QJsonObject QMetaObjectPublisher::classInfoForObject(const QObject *object, QWeb signalInfo.append(QString::fromLatin1(notifySignal)); } signalInfo.append(prop.notifySignalIndex()); - } else if (!prop.isConstant()) { - qWarning("Property '%s'' of object '%s' has no notify signal and is not constant, " + } else if (!prop.isConstant() && !prop.isBindable()) { + qWarning("Property '%s'' of object '%s' has no notify signal, is not bindable and is not constant, " "value updates in HTML will be broken!", prop.name(), object->metaObject()->className()); } @@ -308,8 +326,8 @@ void QMetaObjectPublisher::setClientIsIdle(bool isIdle) clientIsIdle = isIdle; if (!isIdle && timer.isActive()) { timer.stop(); - } else if (isIdle && !timer.isActive()) { - timer.start(PROPERTY_UPDATE_INTERVAL, this); + } else { + startPropertyUpdateTimer(); } } @@ -330,8 +348,9 @@ QJsonObject QMetaObjectPublisher::initializeClient(QWebChannelAbstractTransport return objectInfos; } -void QMetaObjectPublisher::initializePropertyUpdates(const QObject *const object, const QJsonObject &objectInfo) +void QMetaObjectPublisher::initializePropertyUpdates(QObject *const object, const QJsonObject &objectInfo) { + auto *metaObject = object->metaObject(); auto *signalHandler = signalHandlerFor(object); for (const auto propertyInfoVar : objectInfo[KEY_PROPERTIES].toArray()) { const QJsonArray &propertyInfo = propertyInfoVar.toArray(); @@ -341,22 +360,32 @@ void QMetaObjectPublisher::initializePropertyUpdates(const QObject *const object } const int propertyIndex = propertyInfo.at(0).toInt(); const QJsonArray &signalData = propertyInfo.at(2).toArray(); + const QMetaProperty metaProp = metaObject->property(propertyIndex); - if (signalData.isEmpty()) { - // Property without NOTIFY signal - continue; - } + if (!signalData.isEmpty()) { + // Property with a NOTIFY signal + const int signalIndex = signalData.at(1).toInt(); - const int signalIndex = signalData.at(1).toInt(); + QSet &connectedProperties = signalToPropertyMap[object][signalIndex]; - QSet &connectedProperties = signalToPropertyMap[object][signalIndex]; + // Only connect for a property update once + if (connectedProperties.isEmpty()) { + signalHandler->connectTo(object, signalIndex); + } - // Only connect for a property update once - if (connectedProperties.isEmpty()) { - signalHandler->connectTo(object, signalIndex); + connectedProperties.insert(propertyIndex); + } else if (metaProp.isBindable()) { + const auto [begin, end] = propertyObservers.equal_range(object); + const auto it = std::find_if(begin, end, [&](auto &n) { + return n.second.propertyIndex == propertyIndex; + }); + // Only connect for a property update once + if (it == end) { + auto it = propertyObservers.emplace( + object, QWebChannelPropertyChangeNotifier{this, object, propertyIndex}); + metaProp.bindable(object).observe(&it->second); + } } - - connectedProperties.insert(propertyIndex); } // also always connect to destroyed signal @@ -383,16 +412,21 @@ void QMetaObjectPublisher::sendPendingPropertyUpdates() QJsonObject properties; // maps signal index to list of arguments of the last emit QJsonObject sigs; - const SignalToArgumentsMap::const_iterator sigEnd = it.value().constEnd(); - for (SignalToArgumentsMap::const_iterator sigIt = it.value().constBegin(); sigIt != sigEnd; ++sigIt) { - // TODO: can we get rid of the int <-> string conversions here? - for (const int propertyIndex : objectsSignalToPropertyMap.value(sigIt.key())) { - const QMetaProperty &property = metaObject->property(propertyIndex); - Q_ASSERT(property.isValid()); - properties[QString::number(propertyIndex)] = wrapResult(property.read(object), nullptr, objectId); - } + + const auto indexes = it.value().propertyIndices(objectsSignalToPropertyMap); + + // TODO: can we get rid of the int <-> string conversions here? + for (const int propertyIndex : indexes) { + const QMetaProperty &property = metaObject->property(propertyIndex); + Q_ASSERT(property.isValid()); + properties[QString::number(propertyIndex)] = wrapResult(property.read(object), nullptr, objectId); + } + + const auto sigMap = it.value().signalMap; + for (auto sigIt = sigMap.constBegin(); sigIt != sigMap.constEnd(); ++sigIt) { sigs[QString::number(sigIt.key())] = QJsonArray::fromVariantList(sigIt.value()); } + QJsonObject obj; obj[KEY_OBJECT] = objectId; obj[KEY_SIGNALS] = sigs; @@ -572,10 +606,22 @@ void QMetaObjectPublisher::signalEmitted(const QObject *object, const int signal objectDestroyed(object); } } else { - pendingPropertyUpdates[object][signalIndex] = arguments; - if (clientIsIdle && !blockUpdates && !timer.isActive()) { - timer.start(PROPERTY_UPDATE_INTERVAL, this); - } + auto &propertyUpdate = pendingPropertyUpdates[object]; + propertyUpdate.signalMap[signalIndex] = arguments; + startPropertyUpdateTimer(); + } +} + +void QMetaObjectPublisher::propertyValueChanged(const QObject *object, const int propertyIndex) +{ + pendingPropertyUpdates[object].plainProperties.insert(propertyIndex); + startPropertyUpdateTimer(); +} + +void QMetaObjectPublisher::startPropertyUpdateTimer() +{ + if (clientIsIdle && !blockUpdates && !timer.isActive()) { + timer.start(PROPERTY_UPDATE_INTERVAL, this); } } @@ -595,6 +641,7 @@ void QMetaObjectPublisher::objectDestroyed(const QObject *object) signalToPropertyMap.remove(object); } pendingPropertyUpdates.remove(object); + propertyObservers.erase(object); } QObject *QMetaObjectPublisher::unwrapObject(const QString &objectId) const diff --git a/src/webchannel/qmetaobjectpublisher_p.h b/src/webchannel/qmetaobjectpublisher_p.h index e87e4f0..f54f569 100644 --- a/src/webchannel/qmetaobjectpublisher_p.h +++ b/src/webchannel/qmetaobjectpublisher_p.h @@ -86,8 +86,24 @@ enum MessageType { TYPES_LAST_VALUE = 10 }; +class QMetaObjectPublisher; class QWebChannel; class QWebChannelAbstractTransport; + +struct QWebChannelPropertyChangeNotifier : QPropertyObserver +{ + QWebChannelPropertyChangeNotifier(QMetaObjectPublisher *publisher, const QObject *object, int propertyIndex) + : QPropertyObserver(&QWebChannelPropertyChangeNotifier::notify), + publisher(publisher), object(object), propertyIndex(propertyIndex) + { + } + + QMetaObjectPublisher *publisher = nullptr; + const QObject *object = nullptr; + int propertyIndex = 0; + static void notify(QPropertyObserver *, QUntypedPropertyData *); +}; + class Q_WEBCHANNEL_EXPORT QMetaObjectPublisher : public QObject { Q_OBJECT @@ -136,7 +152,7 @@ public: * When receiving a notify signal, it will store the information in pendingPropertyUpdates which * gets send via a Qt.propertyUpdate message to the server when the grouping timer timeouts. */ - void initializePropertyUpdates(const QObject *const object, const QJsonObject &objectInfo); + void initializePropertyUpdates(QObject *const object, const QJsonObject &objectInfo); /** * Send the clients the new property values since the last time this function was invoked. @@ -185,6 +201,17 @@ public: */ void signalEmitted(const QObject *object, const int signalIndex, const QVariantList &arguments); + /** + * Callback for bindable property value changes which forwards the change to the webchannel clients. + */ + void propertyValueChanged(const QObject *object, const int propertyIndex); + + /** + * Called after a property has been updated. Starts the update timer if + * the client is idle and updates are not blocked. + */ + void startPropertyUpdateTimer(); + /** * Callback for registered or wrapped objects which erases all data related to @p object. * @@ -319,10 +346,32 @@ private: typedef QHash > SignalToPropertyNameMap; QHash signalToPropertyMap; + // Keeps property observers alive for as long as we track an object + std::unordered_multimap propertyObservers; + // Objects that changed their properties and are waiting for idle client. - // map of object name to map of signal index to arguments typedef QHash SignalToArgumentsMap; - typedef QHash PendingPropertyUpdates; + + // A set of plain property index (for bindable properties) and a map of + // signal index to arguments (for property updates from a notify signal). + // NOTIFY signals and their arguments are first collected and then mapped to + // the corresponding property in sendPendingPropertyUpdates() + struct PropertyUpdate + { + public: + SignalToArgumentsMap signalMap; + QSet plainProperties; + + /** + * Given a SignalToPropertyNameMap, returns the set of all property + * indices of properties that were changed in this PropertyUpdate. + */ + QSet propertyIndices(const SignalToPropertyNameMap &map) const; + }; + + // map of object to either a property index for plain bindable properties + // or a to map of signal index to arguments + typedef QHash PendingPropertyUpdates; PendingPropertyUpdates pendingPropertyUpdates; // Aggregate property updates since we get multiple Qt.idle message when we have multiple @@ -331,6 +380,14 @@ private: QBasicTimer timer; }; +inline QSet QMetaObjectPublisher::PropertyUpdate::propertyIndices(const SignalToPropertyNameMap &map) const { + auto indexes = plainProperties; + for (auto it = signalMap.cbegin(); it != signalMap.cend(); ++it) { + indexes += map.value(it.key()); + } + return indexes; +} + QT_END_NAMESPACE #endif // QMETAOBJECTPUBLISHER_P_H diff --git a/src/webchannel/qwebchannel.cpp b/src/webchannel/qwebchannel.cpp index 050d04e..2d7cc2d 100644 --- a/src/webchannel/qwebchannel.cpp +++ b/src/webchannel/qwebchannel.cpp @@ -175,6 +175,9 @@ QHash QWebChannel::registeredObjects() const The properties, signals and public methods of the \a object are published to the remote clients. There, an object with the identifier \a id is then constructed. + \note A property that is \c BINDABLE but does not have a \c NOTIFY signal will have working property + updates on the client side, but no mechanism to register a callback for the change notifications. + \note A current limitation is that objects must be registered before any client is initialized. \sa QWebChannel::registerObjects(), QWebChannel::deregisterObject(), QWebChannel::registeredObjects() -- cgit v1.2.3