From aa3b04de39c35eef8eecf9d1e512965516815e2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCri=20Valdmann?= Date: Wed, 16 Sep 2020 14:54:39 +0200 Subject: Add more tests to tst_loadsignals Add new cases for non-same-page navigations, for navigations triggered from the DOMContentLoaded event handler, for navigations triggered by user action (clicks), and for navigation rejected by the acceptNavigationRequest API. Drop the 'no more signals' waiting time from 10 to 1 seconds. Task-number: QTBUG-65223 Change-Id: Ic074eaf5aa58f779e31927296ae84d9e4faeaaae Reviewed-by: Allan Sandfeld Jensen Reviewed-by: Peter Varga --- tests/auto/shared/data/loadprogress/page1.html | 2 +- tests/auto/shared/data/loadprogress/page2.html | 1 + tests/auto/shared/data/loadprogress/page5.html | 20 ++ tests/auto/shared/data/loadprogress/page6.html | 13 ++ tests/auto/shared/data/loadprogress/page7.html | 13 ++ tests/auto/shared/data/loadprogress/page8.html | 20 ++ tests/auto/widgets/loadsignals/BLACKLIST | 16 +- tests/auto/widgets/loadsignals/tst_loadsignals.cpp | 212 ++++++++++++++++++--- tests/auto/widgets/loadsignals/tst_loadsignals.qrc | 4 + 9 files changed, 268 insertions(+), 33 deletions(-) create mode 100644 tests/auto/shared/data/loadprogress/page5.html create mode 100644 tests/auto/shared/data/loadprogress/page6.html create mode 100644 tests/auto/shared/data/loadprogress/page7.html create mode 100644 tests/auto/shared/data/loadprogress/page8.html (limited to 'tests') diff --git a/tests/auto/shared/data/loadprogress/page1.html b/tests/auto/shared/data/loadprogress/page1.html index 5cd479ab6..9b11ce887 100644 --- a/tests/auto/shared/data/loadprogress/page1.html +++ b/tests/auto/shared/data/loadprogress/page1.html @@ -3,6 +3,6 @@ page1 -

page1

