Skip to content

Commit badcb13

Browse files
Kirill Burtsevpatricia-gallardo
authored andcommitted
Suppress error pages also for http errors if they are disabled
Load with client or server http error results in successful navigation, which leads to 'true' loadFinished result, and subsequent chromium's error page load and display with second set of loadStarted/loadFinished signals. This effectively ignores QWebEngineSettings::ErrorPageEnabled. Fixing it requires submodule change to ask embedder if error pages should also be suppressed for http errors. Also update chromium for required change, which pulls in the following changes: * e71010069b4 Fix embedded builds with printing enabled * f5a93d251cc Allow the embedder to suppress an error page for http errors Change-Id: I731678575439a6dad90dfb89e79b0083c63b49c2 Reviewed-by: Allan Sandfeld Jensen <[email protected]>
1 parent b51f8c0 commit badcb13

File tree

3 files changed

+38
-9
lines changed

3 files changed

+38
-9
lines changed

src/core/web_contents_delegate_qt.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,6 @@ void WebContentsDelegateQt::DidStartNavigation(content::NavigationHandle *naviga
373373
if (!navigation_handle->IsInMainFrame())
374374
return;
375375

376-
// Error-pages are not reported as separate started navigations.
377-
Q_ASSERT(!navigation_handle->IsErrorPage());
378376

379377
m_loadingErrorFrameList.clear();
380378
m_faviconManager->resetCandidates();
@@ -383,6 +381,8 @@ void WebContentsDelegateQt::DidStartNavigation(content::NavigationHandle *naviga
383381

384382
void WebContentsDelegateQt::EmitLoadFinished(bool success, const QUrl &url, bool isErrorPage, int errorCode, const QString &errorDescription)
385383
{
384+
Q_ASSERT(!isErrorPage || webEngineSettings()->testAttribute(WebEngineSettings::ErrorPageEnabled));
385+
386386
// When error page enabled we don't need to send the error page load finished signal
387387
if (m_loadProgressMap[url] == 100)
388388
return;
@@ -537,10 +537,8 @@ void WebContentsDelegateQt::DidFinishLoad(content::RenderFrameHost* render_frame
537537
m_viewClient->iconChanged(QUrl());
538538

539539
content::NavigationEntry *entry = web_contents()->GetController().GetActiveEntry();
540-
int http_statuscode = 0;
541-
if (entry)
542-
http_statuscode = entry->GetHttpStatusCode();
543-
EmitLoadFinished(true /* success */ , toQt(validated_url), false /* isErrorPage */, http_statuscode);
540+
int http_statuscode = entry ? http_statuscode = entry->GetHttpStatusCode() : 0;
541+
EmitLoadFinished(http_statuscode < 400, toQt(validated_url), false /* isErrorPage */, http_statuscode);
544542
}
545543

546544
void WebContentsDelegateQt::DidUpdateFaviconURL(const std::vector<blink::mojom::FaviconURLPtr> &candidates)

src/webenginewidgets/api/qwebenginepage.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,6 @@ void QWebEnginePagePrivate::loadFinished(bool success, const QUrl &url, bool isE
290290
Q_UNUSED(errorDescription);
291291

292292
if (isErrorPage) {
293-
Q_ASSERT(settings->testAttribute(QWebEngineSettings::ErrorPageEnabled));
294293
QTimer::singleShot(0, q, [q](){
295294
emit q->loadFinished(false);
296295
});

tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ private Q_SLOTS:
174174
void setUrlUsingStateObject();
175175
void setUrlThenLoads_data();
176176
void setUrlThenLoads();
177+
void loadFinishedAfterNotFoundError_data();
177178
void loadFinishedAfterNotFoundError();
178179
void loadInSignalHandlers_data();
179180
void loadInSignalHandlers();
@@ -2820,18 +2821,49 @@ void tst_QWebEnginePage::setUrlThenLoads()
28202821
QCOMPARE(baseUrlSync(m_page), extractBaseUrl(urlToLoad2));
28212822
}
28222823

2824+
void tst_QWebEnginePage::loadFinishedAfterNotFoundError_data()
2825+
{
2826+
QTest::addColumn<bool>("rfcInvalid");
2827+
QTest::addColumn<bool>("withServer");
2828+
QTest::addRow("rfc_invalid") << true << false;
2829+
QTest::addRow("non_existent") << false << false;
2830+
QTest::addRow("server_404") << false << true;
2831+
}
2832+
28232833
void tst_QWebEnginePage::loadFinishedAfterNotFoundError()
28242834
{
2835+
QFETCH(bool, withServer);
2836+
QFETCH(bool, rfcInvalid);
2837+
2838+
QScopedPointer<HttpServer> server;
2839+
if (withServer) {
2840+
server.reset(new HttpServer);
2841+
QVERIFY(server->start());
2842+
}
2843+
28252844
QWebEnginePage page;
28262845
QSignalSpy spy(&page, SIGNAL(loadFinished(bool)));
28272846

28282847
page.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, false);
2829-
page.setUrl(QUrl("http://non.existent/url"));
2848+
auto url = server
2849+
? server->url("/not-found-page.html")
2850+
: QUrl(rfcInvalid ? "http://some.invalid" : "http://non.existent/url");
2851+
page.setUrl(url);
28302852
QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 1, 20000);
2853+
QVERIFY(!spy.at(0).at(0).toBool());
2854+
QCOMPARE(toPlainTextSync(&page), QString());
2855+
QCOMPARE(spy.count(), 1);
28312856

28322857
page.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, true);
2833-
page.setUrl(QUrl("http://another.non.existent/url"));
2858+
url = server
2859+
? server->url("/another-missing-one.html")
2860+
: QUrl(rfcInvalid ? "http://some.other.invalid" : "http://another.non.existent/url");
2861+
page.setUrl(url);
28342862
QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 2, 20000);
2863+
QVERIFY(!spy.at(1).at(0).toBool());
2864+
2865+
QEXPECT_FAIL("", "No more loads (like separate load for error pages) are expected", Continue);
2866+
QTRY_COMPARE_WITH_TIMEOUT(spy.count(), 3, 1000);
28352867
}
28362868

28372869
class URLSetter : public QObject {

0 commit comments

Comments
 (0)