From 68511d41d50b1a136fb0e74fdabfd362e0b21a1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Wed, 5 Sep 2018 13:47:05 +0200 Subject: [PATCH] qWaitFor: Prevent being stuck in QCoreApplication::processEvents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using the overload of QCoreApplication::processEvents that takes a maxtime argument, the function will keep processing events until there are no more events, or until it times out. The problem is that the function doesn't distinguish between events that were on the event queue when the function was called, and events generated by processing events as part of its own execution. If for example a widget calls update() in its paintEvent, the function will spin for the entire duration of maxtime. That doesn't work for qWaitFor, where we need to check the predicate between each pass, so we use the overload of processEvents that doesn't take a maxtime. That's fine, as we have our own timeout logic. Change-Id: I9738d7d0187c36d4a5ddfcd3fd075b0bd84583c4 Reviewed-by: Qt CI Bot Reviewed-by: Tor Arne Vestbø --- src/corelib/kernel/qtestsupport_core.h | 8 ++++- tests/auto/opengl/qgl/tst_qgl.cpp | 1 + .../dialogs/qfiledialog2/tst_qfiledialog2.cpp | 5 ++++ .../qgraphicseffect/tst_qgraphicseffect.cpp | 1 + .../tst_qgraphicseffectsource.cpp | 1 + .../qgraphicsitem/tst_qgraphicsitem.cpp | 29 +++++++++++++------ .../qgraphicsscene/tst_qgraphicsscene.cpp | 1 + .../qgraphicsview/tst_qgraphicsview.cpp | 1 + .../itemviews/qtreewidget/tst_qtreewidget.cpp | 1 + 9 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/corelib/kernel/qtestsupport_core.h b/src/corelib/kernel/qtestsupport_core.h index c8b664b6d34..c8209b5ae47 100644 --- a/src/corelib/kernel/qtestsupport_core.h +++ b/src/corelib/kernel/qtestsupport_core.h @@ -67,7 +67,13 @@ Q_REQUIRED_RESULT static bool qWaitFor(Functor predicate, int timeout = 5000) QDeadlineTimer deadline(remaining, Qt::PreciseTimer); do { - QCoreApplication::processEvents(QEventLoop::AllEvents, remaining); + // We explicitly do not pass the remaining time to processEvents, as + // that would keep spinning processEvents for the whole duration if + // new events were posted as part of processing events, and we need + // to return back to this function to check the predicate between + // each pass of processEvents. Our own timer will take care of the + // timeout. + QCoreApplication::processEvents(QEventLoop::AllEvents); QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); remaining = deadline.remainingTime(); diff --git a/tests/auto/opengl/qgl/tst_qgl.cpp b/tests/auto/opengl/qgl/tst_qgl.cpp index 7666bf81e4b..053c4b026b4 100644 --- a/tests/auto/opengl/qgl/tst_qgl.cpp +++ b/tests/auto/opengl/qgl/tst_qgl.cpp @@ -924,6 +924,7 @@ void tst_QGL::partialGLWidgetUpdates() widget.setAutoFillBackground(autoFillBackground); widget.show(); QVERIFY(QTest::qWaitForWindowExposed(&widget)); + QCoreApplication::processEvents(); // Process all queued paint events if (widget.format().doubleBuffer() != doubleBufferedContext) QSKIP("Platform does not support requested format"); diff --git a/tests/auto/widgets/dialogs/qfiledialog2/tst_qfiledialog2.cpp b/tests/auto/widgets/dialogs/qfiledialog2/tst_qfiledialog2.cpp index 8f9a8c11a7f..24ce5982797 100644 --- a/tests/auto/widgets/dialogs/qfiledialog2/tst_qfiledialog2.cpp +++ b/tests/auto/widgets/dialogs/qfiledialog2/tst_qfiledialog2.cpp @@ -601,6 +601,11 @@ void tst_QFileDialog2::task227930_correctNavigationKeyboardBehavior() fd.setDirectory(current.absolutePath()); fd.show(); QVERIFY(QTest::qWaitForWindowActive(&fd)); + + // Ensure LayoutRequest event is processed so that the list view + // is sorted correctly to have the directory entires at the top. + QCoreApplication::processEvents(); + QListView *list = fd.findChild("listView"); QVERIFY(list); QTest::keyClick(list, Qt::Key_Down); diff --git a/tests/auto/widgets/effects/qgraphicseffect/tst_qgraphicseffect.cpp b/tests/auto/widgets/effects/qgraphicseffect/tst_qgraphicseffect.cpp index 95662a49a08..c4b6e22c374 100644 --- a/tests/auto/widgets/effects/qgraphicseffect/tst_qgraphicseffect.cpp +++ b/tests/auto/widgets/effects/qgraphicseffect/tst_qgraphicseffect.cpp @@ -313,6 +313,7 @@ void tst_QGraphicsEffect::draw() view.show(); QVERIFY(QTest::qWaitForWindowActive(&view)); QTRY_VERIFY(item->numRepaints > 0); + QCoreApplication::processEvents(); // Process all queued paint events item->reset(); // Make sure installing the effect triggers a repaint. diff --git a/tests/auto/widgets/graphicsview/qgraphicseffectsource/tst_qgraphicseffectsource.cpp b/tests/auto/widgets/graphicsview/qgraphicseffectsource/tst_qgraphicseffectsource.cpp index e7d26622c95..973a73a4a99 100644 --- a/tests/auto/widgets/graphicsview/qgraphicseffectsource/tst_qgraphicseffectsource.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicseffectsource/tst_qgraphicseffectsource.cpp @@ -183,6 +183,7 @@ void tst_QGraphicsEffectSource::init() effect->storeDeviceDependentStuff = false; effect->doNothingInDraw = false; item->reset(); + QCoreApplication::processEvents(); // Process all queued paint events } void tst_QGraphicsEffectSource::graphicsItem() diff --git a/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp index bd08461544d..5f269bd5207 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp @@ -3231,6 +3231,7 @@ void tst_QGraphicsItem::hoverEventsGenerateRepaints() view.show(); QVERIFY(QTest::qWaitForWindowExposed(&view)); QVERIFY(QTest::qWaitForWindowActive(&view)); + QCoreApplication::processEvents(); // Process all queued paint events EventTester *tester = new EventTester; scene.addItem(tester); @@ -5074,13 +5075,11 @@ void tst_QGraphicsItem::paint() QGraphicsView view2(&scene2); view2.show(); QVERIFY(QTest::qWaitForWindowExposed(&view2)); + QCoreApplication::processEvents(); // Process all queued paint events PaintTester tester2; scene2.addItem(&tester2); -#ifdef Q_OS_WINRT - QEXPECT_FAIL("", "Fails on WinRT. Figure out why - QTBUG-68297", Abort); -#endif //First show one paint QTRY_COMPARE(tester2.painted, 1); @@ -6534,11 +6533,9 @@ void tst_QGraphicsItem::ensureUpdateOnTextItem() QGraphicsView view(&scene); view.show(); QVERIFY(QTest::qWaitForWindowExposed(&view)); + QCoreApplication::processEvents(); // Process all queued paint events TextItem *text1 = new TextItem(QLatin1String("123")); scene.addItem(text1); -#ifdef Q_OS_WINRT - QEXPECT_FAIL("", "Fails on WinRT. Figure out why - QTBUG-68297", Abort); -#endif QTRY_COMPARE(text1->updates,1); //same bouding rect but we have to update @@ -6810,6 +6807,7 @@ void tst_QGraphicsItem::opacity2() view.show(); QVERIFY(QTest::qWaitForWindowExposed(&view)); QVERIFY(QTest::qWaitForWindowActive(&view)); + QCoreApplication::processEvents(); // Process all queued paint events QTRY_VERIFY(view.repaints >= 1); #define RESET_REPAINT_COUNTERS \ @@ -6883,6 +6881,7 @@ void tst_QGraphicsItem::opacityZeroUpdates() view.show(); QVERIFY(QTest::qWaitForWindowExposed(&view)); QVERIFY(QTest::qWaitForWindowActive(&view)); + QCoreApplication::processEvents(); // Process all queued paint events QTRY_VERIFY(view.repaints > 0); view.reset(); @@ -7275,6 +7274,7 @@ void tst_QGraphicsItem::cacheMode() QApplication::setActiveWindow(&view); QVERIFY(QTest::qWaitForWindowExposed(&view)); QVERIFY(QTest::qWaitForWindowActive(&view)); + QCoreApplication::processEvents(); // Process all queued paint events EventTester *tester = new EventTester; EventTester *testerChild = new EventTester; @@ -7454,6 +7454,7 @@ void tst_QGraphicsItem::cacheMode2() QApplication::setActiveWindow(&view); QVERIFY(QTest::qWaitForWindowExposed(&view)); QVERIFY(QTest::qWaitForWindowActive(&view)); + QCoreApplication::processEvents(); // Process all queued paint events EventTester *tester = new EventTester; scene.addItem(tester); @@ -7944,6 +7945,7 @@ void tst_QGraphicsItem::itemUsesExtendedStyleOption() rect->startTrack = false; topLevel.show(); QVERIFY(QTest::qWaitForWindowExposed(&topLevel)); + QCoreApplication::processEvents(); // Process all queued paint events QTRY_VERIFY(rect->repaints > 0); rect->repaints = 0; rect->startTrack = true; @@ -8030,6 +8032,7 @@ void tst_QGraphicsItem::moveItem() MyGraphicsView view(&scene); view.show(); QVERIFY(QTest::qWaitForWindowExposed(&view)); + QCoreApplication::processEvents(); // Process all queued paint events EventTester *parent = new EventTester; EventTester *child = new EventTester(parent); @@ -8108,6 +8111,7 @@ void tst_QGraphicsItem::moveLineItem() view.show(); QVERIFY(QTest::qWaitForWindowExposed(&view)); QVERIFY(QTest::qWaitForWindowActive(&view)); + QCoreApplication::processEvents(); // Process all queued paint events view.reset(); QRectF brect = item->boundingRect(); @@ -8288,6 +8292,7 @@ void tst_QGraphicsItem::hitTestGraphicsEffectItem() toplevel.resize(300, 300); toplevel.show(); QVERIFY(QTest::qWaitForWindowExposed(&toplevel)); + QCoreApplication::processEvents(); // Process all queued paint events // Confuse the BSP with dummy items. QGraphicsRectItem *dummy = new QGraphicsRectItem(0, 0, 20, 20); @@ -11266,6 +11271,7 @@ void tst_QGraphicsItem::QTBUG_6738_missingUpdateWithSetParent() qApp->setActiveWindow(&view); QVERIFY(QTest::qWaitForWindowExposed(&view)); QVERIFY(QTest::qWaitForWindowActive(&view)); + QCoreApplication::processEvents(); // Process all queued paint events QTRY_VERIFY(view.repaints > 0); // test case #1 @@ -11315,6 +11321,7 @@ void tst_QGraphicsItem::QT_2653_fullUpdateDiscardingOpacityUpdate() view.show(); QVERIFY(QTest::qWaitForWindowExposed(&view)); QVERIFY(QTest::qWaitForWindowActive(&view)); + QCoreApplication::processEvents(); // Process all queued paint events view.reset(); parentGreen->setOpacity(1.0); @@ -11348,6 +11355,8 @@ void tst_QGraphicsItem::QTBUG_7714_fullUpdateDiscardingOpacityUpdate2() origView.show(); QVERIFY(QTest::qWaitForWindowActive(&origView)); + QCoreApplication::processEvents(); // Process all queued paint events + origView.setGeometry(origView.x() + origView.width() + 20, origView.y() + 20, origView.width(), origView.height()); @@ -11356,9 +11365,6 @@ void tst_QGraphicsItem::QTBUG_7714_fullUpdateDiscardingOpacityUpdate2() origView.reset(); childYellow->setOpacity(0.0); -#ifdef Q_OS_WINRT - QEXPECT_FAIL("", "Fails on WinRT. Figure out why - QTBUG-68297", Abort); -#endif QTRY_COMPARE(origView.repaints, 1); view.show(); @@ -11370,6 +11376,10 @@ void tst_QGraphicsItem::QTBUG_7714_fullUpdateDiscardingOpacityUpdate2() childYellow->setOpacity(1.0); +#ifdef Q_OS_WINRT + QEXPECT_FAIL("", "Fails on WinRT. Figure out why - QTBUG-68297", Abort); +#endif + QTRY_COMPARE(origView.repaints, 1); QTRY_COMPARE(view.repaints, 1); } @@ -11505,6 +11515,7 @@ void tst_QGraphicsItem::doNotMarkFullUpdateIfNotInScene() view.show(); QVERIFY(QTest::qWaitForWindowExposed(view.windowHandle())); QVERIFY(QTest::qWaitForWindowActive(view.windowHandle())); + QCoreApplication::processEvents(); // Process all queued paint events view.activateWindow(); QTRY_VERIFY(view.isActiveWindow()); QTRY_VERIFY(view.repaints >= 1); diff --git a/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp b/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp index 838b1f4be6a..48488abfb8b 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp @@ -4318,6 +4318,7 @@ void tst_QGraphicsScene::removeFullyTransparentItem() view.show(); qApp->setActiveWindow(&view); QVERIFY(QTest::qWaitForWindowActive(&view)); + QCoreApplication::processEvents(); // Process all queued paint events // NB! The parent has the ItemHasNoContents flag set, which means // the parent itself doesn't generate any update requests, only the diff --git a/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp b/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp index 3dc110298a0..55139ff99ac 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicsview/tst_qgraphicsview.cpp @@ -1650,6 +1650,7 @@ void tst_QGraphicsView::itemsInRect_cosmeticAdjust() QVERIFY(QTest::qWaitForWindowActive(&view)); QTRY_VERIFY(rect->numPaints > 0); + QCoreApplication::processEvents(); // Process all queued paint events rect->numPaints = 0; if (updateRect.isNull()) view.viewport()->update(); diff --git a/tests/auto/widgets/itemviews/qtreewidget/tst_qtreewidget.cpp b/tests/auto/widgets/itemviews/qtreewidget/tst_qtreewidget.cpp index 8c93df90730..adb2c54751f 100644 --- a/tests/auto/widgets/itemviews/qtreewidget/tst_qtreewidget.cpp +++ b/tests/auto/widgets/itemviews/qtreewidget/tst_qtreewidget.cpp @@ -3368,6 +3368,7 @@ void tst_QTreeWidget::setChildIndicatorPolicy() treeWidget.setItemDelegate(&delegate); treeWidget.show(); QVERIFY(QTest::qWaitForWindowExposed(&treeWidget)); + QCoreApplication::processEvents(); // Process all queued paint events QTreeWidgetItem *item = new QTreeWidgetItem(QStringList("Hello")); treeWidget.insertTopLevelItem(0, item);