From a193b3b9abb6d742b40d0a8067155932a5c2c2d3 Mon Sep 17 00:00:00 2001
From: Adam Kallai <kadam@inf.u-szeged.hu>
Date: Mon, 15 Sep 2014 02:18:01 -0700
Subject: [PATCH] Fix an assertion in web_content_delegates_qt.cpp
If we get a replacement content, we can see a DidFinishLoad event for a frame.
This error page should be ignored based on the frame.
Change-Id: I3e1cd1773e8c5fc608605197c957011ddf258123
Reviewed-by: Jocelyn Turcotte <jocelyn.turcotte@digia.com>
---
src/core/web_contents_delegate_qt.cpp | 51 ++++++++++---------
src/core/web_contents_delegate_qt.h | 3 +-
.../auto/quick/qmltests/data/tst_loadFail.qml | 11 ++++
3 files changed, 41 insertions(+), 24 deletions(-)
diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp
index c78305876..ff31e081a 100644
--- a/src/core/web_contents_delegate_qt.cpp
+++ b/src/core/web_contents_delegate_qt.cpp
@@ -72,7 +72,6 @@ static WebContentsAdapterClient::JavaScriptConsoleMessageLevel mapToJavascriptCo
WebContentsDelegateQt::WebContentsDelegateQt(content::WebContents *webContents, WebContentsAdapterClient *adapterClient)
: m_viewClient(adapterClient)
, m_lastReceivedFindReply(0)
- , m_isLoadingErrorPage(false)
{
webContents->SetDelegate(this);
Observe(webContents);
@@ -125,18 +124,22 @@ void WebContentsDelegateQt::CloseContents(content::WebContents *source)
void WebContentsDelegateQt::LoadProgressChanged(content::WebContents* source, double progress)
{
- if (m_isLoadingErrorPage)
+ if (!m_loadingErrorFrameList.isEmpty())
return;
m_viewClient->loadProgressChanged(qRound(progress * 100));
}
-void WebContentsDelegateQt::DidStartProvisionalLoadForFrame(int64, int64, bool is_main_frame, const GURL &validated_url, bool isErrorPage, bool, content::RenderViewHost*)
+void WebContentsDelegateQt::DidStartProvisionalLoadForFrame(int64 frame_id, int64 parent_frame_id, bool is_main_frame, const GURL &validated_url, bool isErrorPage, bool, content::RenderViewHost*)
{
- m_isLoadingErrorPage = isErrorPage;
- if (isErrorPage)
+ if (isErrorPage) {
+ m_loadingErrorFrameList.append(frame_id);
return;
- if (is_main_frame)
- m_viewClient->loadStarted(toQt(validated_url));
+ }
+
+ if (!is_main_frame)
+ return;
+
+ m_viewClient->loadStarted(toQt(validated_url));
}
void WebContentsDelegateQt::DidCommitProvisionalLoadForFrame(int64 frame_id, const base::string16& frame_unique_name, bool is_main_frame, const GURL& url, content::PageTransition transition_type, content::RenderViewHost* render_view_host)
@@ -153,34 +156,36 @@ void WebContentsDelegateQt::DidFailProvisionalLoad(int64 frame_id, const base::s
DidFailLoad(frame_id, validated_url, is_main_frame, error_code, error_description, render_view_host);
}
-void WebContentsDelegateQt::DidFailLoad(int64, const GURL &validated_url, bool is_main_frame, int error_code, const base::string16 &error_description, content::RenderViewHost *rvh)
+void WebContentsDelegateQt::DidFailLoad(int64 frame_id, const GURL &validated_url, bool is_main_frame, int error_code, const base::string16 &error_description, content::RenderViewHost *rvh)
{
- if (!is_main_frame || m_isLoadingErrorPage)
+ if (m_loadingErrorFrameList.removeOne(frame_id) || !is_main_frame)
return;
+
m_viewClient->loadFinished(false, toQt(validated_url), error_code, toQt(error_description));
m_viewClient->loadProgressChanged(0);
}
-void WebContentsDelegateQt::DidFinishLoad(int64, const GURL &url, bool is_main_frame, content::RenderViewHost*)
+void WebContentsDelegateQt::DidFinishLoad(int64 frame_id, const GURL &url, bool is_main_frame, content::RenderViewHost*)
{
- if (m_isLoadingErrorPage) {
+ if (m_loadingErrorFrameList.removeOne(frame_id)) {
Q_ASSERT(url.is_valid() && url.spec() == content::kUnreachableWebDataURL);
m_viewClient->iconChanged(QUrl());
return;
}
- if (is_main_frame) {
- m_viewClient->loadFinished(true, toQt(url));
-
- content::NavigationEntry *entry = web_contents()->GetController().GetActiveEntry();
- if (!entry)
- return;
- content::FaviconStatus &favicon = entry->GetFavicon();
- if (favicon.valid)
- m_viewClient->iconChanged(toQt(favicon.url));
- else
- m_viewClient->iconChanged(QUrl());
- }
+ if (!is_main_frame)
+ return;
+
+ m_viewClient->loadFinished(true, toQt(url));
+
+ content::NavigationEntry *entry = web_contents()->GetController().GetActiveEntry();
+ if (!entry)
+ return;
+ content::FaviconStatus &favicon = entry->GetFavicon();
+ if (favicon.valid)
+ m_viewClient->iconChanged(toQt(favicon.url));
+ else
+ m_viewClient->iconChanged(QUrl());
}
void WebContentsDelegateQt::DidUpdateFaviconURL(const std::vector<content::FaviconURL>& candidates)
diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h
index b7a23d3c2..2ab5fc8cd 100644
--- a/src/core/web_contents_delegate_qt.h
+++ b/src/core/web_contents_delegate_qt.h
@@ -60,6 +60,7 @@ class WebContentsDelegateQt : public content::WebContentsDelegate
{
public:
WebContentsDelegateQt(content::WebContents*, WebContentsAdapterClient *adapterClient);
+ ~WebContentsDelegateQt() { Q_ASSERT(m_loadingErrorFrameList.isEmpty()); }
QString lastSearchedString() const { return m_lastSearchedString; }
void setLastSearchedString(const QString &s) { m_lastSearchedString = s; }
int lastReceivedFindReply() const { return m_lastReceivedFindReply; }
@@ -94,7 +95,7 @@ private:
WebContentsAdapterClient *m_viewClient;
QString m_lastSearchedString;
int m_lastReceivedFindReply;
- bool m_isLoadingErrorPage;
+ QList<int64> m_loadingErrorFrameList;
};
#endif // WEB_CONTENTS_DELEGATE_QT_H
diff --git a/tests/auto/quick/qmltests/data/tst_loadFail.qml b/tests/auto/quick/qmltests/data/tst_loadFail.qml
index e2282129f..7b0a1849e 100644
--- a/tests/auto/quick/qmltests/data/tst_loadFail.qml
+++ b/tests/auto/quick/qmltests/data/tst_loadFail.qml
@@ -50,6 +50,12 @@ TestWebEngineView {
property variant testUrl
+ SignalSpy {
+ id: spyIconChanged
+ target: webEngineView
+ signalName: "iconChanged"
+ }
+
TestCase {
id: test
name: "WebEngineViewLoadFail"
@@ -57,6 +63,11 @@ TestWebEngineView {
testUrl = Qt.resolvedUrl("file_that_does_not_exist.html")
webEngineView.url = testUrl
verify(webEngineView.waitForLoadFailed())
+ spyIconChanged.clear()
+
+ // If this testcase finishes too early, we can not handle the received replacement content.
+ // So we should wait to ignore this error page.
+ spyIconChanged.wait()
}
}
--
GitLab