From 59414b7c586d7e75b1738f70d58a34f26bf17338 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 27 Sep 2016 09:03:28 +0200 Subject: [PATCH 01/21] Plug memleak in tst_QStackedWidget To keep the change minimal, keep 'sw' as a pointer variable, but back it by a stack-allocated QStackedWidget instead of a heap-allocated one that's never deleted. Change-Id: I9e2a8c07979b861eb7e7040c144d8e75c90d0bc9 Reviewed-by: Giuseppe D'Angelo --- .../auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp b/tests/auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp index d060c4ceae..d9219f8941 100644 --- a/tests/auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp +++ b/tests/auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp @@ -168,7 +168,8 @@ private: void tst_QStackedWidget::dynamicPages() { - QStackedWidget *sw = new QStackedWidget; + QStackedWidget stackedWidget; + QStackedWidget *sw = &stackedWidget; TestPage *w1 = new TestPage(true); w1->setN(3); From dc737fa0d77a7549396efcf9ffa6e917dd40595f Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 27 Sep 2016 09:59:46 +0200 Subject: [PATCH 02/21] tst_QShortcut: Fix UB (invalid cast) in shortcutDestroyed() The slot is invoked from QObject::destroyed(), which is emitted from ~QObject. By that time the object is no longer a QShortcut, so the static_cast it invalid. Found by UBSan: tst_qshortcut.cpp:1210:53: runtime error: downcast of address 0x6020000289d0 which does not point to an object of type 'QShortcut' 0x6020000289d0: note: object is of type 'QObject' 10 00 80 17 c0 ce 63 df 93 2b 00 00 b0 02 00 00 d0 60 00 00 02 00 00 00 ff ff ff 04 04 00 00 00 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QObject' #0 0x42b3bb in tst_QShortcut::shortcutDestroyed(QObject*) tst_qshortcut.cpp:1210 #1 0x446cc9 in tst_QShortcut::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) .moc/tst_qshortcut.moc:186 #2 0x2b93dba52c86 in QMetaObject::activate(QObject*, int, int, void**) qobject.cpp:3787 #3 0x2b93dba55400 in QObject::destroyed(QObject*) .moc/moc_qobject.cpp:213 #4 0x2b93dba8d80d in QObject::~QObject() qobject.cpp:967 #5 0x2b93c6b6e032 in QShortcut::~QShortcut() qshortcut.cpp:476 #6 0x2b93c6b6e370 in QShortcut::~QShortcut() qshortcut.cpp:481 #7 0x42a5de in void qDeleteAll::const_iterator>(QList::const_iterator, QList::const_iterator) qalgorithms.h:317 #8 0x42a5de in void qDeleteAll >(QList const&) qalgorithms.h:325 #9 0x42a5de in tst_QShortcut::clearAllShortcuts() tst_qshortcut.cpp:1136 Fix by replacing QVector::replaceAll() with the erase-remove idiom, which does not require the cast, because it can perform mixed-type lookups. Change-Id: I4251c1895fa4398023f489dbfd7108d90c1a6c94 Reviewed-by: Giuseppe D'Angelo --- tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp b/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp index 15aef8d503..8fcc14bf00 100644 --- a/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp +++ b/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp @@ -1211,7 +1211,8 @@ QShortcut *tst_QShortcut::setupShortcut(QWidget *parent, const char *name, int t void tst_QShortcut::shortcutDestroyed(QObject* obj) { - shortcuts.removeAll(static_cast(obj)); + shortcuts.erase(std::remove(shortcuts.begin(), shortcuts.end(), obj), + shortcuts.end()); } void tst_QShortcut::sendKeyEvents(int k1, QChar c1, int k2, QChar c2, int k3, QChar c3, int k4, QChar c4) From 840e2dd2f04a32120e2ca450fba6f79b06ca2515 Mon Sep 17 00:00:00 2001 From: Vladimir Prus Date: Fri, 20 Nov 2015 11:20:02 +0300 Subject: [PATCH 03/21] Make sure SSL configuration is correct in QNetworkReply::encrypted. In some cases, when QNetworkReply::encrypted is emitted, QNetworkReply::sslConfiguration is not yet initialized, in particular certificate chain is empty, which breaks the documented usage of 'encrypted' to perform additional checks on certificate chain. It looks to be caused by the fact that QHttpNetworkReply is originally associated with 0th QHttpNetworkConnectionChannel, and this association is not updated if HTTP pipelining is not used. Therefore, a reply on channel >0 might arrive before reply on channel 0, and then using ssl configuration from channel 0, which not made it through handshake, is not usable. Task-number: QTBUG-49554 Change-Id: Ie5d4b5a0c503d5bdc44761ce8581f6ffe4e3bac2 Reviewed-by: Markus Goetz (Woboq GmbH) --- src/network/access/qhttpnetworkconnection.cpp | 15 +++++++++++---- src/network/access/qhttpnetworkconnection_p.h | 1 + .../access/qhttpnetworkconnectionchannel.cpp | 3 +-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp index c4cb8e65c0..452b998b6f 100644 --- a/src/network/access/qhttpnetworkconnection.cpp +++ b/src/network/access/qhttpnetworkconnection.cpp @@ -666,8 +666,7 @@ bool QHttpNetworkConnectionPrivate::dequeueRequest(QAbstractSocket *socket) HttpMessagePair messagePair = highPriorityQueue.takeLast(); if (!messagePair.second->d_func()->requestIsPrepared) prepareRequest(messagePair); - channels[i].request = messagePair.first; - channels[i].reply = messagePair.second; + updateChannel(i, messagePair); return true; } @@ -676,13 +675,21 @@ bool QHttpNetworkConnectionPrivate::dequeueRequest(QAbstractSocket *socket) HttpMessagePair messagePair = lowPriorityQueue.takeLast(); if (!messagePair.second->d_func()->requestIsPrepared) prepareRequest(messagePair); - channels[i].request = messagePair.first; - channels[i].reply = messagePair.second; + updateChannel(i, messagePair); return true; } return false; } +void QHttpNetworkConnectionPrivate::updateChannel(int i, const HttpMessagePair &messagePair) +{ + channels[i].request = messagePair.first; + channels[i].reply = messagePair.second; + // Now that reply is assigned a channel, correct reply to channel association + // previously set in queueRequest. + channels[i].reply->d_func()->connectionChannel = &channels[i]; +} + QHttpNetworkRequest QHttpNetworkConnectionPrivate::predictNextRequest() { if (!highPriorityQueue.isEmpty()) diff --git a/src/network/access/qhttpnetworkconnection_p.h b/src/network/access/qhttpnetworkconnection_p.h index 9af39d416a..d63e722be1 100644 --- a/src/network/access/qhttpnetworkconnection_p.h +++ b/src/network/access/qhttpnetworkconnection_p.h @@ -207,6 +207,7 @@ public: void requeueRequest(const HttpMessagePair &pair); // e.g. after pipeline broke bool dequeueRequest(QAbstractSocket *socket); void prepareRequest(HttpMessagePair &request); + void updateChannel(int i, const HttpMessagePair &messagePair); QHttpNetworkRequest predictNextRequest(); void fillPipeline(QAbstractSocket *socket); diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp index dfa5d0e74a..946df8d8f9 100644 --- a/src/network/access/qhttpnetworkconnectionchannel.cpp +++ b/src/network/access/qhttpnetworkconnectionchannel.cpp @@ -1070,6 +1070,7 @@ void QHttpNetworkConnectionChannel::_q_encrypted() connection->d_func()->dequeueRequest(socket); if (reply) { reply->setSpdyWasUsed(false); + Q_ASSERT(reply->d_func()->connectionChannel == this); emit reply->encrypted(); } if (reply) @@ -1109,8 +1110,6 @@ void QHttpNetworkConnectionChannel::_q_sslErrors(const QList &errors) connection->d_func()->pauseConnection(); if (pendingEncrypt && !reply) connection->d_func()->dequeueRequest(socket); - if (reply) // a reply was actually dequeued. - reply->d_func()->connectionChannel = this; // set correct channel like in sendRequest() and queueRequest(); if (connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP) { if (reply) emit reply->sslErrors(errors); From 98cb4977722da9d1acd764a05173529fe1618acf Mon Sep 17 00:00:00 2001 From: BogDan Vatra Date: Tue, 4 Oct 2016 15:36:58 +0300 Subject: [PATCH 04/21] Add QMAKE_LFLAGS_SONAME to linker flags for Android plugins It's needed to shut up Android 6+ warnings Task-number: QTBUG-52112 Change-Id: I21ff53d687bf545250ec7fcdc059db16d4cecbc9 Reviewed-by: Oswald Buddenhagen Reviewed-by: Eskil Abrahamsen Blomfeldt --- mkspecs/android-clang/qmake.conf | 2 +- mkspecs/android-g++/qmake.conf | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mkspecs/android-clang/qmake.conf b/mkspecs/android-clang/qmake.conf index 9758546fd4..b25a4399f3 100644 --- a/mkspecs/android-clang/qmake.conf +++ b/mkspecs/android-clang/qmake.conf @@ -3,7 +3,7 @@ MAKEFILE_GENERATOR = UNIX QMAKE_PLATFORM = android QMAKE_COMPILER = gcc clang llvm -CONFIG += android_install unversioned_soname unversioned_libname android_deployment_settings +CONFIG += android_install unversioned_soname unversioned_libname plugin_with_soname android_deployment_settings include(../common/linux.conf) include(../common/clang.conf) diff --git a/mkspecs/android-g++/qmake.conf b/mkspecs/android-g++/qmake.conf index 50d1fd6cf2..2b81ac58f0 100644 --- a/mkspecs/android-g++/qmake.conf +++ b/mkspecs/android-g++/qmake.conf @@ -3,7 +3,7 @@ MAKEFILE_GENERATOR = UNIX QMAKE_PLATFORM = android QMAKE_COMPILER = gcc -CONFIG += android_install unversioned_soname unversioned_libname android_deployment_settings +CONFIG += android_install unversioned_soname unversioned_libname plugin_with_soname android_deployment_settings include(../common/linux.conf) include(../common/gcc-base-unix.conf) From 8f4054a7e6d5052a1030ce42c680f933c6f3a896 Mon Sep 17 00:00:00 2001 From: Kai Koehne Date: Thu, 29 Sep 2016 10:42:20 +0200 Subject: [PATCH 05/21] Doc: Document that by default, QTimer is not single-shot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I586997ddb5ed55d68f53ddfe9302b961296cc4eb Reviewed-by: Topi Reiniö --- src/corelib/kernel/qtimer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/corelib/kernel/qtimer.cpp b/src/corelib/kernel/qtimer.cpp index af9a1be6ab..f6c59055a8 100644 --- a/src/corelib/kernel/qtimer.cpp +++ b/src/corelib/kernel/qtimer.cpp @@ -532,6 +532,8 @@ void QTimer::singleShot(int msec, Qt::TimerType timerType, const QObject *receiv A single-shot timer fires only once, non-single-shot timers fire every \l interval milliseconds. + The default value for this property is \c false. + \sa interval, singleShot() */ From 4b975405d70529408b2422e42720a80152746c20 Mon Sep 17 00:00:00 2001 From: Richard Moe Gustavsen Date: Wed, 5 Oct 2016 14:27:55 +0200 Subject: [PATCH 06/21] QTextEdit: don't show placeholder text while composing text If using IM to compose text in QTextEdit, the placeholder text will show underneath until the text is committed. This patch will additionally check if the user is currently composing preedit text before deciding whether or not to draw the placeholder text. Task-number: QTBUG-55758 Change-Id: If7943c6c94fb96d46514a81caa118829e6e6a0f9 Reviewed-by: Liang Qi --- src/widgets/widgets/qtextedit.cpp | 2 +- src/widgets/widgets/qwidgettextcontrol.cpp | 5 +++++ src/widgets/widgets/qwidgettextcontrol_p.h | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/widgets/widgets/qtextedit.cpp b/src/widgets/widgets/qtextedit.cpp index 28666f76ce..7e4b86d8fd 100644 --- a/src/widgets/widgets/qtextedit.cpp +++ b/src/widgets/widgets/qtextedit.cpp @@ -1506,7 +1506,7 @@ void QTextEditPrivate::paint(QPainter *p, QPaintEvent *e) if (layout) layout->setViewport(QRect()); - if (!placeholderText.isEmpty() && doc->isEmpty()) { + if (!placeholderText.isEmpty() && doc->isEmpty() && !control->isPreediting()) { QColor col = control->palette().text().color(); col.setAlpha(128); p->setPen(col); diff --git a/src/widgets/widgets/qwidgettextcontrol.cpp b/src/widgets/widgets/qwidgettextcontrol.cpp index deca002bf5..4199ae537d 100644 --- a/src/widgets/widgets/qwidgettextcontrol.cpp +++ b/src/widgets/widgets/qwidgettextcontrol.cpp @@ -2557,6 +2557,11 @@ bool QWidgetTextControl::isWordSelectionEnabled() const return d->wordSelectionEnabled; } +bool QWidgetTextControl::isPreediting() +{ + return d_func()->isPreediting(); +} + #ifndef QT_NO_PRINTER void QWidgetTextControl::print(QPagedPaintDevice *printer) const { diff --git a/src/widgets/widgets/qwidgettextcontrol_p.h b/src/widgets/widgets/qwidgettextcontrol_p.h index c1180fd665..cd5abf4f5d 100644 --- a/src/widgets/widgets/qwidgettextcontrol_p.h +++ b/src/widgets/widgets/qwidgettextcontrol_p.h @@ -166,6 +166,8 @@ public: bool isWordSelectionEnabled() const; void setWordSelectionEnabled(bool enabled); + bool isPreediting(); + void print(QPagedPaintDevice *printer) const; virtual int hitTest(const QPointF &point, Qt::HitTestAccuracy accuracy) const; From 0bf0c3e184448e91a7ddbd20b03997538ae19f41 Mon Sep 17 00:00:00 2001 From: Frederik Schwarzer Date: Thu, 29 Sep 2016 17:07:00 +0200 Subject: [PATCH 07/21] Use the same object in description as in described code Change-Id: If52ecfc8d29a83cb2949fbbf4672ae386ae5d739 Reviewed-by: Shawn Rutledge Reviewed-by: Venugopal Shivashankar --- src/widgets/doc/src/model-view-programming.qdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/widgets/doc/src/model-view-programming.qdoc b/src/widgets/doc/src/model-view-programming.qdoc index 784e25b1d9..7e278b8c91 100644 --- a/src/widgets/doc/src/model-view-programming.qdoc +++ b/src/widgets/doc/src/model-view-programming.qdoc @@ -737,7 +737,7 @@ \image spinboxdelegate-example.png - We subclass the delegate from \l QItemDelegate because we do not want + We subclass the delegate from \l QStyledItemDelegate because we do not want to write custom display functions. However, we must still provide functions to manage the editor widget: From 0f21e0bc198af106421b02275a053c82a2eded3d Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 15 Sep 2016 09:43:01 +0200 Subject: [PATCH 08/21] QDBusDemarshaller: use RAII in duplicate() QtDBus is compiled with exceptions disabled, but checkers don't know that, and it's not 100% certain it will stay that way until eternity. So do the simple change and hold the new'ed pointer in a QScopedPointer until handing it off to create(). Coverity-Id: 154477 Change-Id: I91a763ca4e93585c97cb9e794312b53046971161 Reviewed-by: Thiago Macieira --- src/dbus/qdbusdemarshaller.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/dbus/qdbusdemarshaller.cpp b/src/dbus/qdbusdemarshaller.cpp index cf631baa4f..3852e53d57 100644 --- a/src/dbus/qdbusdemarshaller.cpp +++ b/src/dbus/qdbusdemarshaller.cpp @@ -33,6 +33,9 @@ #include "qdbusargument_p.h" #include "qdbusconnection.h" + +#include + #include QT_BEGIN_NAMESPACE @@ -417,12 +420,12 @@ QDBusDemarshaller *QDBusDemarshaller::endCommon() QDBusArgument QDBusDemarshaller::duplicate() { - QDBusDemarshaller *d = new QDBusDemarshaller(capabilities); + QScopedPointer d(new QDBusDemarshaller(capabilities)); d->iterator = iterator; d->message = q_dbus_message_ref(message); q_dbus_message_iter_next(&iterator); - return QDBusArgumentPrivate::create(d); + return QDBusArgumentPrivate::create(d.take()); } QT_END_NAMESPACE From daaa1a287bcccda61ce81941f8b3a69d2371e04a Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Wed, 28 Sep 2016 10:37:59 +0200 Subject: [PATCH 09/21] Plug leak in QFormLayout::setWidget() Unlike layouts and spacer items, a QWidget is-not-a QLayoutItem. QWidgetItem simply wraps the QWidget, so in QFormLayout::setWidget(), we allocate a widget item for the widget passed, and hand that down to Private::setItem() for adding to the various data structures. Private::setItem() has a bunch of guard clauses, though, that return without deleting the item. A test triggered this code path and made asan complain. This is just one part of a larger problem: QFormLayout::setLayout() normally takes ownership of the layout passed, because QLayouts own their QLayoutItems, and QLayout is-a QLayoutItem. But setLayout() fails to live up to the owner role when it fails to add a layout, and there's no easy way for the API user to check for success. A fix for this breaks tst_qformlayout, and while those checks that break deserve to be broken, I'll refrain from proposing the larger fix for 5.6 LTS, but will propose it for 5.8 or 5.9 instead. This fix here only fixes the leak in setWidget() by adding a bool return to Private::setItem() informing Private::setWidget() of the need to manually delete the item it allocated for the widget. Change-Id: I81409c260f9bee2e95c9a98542d8c60bc19a1332 Reviewed-by: Thiago Macieira Reviewed-by: Giuseppe D'Angelo --- src/widgets/kernel/qformlayout.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/widgets/kernel/qformlayout.cpp b/src/widgets/kernel/qformlayout.cpp index 24bf80f16c..0ae617d945 100644 --- a/src/widgets/kernel/qformlayout.cpp +++ b/src/widgets/kernel/qformlayout.cpp @@ -179,7 +179,7 @@ public: int insertRow(int row); void insertRows(int row, int count); - void setItem(int row, QFormLayout::ItemRole role, QLayoutItem *item); + bool setItem(int row, QFormLayout::ItemRole role, QLayoutItem *item); void setLayout(int row, QFormLayout::ItemRole role, QLayout *layout); void setWidget(int row, QFormLayout::ItemRole role, QWidget *widget); @@ -941,21 +941,21 @@ void QFormLayoutPrivate::insertRows(int row, int count) } } -void QFormLayoutPrivate::setItem(int row, QFormLayout::ItemRole role, QLayoutItem *item) +bool QFormLayoutPrivate::setItem(int row, QFormLayout::ItemRole role, QLayoutItem *item) { const bool fullRow = role == QFormLayout::SpanningRole; const int column = role == QFormLayout::SpanningRole ? 1 : static_cast(role); if (uint(row) >= uint(m_matrix.rowCount()) || uint(column) > 1U) { qWarning("QFormLayoutPrivate::setItem: Invalid cell (%d, %d)", row, column); - return; + return false; } if (!item) - return; + return false; if (m_matrix(row, column)) { qWarning("QFormLayoutPrivate::setItem: Cell (%d, %d) already occupied", row, column); - return; + return false; } QFormLayoutItem *i = new QFormLayoutItem(item); @@ -963,6 +963,7 @@ void QFormLayoutPrivate::setItem(int row, QFormLayout::ItemRole role, QLayoutIte m_matrix(row, column) = i; m_things.append(i); + return true; } void QFormLayoutPrivate::setLayout(int row, QFormLayout::ItemRole role, QLayout *layout) @@ -979,7 +980,9 @@ void QFormLayoutPrivate::setWidget(int row, QFormLayout::ItemRole role, QWidget if (widget) { Q_Q(QFormLayout); q->addChildWidget(widget); - setItem(row, role, QLayoutPrivate::createWidgetItem(q, widget)); + QWidgetItem *item = QLayoutPrivate::createWidgetItem(q, widget); + if (!setItem(row, role, item)) + delete item; } } From 237b36a72cf0646ba28e762bfde0cb398f4041e8 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Wed, 21 Sep 2016 09:15:40 +0200 Subject: [PATCH 10/21] QGraphicsWidget: Fix UB (invalid member calls) in destruction sequence Found by UBSan: qgraphicswidget_p.h:72:5: runtime error: downcast of address 0x2ab6a8021400 which does not point to an object of type 'QGraphicsWidget' 0x2ab6a8021400: note: object is of type 'QGraphicsObject' 00 00 00 00 70 93 5c 91 b6 2a 00 00 f0 c0 01 a8 b6 2a 00 00 e8 81 5c 91 b6 2a 00 00 10 bf 01 a8 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QGraphicsObject' #0 0x2ab68f2fdd7c in QGraphicsWidgetPrivate::q_func() qgraphicswidget_p.h:72 #1 0x2ab68f2fdd7c in QGraphicsWidgetPrivate::fixFocusChainBeforeReparenting(QGraphicsWidget*, QGraphicsScene*, QGraphicsScene*) qgraphicswidget_p.cpp:775 #2 0x2ab68f020d2a in QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem*, QVariant const*, QVariant const*) qgraphicsitem.cpp:1181 #3 0x2ab68f024f73 in QGraphicsItem::setParentItem(QGraphicsItem*) qgraphicsitem.cpp:1781 #4 0x2ab68f168401 in QGraphicsScenePrivate::removeItemHelper(QGraphicsItem*) qgraphicsscene.cpp:620 #5 0x2ab68f02c166 in QGraphicsItem::~QGraphicsItem() qgraphicsitem.cpp:1555 #6 0x2ab68f02ebb8 in QGraphicsObject::~QGraphicsObject() qgraphicsitem.cpp:7766 #7 0x2ab68f2d8888 in QGraphicsWidget::~QGraphicsWidget() qgraphicswidget.cpp:231 #8 0x4bce62 in SubQGraphicsWidget::~SubQGraphicsWidget() /tst_qgraphicswidget.cpp:175 #9 0x4bce62 in SubQGraphicsWidget::~SubQGraphicsWidget() /tst_qgraphicswidget.cpp:175 #10 0x2ab68f02c9ec in QGraphicsItem::~QGraphicsItem() qgraphicsitem.cpp:1550 #11 0x2ab68f02ebb8 in QGraphicsObject::~QGraphicsObject() qgraphicsitem.cpp:7766 #12 0x2ab68f2d8888 in QGraphicsWidget::~QGraphicsWidget() qgraphicswidget.cpp:231 #13 0x4bce62 in SubQGraphicsWidget::~SubQGraphicsWidget() /tst_qgraphicswidget.cpp:175 #14 0x4bce62 in SubQGraphicsWidget::~SubQGraphicsWidget() /tst_qgraphicswidget.cpp:175 #15 0x2ab68f128da4 in QGraphicsScene::clear() qgraphicsscene.cpp:2388 #16 0x2ab68f12936c in QGraphicsScene::~QGraphicsScene() qgraphicsscene.cpp:1682 #17 0x44d44c in tst_QGraphicsWidget::focusWidget() /tst_qgraphicswidget.cpp:435 qgraphicswidget_p.cpp:805:24: runtime error: member call on address 0x2ab6a8021400 which does not point to an object of type 'QGraphicsWidget' 0x2ab6a8021400: note: object is of type 'QGraphicsObject' 00 00 00 00 70 93 5c 91 b6 2a 00 00 f0 c0 01 a8 b6 2a 00 00 e8 81 5c 91 b6 2a 00 00 10 bf 01 a8 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QGraphicsObject' #0 0x2ab68f2fdc68 in QGraphicsWidgetPrivate::fixFocusChainBeforeReparenting(QGraphicsWidget*, QGraphicsScene*, QGraphicsScene*) qgraphicswidget_p.cpp:805 #1 0x2ab68f020d2a in QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem*, QVariant const*, QVariant const*) qgraphicsitem.cpp:1181 [... identical lines omitted ...] qgraphicswidget_p.cpp:806:23: runtime error: member call on address 0x2ab6a8021400 which does not point to an object of type 'QGraphicsWidget' 0x2ab6a8021400: note: object is of type 'QGraphicsObject' 00 00 00 00 70 93 5c 91 b6 2a 00 00 f0 c0 01 a8 b6 2a 00 00 e8 81 5c 91 b6 2a 00 00 10 bf 01 a8 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QGraphicsObject' #0 0x2ab68f2fdb6b in QGraphicsWidgetPrivate::fixFocusChainBeforeReparenting(QGraphicsWidget*, QGraphicsScene*, QGraphicsScene*) qgraphicswidget_p.cpp:806 #1 0x2ab68f020d2a in QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem*, QVariant const*, QVariant const*) qgraphicsitem.cpp:1181 [... identical lines omitted ...] qgraphicswidget_p.cpp:827:26: runtime error: member call on address 0x2ab6a8021400 which does not point to an object of type 'QGraphicsWidget' 0x2ab6a8021400: note: object is of type 'QGraphicsObject' 00 00 00 00 70 93 5c 91 b6 2a 00 00 f0 c0 01 a8 b6 2a 00 00 e8 81 5c 91 b6 2a 00 00 10 bf 01 a8 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QGraphicsObject' #0 0x2ab68f2fdf91 in QGraphicsWidgetPrivate::fixFocusChainBeforeReparenting(QGraphicsWidget*, QGraphicsScene*, QGraphicsScene*) qgraphicswidget_p.cpp:827 #1 0x2ab68f020d2a in QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem*, QVariant const*, QVariant const*) qgraphicsitem.cpp:1181 [... identical lines omitted ...] Fix by moving the setParentItem(nullptr) call up the call stack into ~QGraphicsWidget(), ensuring that the object is still a QGraphicsWidget when these calls are made. Change-Id: I264779e33098e9752de9a312a146fb203578a3cc Reviewed-by: Thiago Macieira Reviewed-by: Giuseppe D'Angelo --- src/widgets/graphicsview/qgraphicswidget.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/widgets/graphicsview/qgraphicswidget.cpp b/src/widgets/graphicsview/qgraphicswidget.cpp index 2700605a71..125174627d 100644 --- a/src/widgets/graphicsview/qgraphicswidget.cpp +++ b/src/widgets/graphicsview/qgraphicswidget.cpp @@ -267,6 +267,11 @@ QGraphicsWidget::~QGraphicsWidget() // Remove this graphics widget from widgetStyles widgetStyles()->setStyleForWidget(this, 0); + + // Unset the parent here, when we're still a QGraphicsWidget. + // It is otherwise done in ~QGraphicsItem() where we'd be + // calling QGraphicsWidget members on an ex-QGraphicsWidget object + setParentItem(Q_NULLPTR); } /*! From f9acbaccde278eaf44a5324c2a63a99a4cccfb1c Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 26 Aug 2016 12:29:48 +0200 Subject: [PATCH 11/21] QPixmap::load: ensure QBitmap stays a QBitmap even on failure ... and avoid detach()ing potentially large data for just preserving the QPlatformPixmap::pixelType(). A QBitmap differs from a QPixmap (its base class, urgh) by always having a data != nullptr and a Bitmap pixel type, yet load() was unconditionally setting 'data' to nullptr on failure, turning a QBitmap into a non-QBitmap. Fix by move-assigning a null QBitmap instead of resetting 'data'. Add some tests. Change-Id: Ida58b3b24d96472a5f9d0f18f81cc763edcf3c16 Reviewed-by: Anton Kudryavtsev Reviewed-by: Edward Welbourne Reviewed-by: Ulf Hermann --- src/gui/image/qpixmap.cpp | 58 ++++++++++---------- tests/auto/gui/image/qpixmap/tst_qpixmap.cpp | 27 +++++++++ 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/gui/image/qpixmap.cpp b/src/gui/image/qpixmap.cpp index 78031bca97..3726b2124c 100644 --- a/src/gui/image/qpixmap.cpp +++ b/src/gui/image/qpixmap.cpp @@ -762,39 +762,37 @@ QBitmap QPixmap::createMaskFromColor(const QColor &maskColor, Qt::MaskMode mode) bool QPixmap::load(const QString &fileName, const char *format, Qt::ImageConversionFlags flags) { - if (fileName.isEmpty()) { - data.reset(); - return false; + if (!fileName.isEmpty()) { + + QFileInfo info(fileName); + // Note: If no extension is provided, we try to match the + // file against known plugin extensions + if (info.completeSuffix().isEmpty() || info.exists()) { + + QString key = QLatin1String("qt_pixmap") + % info.absoluteFilePath() + % HexString(info.lastModified().toTime_t()) + % HexString(info.size()) + % HexString(data ? data->pixelType() : QPlatformPixmap::PixmapType); + + if (QPixmapCache::find(key, this)) + return true; + + data = QPlatformPixmap::create(0, 0, data ? data->pixelType() : QPlatformPixmap::PixmapType); + + if (data->fromFile(fileName, format, flags)) { + QPixmapCache::insert(key, *this); + return true; + } + } } - detach(); - - QFileInfo info(fileName); - QString key = QLatin1String("qt_pixmap") - % info.absoluteFilePath() - % HexString(info.lastModified().toTime_t()) - % HexString(info.size()) - % HexString(data ? data->pixelType() : QPlatformPixmap::PixmapType); - - // Note: If no extension is provided, we try to match the - // file against known plugin extensions - if (!info.completeSuffix().isEmpty() && !info.exists()) { - data.reset(); - return false; + if (!isNull()) { + if (isQBitmap()) + *this = QBitmap(); + else + data.reset(); } - - if (QPixmapCache::find(key, this)) - return true; - - if (!data) - data = QPlatformPixmap::create(0, 0, QPlatformPixmap::PixmapType); - - if (data->fromFile(fileName, format, flags)) { - QPixmapCache::insert(key, *this); - return true; - } - - data.reset(); return false; } diff --git a/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp b/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp index 4ffe357d09..286f00c111 100644 --- a/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp +++ b/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp @@ -1508,6 +1508,33 @@ void tst_QPixmap::loadAsBitmapOrPixmap() QVERIFY(!bitmap.isNull()); QCOMPARE(bitmap.depth(), 1); QVERIFY(bitmap.isQBitmap()); + + // check that a QBitmap stays a QBitmap even when loading fails: + ok = bitmap.load(QString()); + QVERIFY(!ok); + QVERIFY(bitmap.isNull()); + QVERIFY(bitmap.isQBitmap()); + + ok = bitmap.load("does not exist"); + QVERIFY(!ok); + QVERIFY(bitmap.isNull()); + QVERIFY(bitmap.isQBitmap()); + + ok = bitmap.load("does not exist.png"); + QVERIFY(!ok); + QVERIFY(bitmap.isNull()); + QVERIFY(bitmap.isQBitmap()); + + QTemporaryFile garbage; + QVERIFY(garbage.open()); + const QString garbagePath = garbage.fileName(); + garbage.write(reinterpret_cast(&garbage), sizeof garbage); + garbage.close(); + + ok = bitmap.load(garbagePath); + QVERIFY(!ok); + QVERIFY(bitmap.isNull()); + QVERIFY(bitmap.isQBitmap()); } void tst_QPixmap::toImageDeepCopy() From dd00f6dd91a28f3e76bc4c3e894174efffc7f201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Martins?= Date: Sun, 28 Aug 2016 21:34:32 +0100 Subject: [PATCH 12/21] Handle short reads in the local sockets example Protection against short reads was already half implemented, blockSize was being sent by the server but never used by the client. Also, blockSize was bumped to quint32: If you're in a position where short reads can happen then quint16 is probably not enough to hold the size of your data. On Linux I could only reproduce short reads for messages > 500K. Change-Id: I191a3d781da1d8a119debbdafae641c8340a1da2 Reviewed-by: Giuseppe D'Angelo --- examples/corelib/ipc/localfortuneclient/client.cpp | 5 +++-- examples/corelib/ipc/localfortuneclient/client.h | 2 +- examples/corelib/ipc/localfortuneserver/server.cpp | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/examples/corelib/ipc/localfortuneclient/client.cpp b/examples/corelib/ipc/localfortuneclient/client.cpp index 853cb2b087..84c0db9a0e 100644 --- a/examples/corelib/ipc/localfortuneclient/client.cpp +++ b/examples/corelib/ipc/localfortuneclient/client.cpp @@ -100,12 +100,13 @@ void Client::readFortune() in.setVersion(QDataStream::Qt_4_0); if (blockSize == 0) { - if (socket->bytesAvailable() < (int)sizeof(quint16)) + // Relies on the fact that QDataStream format streams a quint32 into sizeof(quint32) bytes + if (socket->bytesAvailable() < (int)sizeof(quint32)) return; in >> blockSize; } - if (in.atEnd()) + if (socket->bytesAvailable() < blockSize || in.atEnd()) return; QString nextFortune; diff --git a/examples/corelib/ipc/localfortuneclient/client.h b/examples/corelib/ipc/localfortuneclient/client.h index f12957fe33..8ba767b4fa 100644 --- a/examples/corelib/ipc/localfortuneclient/client.h +++ b/examples/corelib/ipc/localfortuneclient/client.h @@ -76,7 +76,7 @@ private: QLocalSocket *socket; QString currentFortune; - quint16 blockSize; + quint32 blockSize; }; #endif diff --git a/examples/corelib/ipc/localfortuneserver/server.cpp b/examples/corelib/ipc/localfortuneserver/server.cpp index 5b64e917d9..fa867eb737 100644 --- a/examples/corelib/ipc/localfortuneserver/server.cpp +++ b/examples/corelib/ipc/localfortuneserver/server.cpp @@ -96,10 +96,10 @@ void Server::sendFortune() QByteArray block; QDataStream out(&block, QIODevice::WriteOnly); out.setVersion(QDataStream::Qt_4_0); - out << (quint16)0; + out << (quint32)0; out << fortunes.at(qrand() % fortunes.size()); out.device()->seek(0); - out << (quint16)(block.size() - sizeof(quint16)); + out << (quint32)(block.size() - sizeof(quint32)); QLocalSocket *clientConnection = server->nextPendingConnection(); connect(clientConnection, SIGNAL(disconnected()), From 4268dde1f13a4b8867fd439e8e24f85f8e4741a0 Mon Sep 17 00:00:00 2001 From: Sune Vuorela Date: Fri, 9 Sep 2016 09:12:54 +0200 Subject: [PATCH 13/21] QTemporaryFile's setFileTemplate operates not only on XXXXXX in the end Is even covered by unit tests. Change-Id: I7b22da2a338868fdb99c6238925f944bfea88190 Reviewed-by: Thiago Macieira --- src/corelib/io/qtemporaryfile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp index e8e8d8c878..bef900969d 100644 --- a/src/corelib/io/qtemporaryfile.cpp +++ b/src/corelib/io/qtemporaryfile.cpp @@ -666,7 +666,7 @@ QString QTemporaryFile::fileTemplate() const /*! Sets the static portion of the file name to \a name. If the file - template ends in XXXXXX that will automatically be replaced with + template contains XXXXXX that will automatically be replaced with the unique part of the filename, otherwise a filename will be determined automatically based on the static portion specified. From 62d13e2e9bbddf5f3e2ab24fd1138b970bc89805 Mon Sep 17 00:00:00 2001 From: Samuli Piippo Date: Wed, 5 Oct 2016 13:31:51 +0300 Subject: [PATCH 14/21] Skip chmod for separate_debug_info when cross compiling on Windows Task-number: QTBUG-56289 Change-Id: If6dd07182ed77e87865004c14240bf7f0c764772 Reviewed-by: Oswald Buddenhagen --- mkspecs/features/unix/separate_debug_info.prf | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mkspecs/features/unix/separate_debug_info.prf b/mkspecs/features/unix/separate_debug_info.prf index 272cc8ef79..ebb37bdfc7 100644 --- a/mkspecs/features/unix/separate_debug_info.prf +++ b/mkspecs/features/unix/separate_debug_info.prf @@ -87,8 +87,9 @@ have_target:!static:if(darwin|!isEmpty(QMAKE_OBJCOPY)) { QMAKE_POST_LINK = $$mkdir_debug_info && $$copy_debug_info && $$strip_debug_info $$QMAKE_POST_LINK } else { link_debug_info = $$QMAKE_OBJCOPY --add-gnu-debuglink=$$shell_target_debug_info $$shell_target - chmod_debug_info = chmod -x $$shell_target_debug_info - QMAKE_POST_LINK = $$copy_debug_info && $$strip_debug_info && $$link_debug_info && $$chmod_debug_info $$QMAKE_POST_LINK + !contains(QMAKE_HOST.os, Windows): \ + QMAKE_POST_LINK = && chmod -x $$shell_target_debug_info $$QMAKE_POST_LINK + QMAKE_POST_LINK = $$copy_debug_info && $$strip_debug_info && $$link_debug_info $$QMAKE_POST_LINK } silent:QMAKE_POST_LINK = @echo creating $@.$$debug_info_suffix && $$QMAKE_POST_LINK From c353bf0033b0f1b433f803bd81922e484f1e3a98 Mon Sep 17 00:00:00 2001 From: Sergio Martins Date: Thu, 6 Oct 2016 18:36:12 +0100 Subject: [PATCH 15/21] Link to topLevelChanged() in the docs of QDockWidget::floating topLevelChanged() is emitted when the floating property changes. It's not very well named, it's easy to miss. Change-Id: Iabaa4fb3dc6190df43d719ed7565f0586816c6de Reviewed-by: Martin Smith --- src/widgets/widgets/qdockwidget.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/widgets/widgets/qdockwidget.cpp b/src/widgets/widgets/qdockwidget.cpp index 20f35cb211..03423be647 100644 --- a/src/widgets/widgets/qdockwidget.cpp +++ b/src/widgets/widgets/qdockwidget.cpp @@ -1311,7 +1311,9 @@ QDockWidget::DockWidgetFeatures QDockWidget::features() const By default, this property is \c true. - \sa isWindow() + When this property changes, the \c {topLevelChanged()} signal is emitted. + + \sa isWindow(), topLevelChanged() */ void QDockWidget::setFloating(bool floating) { From eda095ebb47aa3549906ad9d65c6edad438c3825 Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Fri, 23 Sep 2016 15:01:06 +0200 Subject: [PATCH 16/21] Darwin: correct state restore when FSEventsStream starting fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous state was not restored completely when adding/removing paths resulted in a stream start failure. It also removes an autoreleasepool in restartStream, because both stopStream and startStream do already create an autoreleasepool of their own. (So, this pool will always be empty.) Change-Id: Idc674e9c040f346703ab3ec256957e787a0ade73 Reviewed-by: Marc Mutz Reviewed-by: Morten Johan Sørvig --- src/corelib/io/qfilesystemwatcher_fsevents.mm | 103 +++++++++++------- .../io/qfilesystemwatcher_fsevents_p.h | 19 +++- 2 files changed, 77 insertions(+), 45 deletions(-) diff --git a/src/corelib/io/qfilesystemwatcher_fsevents.mm b/src/corelib/io/qfilesystemwatcher_fsevents.mm index 7656530a46..c6fa6c9846 100644 --- a/src/corelib/io/qfilesystemwatcher_fsevents.mm +++ b/src/corelib/io/qfilesystemwatcher_fsevents.mm @@ -81,7 +81,7 @@ bool QFseventsFileSystemWatcherEngine::checkDir(DirsByName::iterator &it) if (res == -1) { needsRestart |= derefPath(info.watchedPath); emit emitDirectoryChanged(info.origPath, true); - it = watchedDirectories.erase(it); + it = watchingState.watchedDirectories.erase(it); } else if (st.st_ctimespec != info.ctime || st.st_mode != info.mode) { info.ctime = st.st_ctimespec; info.mode = st.st_mode; @@ -132,7 +132,8 @@ bool QFseventsFileSystemWatcherEngine::rescanDirs(const QString &path) { bool needsRestart = false; - for (DirsByName::iterator it = watchedDirectories.begin(); it != watchedDirectories.end(); ) { + for (DirsByName::iterator it = watchingState.watchedDirectories.begin(); + it != watchingState.watchedDirectories.end(); ) { if (it.key().startsWith(path)) needsRestart |= checkDir(it); else @@ -171,11 +172,12 @@ bool QFseventsFileSystemWatcherEngine::rescanFiles(const QString &path) { bool needsRestart = false; - for (FilesByPath::iterator i = watchedFiles.begin(); i != watchedFiles.end(); ) { + for (FilesByPath::iterator i = watchingState.watchedFiles.begin(); + i != watchingState.watchedFiles.end(); ) { if (i.key().startsWith(path)) { needsRestart |= rescanFiles(i.value()); if (i.value().isEmpty()) { - i = watchedFiles.erase(i); + i = watchingState.watchedFiles.erase(i); continue; } } @@ -226,8 +228,8 @@ void QFseventsFileSystemWatcherEngine::processEvent(ConstFSEventStreamRef stream if (eFlags & kFSEventStreamEventFlagRootChanged) { // re-check everything: - DirsByName::iterator dirIt = watchedDirectories.find(path); - if (dirIt != watchedDirectories.end()) + DirsByName::iterator dirIt = watchingState.watchedDirectories.find(path); + if (dirIt != watchingState.watchedDirectories.end()) needsRestart |= checkDir(dirIt); needsRestart |= rescanFiles(path); continue; @@ -237,13 +239,13 @@ void QFseventsFileSystemWatcherEngine::processEvent(ConstFSEventStreamRef stream needsRestart |= rescanDirs(path); // check watched directories: - DirsByName::iterator dirIt = watchedDirectories.find(path); - if (dirIt != watchedDirectories.end()) + DirsByName::iterator dirIt = watchingState.watchedDirectories.find(path); + if (dirIt != watchingState.watchedDirectories.end()) needsRestart |= checkDir(dirIt); // check watched files: - FilesByPath::iterator pIt = watchedFiles.find(path); - if (pIt != watchedFiles.end()) + FilesByPath::iterator pIt = watchingState.watchedFiles.find(path); + if (pIt != watchingState.watchedFiles.end()) needsRestart |= rescanFiles(pIt.value()); } @@ -276,12 +278,11 @@ void QFseventsFileSystemWatcherEngine::doEmitDirectoryChanged(const QString &pat emit directoryChanged(path, removed); } -void QFseventsFileSystemWatcherEngine::restartStream() +bool QFseventsFileSystemWatcherEngine::restartStream() { - QMacAutoReleasePool pool; QMutexLocker locker(&lock); stopStream(); - startStream(); + return startStream(); } QFseventsFileSystemWatcherEngine *QFseventsFileSystemWatcherEngine::create(QObject *parent) @@ -311,6 +312,7 @@ QFseventsFileSystemWatcherEngine::~QFseventsFileSystemWatcherEngine() { QMacAutoReleasePool pool; + // Stop the stream in case we have to wait for the lock below to be acquired. if (stream) FSEventStreamStop(stream); @@ -334,8 +336,10 @@ QStringList QFseventsFileSystemWatcherEngine::addPaths(const QStringList &paths, QMutexLocker locker(&lock); + bool wasRunning = stream != Q_NULLPTR; bool needsRestart = false; + WatchingState oldState = watchingState; QStringList p = paths; QMutableListIterator it(p); while (it.hasNext()) { @@ -356,7 +360,7 @@ QStringList QFseventsFileSystemWatcherEngine::addPaths(const QStringList &paths, const bool isDir = S_ISDIR(st.st_mode); if (isDir) { - if (watchedDirectories.contains(realPath)) + if (watchingState.watchedDirectories.contains(realPath)) continue; directories->append(origPath); watchedPath = realPath; @@ -371,17 +375,18 @@ QStringList QFseventsFileSystemWatcherEngine::addPaths(const QStringList &paths, parentPath = watchedPath; } - for (PathRefCounts::const_iterator i = watchedPaths.begin(), ei = watchedPaths.end(); i != ei; ++i) { + for (PathRefCounts::const_iterator i = watchingState.watchedPaths.begin(), + ei = watchingState.watchedPaths.end(); i != ei; ++i) { if (watchedPath.startsWith(i.key())) { watchedPath = i.key(); break; } } - PathRefCounts::iterator it = watchedPaths.find(watchedPath); - if (it == watchedPaths.end()) { + PathRefCounts::iterator it = watchingState.watchedPaths.find(watchedPath); + if (it == watchingState.watchedPaths.end()) { needsRestart = true; - watchedPaths.insert(watchedPath, 1); + watchingState.watchedPaths.insert(watchedPath, 1); DEBUG("Adding '%s' to watchedPaths", qPrintable(watchedPath)); } else { ++it.value(); @@ -392,18 +397,25 @@ QStringList QFseventsFileSystemWatcherEngine::addPaths(const QStringList &paths, DirInfo dirInfo; dirInfo.dirInfo = info; dirInfo.entries = scanForDirEntries(realPath); - watchedDirectories.insert(realPath, dirInfo); + watchingState.watchedDirectories.insert(realPath, dirInfo); DEBUG("-- Also adding '%s' to watchedDirectories", qPrintable(realPath)); } else { - watchedFiles[parentPath].insert(realPath, info); + watchingState.watchedFiles[parentPath].insert(realPath, info); DEBUG("-- Also adding '%s' to watchedFiles", qPrintable(realPath)); } } if (needsRestart) { stopStream(); - if (!startStream()) + if (!startStream()) { + // ok, something went wrong, let's try to restore the previous state + watchingState = qMove(oldState); + // and because we don't know which path caused the issue (if any), fail on all of them p = paths; + + if (wasRunning) + startStream(); + } } return p; @@ -419,6 +431,7 @@ QStringList QFseventsFileSystemWatcherEngine::removePaths(const QStringList &pat bool needsRestart = false; + WatchingState oldState = watchingState; QStringList p = paths; QMutableListIterator it(p); while (it.hasNext()) { @@ -431,10 +444,10 @@ QStringList QFseventsFileSystemWatcherEngine::removePaths(const QStringList &pat realPath = fi.canonicalFilePath(); if (fi.isDir()) { - DirsByName::iterator dirIt = watchedDirectories.find(realPath); - if (dirIt != watchedDirectories.end()) { + DirsByName::iterator dirIt = watchingState.watchedDirectories.find(realPath); + if (dirIt != watchingState.watchedDirectories.end()) { needsRestart |= derefPath(dirIt->dirInfo.watchedPath); - watchedDirectories.erase(dirIt); + watchingState.watchedDirectories.erase(dirIt); directories->removeAll(origPath); it.remove(); DEBUG("Removed directory '%s'", qPrintable(realPath)); @@ -442,15 +455,15 @@ QStringList QFseventsFileSystemWatcherEngine::removePaths(const QStringList &pat } else { QFileInfo fi(realPath); QString parentPath = fi.path(); - FilesByPath::iterator pIt = watchedFiles.find(parentPath); - if (pIt != watchedFiles.end()) { + FilesByPath::iterator pIt = watchingState.watchedFiles.find(parentPath); + if (pIt != watchingState.watchedFiles.end()) { InfoByName &filesInDir = pIt.value(); InfoByName::iterator fIt = filesInDir.find(realPath); if (fIt != filesInDir.end()) { needsRestart |= derefPath(fIt->watchedPath); filesInDir.erase(fIt); if (filesInDir.isEmpty()) - watchedFiles.erase(pIt); + watchingState.watchedFiles.erase(pIt); files->removeAll(origPath); it.remove(); DEBUG("Removed file '%s'", qPrintable(realPath)); @@ -461,26 +474,33 @@ QStringList QFseventsFileSystemWatcherEngine::removePaths(const QStringList &pat locker.unlock(); - if (needsRestart) - restartStream(); + if (needsRestart) { + if (!restartStream()) { + watchingState = qMove(oldState); + startStream(); + } + } return p; } +// Returns false if FSEventStream* calls failed for some mysterious reason, true if things got a +// thumbs-up. bool QFseventsFileSystemWatcherEngine::startStream() { Q_ASSERT(stream == 0); - QMacAutoReleasePool pool; - if (stream) // This shouldn't happen, but let's be nice and handle it. + if (stream) // Ok, this really shouldn't happen, esp. not after the assert. But let's be nice in release mode and still handle it. stopStream(); - if (watchedPaths.isEmpty()) - return false; + QMacAutoReleasePool pool; - DEBUG() << "Starting stream with paths" << watchedPaths.keys(); + if (watchingState.watchedPaths.isEmpty()) + return true; // we succeeded in doing nothing - NSMutableArray *pathsToWatch = [NSMutableArray arrayWithCapacity:watchedPaths.size()]; - for (PathRefCounts::const_iterator i = watchedPaths.begin(), ei = watchedPaths.end(); i != ei; ++i) + DEBUG() << "Starting stream with paths" << watchingState.watchedPaths.keys(); + + NSMutableArray *pathsToWatch = [NSMutableArray arrayWithCapacity:watchingState.watchedPaths.size()]; + for (PathRefCounts::const_iterator i = watchingState.watchedPaths.begin(), ei = watchingState.watchedPaths.end(); i != ei; ++i) [pathsToWatch addObject:i.key().toNSString()]; struct FSEventStreamContext callBackInfo = { @@ -504,7 +524,7 @@ bool QFseventsFileSystemWatcherEngine::startStream() latency, FSEventStreamCreateFlags(0)); - if (!stream) { + if (!stream) { // nope, no way to know what went wrong, so just fail DEBUG() << "Failed to create stream!"; return false; } @@ -514,7 +534,7 @@ bool QFseventsFileSystemWatcherEngine::startStream() if (FSEventStreamStart(stream)) { DEBUG() << "Stream started successfully with sinceWhen =" << lastReceivedEvent; return true; - } else { + } else { // again, no way to know what went wrong, so just clean up and fail DEBUG() << "Stream failed to start!"; FSEventStreamInvalidate(stream); FSEventStreamRelease(stream); @@ -525,6 +545,7 @@ bool QFseventsFileSystemWatcherEngine::startStream() void QFseventsFileSystemWatcherEngine::stopStream(bool isStopped) { + QMacAutoReleasePool pool; if (stream) { if (!isStopped) FSEventStreamStop(stream); @@ -554,9 +575,9 @@ QFseventsFileSystemWatcherEngine::InfoByName QFseventsFileSystemWatcherEngine::s bool QFseventsFileSystemWatcherEngine::derefPath(const QString &watchedPath) { - PathRefCounts::iterator it = watchedPaths.find(watchedPath); - if (it != watchedPaths.end() && --it.value() < 1) { - watchedPaths.erase(it); + PathRefCounts::iterator it = watchingState.watchedPaths.find(watchedPath); + if (it != watchingState.watchedPaths.end() && --it.value() < 1) { + watchingState.watchedPaths.erase(it); DEBUG("Removing '%s' from watchedPaths.", qPrintable(watchedPath)); return true; } diff --git a/src/corelib/io/qfilesystemwatcher_fsevents_p.h b/src/corelib/io/qfilesystemwatcher_fsevents_p.h index b4640afc4e..b99c026fad 100644 --- a/src/corelib/io/qfilesystemwatcher_fsevents_p.h +++ b/src/corelib/io/qfilesystemwatcher_fsevents_p.h @@ -81,7 +81,7 @@ Q_SIGNALS: private slots: void doEmitFileChanged(const QString &path, bool removed); void doEmitDirectoryChanged(const QString &path, bool removed); - void restartStream(); + bool restartStream(); private: struct Info { @@ -112,6 +112,19 @@ private: typedef QHash DirsByName; typedef QHash PathRefCounts; + struct WatchingState { + // These fields go hand-in-hand. FSEvents watches paths, and there is no use in watching + // the same path multiple times. So, the "refcount" on a path is the number of watched + // files that have the same path, plus the number of directories that have the same path. + // + // If the stream fails to start after adding files/directories, the watcher will try to + // keep watching files/directories that it was already watching. It does that by restoring + // the previous WatchingState and restarting the stream. + FilesByPath watchedFiles; + DirsByName watchedDirectories; + PathRefCounts watchedPaths; + }; + QFseventsFileSystemWatcherEngine(QObject *parent); bool startStream(); void stopStream(bool isStopped = false); @@ -125,10 +138,8 @@ private: QMutex lock; dispatch_queue_t queue; FSEventStreamRef stream; - FilesByPath watchedFiles; - DirsByName watchedDirectories; - PathRefCounts watchedPaths; FSEventStreamEventId lastReceivedEvent; + WatchingState watchingState; }; QT_END_NAMESPACE From c6f5e4b47c3f27c80768e735bb4109a8dc83d2e5 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Tue, 4 Oct 2016 12:44:12 +0200 Subject: [PATCH 17/21] Fix multimedia print key mapping Qt::Key_Print is the PrintScreen key-mapping. Instead use Qt::Key_Printer which is also what VK_PRINT is mapped to. Change-Id: I60a0181ed118253b6681ae0e5847812f73d63119 Reviewed-by: Oliver Wolff --- src/plugins/platforms/windows/qwindowskeymapper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/platforms/windows/qwindowskeymapper.cpp b/src/plugins/platforms/windows/qwindowskeymapper.cpp index 1e58b9b3d4..60361e436d 100644 --- a/src/plugins/platforms/windows/qwindowskeymapper.cpp +++ b/src/plugins/platforms/windows/qwindowskeymapper.cpp @@ -501,7 +501,7 @@ static const uint CmdTbl[] = { // Multimedia keys mapping table Qt::Key_Open, // 30 0x1e APPCOMMAND_OPEN Qt::Key_Close, // 31 0x1f APPCOMMAND_CLOSE Qt::Key_Save, // 32 0x20 APPCOMMAND_SAVE - Qt::Key_Print, // 33 0x21 APPCOMMAND_PRINT + Qt::Key_Printer, // 33 0x21 APPCOMMAND_PRINT Qt::Key_Undo, // 34 0x22 APPCOMMAND_UNDO Qt::Key_Redo, // 35 0x23 APPCOMMAND_REDO Qt::Key_Copy, // 36 0x24 APPCOMMAND_COPY From 5cc0d92c2493aea8397a51eea80167abdd6a1fa6 Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Wed, 18 May 2016 19:25:36 +0200 Subject: [PATCH 18/21] QAbstractItemView: use only a 1x1 QRect for selecting on mouse press MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before commit f1e90768095be37419ba4bf3c69ec5c860bdbcb6, mousePressEvent called the virtual method setSelection(const QRect&, SelectionFlags) with the 1x1 rectangle which contains only the clicked QPoint, unless the SelectionFlag "Current" was set because Shift was pressed during the mouse press. Since that commit, the behavior has been changed such that the rectangle is the one that is spanned by the center of the clicked item and the clicked pixel. In theory, the result should be the same (i.e., only the clicked item should be selected), but * the code path in QListView::setSelection for 1x1 QRects is more efficient, and * using a larger QRect can cause problems with custom views, see the comments in QTBUG-18009 This commit ensures that the 1x1 QRect is used again, unless the SelectionFlag "Current" is used. Change-Id: I70dd70c083c20a3af6cd6095aa89a489756b505f Task-number: QTBUG-18009 Reviewed-by: Olivier Goffart (Woboq GmbH) Reviewed-by: Thorbjørn Lund Martsum --- src/widgets/itemviews/qabstractitemview.cpp | 9 +++- .../tst_qabstractitemview.cpp | 52 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp index d36431c716..4dd8ad0215 100644 --- a/src/widgets/itemviews/qabstractitemview.cpp +++ b/src/widgets/itemviews/qabstractitemview.cpp @@ -1735,13 +1735,18 @@ void QAbstractItemView::mousePressEvent(QMouseEvent *event) d->autoScroll = false; d->selectionModel->setCurrentIndex(index, QItemSelectionModel::NoUpdate); d->autoScroll = autoScroll; - QRect rect(visualRect(d->currentSelectionStartIndex).center(), pos); if (command.testFlag(QItemSelectionModel::Toggle)) { command &= ~QItemSelectionModel::Toggle; d->ctrlDragSelectionFlag = d->selectionModel->isSelected(index) ? QItemSelectionModel::Deselect : QItemSelectionModel::Select; command |= d->ctrlDragSelectionFlag; } - setSelection(rect, command); + + if ((command & QItemSelectionModel::Current) == 0) { + setSelection(QRect(pos, QSize(1, 1)), command); + } else { + QRect rect(visualRect(d->currentSelectionStartIndex).center(), pos); + setSelection(rect, command); + } // signal handlers may change the model emit pressed(index); diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp index 27c9e07037..90ea6f3ef7 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp +++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp @@ -152,6 +152,7 @@ private slots: void QTBUG48968_reentrant_updateEditorGeometries(); void QTBUG50535_update_on_new_selection_model(); void testSelectionModelInSyncWithView(); + void testClickToSelect(); }; class MyAbstractItemDelegate : public QAbstractItemDelegate @@ -2074,5 +2075,56 @@ void tst_QAbstractItemView::testSelectionModelInSyncWithView() QCOMPARE(view.selectionModel()->selection().indexes(), QModelIndexList() << model.index(0, 0)); } +class SetSelectionTestView : public QListView +{ + Q_OBJECT +public: + SetSelectionTestView() : QListView() {} + +signals: + void setSelectionCalled(const QRect &rect); + +protected: + void setSelection(const QRect &rect, QItemSelectionModel::SelectionFlags flags) Q_DECL_OVERRIDE + { + emit setSelectionCalled(rect); + QListView::setSelection(rect, flags); + } +}; + +void tst_QAbstractItemView::testClickToSelect() +{ + // This test verifies that the QRect that is passed from QAbstractItemView::mousePressEvent + // to the virtual method QAbstractItemView::setSelection(const QRect &, SelectionFlags) + // is the 1x1 rect which conains exactly the clicked pixel if no modifiers are pressed. + + QStringList list; + list << "A" << "B" << "C"; + QStringListModel model(list); + + SetSelectionTestView view; + view.setModel(&model); + view.show(); + QTest::qWaitForWindowExposed(&view); + + QSignalSpy spy(&view, &SetSelectionTestView::setSelectionCalled); + + const QModelIndex indexA(model.index(0, 0)); + const QRect visualRectA = view.visualRect(indexA); + const QPoint centerA = visualRectA.center(); + + // Click the center of the visualRect of item "A" + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, centerA); + QCOMPARE(spy.count(), 1); + QCOMPARE(spy.back().front().value(), QRect(centerA, QSize(1, 1))); + + // Click a point slightly away from the center + const QPoint nearCenterA = centerA + QPoint(1, 1); + QVERIFY(visualRectA.contains(nearCenterA)); + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, nearCenterA); + QCOMPARE(spy.count(), 2); + QCOMPARE(spy.back().front().value(), QRect(nearCenterA, QSize(1, 1))); +} + QTEST_MAIN(tst_QAbstractItemView) #include "tst_qabstractitemview.moc" From 75b49a59db98254ce85bc8f1da08938f4fbb63f8 Mon Sep 17 00:00:00 2001 From: Dmitry Shachnev Date: Thu, 29 Sep 2016 22:22:35 +0300 Subject: [PATCH 19/21] dbustray: Support replacing menu on QDBusTrayIcon If a new menu is set via the updateMenu() method, properly unregister the old menu and register the new one. Task-number: QTBUG-53676 Change-Id: I8c1ea2d171caec01488f0fe8a565bc9b2f7e431e Reviewed-by: Shawn Rutledge --- .../dbusmenu/qdbusmenuconnection.cpp | 8 +++++++- .../dbusmenu/qdbusmenuconnection_p.h | 1 + src/platformsupport/dbustray/qdbustrayicon.cpp | 15 ++++++++------- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/platformsupport/dbusmenu/qdbusmenuconnection.cpp b/src/platformsupport/dbusmenu/qdbusmenuconnection.cpp index 2a0bb8878e..b6682d6b67 100644 --- a/src/platformsupport/dbusmenu/qdbusmenuconnection.cpp +++ b/src/platformsupport/dbusmenu/qdbusmenuconnection.cpp @@ -89,6 +89,12 @@ bool QDBusMenuConnection::registerTrayIconMenu(QDBusTrayIcon *item) return success; } +void QDBusMenuConnection::unregisterTrayIconMenu(QDBusTrayIcon *item) +{ + if (item->menu()) + connection().unregisterObject(MenuBarPath); +} + bool QDBusMenuConnection::registerTrayIcon(QDBusTrayIcon *item) { bool success = connection().registerService(item->instanceId()); @@ -118,7 +124,7 @@ bool QDBusMenuConnection::registerTrayIcon(QDBusTrayIcon *item) bool QDBusMenuConnection::unregisterTrayIcon(QDBusTrayIcon *item) { - connection().unregisterObject(MenuBarPath); + unregisterTrayIconMenu(item); connection().unregisterObject(StatusNotifierItemPath); bool success = connection().unregisterService(item->instanceId()); if (!success) diff --git a/src/platformsupport/dbusmenu/qdbusmenuconnection_p.h b/src/platformsupport/dbusmenu/qdbusmenuconnection_p.h index e168d73791..b09514d560 100644 --- a/src/platformsupport/dbusmenu/qdbusmenuconnection_p.h +++ b/src/platformsupport/dbusmenu/qdbusmenuconnection_p.h @@ -66,6 +66,7 @@ public: bool isStatusNotifierHostRegistered() const { return m_statusNotifierHostRegistered; } #ifndef QT_NO_SYSTEMTRAYICON bool registerTrayIconMenu(QDBusTrayIcon *item); + void unregisterTrayIconMenu(QDBusTrayIcon *item); bool registerTrayIcon(QDBusTrayIcon *item); bool unregisterTrayIcon(QDBusTrayIcon *item); #endif // QT_NO_SYSTEMTRAYICON diff --git a/src/platformsupport/dbustray/qdbustrayicon.cpp b/src/platformsupport/dbustray/qdbustrayicon.cpp index 859047424d..ac250cd34c 100644 --- a/src/platformsupport/dbustray/qdbustrayicon.cpp +++ b/src/platformsupport/dbustray/qdbustrayicon.cpp @@ -206,20 +206,21 @@ QPlatformMenu *QDBusTrayIcon::createMenu() const void QDBusTrayIcon::updateMenu(QPlatformMenu * menu) { qCDebug(qLcTray) << menu; - bool needsRegistering = !m_menu; - if (!m_menu) - m_menu = qobject_cast(menu); - if (!m_menuAdaptor) { + QDBusPlatformMenu *newMenu = qobject_cast(menu); + if (m_menu != newMenu) { + if (m_menu) { + dBusConnection()->unregisterTrayIconMenu(this); + delete m_menuAdaptor; + } + m_menu = newMenu; m_menuAdaptor = new QDBusMenuAdaptor(m_menu); // TODO connect(m_menu, , m_menuAdaptor, SIGNAL(ItemActivationRequested(int,uint))); connect(m_menu, SIGNAL(propertiesUpdated(QDBusMenuItemList,QDBusMenuItemKeysList)), m_menuAdaptor, SIGNAL(ItemsPropertiesUpdated(QDBusMenuItemList,QDBusMenuItemKeysList))); connect(m_menu, SIGNAL(updated(uint,int)), m_menuAdaptor, SIGNAL(LayoutUpdated(uint,int))); - } - m_menu->emitUpdated(); - if (needsRegistering) dBusConnection()->registerTrayIconMenu(this); + } } void QDBusTrayIcon::showMessage(const QString &title, const QString &msg, const QIcon &icon, From ebd1046323615f22192055a94438bf85fc360ca3 Mon Sep 17 00:00:00 2001 From: Frederik Schwarzer Date: Fri, 7 Oct 2016 12:09:54 +0200 Subject: [PATCH 20/21] QEvent: fix typo in apidoc Change-Id: I43911d781024b5e76ff5065964a570663de6e33c Reviewed-by: Giuseppe D'Angelo --- src/gui/kernel/qevent.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/kernel/qevent.cpp b/src/gui/kernel/qevent.cpp index a3d39f13b2..d8b103b3f0 100644 --- a/src/gui/kernel/qevent.cpp +++ b/src/gui/kernel/qevent.cpp @@ -1592,7 +1592,7 @@ QResizeEvent::~QResizeEvent() The event handler QWidget::closeEvent() receives close events. The default implementation of this event handler accepts the close event. If you do not want your widget to be hidden, or want some - special handing, you should reimplement the event handler and + special handling, you should reimplement the event handler and ignore() the event. The \l{mainwindows/application#close event handler}{closeEvent() in the From 1a78ef09b93b0a7337075555dc91032f39fab2a9 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Fri, 7 Oct 2016 14:13:00 +0000 Subject: [PATCH 21/21] Revert "QCocoaKeyMapper - correctly update key layouts" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 26961e32f34c06f083fe441c23be6874f03446a3. This patch was apparently a bit ill-considered and while fixed one problem introduced others. Task-number: QTBUG-50865 Change-Id: I2e3569d16c8fc47b4a492d4aed6e747d7ff93a55 Reviewed-by: Morten Johan Sørvig --- src/plugins/platforms/cocoa/qcocoakeymapper.mm | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoakeymapper.mm b/src/plugins/platforms/cocoa/qcocoakeymapper.mm index ebe547f64b..250f3f3bf7 100644 --- a/src/plugins/platforms/cocoa/qcocoakeymapper.mm +++ b/src/plugins/platforms/cocoa/qcocoakeymapper.mm @@ -390,11 +390,6 @@ bool QCocoaKeyMapper::updateKeyboard() keyboardInputLocale = QLocale::c(); keyboardInputDirection = Qt::LeftToRight; } - - const auto newMode = keyboard_mode; - deleteLayouts(); - keyboard_mode = newMode; - return true; } @@ -417,8 +412,10 @@ void QCocoaKeyMapper::clearMappings() void QCocoaKeyMapper::updateKeyMap(unsigned short macVirtualKey, QChar unicodeKey) { - updateKeyboard(); - + if (updateKeyboard()) { + // ### Qt 4 did this: + // QKeyMapper::changeKeyboard(); + } if (keyLayout[macVirtualKey]) return; @@ -474,8 +471,9 @@ QList QCocoaKeyMapper::possibleKeys(const QKeyEvent *event) const for (int i = 1; i < 8; ++i) { Qt::KeyboardModifiers neededMods = ModsTbl[i]; int key = kbItem->qtKey[i]; - if (key && key != baseKey && ((keyMods & neededMods) == neededMods)) - ret << int(key + neededMods); + if (key && key != baseKey && ((keyMods & neededMods) == neededMods)) { + ret << int(key + (keyMods & ~neededMods)); + } } return ret; }