diff options
author | Kirill Burtsev <kirill.burtsev@qt.io> | 2021-04-30 21:30:16 +0200 |
---|---|---|
committer | Kirill Burtsev <kirill.burtsev@qt.io> | 2021-05-06 09:07:13 +0200 |
commit | 4d4fc9cd120376f30ce0630b1e8c7bf174d44fae (patch) | |
tree | 30424dfd873ff93ea3ff4f1fdf5791440c4fd69a | |
parent | 88c9dc6801a583956e2eb5063544303b6f12f95d (diff) |
Fix inconsistent number of load signals and their order
This change tries to match how chromium treats one single load. Before
the pair of loadStarted/loadFinished methods for api classes was called
on delegate's DidStartNavigation/DidFinishNavigation, which might be many
within one single logical load. This is true for multiple usecases (like
multiple redirects on load, immediate form submit on DOM load, page's
subresource load, or just an error page load on failure). Tracking these
methods and deciding when to emit signals based on states are error-prone
and complicates logic for no benefits. Also it somewhat lies about when
real load is done, which is only started and finished on outer methods
DidStartLoading/DidStopLoading, which are conveniently called only once
for all mentioned usecases. So, this change uses these methods to issue
signals for load start/finish, and only makes exception for error page,
which is needed for quick's private test support.
Fixes: QTBUG-65223
Fixes: QTBUG-76802
Fixes: QTBUG-87089
Fixes: QTBUG-90342
Fixes: QTBUG-91773
Fixes: QTBUG-92063
Change-Id: I9cc99b639030fedd8cf6a9dc04d0869d6be6357d
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Peter Varga <pvarga@inf.u-szeged.hu>
-rw-r--r-- | src/core/web_contents_adapter.cpp | 6 | ||||
-rw-r--r-- | src/core/web_contents_adapter_client.h | 3 | ||||
-rw-r--r-- | src/core/web_contents_delegate_qt.cpp | 110 | ||||
-rw-r--r-- | src/core/web_contents_delegate_qt.h | 20 | ||||
-rw-r--r-- | src/webengine/api/qquickwebengineview.cpp | 4 | ||||
-rw-r--r-- | src/webengine/api/qquickwebengineview_p_p.h | 3 | ||||
-rw-r--r-- | src/webenginewidgets/api/qwebenginepage.cpp | 18 | ||||
-rw-r--r-- | src/webenginewidgets/api/qwebenginepage_p.h | 3 | ||||
-rw-r--r-- | tests/auto/quick/qmltests/data/test4.html | 1 | ||||
-rw-r--r-- | tests/auto/quick/qmltests/data/tst_loadUrl.qml | 17 | ||||
-rw-r--r-- | tests/auto/quick/qmltests2/data/tst_loadFail.qml | 7 | ||||
-rw-r--r-- | tests/auto/quick/qmltests2/data/tst_viewSoure.qml | 14 | ||||
-rw-r--r-- | tests/auto/widgets/loadsignals/BLACKLIST | 24 | ||||
-rw-r--r-- | tests/auto/widgets/loadsignals/tst_loadsignals.cpp | 73 | ||||
-rw-r--r-- | tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp | 13 |
15 files changed, 143 insertions, 173 deletions
diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp index 5597c69f9..2cd48907b 100644 --- a/src/core/web_contents_adapter.cpp +++ b/src/core/web_contents_adapter.cpp @@ -716,8 +716,7 @@ void WebContentsAdapter::load(const QWebEngineHttpRequest &request) m_adapterClient->loadFinished(false, request.url(), false, net::ERR_DISALLOWED_URL_SCHEME, QCoreApplication::translate("WebContentsAdapter", - "HTTP-POST data can only be sent over HTTP(S) protocol"), - false); + "HTTP-POST data can only be sent over HTTP(S) protocol")); return; } params.post_data = network::ResourceRequestBody::CreateFromBytes( @@ -773,7 +772,7 @@ void WebContentsAdapter::setContent(const QByteArray &data, const QString &mimeT GURL dataUrlToLoad(urlString); if (dataUrlToLoad.spec().size() > url::kMaxURLChars) { - m_adapterClient->loadFinished(false, baseUrl, false, net::ERR_ABORTED, QString(), false); + m_adapterClient->loadFinished(false, baseUrl, false, net::ERR_ABORTED, QString()); return; } content::NavigationController::LoadURLParams params((dataUrlToLoad)); @@ -1995,6 +1994,7 @@ void WebContentsAdapter::discard() if (m_webContents->IsLoading()) { m_webContentsDelegate->didFailLoad(m_webContentsDelegate->url(webContents()), net::Error::ERR_ABORTED, QStringLiteral("Discarded")); + m_webContentsDelegate->DidStopLoading(); } content::WebContents::CreateParams createParams(m_profileAdapter->profile()); diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h index 267266d81..afc43806a 100644 --- a/src/core/web_contents_adapter_client.h +++ b/src/core/web_contents_adapter_client.h @@ -466,8 +466,7 @@ public: virtual void loadStarted(const QUrl &provisionalUrl, bool isErrorPage = false) = 0; virtual void loadCommitted() = 0; virtual void loadVisuallyCommitted() = 0; - virtual void loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, - const QString &errorDescription, bool triggersErrorPage) = 0; + virtual void loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, const QString &errorDescription) = 0; virtual void focusContainer() = 0; virtual void unhandledKeyEvent(QKeyEvent *event) = 0; virtual QSharedPointer<WebContentsAdapter> diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp index 1e92a46f8..f0e4130e8 100644 --- a/src/core/web_contents_delegate_qt.cpp +++ b/src/core/web_contents_delegate_qt.cpp @@ -259,14 +259,12 @@ void WebContentsDelegateQt::CloseContents(content::WebContents *source) void WebContentsDelegateQt::LoadProgressChanged(double progress) { - QUrl current_url(m_viewClient->webContentsAdapter()->getNavigationEntryOriginalUrl(m_viewClient->webContentsAdapter()->currentNavigationEntryIndex())); - int p = qMin(qRound(progress * 100), 100); - - if (!m_loadingErrorFrameList.isEmpty() || !m_loadProgressMap.contains(current_url) || m_loadProgressMap[current_url] == 100 || p == 100) + if (!m_loadingErrorFrameList.isEmpty() || !m_loadingInfo.isLoading()) // suppress signals that aren't between loadStarted and loadFinished return; - if (p > m_loadProgressMap[current_url]) { // ensure strict monotonic increase - m_loadProgressMap[current_url] = p; + int p = qMin(qRound(progress * 100), 100); + if (p > m_loadingInfo.progress) { // ensure strict monotonic increase + m_loadingInfo.progress = p; m_viewClient->loadProgressChanged(p); } } @@ -339,35 +337,21 @@ void WebContentsDelegateQt::RenderViewHostChanged(content::RenderViewHost *, con } } -void WebContentsDelegateQt::EmitLoadStarted(const QUrl &url, bool isErrorPage) +void WebContentsDelegateQt::emitLoadStarted(bool isErrorPage) { - m_isDocumentEmpty = true; - m_viewClient->loadStarted(url, isErrorPage); - m_viewClient->updateNavigationActions(); - - if ((url.hasFragment() || m_lastLoadedUrl.hasFragment()) - && url.adjusted(QUrl::RemoveFragment) == m_lastLoadedUrl.adjusted(QUrl::RemoveFragment) - && !m_isNavigationCommitted) { - m_loadProgressMap.insert(url, 100); - m_lastLoadedUrl = url; - m_viewClient->loadProgressChanged(100); + // only report first ever load start or separate one for error page only + if (!isErrorPage && m_loadingInfo.isLoading()) // already running return; - } - if (!m_loadProgressMap.isEmpty()) { - QMap<QUrl, int>::iterator it = m_loadProgressMap.begin(); - while (it != m_loadProgressMap.end()) { - if (it.value() == 100) { - it = m_loadProgressMap.erase(it); - continue; - } - ++it; - } + m_isDocumentEmpty = true; // reset to default which may only be overridden on actual resource load complete + if (!isErrorPage) { + m_loadingInfo.progress = 0; + m_viewClient->loadStarted(m_loadingInfo.url, false); + m_viewClient->updateNavigationActions(); + m_viewClient->loadProgressChanged(0); + } else { + m_viewClient->loadStarted(toQt(GURL(content::kUnreachableWebDataURL)), true); } - - m_lastLoadedUrl = url; - m_loadProgressMap.insert(url, 0); - m_viewClient->loadProgressChanged(0); } void WebContentsDelegateQt::DidStartNavigation(content::NavigationHandle *navigation_handle) @@ -375,34 +359,41 @@ void WebContentsDelegateQt::DidStartNavigation(content::NavigationHandle *naviga if (!webEngineSettings()->testAttribute(WebEngineSettings::ErrorPageEnabled)) navigation_handle->SetSilentlyIgnoreErrors(); - if (!navigation_handle->IsInMainFrame()) + if (!navigation_handle->IsInMainFrame() || !web_contents()->IsLoadingToDifferentDocument()) return; m_loadingErrorFrameList.clear(); m_faviconManager->resetCandidates(); - EmitLoadStarted(toQt(navigation_handle->GetURL())); + + m_loadingInfo.url = toQt(navigation_handle->GetURL()); + // IsErrorPage is only set after navigation commit, so check it otherwise: error page shouldn't have navigation entry + bool isErrorPage = m_loadingInfo.triggersErrorPage && !navigation_handle->GetNavigationEntry(); + emitLoadStarted(isErrorPage); } -void WebContentsDelegateQt::EmitLoadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, const QString &errorDescription, bool triggersErrorPage) +void WebContentsDelegateQt::emitLoadFinished(bool isErrorPage) { - Q_ASSERT(!isErrorPage || webEngineSettings()->testAttribute(WebEngineSettings::ErrorPageEnabled)); - Q_ASSERT((triggersErrorPage && webEngineSettings()->testAttribute(WebEngineSettings::ErrorPageEnabled)) || !triggersErrorPage); - - // When error page enabled we don't need to send the error page load finished signal - if (m_loadProgressMap[url] == 100) + if (!m_loadingInfo.isLoading()) // not currently running return; - m_lastLoadedUrl = url; - m_loadProgressMap[url] = 100; - m_isNavigationCommitted = false; - m_viewClient->loadProgressChanged(100); + Q_ASSERT(!isErrorPage || webEngineSettings()->testAttribute(WebEngineSettings::ErrorPageEnabled)); + Q_ASSERT((m_loadingInfo.triggersErrorPage && webEngineSettings()->testAttribute(WebEngineSettings::ErrorPageEnabled)) || !m_loadingInfo.triggersErrorPage); + + if (!isErrorPage) { + if (m_loadingInfo.progress < 100) { + m_loadingInfo.progress = 100; + m_viewClient->loadProgressChanged(100); + } - m_viewClient->loadFinished(success, url, isErrorPage, errorCode, errorDescription, triggersErrorPage); - m_viewClient->updateNavigationActions(); + m_viewClient->loadFinished(m_loadingInfo.success, m_loadingInfo.url, false, m_loadingInfo.errorCode, m_loadingInfo.errorDescription); + m_viewClient->updateNavigationActions(); + } else { + m_viewClient->loadFinished(false, toQt(GURL(content::kUnreachableWebDataURL)), true, 0, QString()); + } } -void WebContentsDelegateQt::EmitLoadCommitted() +void WebContentsDelegateQt::emitLoadCommitted() { m_findTextHelper->handleLoadCommitted(); m_viewClient->loadCommitted(); @@ -422,8 +413,7 @@ void WebContentsDelegateQt::DidFinishNavigation(content::NavigationHandle *navig profileAdapter->visitedLinksManager()->addUrl(url); } - m_isNavigationCommitted = true; - EmitLoadCommitted(); + emitLoadCommitted(); } // Success is reported by DidFinishLoad, but DidFailLoad is now dead code and needs to be handled below @@ -440,11 +430,11 @@ void WebContentsDelegateQt::DidFinishNavigation(content::NavigationHandle *navig // Now report we are starting to load an error-page. m_loadingErrorFrameList.append(navigation_handle->GetRenderFrameHost()->GetRoutingID()); m_faviconManager->resetCandidates(); - EmitLoadStarted(toQt(GURL(content::kUnreachableWebDataURL)), true); + emitLoadStarted(true); // If it is already committed we will not see another DidFinishNavigation call or a DidFinishLoad call. if (navigation_handle->HasCommitted()) - EmitLoadCommitted(); + emitLoadCommitted(); } } @@ -486,6 +476,9 @@ void WebContentsDelegateQt::DidStopLoading() if (m_loadingState == LoadingState::Loading) setLoadingState(LoadingState::Loaded); + + emitLoadFinished(); + m_loadingInfo.clear(); } void WebContentsDelegateQt::didFailLoad(const QUrl &url, int errorCode, const QString &errorDescription) @@ -495,7 +488,11 @@ void WebContentsDelegateQt::didFailLoad(const QUrl &url, int errorCode, const QS // Delay notifying failure until the error-page is done loading. // Error-pages are not loaded on failures due to abort. bool aborted = (errorCode == -3 /* ERR_ABORTED*/ ); - EmitLoadFinished(false /* success */ , url, false /* isErrorPage */, errorCode, errorDescription, errorPageEnabled && !aborted); + m_loadingInfo.success = false; + m_loadingInfo.url = url; + m_loadingInfo.errorCode = errorCode; + m_loadingInfo.errorDescription = errorDescription; + m_loadingInfo.triggersErrorPage = errorPageEnabled && !aborted; } void WebContentsDelegateQt::DidFailLoad(content::RenderFrameHost* render_frame_host, const GURL& validated_url, int error_code) @@ -511,8 +508,7 @@ void WebContentsDelegateQt::DidFailLoad(content::RenderFrameHost* render_frame_h Q_ASSERT(error_code == -3 /* ERR_ABORTED */); m_loadingErrorFrameList.removeOne(render_frame_host->GetRoutingID()); m_viewClient->iconChanged(QUrl()); - - EmitLoadFinished(false /* success */, toQt(validated_url), true /* isErrorPage */); + emitLoadFinished(/* isErrorPage = */true); return; } // Qt6: Consider getting rid of the error_description (Chromium already has) @@ -532,7 +528,7 @@ void WebContentsDelegateQt::DidFinishLoad(content::RenderFrameHost* render_frame // Trigger LoadFinished signal for main frame's error page only. if (!render_frame_host->GetParent()) { m_viewClient->iconChanged(QUrl()); - EmitLoadFinished(true /* success */, toQt(validated_url), true /* isErrorPage */); + emitLoadFinished(/* isErrorPage = */true); } return; @@ -550,7 +546,11 @@ void WebContentsDelegateQt::DidFinishLoad(content::RenderFrameHost* render_frame int http_statuscode = entry ? entry->GetHttpStatusCode() : 0; bool errorPageEnabled = webEngineSettings()->testAttribute(WebEngineSettings::ErrorPageEnabled); bool triggersErrorPage = errorPageEnabled && (http_statuscode >= 400) && m_isDocumentEmpty; - EmitLoadFinished(http_statuscode < 400, toQt(validated_url), false /* isErrorPage */, http_statuscode, QString(), triggersErrorPage); + + m_loadingInfo.success = http_statuscode < 400; + m_loadingInfo.url = toQt(validated_url); + m_loadingInfo.errorCode = http_statuscode; + m_loadingInfo.triggersErrorPage = triggersErrorPage; } void WebContentsDelegateQt::DidUpdateFaviconURL(content::RenderFrameHost *render_frame_host, const std::vector<blink::mojom::FaviconURLPtr> &candidates) diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h index 5a3dff6e9..7149f6bff 100644 --- a/src/core/web_contents_delegate_qt.h +++ b/src/core/web_contents_delegate_qt.h @@ -216,9 +216,9 @@ private: WindowOpenDisposition disposition, const gfx::Rect &initial_pos, const QUrl &url, bool user_gesture); - void EmitLoadStarted(const QUrl &url, bool isErrorPage = false); - void EmitLoadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString(), bool triggersErrorPage = false); - void EmitLoadCommitted(); + void emitLoadStarted(bool isErrorPage = false); + void emitLoadFinished(bool isErrorPage = false); + void emitLoadCommitted(); LoadingState determineLoadingState(content::WebContents *contents); void setLoadingState(LoadingState state); @@ -242,9 +242,17 @@ private: int m_desktopStreamCount = 0; mutable bool m_pendingUrlUpdate = false; - QMap<QUrl, int> m_loadProgressMap; - QUrl m_lastLoadedUrl; - bool m_isNavigationCommitted = false; + struct LoadingInfo { + bool success = false; + int progress = -1; + bool isLoading() const { return progress >= 0; } + QUrl url; + int errorCode = 0; + QString errorDescription; + bool triggersErrorPage = false; + void clear() { *this = LoadingInfo(); } + } m_loadingInfo; + bool m_isDocumentEmpty = true; base::WeakPtrFactory<WebContentsDelegateQt> m_weakPtrFactory { this }; }; diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp index 6ab1c97cb..1de7f1c7f 100644 --- a/src/webengine/api/qquickwebengineview.cpp +++ b/src/webengine/api/qquickwebengineview.cpp @@ -496,11 +496,9 @@ Q_STATIC_ASSERT(static_cast<int>(WebEngineError::NoErrorDomain) == static_cast<i Q_STATIC_ASSERT(static_cast<int>(WebEngineError::CertificateErrorDomain) == static_cast<int>(QQuickWebEngineView::CertificateErrorDomain)); Q_STATIC_ASSERT(static_cast<int>(WebEngineError::DnsErrorDomain) == static_cast<int>(QQuickWebEngineView::DnsErrorDomain)); -void QQuickWebEngineViewPrivate::loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, - const QString &errorDescription, bool triggersErrorPage) +void QQuickWebEngineViewPrivate::loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, const QString &errorDescription) { Q_Q(QQuickWebEngineView); - Q_UNUSED(triggersErrorPage); if (isErrorPage) { #if QT_CONFIG(webengine_testsupport) diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h index ebe55c345..ec535298b 100644 --- a/src/webengine/api/qquickwebengineview_p_p.h +++ b/src/webengine/api/qquickwebengineview_p_p.h @@ -116,8 +116,7 @@ public: void loadStarted(const QUrl &provisionalUrl, bool isErrorPage = false) override; void loadCommitted() override; void loadVisuallyCommitted() override; - void loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, - const QString &errorDescription, bool triggersErrorPage) override; + void loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, const QString &errorDescription) override; void focusContainer() override; void unhandledKeyEvent(QKeyEvent *event) override; QSharedPointer<QtWebEngineCore::WebContentsAdapter> diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index e08afed44..b32c15039 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -282,28 +282,20 @@ void QWebEnginePagePrivate::loadStarted(const QUrl &provisionalUrl, bool isError QTimer::singleShot(0, q, &QWebEnginePage::loadStarted); } -void QWebEnginePagePrivate::loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, - const QString &errorDescription, bool triggersErrorPage) +void QWebEnginePagePrivate::loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, const QString &errorDescription) { Q_Q(QWebEnginePage); Q_UNUSED(url); Q_UNUSED(errorCode); Q_UNUSED(errorDescription); - if (isErrorPage) { - QTimer::singleShot(0, q, [q](){ - emit q->loadFinished(false); - }); + if (isErrorPage) return; - } isLoading = false; - Q_ASSERT((success && !triggersErrorPage) || !success); - if (!triggersErrorPage) { - QTimer::singleShot(0, q, [q, success](){ - emit q->loadFinished(success); - }); - } + QTimer::singleShot(0, q, [q, success](){ + emit q->loadFinished(success); + }); } void QWebEnginePagePrivate::didPrintPageToPdf(const QString &filePath, bool success) diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h index 82ce99503..ae3ab5d25 100644 --- a/src/webenginewidgets/api/qwebenginepage_p.h +++ b/src/webenginewidgets/api/qwebenginepage_p.h @@ -107,8 +107,7 @@ public: void loadStarted(const QUrl &provisionalUrl, bool isErrorPage = false) override; void loadCommitted() override { } void loadVisuallyCommitted() override { } - void loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, - const QString &errorDescription, bool triggersErrorPage) override; + void loadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, const QString &errorDescription) override; void focusContainer() override; void unhandledKeyEvent(QKeyEvent *event) override; QSharedPointer<QtWebEngineCore::WebContentsAdapter> diff --git a/tests/auto/quick/qmltests/data/test4.html b/tests/auto/quick/qmltests/data/test4.html index c9b395ee5..cf5708994 100644 --- a/tests/auto/quick/qmltests/data/test4.html +++ b/tests/auto/quick/qmltests/data/test4.html @@ -24,6 +24,7 @@ } </script> <div id="content"> + <p><a id='anchor' href='#anchor'>anchor</a> <p>bla00 <p>bla01 <p>bla02 diff --git a/tests/auto/quick/qmltests/data/tst_loadUrl.qml b/tests/auto/quick/qmltests/data/tst_loadUrl.qml index 47dbbc087..8f589cc52 100644 --- a/tests/auto/quick/qmltests/data/tst_loadUrl.qml +++ b/tests/auto/quick/qmltests/data/tst_loadUrl.qml @@ -298,20 +298,19 @@ TestWebEngineView { compare(loadRequestArray[0].status, WebEngineView.LoadStartedStatus); compare(loadRequestArray[1].status, WebEngineView.LoadSucceededStatus); - // In-page navigation. - webEngineView.url = Qt.resolvedUrl("test4.html#content"); - // In-page navigation doesn't trigger load succeeded, wait for load progress instead. - tryCompare(loadRequestArray, "length", 3); - tryCompare(webEngineView, "loadProgress", 100); - compare(loadRequestArray[2].status, WebEngineView.LoadStartedStatus); + // In-page navigation shouldn't trigger load + let anchorUrl = Qt.resolvedUrl("test4.html#anchor"); + let c = webEngineView.getElementCenter('anchor') + mouseClick(webEngineView, c.x, c.y) + tryCompare(webEngineView, 'url', anchorUrl) // Load after in-page navigation. webEngineView.url = Qt.resolvedUrl("test4.html"); verify(webEngineView.waitForLoadSucceeded()); compare(webEngineView.loadProgress, 100); - compare(loadRequestArray.length, 5); - compare(loadRequestArray[3].status, WebEngineView.LoadStartedStatus); - compare(loadRequestArray[4].status, WebEngineView.LoadSucceededStatus); + compare(loadRequestArray.length, 4); + compare(loadRequestArray[2].status, WebEngineView.LoadStartedStatus); + compare(loadRequestArray[3].status, WebEngineView.LoadSucceededStatus); webEngineView.clear(); } diff --git a/tests/auto/quick/qmltests2/data/tst_loadFail.qml b/tests/auto/quick/qmltests2/data/tst_loadFail.qml index db412f252..298f5b0fe 100644 --- a/tests/auto/quick/qmltests2/data/tst_loadFail.qml +++ b/tests/auto/quick/qmltests2/data/tst_loadFail.qml @@ -119,21 +119,22 @@ TestWebEngineView { verify(!loadRequest.isErrorPage) // Loading of the unavailableUrl must fail - loadRequest = loadRequestArray[1] + loadRequest = loadRequestArray[3] compare(loadRequest.status, WebEngineView.LoadFailedStatus) compare(loadRequest.errorDomain, WebEngineView.InternalErrorDomain) compare(loadRequest.url, unavailableUrl) verify(!loadRequest.isErrorPage) + // error page load is done inside main load through test support // Start to load error page - loadRequest = loadRequestArray[2] + loadRequest = loadRequestArray[1] compare(loadRequest.status, WebEngineView.LoadStartedStatus) compare(loadRequest.errorDomain, WebEngineView.NoErrorDomain) compare(loadRequest.url, "chrome-error://chromewebdata/") verify(loadRequest.isErrorPage) // Loading of the error page must be successful - loadRequest = loadRequestArray[3] + loadRequest = loadRequestArray[2] compare(loadRequest.status, WebEngineView.LoadSucceededStatus) compare(loadRequest.errorDomain, WebEngineView.NoErrorDomain) compare(loadRequest.url, "chrome-error://chromewebdata/") diff --git a/tests/auto/quick/qmltests2/data/tst_viewSoure.qml b/tests/auto/quick/qmltests2/data/tst_viewSoure.qml index 04b40f544..74b7f803d 100644 --- a/tests/auto/quick/qmltests2/data/tst_viewSoure.qml +++ b/tests/auto/quick/qmltests2/data/tst_viewSoure.qml @@ -44,6 +44,7 @@ TestWebEngineView { errorPage.onLoadingChanged: { loadRequestArray.push({ "status": loadRequest.status, + "url": loadRequest.url }) } } @@ -51,6 +52,7 @@ TestWebEngineView { onLoadingChanged: { loadRequestArray.push({ "status": loadRequest.status, + "url": loadRequest.url }); } @@ -109,14 +111,16 @@ TestWebEngineView { WebEngine.settings.errorPageEnabled = true webEngineView.url = row.userInputUrl; + if (row.loadSucceed) { - tryVerify(function() { return loadRequestArray.length >= 2 }); + tryVerify(function() { return loadRequestArray.length == 2 }); compare(loadRequestArray[1].status, WebEngineView.LoadSucceededStatus); } else { - tryVerify(function() { return loadRequestArray.length >= 2 }); - compare(loadRequestArray[1].status, WebEngineView.LoadFailedStatus); - tryVerify(function() { return loadRequestArray.length == 4 }); - compare(loadRequestArray[3].status, WebEngineView.LoadSucceededStatus); + tryVerify(function() { return loadRequestArray.length == 4 }, 90000); + // error page load is done inside main load through test support + compare(loadRequestArray[2].status, WebEngineView.LoadSucceededStatus); + compare(loadRequestArray[2].url, "chrome-error://chromewebdata/") + compare(loadRequestArray[3].status, WebEngineView.LoadFailedStatus); } tryVerify(function() { return titleChangedSpy.count == 1; }); diff --git a/tests/auto/widgets/loadsignals/BLACKLIST b/tests/auto/widgets/loadsignals/BLACKLIST deleted file mode 100644 index d72d35d98..000000000 --- a/tests/auto/widgets/loadsignals/BLACKLIST +++ /dev/null @@ -1,24 +0,0 @@ -# QTBUG-65223 -[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) -[loadAfterInPageNavigation_qtbug66869] -* - -[numberOfStartedAndFinishedSignalsIsSame] -b2qt diff --git a/tests/auto/widgets/loadsignals/tst_loadsignals.cpp b/tests/auto/widgets/loadsignals/tst_loadsignals.cpp index e7dc77e95..bee6df486 100644 --- a/tests/auto/widgets/loadsignals/tst_loadsignals.cpp +++ b/tests/auto/widgets/loadsignals/tst_loadsignals.cpp @@ -91,6 +91,7 @@ private Q_SLOTS: void rejectNavigationRequest(); void loadAfterInPageNavigation_qtbug66869(); void fileDownload(); + void numberOfStartedAndFinishedSignalsIsSame_data(); void numberOfStartedAndFinishedSignalsIsSame(); void loadFinishedAfterNotFoundError_data(); void loadFinishedAfterNotFoundError(); @@ -311,19 +312,14 @@ void tst_LoadSignals::loadAfterInPageNavigation_qtbug66869() QVERIFY(loadFinishedSpy[0][0].toBool()); // page3 does an in-page navigation after 500ms - QTest::qWait(2000); - loadFinishedSpy.clear(); - loadStartedSpy.clear(); + QTRY_COMPARE(view.url(), QUrl("qrc:///resources/page3.html#anchor")); // second load view.load(QUrl("qrc:///resources/page1.html")); - QTRY_COMPARE(loadFinishedSpy.size(), 1); + QTRY_COMPARE(loadFinishedSpy.size(), 2); QVERIFY(loadFinishedSpy[0][0].toBool()); // loadStarted and loadFinished should have been signalled - QCOMPARE(loadStartedSpy.size(), 1); - - // reminder that we still need to solve the core issue - QFAIL("https://codereview.qt-project.org/#/c/222112/ only hides the symptom, the core issue still needs to be solved"); + QCOMPARE(loadStartedSpy.size(), 2); } void tst_LoadSignals::fileDownload() @@ -361,42 +357,42 @@ void tst_LoadSignals::fileDownload() QCOMPARE(page.signalsOrder, SignalsOrderTwiceWithFailure); } -void tst_LoadSignals::numberOfStartedAndFinishedSignalsIsSame() { +void tst_LoadSignals::numberOfStartedAndFinishedSignalsIsSame_data() +{ + QTest::addColumn<bool>("imageFromServer"); + QTest::addColumn<QString>("imageResourceUrl"); + // triggers these calls in delegate internally: + // just two ordered triples DidStartNavigation/DidFinishNavigation/DidFinishLoad + QTest::newRow("no_image_resource") << false << ""; + // out of order: DidStartNavigation/DidFinishNavigation/DidStartNavigation/DidFailLoad/DidFinishNavigation/DidFinishLoad + QTest::newRow("with_invalid_image") << false << "https://non.existent.locahost/image.png"; + // out of order: DidStartNavigation/DidFinishNavigation/DidStartNavigation/DidFinishLoad/DidFinishNavigation/DidFinishLoad + QTest::newRow("with_server_image") << true << ""; +} + +void tst_LoadSignals::numberOfStartedAndFinishedSignalsIsSame() +{ + QFETCH(bool, imageFromServer); + QFETCH(QString, imageResourceUrl); HttpServer server; server.setResourceDirs({ TESTS_SOURCE_DIR "/qwebengineprofile/resources" }); - connect(&server, &HttpServer::newRequest, [] (HttpReqRep *) { - QTest::qWait(250); // just add delay to trigger some progress for every sub resource - }); QVERIFY(server.start()); - view.load(server.url("/hedgehog.png")); - QTRY_COMPARE(loadFinishedSpy.size(), 1); - QVERIFY(loadFinishedSpy[0][0].toBool()); - - loadStartedSpy.clear(); - loadFinishedSpy.clear(); + QUrl serverImage = server.url("/hedgehog.png"); + QString imageUrl(!imageFromServer && imageResourceUrl.isEmpty() + ? "" : (imageFromServer ? serverImage.toEncoded() : imageResourceUrl)); - view.page()->setHtml("<html><body>" - "<img src=\"" + server.url("/hedgehog.png").toEncoded() + "\">" - "<form method='GET' name='hiddenform' action='qrc:///resources/page1.html' />" - "<script language='javascript'>document.forms[0].submit();</script>" - "</body></html>"); - - QTRY_COMPARE(loadStartedSpy.size(), 2); - QTRY_COMPARE(loadFinishedSpy.size(), 2); + auto html = "<html><head><link rel='icon' href='data:,'></head><body>" + "%1" "<form method='GET' name='hiddenform' action='qrc:///resources/page1.html' />" + "<script language='javascript'>document.forms[0].submit();</script>" + "</body></html>"; + view.page()->setHtml(QString(html).arg(imageUrl.isEmpty() ? "" : "<img src='" + imageUrl + "'>")); + QTRY_COMPARE(loadFinishedSpy.size(), 1); - QTRY_VERIFY(!loadFinishedSpy[0][0].toBool()); - QTRY_VERIFY(loadFinishedSpy[1][0].toBool()); - - view.page()->setHtml("<html><body>" - "<form method='GET' name='hiddenform' action='qrc:///resources/page1.html' />" - "<script language='javascript'>document.forms[0].submit();</script>" - "</body></html>"); - QTRY_COMPARE(loadStartedSpy.size(), 4); - QTRY_COMPARE(loadFinishedSpy.size(), 4); - QVERIFY(loadFinishedSpy[2][0].toBool()); - QVERIFY(loadFinishedSpy[3][0].toBool()); + resetSpies(); + QTRY_LOOP_IMPL(loadStartedSpy.size() || loadFinishedSpy.size(), 1000, 100); + QCOMPARE(page.signalsOrder, SignalsOrderOnce); } void tst_LoadSignals::loadFinishedAfterNotFoundError_data() @@ -428,6 +424,7 @@ void tst_LoadSignals::loadFinishedAfterNotFoundError() QVERIFY(!loadFinishedSpy.at(0).at(0).toBool()); QCOMPARE(toPlainTextSync(view.page()), QString()); QCOMPARE(loadFinishedSpy.count(), 1); + QCOMPARE(loadStartedSpy.count(), 1); view.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, true); url = server @@ -436,9 +433,11 @@ void tst_LoadSignals::loadFinishedAfterNotFoundError() view.load(url); QTRY_COMPARE_WITH_TIMEOUT(loadFinishedSpy.count(), 2, 20000); QVERIFY(!loadFinishedSpy.at(1).at(0).toBool()); + QCOMPARE(loadStartedSpy.count(), 2); QEXPECT_FAIL("", "No more loads (like separate load for error pages) are expected", Continue); QTRY_COMPARE_WITH_TIMEOUT(loadFinishedSpy.count(), 3, 1000); + QCOMPARE(loadStartedSpy.count(), 2); } void tst_LoadSignals::errorPageTriggered_data() diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp index e5975edcd..5df09357f 100644 --- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp +++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp @@ -599,7 +599,7 @@ void tst_QWebEnginePage::acceptNavigationRequestNavigationType() // client side redirect page.load(QUrl("qrc:///resources/redirect.html")); - QTRY_COMPARE_WITH_TIMEOUT(loadSpy.count(), 8, 20000); + QTRY_COMPARE_WITH_TIMEOUT(loadSpy.count(), 7, 20000); QTRY_COMPARE(page.navigations.count(), 8); expectedList += { QWebEnginePage::NavigationTypeTyped, QWebEnginePage::NavigationTypeRedirect }; @@ -621,7 +621,7 @@ void tst_QWebEnginePage::acceptNavigationRequestNavigationType() }); QVERIFY(server.start()); page.load(QUrl(server.url("/redirect1.html"))); - QTRY_COMPARE_WITH_TIMEOUT(loadSpy.count(), 9, 20000); + QTRY_COMPARE_WITH_TIMEOUT(loadSpy.count(), 8, 20000); QTRY_COMPARE(page.navigations.count(), 11); expectedList += { QWebEnginePage::NavigationTypeTyped, @@ -2936,11 +2936,7 @@ void tst_QWebEnginePage::loadInSignalHandlers() URLSetter setter(m_page, signal, type, urlForSetter); QSignalSpy spy(&setter, &URLSetter::finished); m_page->load(url); - // every loadStarted() call should have also loadFinished() - if (signal == URLSetter::LoadStarted) - QTRY_COMPARE(spy.count(), 2); - else - QTRY_COMPARE(spy.count(), 1); + QTRY_COMPARE(spy.count(), 1); QCOMPARE(m_page->url(), urlForSetter); } @@ -4091,8 +4087,7 @@ void tst_QWebEnginePage::setLifecycleStateWithDevTools() devToolsPage.setLifecycleState(QWebEnginePage::LifecycleState::Discarded); devToolsPage.setInspectedPage(&inspectedPage); QCOMPARE(devToolsPage.lifecycleState(), QWebEnginePage::LifecycleState::Active); - QTRY_COMPARE_WITH_TIMEOUT(devToolsSpy.count(), 2, 90000); - QCOMPARE(devToolsSpy.takeFirst().value(0), QVariant(false)); + QTRY_COMPARE_WITH_TIMEOUT(devToolsSpy.count(), 1, 90000); QCOMPARE(devToolsSpy.takeFirst().value(0), QVariant(true)); // keep DevTools open |