+
page2
diff --git a/tests/auto/shared/data/loadprogress/page2.html b/tests/auto/shared/data/loadprogress/page2.html index e3031f56a..223817c8c 100644 --- a/tests/auto/shared/data/loadprogress/page2.html +++ b/tests/auto/shared/data/loadprogress/page2.html @@ -9,6 +9,7 @@ } +
page2
page2 anchor
diff --git a/tests/auto/shared/data/loadprogress/page5.html b/tests/auto/shared/data/loadprogress/page5.html new file mode 100644 index 000000000..47709ff08 --- /dev/null +++ b/tests/auto/shared/data/loadprogress/page5.html @@ -0,0 +1,20 @@ + + + page5 + + + + +
go to the anchor
+
here is the anchor
+ + diff --git a/tests/auto/shared/data/loadprogress/page6.html b/tests/auto/shared/data/loadprogress/page6.html new file mode 100644 index 000000000..98042701a --- /dev/null +++ b/tests/auto/shared/data/loadprogress/page6.html @@ -0,0 +1,13 @@ + + + page6 + + + +
go to another page
+ + diff --git a/tests/auto/shared/data/loadprogress/page7.html b/tests/auto/shared/data/loadprogress/page7.html new file mode 100644 index 000000000..42538c5de --- /dev/null +++ b/tests/auto/shared/data/loadprogress/page7.html @@ -0,0 +1,13 @@ + + + page6 + + + +
go to another page
+ + diff --git a/tests/auto/shared/data/loadprogress/page8.html b/tests/auto/shared/data/loadprogress/page8.html new file mode 100644 index 000000000..8ebdddf97 --- /dev/null +++ b/tests/auto/shared/data/loadprogress/page8.html @@ -0,0 +1,20 @@ + + + Page with js navigation in the end of document to anchor within the page + + + +
go to the anchor
+
here is the anchor
+ + + diff --git a/tests/auto/widgets/loadsignals/BLACKLIST b/tests/auto/widgets/loadsignals/BLACKLIST index ffe1f71fe..71bb71b9e 100644 --- a/tests/auto/widgets/loadsignals/BLACKLIST +++ b/tests/auto/widgets/loadsignals/BLACKLIST @@ -1,5 +1,19 @@ # QTBUG-65223 -[loadStartedAndFinishedCount:WithAnchorClickedFromJS] +[loadStartedAndFinishedCount:SamePageImmediate] +* +[loadStartedAndFinishedCount:SamePageDeferred] +* +[loadStartedAndFinishedCount:OtherPageImmediate] +* +[loadStartedAndFinishedCount:SamePageImmediateJS] +* +[loadStartedAndFinishedCountClick:SamePage] +* +[rejectNavigationRequest:SamePageDeferred] +* +[rejectNavigationRequest:SamePageImmediate] +* +[rejectNavigationRequest:OtherPageImmediate] * # QTBUG-66869 (https://codereview.qt-project.org/#/c/222112/ is only a workaround) diff --git a/tests/auto/widgets/loadsignals/tst_loadsignals.cpp b/tests/auto/widgets/loadsignals/tst_loadsignals.cpp index 8112a274c..3f5d975c0 100644 --- a/tests/auto/widgets/loadsignals/tst_loadsignals.cpp +++ b/tests/auto/widgets/loadsignals/tst_loadsignals.cpp @@ -36,6 +36,43 @@ #include "qwebenginesettings.h" #include "qwebengineview.h" +enum { LoadStarted, LoadSucceeded, LoadFailed }; +static const QList SignalsOrderOnce({ LoadStarted, LoadSucceeded}); +static const QList SignalsOrderTwice({ LoadStarted, LoadSucceeded, LoadStarted, LoadSucceeded }); +static const QList SignalsOrderOnceFailure({ LoadStarted, LoadFailed }); +static const QList SignalsOrderTwiceWithFailure({ LoadStarted, LoadSucceeded, LoadStarted, LoadFailed }); + +class TestPage : public QWebEnginePage +{ +public: + QSet blacklist; + int navigationRequestCount = 0; + QList signalsOrder; + QList loadProgress; + + explicit TestPage(QObject *parent = nullptr) : TestPage(nullptr, parent) { } + TestPage(QWebEngineProfile *profile, QObject *parent = nullptr) : QWebEnginePage(profile, parent) { + connect(this, &QWebEnginePage::loadStarted, [this] () { signalsOrder.append(LoadStarted); }); + connect(this, &QWebEnginePage::loadProgress, [this] (int p) { loadProgress.append(p); }); + connect(this, &QWebEnginePage::loadFinished, [this] (bool r) { signalsOrder.append(r ? LoadSucceeded : LoadFailed); }); + } + + void reset() + { + blacklist.clear(); + navigationRequestCount = 0; + signalsOrder.clear(); + loadProgress.clear(); + } + +protected: + bool acceptNavigationRequest(const QUrl &url, NavigationType, bool) override + { + ++navigationRequestCount; + return !blacklist.contains(url); + } +}; + class tst_LoadSignals : public QObject { Q_OBJECT @@ -48,6 +85,10 @@ private Q_SLOTS: void monotonicity(); void loadStartedAndFinishedCount_data(); void loadStartedAndFinishedCount(); + void loadStartedAndFinishedCountClick_data(); + void loadStartedAndFinishedCountClick(); + void rejectNavigationRequest_data(); + void rejectNavigationRequest(); void loadAfterInPageNavigation_qtbug66869(); void fileDownloadDoesNotTriggerLoadSignals_qtbug66661(); void numberOfStartedAndFinishedSignalsIsSame(); @@ -57,31 +98,45 @@ private Q_SLOTS: void errorPageTriggered(); private: + void clickLink(QPoint linkPos); + QWebEngineProfile profile; - QWebEnginePage page{&profile}; + TestPage page{&profile}; QWebEngineView view; QSignalSpy loadStartedSpy{&page, &QWebEnginePage::loadStarted}; - QSignalSpy loadProgressSpy{&page, &QWebEnginePage::loadProgress}; QSignalSpy loadFinishedSpy{&page, &QWebEnginePage::loadFinished}; + void resetSpies() { + loadStartedSpy.clear(); + loadFinishedSpy.clear(); + } }; void tst_LoadSignals::initTestCase() { view.setPage(&page); - view.resize(1024,768); + view.resize(640, 480); view.show(); + QVERIFY(QTest::qWaitForWindowExposed(&view)); } void tst_LoadSignals::init() { // Reset content - loadFinishedSpy.clear(); - view.load(QUrl("about:blank")); - QTRY_COMPARE(loadFinishedSpy.count(), 1); + if (!view.url().isEmpty()) { + loadFinishedSpy.clear(); + view.load(QUrl("about:blank")); + QTRY_COMPARE(loadFinishedSpy.count(), 1); + } + resetSpies(); + page.reset(); +} - loadStartedSpy.clear(); - loadProgressSpy.clear(); - loadFinishedSpy.clear(); +void tst_LoadSignals::clickLink(QPoint linkPos) +{ + // Simulate left-clicking on link. + QTRY_VERIFY(view.focusWidget()); + QWidget *renderWidget = view.focusWidget(); + QTest::mouseClick(renderWidget, Qt::LeftButton, {}, linkPos); } /** @@ -90,27 +145,124 @@ void tst_LoadSignals::init() void tst_LoadSignals::loadStartedAndFinishedCount_data() { QTest::addColumn("url"); - QTest::addColumn("expectedLoadCount"); - QTest::newRow("Normal") << QUrl("qrc:///resources/page1.html") << 1; - QTest::newRow("WithAnchor") << QUrl("qrc:///resources/page2.html#anchor") << 1; - - // In this case, we get an unexpected additional loadStarted, but no corresponding - // loadFinished, so expectedLoadCount=2 would also not work. See also QTBUG-65223 - QTest::newRow("WithAnchorClickedFromJS") << QUrl("qrc:///resources/page3.html") << 1; + QTest::addColumn>("expectedSignals"); + QTest::newRow("Simple") << QUrl("qrc:///resources/page1.html") << SignalsOrderOnce; + QTest::newRow("SimpleWithAnchor") << QUrl("qrc:///resources/page2.html#anchor") << SignalsOrderOnce; + QTest::newRow("SamePageImmediate") << QUrl("qrc:///resources/page5.html") << SignalsOrderOnce; + QTest::newRow("SamePageDeferred") << QUrl("qrc:///resources/page3.html") << SignalsOrderOnce; + QTest::newRow("OtherPageImmediate") << QUrl("qrc:///resources/page6.html") << SignalsOrderOnce; + QTest::newRow("OtherPageDeferred") << QUrl("qrc:///resources/page7.html") << SignalsOrderTwice; + QTest::newRow("SamePageImmediateJS") << QUrl("qrc:///resources/page8.html") << SignalsOrderOnce; } void tst_LoadSignals::loadStartedAndFinishedCount() { QFETCH(QUrl, url); - QFETCH(int, expectedLoadCount); + QFETCH(QList, expectedSignals); view.load(url); + + int expectedLoadCount = expectedSignals.size() / 2; + QTRY_COMPARE(loadStartedSpy.size(), expectedLoadCount); QTRY_COMPARE(loadFinishedSpy.size(), expectedLoadCount); + + // verify no more signals is emitted by waiting for another loadStarted or loadFinished + QTRY_LOOP_IMPL(loadStartedSpy.size() != expectedLoadCount || loadFinishedSpy.size() != expectedLoadCount, 1000, 100); + + // No further signals should have occurred within this time and expected number of signals is preserved + QCOMPARE(loadStartedSpy.size(), expectedLoadCount); + QCOMPARE(loadFinishedSpy.size(), expectedLoadCount); + QCOMPARE(page.signalsOrder, expectedSignals); +} + +/** + * Load a URL, then simulate a click to load a different URL. + */ +void tst_LoadSignals::loadStartedAndFinishedCountClick_data() +{ + QTest::addColumn("url"); + QTest::addColumn("numberOfSignals"); + QTest::newRow("SamePage") << QUrl("qrc:///resources/page2.html") << 0; // in-page navigation to anchor shouldn't emit anything + QTest::newRow("OtherPage") << QUrl("qrc:///resources/page1.html") << 1; +} + +void tst_LoadSignals::loadStartedAndFinishedCountClick() +{ + QFETCH(QUrl, url); + QFETCH(int, numberOfSignals); + + view.load(url); + QTRY_COMPARE(loadStartedSpy.size(), 1); + QTRY_COMPARE(loadFinishedSpy.size(), 1); QVERIFY(loadFinishedSpy[0][0].toBool()); + resetSpies(); + + clickLink(QPoint(10, 10)); + if (numberOfSignals > 0) { + QTRY_COMPARE(loadStartedSpy.size(), numberOfSignals); + QTRY_COMPARE(loadFinishedSpy.size(), numberOfSignals); + QVERIFY(loadFinishedSpy[0][0].toBool()); + } - // Wait for 10 seconds (abort waiting if another loadStarted or loadFinished occurs) - QTRY_LOOP_IMPL((loadStartedSpy.size() != expectedLoadCount) - || (loadFinishedSpy.size() != expectedLoadCount), 10000, 100); + // verify no more signals is emitted by waiting for another loadStarted or loadFinished + QTRY_LOOP_IMPL(loadStartedSpy.size() != numberOfSignals || loadFinishedSpy.size() != numberOfSignals, 1000, 100); + + // No further loadStarted should have occurred within this time + QCOMPARE(loadStartedSpy.size(), numberOfSignals); + QCOMPARE(loadFinishedSpy.size(), numberOfSignals); + QCOMPARE(page.signalsOrder, numberOfSignals > 0 ? SignalsOrderTwice : SignalsOrderOnce); +} + +void tst_LoadSignals::rejectNavigationRequest_data() +{ + QTest::addColumn("initialUrl"); + QTest::addColumn("rejectedUrl"); + QTest::addColumn("expectedNavigations"); + QTest::addColumn>("expectedSignals"); + QTest::newRow("Simple") + << QUrl("qrc:///resources/page1.html") + << QUrl("qrc:///resources/page1.html") + << 1 << SignalsOrderOnceFailure; + QTest::newRow("SamePageImmediate") + << QUrl("qrc:///resources/page5.html") + << QUrl("qrc:///resources/page5.html#anchor") + << 1 << SignalsOrderOnce; + QTest::newRow("SamePageDeferred") + << QUrl("qrc:///resources/page3.html") + << QUrl("qrc:///resources/page3.html#anchor") + << 1 << SignalsOrderOnce; + QTest::newRow("OtherPageImmediate") + << QUrl("qrc:///resources/page6.html") + << QUrl("qrc:///resources/page2.html#anchor") + << 2 << SignalsOrderOnceFailure; + QTest::newRow("OtherPageDeferred") + << QUrl("qrc:///resources/page7.html") + << QUrl("qrc:///resources/page2.html#anchor") + << 2 << SignalsOrderTwiceWithFailure; +} + +/** + * Returning false from acceptNavigationRequest means that the load + * fails, not that the load never starts. + * + * See QTBUG-75185. + */ +void tst_LoadSignals::rejectNavigationRequest() +{ + QFETCH(QUrl, initialUrl); + QFETCH(QUrl, rejectedUrl); + QFETCH(int, expectedNavigations); + QFETCH(QList, expectedSignals); + + page.blacklist.insert(rejectedUrl); + page.load(initialUrl); + QTRY_COMPARE(page.navigationRequestCount, expectedNavigations); + int expectedLoadCount = expectedSignals.size() / 2; + QTRY_COMPARE(loadFinishedSpy.size(), expectedLoadCount); + QCOMPARE(page.signalsOrder, expectedSignals); + + // verify no more signals is emitted by waiting for another loadStarted or loadFinished + QTRY_LOOP_IMPL(loadStartedSpy.size() != expectedLoadCount || loadFinishedSpy.size() != expectedLoadCount, 1000, 100); // No further loadStarted should have occurred within this time QCOMPARE(loadStartedSpy.size(), expectedLoadCount); @@ -133,18 +285,19 @@ void tst_LoadSignals::monotonicity() QTRY_COMPARE(loadFinishedSpy.size(), 1); QVERIFY(loadFinishedSpy[0][0].toBool()); + QVERIFY(page.loadProgress.size() >= 3); // first loadProgress should have 0% progress - QCOMPARE(loadProgressSpy.takeFirst()[0].toInt(), 0); + QCOMPARE(page.loadProgress.first(), 0); // every loadProgress should have more progress than the one before - int progress = 0; - for (const auto &item : loadProgressSpy) { - QVERIFY(progress < item[0].toInt()); - progress = item[0].toInt(); + int progress = -1; + for (int p : page.loadProgress) { + QVERIFY(progress < p); + progress = p; } // last loadProgress should have 100% progress - QCOMPARE(loadProgressSpy.last()[0].toInt(), 100); + QCOMPARE(page.loadProgress.last(), 100); } /** @@ -160,7 +313,6 @@ void tst_LoadSignals::loadAfterInPageNavigation_qtbug66869() // page3 does an in-page navigation after 500ms QTest::qWait(2000); loadFinishedSpy.clear(); - loadProgressSpy.clear(); loadStartedSpy.clear(); // second load @@ -204,9 +356,9 @@ void tst_LoadSignals::fileDownloadDoesNotTriggerLoadSignals_qtbug66661() QTest::sendKeyEvent(QTest::Press, view.focusProxy(), Qt::Key_Return, QString("\r"), Qt::NoModifier); QTest::sendKeyEvent(QTest::Release, view.focusProxy(), Qt::Key_Return, QString("\r"), Qt::NoModifier); - // Wait for 10 seconds (abort waiting if another loadStarted or loadFinished occurs) + // Wait for 5 seconds (abort waiting if another loadStarted or loadFinished occurs) QTRY_LOOP_IMPL((loadStartedSpy.size() != 1) - || (loadFinishedSpy.size() != 1), 10000, 100); + || (loadFinishedSpy.size() != 1), 5000, 100); // Download must have occurred QTRY_COMPARE(downloadState, QWebEngineDownloadItem::DownloadCompleted); @@ -231,7 +383,6 @@ void tst_LoadSignals::numberOfStartedAndFinishedSignalsIsSame() { loadStartedSpy.clear(); loadFinishedSpy.clear(); - loadProgressSpy.clear(); view.page()->setHtml("" "" @@ -351,6 +502,5 @@ void tst_LoadSignals::errorPageTriggered() loadFinishedSpy.clear(); } - QTEST_MAIN(tst_LoadSignals) #include "tst_loadsignals.moc" diff --git a/tests/auto/widgets/loadsignals/tst_loadsignals.qrc b/tests/auto/widgets/loadsignals/tst_loadsignals.qrc index 21c517154..b4ee36676 100644 --- a/tests/auto/widgets/loadsignals/tst_loadsignals.qrc +++ b/tests/auto/widgets/loadsignals/tst_loadsignals.qrc @@ -4,6 +4,10 @@ ../../shared/data/loadprogress/page2.html ../../shared/data/loadprogress/page3.html ../../shared/data/loadprogress/page4.html + ../../shared/data/loadprogress/page5.html + ../../shared/data/loadprogress/page6.html + ../../shared/data/loadprogress/page7.html + ../../shared/data/loadprogress/page8.html ../../shared/data/loadprogress/downloadable.tar.gz -- cgit v1.2.3