Ensure certificate error callback call for all types

Amends a2a9ea11f9. Actually reject certificate when it's considered to
be fatal. Adapt tests for possible regression but checking if load
really failed (due to missing internet access) or was just halted by
missing callback call internaly.

Pick-to: 6.2
Change-Id: I656525c353ce410f7bda8c55227a29fcd617bfdd
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

5 files changed, 53 insertions, 32 deletions

diff --git a/tests/auto/core/certificateerror/tst_certificateerror.cpp b/tests/auto/core/certificateerror/tst_certificateerror.cpp
index da679fa8a..3c43d997f 100644
--- a/tests/auto/core/certificateerror/tst_certificateerror.cpp
+++ b/tests/auto/core/certificateerror/tst_certificateerror.cpp
@@ -139,13 +139,19 @@ void tst_CertificateError::fatalError()
     QSignalSpy loadFinishedSpy(&page, &QWebEnginePage::loadFinished);
 
     page.setUrl(QUrl("https://revoked.badssl.com"));
-    if (!loadFinishedSpy.wait(10000))
+    if (!loadFinishedSpy.wait(10000)) {
+        QVERIFY2(!page.error, "There shouldn't be any certificate error if not loaded due to missing internet access!");
         QSKIP("Couldn't load page from network, skipping test.");
-    QTRY_VERIFY(page.error);
-    QVERIFY(!page.error->isOverridable());
+    }
 
-    // Fatal certificate errors are implicitly rejected. This should not cause crash.
-    page.error->rejectCertificate();
+    // revoked certificate might not be reported as invalid by chromium and the load will silently succeed
+    bool failed = !loadFinishedSpy.first().first().toBool(), hasError = bool(page.error);
+    QCOMPARE(failed, hasError);
+    if (hasError) {
+        QVERIFY(!page.error->isOverridable());
+        // Fatal certificate errors are implicitly rejected. But second call should not cause crash.
+        page.error->rejectCertificate();
+    }
 }
 
 QTEST_MAIN(tst_CertificateError)
diff --git a/tests/auto/quick/qmltests/data/TestWebEngineView.qml b/tests/auto/quick/qmltests/data/TestWebEngineView.qml
index aa9d67d99..68417c6c5 100644
--- a/tests/auto/quick/qmltests/data/TestWebEngineView.qml
+++ b/tests/auto/quick/qmltests/data/TestWebEngineView.qml
@@ -35,19 +35,29 @@ WebEngineView {
     property bool windowCloseRequestedSignalEmitted: false
     settings.focusOnNavigationEnabled: true
 
+    function loadSucceeded() { return loadStatus == WebEngineView.LoadSucceededStatus }
+    function loadFailed() { return loadStatus == WebEngineView.LoadFailedStatus }
+    function loadStopped() { return loadStatus == WebEngineView.LoadStoppedStatus }
+
+    function waitForLoadResult(timeout) {
+        loadStatus = null
+        var r = _waitFor(function() { return loadStatus != null && loadStatus != WebEngineView.LoadStartedStatus }, timeout)
+        return r
+    }
+
     function waitForLoadSucceeded(timeout) {
-        var success = _waitFor(function() { return loadStatus == WebEngineView.LoadSucceededStatus }, timeout)
         loadStatus = null
+        var success = _waitFor(function() { return loadStatus == WebEngineView.LoadSucceededStatus }, timeout)
         return success
     }
     function waitForLoadFailed(timeout) {
-        var failure = _waitFor(function() { return loadStatus == WebEngineView.LoadFailedStatus }, timeout)
         loadStatus = null
+        var failure = _waitFor(function() { return loadStatus == WebEngineView.LoadFailedStatus }, timeout)
         return failure
     }
     function waitForLoadStopped(timeout) {
-        var stop = _waitFor(function() { return loadStatus == WebEngineView.LoadStoppedStatus }, timeout)
         loadStatus = null
+        var stop = _waitFor(function() { return loadStatus == WebEngineView.LoadStoppedStatus }, timeout)
         return stop
     }
     function waitForWindowCloseRequested() {
diff --git a/tests/auto/quick/qmltests/data/tst_certificateError.qml b/tests/auto/quick/qmltests/data/tst_certificateError.qml
index bad24bf0f..b254bb621 100644
--- a/tests/auto/quick/qmltests/data/tst_certificateError.qml
+++ b/tests/auto/quick/qmltests/data/tst_certificateError.qml
@@ -123,20 +123,25 @@ TestWebEngineView {
         }
 
         function test_fatalError() {
-            var handleCertificateError = function(error) {
-                verify(!error.overrideable);
-                // QQuickWebEngineViewPrivate::allowCertificateError() will implicitly reject
-                // fatal errors and it should not crash if already rejected in handler.
-                error.rejectCertificate();
-            }
+            let error = undefined
+            var handleCertificateError = function(e) { error = e; }
             view.certificateError.connect(handleCertificateError);
 
             view.url = Qt.resolvedUrl('https://revoked.badssl.com');
-            if (!view.waitForLoadFailed(10000))
+            if (!view.waitForLoadResult()) {
+                verify(!error, "There shouldn't be any certificate error if not loaded due to missing internet access!");
                 skip("Couldn't load page from network, skipping test.");
-            compare(spyError.count, 1);
-
+            }
             view.certificateError.disconnect(handleCertificateError);
+
+            // revoked certificate might not be reported as invalid by chromium and the load will silently succeed
+            const failed = view.loadStatus == WebEngineView.LoadFailedStatus, hasError = Boolean(error)
+            compare(hasError, failed)
+            if (failed) {
+                verify(!error.overrideable);
+                // Fatal certificate errors are implicitly rejected. But second call should not cause crash.
+                error.rejectCertificate();
+            }
         }
     }
 }
