From 0431e462dff57bc6a9010649c0d7f153d91cab2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Arve=20S=C3=A6ther?= Date: Mon, 10 Sep 2018 16:30:26 +0200 Subject: Add some null pointer checks to avoid some rare crashes The crashes happened because somebody was calling processEvents() from a mouse/touch event handler that was running for a long time. If we called processEvents() from the onPressed handler, and we released the mouse while still not having returned from the onPressed handler, it meant that we were actually delivering the release event before the press event was fully delivered... This should normally not be a problem, but QQuickWindow is reusing the QQuickPointerEvent object for each incoming QEvent, which meant that when we were delivering the release event, it would reuse (and overwrite) the QQuickPointerEvent that the press event handler is still using.... This then caused some assumptions that the code made to be wrong. This only avoids the crashes, and doesn't really fix the "out-of-order" delivery and the state inconsistency that this can lead to (e.g. mouse button states might be still wrong). But on the other hand, it is not the recommended way of making a long-running handler not block the application. The proper way is to create a thread that will run in the background, so that there would be no need to call processEvents() in the event handler. Change-Id: I6fa5d4f5748ef30d082a210f03ef382922bd4b29 Task-number: QTBUG-65454 Reviewed-by: Shawn Rutledge --- src/quick/items/qquickevents_p_p.h | 2 +- src/quick/items/qquickwindow.cpp | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/quick/items/qquickevents_p_p.h b/src/quick/items/qquickevents_p_p.h index 3f70ad8c2d..4097845ec9 100644 --- a/src/quick/items/qquickevents_p_p.h +++ b/src/quick/items/qquickevents_p_p.h @@ -429,7 +429,7 @@ public: // helpers for C++ only (during event delivery) virtual bool allUpdatedPointsAccepted() const = 0; virtual bool allPointsGrabbed() const = 0; bool isAccepted() { return m_event->isAccepted(); } - void setAccepted(bool accepted) { m_event->setAccepted(accepted); } + void setAccepted(bool accepted) { if (m_event) m_event->setAccepted(accepted); } QVector unacceptedPressedPointScenePositions() const; virtual int pointCount() const = 0; diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index d9db8b56b4..5cd04decf7 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -690,8 +690,8 @@ bool QQuickWindowPrivate::deliverTouchAsMouse(QQuickItem *item, QQuickPointerEve touchMouseId = p.id(); if (!q->mouseGrabberItem()) item->grabMouse(); - auto pointerEventPoint = pointerEvent->pointById(p.id()); - pointerEventPoint->setGrabberItem(item); + if (auto pointerEventPoint = pointerEvent->pointById(p.id())) + pointerEventPoint->setGrabberItem(item); if (checkIfDoubleClicked(event->timestamp())) { QScopedPointer mouseDoubleClick(touchToMouseEvent(QEvent::MouseButtonDblClick, p, event.data(), item, false)); @@ -2607,19 +2607,22 @@ void QQuickWindowPrivate::deliverMatchingPointsToItem(QQuickItem *item, QQuickPo // update accepted new points. bool isPressOrRelease = pointerEvent->isPressEvent() || pointerEvent->isReleaseEvent(); for (auto point: qAsConst(touchEvent->touchPoints())) { - auto pointerEventPoint = ptEvent->pointById(point.id()); - pointerEventPoint->setAccepted(); - if (isPressOrRelease) - pointerEventPoint->setGrabberItem(item); + if (auto pointerEventPoint = ptEvent->pointById(point.id())) { + pointerEventPoint->setAccepted(); + if (isPressOrRelease) + pointerEventPoint->setGrabberItem(item); + } } } else { // But if the event was not accepted then we know this item // will not be interested in further updates for those touchpoint IDs either. for (auto point: qAsConst(touchEvent->touchPoints())) { if (point.state() == Qt::TouchPointPressed) { - if (ptEvent->pointById(point.id())->exclusiveGrabber() == item) { - qCDebug(DBG_TOUCH_TARGET) << "TP" << hex << point.id() << "disassociated"; - ptEvent->pointById(point.id())->setGrabberItem(nullptr); + if (auto *tp = ptEvent->pointById(point.id())) { + if (tp->exclusiveGrabber() == item) { + qCDebug(DBG_TOUCH_TARGET) << "TP" << hex << point.id() << "disassociated"; + tp->setGrabberItem(nullptr); + } } } } -- cgit v1.2.3