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

@@ -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)

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

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()
         }
     }