diff --git a/tests/auto/quick/qmltests/data/tst_navigationRequested.qml b/tests/auto/quick/qmltests/data/tst_navigationRequested.qml
index 462dc8297..11ea69e11 100644
--- a/tests/auto/quick/qmltests/data/tst_navigationRequested.qml
+++ b/tests/auto/quick/qmltests/data/tst_navigationRequested.qml
@@ -94,19 +94,17 @@ TestWebEngineView {
             // Test if we get notified about main frame and iframe loads
             compare(navigationSpy.count, 0)
             webEngineView.url = Qt.resolvedUrl("test-iframe.html")
-            navigationSpy.wait()
+            verify(webEngineView.waitForLoadSucceeded())
             compare(attributes.mainUrl, Qt.resolvedUrl("test-iframe.html"))
-            navigationSpy.wait()
             compare(attributes.iframeUrl, Qt.resolvedUrl("test1.html"))
             compare(navigationSpy.count, 2)
-            verify(webEngineView.waitForLoadSucceeded())
 
             // Test if we get notified about clicked links
             mouseClick(webEngineView, 100, 100)
-            tryCompare(navigationSpy, "count", 3)
+            verify(webEngineView.waitForLoadSucceeded())
             compare(attributes.mainUrl, Qt.resolvedUrl("test1.html"))
             verify(attributes.linkClickedNavigationRequested)
-            verify(webEngineView.waitForLoadSucceeded())
+            compare(navigationSpy.count, 3)
         }
 
         function test_ignoreLinkClickedRequest() {
@@ -117,26 +115,28 @@ TestWebEngineView {
 
             shouldIgnoreLinkClicks = true
             mouseClick(webEngineView, 100, 100)
-            tryCompare(navigationSpy, "count", 3)
-            compare(attributes.mainUrl, Qt.resolvedUrl("test1.html"))
-            verify(attributes.linkClickedNavigationRequested)
-            verify(attributes.linkClickedNavigationIgnored)
             // We ignored the main frame request, so we should
             // get notified that the load has been stopped.
             verify(webEngineView.waitForLoadStopped())
             verify(!webEngineView.loading)
+
+            compare(navigationSpy.count, 3)
+            compare(attributes.mainUrl, Qt.resolvedUrl("test1.html"))
+            verify(attributes.linkClickedNavigationRequested)
+            verify(attributes.linkClickedNavigationIgnored)
         }
 
         function test_ignoreSubFrameRequest() {
             // Test if we can ignore sub frame requests
             shouldIgnoreSubFrameRequests = true
             webEngineView.url = Qt.resolvedUrl("test-iframe.html")
-            tryCompare(navigationSpy, "count", 2)
-            compare(attributes.mainUrl, Qt.resolvedUrl("test-iframe.html"))
-            compare(attributes.iframeUrl, Qt.resolvedUrl("test1.html"))
             // We ignored the sub frame request, so
             // the main frame load should still succeed.
             verify(webEngineView.waitForLoadSucceeded())
+
+            compare(navigationSpy.count, 2)
+            compare(attributes.mainUrl, Qt.resolvedUrl("test-iframe.html"))
+            compare(attributes.iframeUrl, Qt.resolvedUrl("test1.html"))
         }
     }
 }
diff --git a/tests/auto/quick/qmltests/data/tst_newViewRequest.qml b/tests/auto/quick/qmltests/data/tst_newViewRequest.qml
index ac402674d..fb99e9cee 100644
--- a/tests/auto/quick/qmltests/data/tst_newViewRequest.qml
+++ b/tests/auto/quick/qmltests/data/tst_newViewRequest.qml
@@ -122,7 +122,7 @@ TestWebEngineView {
             verify(!newViewRequest.userInitiated);
 
             if (viewType === "dialog") {
-                verify(dialog.webEngineView.waitForLoadSucceeded());
+                tryVerify(dialog.webEngineView.loadSucceeded)
                 compare(dialog.webEngineView.url, "");
                 dialog.destroy();
             }
@@ -143,7 +143,7 @@ TestWebEngineView {
             compare(newViewRequest.requestedUrl, url);
             verify(!newViewRequest.userInitiated);
             if (viewType === "dialog") {
-                verify(dialog.webEngineView.waitForLoadSucceeded());
+                tryVerify(dialog.webEngineView.loadSucceeded)
                 dialog.destroy();
             }
             newViewRequestedSpy.clear();
@@ -166,7 +166,7 @@ TestWebEngineView {
                 compare(newViewRequest.destination, WebEngineNewViewRequest.InNewDialog);
                 verify(newViewRequest.userInitiated);
                 if (viewType === "dialog") {
-                    verify(dialog.webEngineView.waitForLoadSucceeded());
+                    tryVerify(dialog.webEngineView.loadSucceeded)
                     dialog.destroy();
                 }
                 newViewRequestedSpy.clear();