From 7644564d754bbee640a091950b77e23586c2d283 Mon Sep 17 00:00:00 2001
From: Michal Klocek <michal.klocek@qt.io>
Date: Tue, 16 Jan 2018 18:44:01 +0100
Subject: [PATCH] Fix crashes of url load qml tests

Currently we can get QuickWebEngineViewPrivate::loadFinished
while still being in RenderFrameHostImpl::OnDidStopLoading,
unfortunately if user connects onLoadingChanged signal
with new url load request this will end up in DiscardUnusedFrame
and delete on RenderFrameHostImpl which is still on the bottom of
the stack. Use QTimer::singleShot to return to the event loop
before emitting load handling signals.

Post all load handling calls with singleShot to avoid out of order
load "signals" delivery in some cases.

Emitting signals should be done from WebContentsAdapterClient to
make sure the destruction of WebConentAdapterClient prevents
emitting signals on already deleted adapter.

Task-numbmer: QTBUG-65813
Change-Id: I93263876fb14bd959ba951463c8aeb5155f04a4f
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
 .../api/qquickwebenginetestsupport.cpp        | 16 +++++----
 src/webengine/api/qquickwebengineview.cpp     | 35 +++++++++++--------
 src/webenginewidgets/api/qwebenginepage.cpp   | 10 ++++--
 3 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/src/webengine/api/qquickwebenginetestsupport.cpp b/src/webengine/api/qquickwebenginetestsupport.cpp
index b3290d3cc..b7b863125 100644
--- a/src/webengine/api/qquickwebenginetestsupport.cpp
+++ b/src/webengine/api/qquickwebenginetestsupport.cpp
@@ -42,6 +42,7 @@
 #include "qquickwebengineloadrequest_p.h"
 #include <QQuickWindow>
 #include <QtTest/qtest.h>
+#include <QtCore/QTimer>
 
 QT_BEGIN_NAMESPACE
 
@@ -56,19 +57,20 @@ QQuickWebEngineErrorPage::QQuickWebEngineErrorPage()
 void QQuickWebEngineErrorPage::loadFinished(bool success, const QUrl &url)
 {
     Q_UNUSED(success);
-
-    QQuickWebEngineLoadRequest loadRequest(url, QQuickWebEngineView::LoadSucceededStatus);
-    Q_EMIT loadingChanged(&loadRequest);
-    return;
+    QTimer::singleShot(0, this, [this, url]() {
+       QQuickWebEngineLoadRequest loadRequest(url, QQuickWebEngineView::LoadSucceededStatus);
+       emit loadingChanged(&loadRequest);
+    });
 }
 
 void QQuickWebEngineErrorPage::loadStarted(const QUrl &provisionalUrl)
 {
-    QQuickWebEngineLoadRequest loadRequest(provisionalUrl, QQuickWebEngineView::LoadStartedStatus);
-    Q_EMIT loadingChanged(&loadRequest);
+    QTimer::singleShot(0, this, [this, provisionalUrl]() {
+        QQuickWebEngineLoadRequest loadRequest(provisionalUrl, QQuickWebEngineView::LoadStartedStatus);
+        emit loadingChanged(&loadRequest);
+    });
 }
 
-
 QQuickWebEngineTestInputContext::QQuickWebEngineTestInputContext()
     : m_visible(false)
 {
diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp
index b4270a876..cb21f4b15 100644
--- a/src/webengine/api/qquickwebengineview.cpp
+++ b/src/webengine/api/qquickwebengineview.cpp
@@ -434,14 +434,14 @@ void QQuickWebEngineViewPrivate::iconChanged(const QUrl &url)
 
     iconUrl = faviconProvider->attach(q, url);
     m_history->reset();
-    Q_EMIT q->iconChanged();
+    QTimer::singleShot(0, q, &QQuickWebEngineView::iconChanged);
 }
 
 void QQuickWebEngineViewPrivate::loadProgressChanged(int progress)
 {
     Q_Q(QQuickWebEngineView);
     loadProgress = progress;
-    Q_EMIT q->loadProgressChanged();
+    QTimer::singleShot(0, q, &QQuickWebEngineView::loadProgressChanged);
 }
 
 void QQuickWebEngineViewPrivate::didUpdateTargetURL(const QUrl &hoveredUrl)
@@ -486,8 +486,11 @@ void QQuickWebEngineViewPrivate::loadStarted(const QUrl &provisionalUrl, bool is
     isLoading = true;
     m_history->reset();
     m_certificateErrorControllers.clear();
-    QQuickWebEngineLoadRequest loadRequest(provisionalUrl, QQuickWebEngineView::LoadStartedStatus);
-    Q_EMIT q->loadingChanged(&loadRequest);
+
+    QTimer::singleShot(0, q, [q, provisionalUrl]() {
+        QQuickWebEngineLoadRequest loadRequest(provisionalUrl, QQuickWebEngineView::LoadStartedStatus);
+        emit q->loadingChanged(&loadRequest);
+    });
 }
 
 void QQuickWebEngineViewPrivate::loadCommitted()
@@ -522,25 +525,27 @@ void QQuickWebEngineViewPrivate::loadFinished(bool success, const QUrl &url, boo
     isLoading = false;
     m_history->reset();
     if (errorCode == WebEngineError::UserAbortedError) {
-        QQuickWebEngineLoadRequest loadRequest(url, QQuickWebEngineView::LoadStoppedStatus);
-        Q_EMIT q->loadingChanged(&loadRequest);
+        QTimer::singleShot(0, q, [q, url]() {
+            QQuickWebEngineLoadRequest loadRequest(url, QQuickWebEngineView::LoadStoppedStatus);
+            emit q->loadingChanged(&loadRequest);
+        });
         return;
     }
     if (success) {
         explicitUrl = QUrl();
-        QQuickWebEngineLoadRequest loadRequest(url, QQuickWebEngineView::LoadSucceededStatus);
-        Q_EMIT q->loadingChanged(&loadRequest);
+        QTimer::singleShot(0, q, [q, url]() {
+            QQuickWebEngineLoadRequest loadRequest(url, QQuickWebEngineView::LoadSucceededStatus);
+            emit q->loadingChanged(&loadRequest);
+        });
         return;
     }
 
     Q_ASSERT(errorCode);
-    QQuickWebEngineLoadRequest loadRequest(
-        url,
-        QQuickWebEngineView::LoadFailedStatus,
-        errorDescription,
-        errorCode,
-        static_cast<QQuickWebEngineView::ErrorDomain>(WebEngineError::toQtErrorDomain(errorCode)));
-    Q_EMIT q->loadingChanged(&loadRequest);
+    QQuickWebEngineView::ErrorDomain errorDomain = static_cast<QQuickWebEngineView::ErrorDomain>(WebEngineError::toQtErrorDomain(errorCode));
+    QTimer::singleShot(0, q, [q, url, errorDescription, errorCode, errorDomain]() {
+        QQuickWebEngineLoadRequest loadRequest(url, QQuickWebEngineView::LoadFailedStatus,errorDescription, errorCode, errorDomain);
+        emit q->loadingChanged(&loadRequest);
+    });
     return;
 }
 
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 3f585cf78..ac8e9c667 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -328,7 +328,7 @@ void QWebEnginePagePrivate::loadStarted(const QUrl &provisionalUrl, bool isError
         return;
 
     isLoading = true;
-    Q_EMIT q->loadStarted();
+    QTimer::singleShot(0, q, &QWebEnginePage::loadStarted);
     updateNavigationActions();
 }
 
@@ -346,7 +346,9 @@ void QWebEnginePagePrivate::loadFinished(bool success, const QUrl &url, bool isE
 
     if (isErrorPage) {
         Q_ASSERT(settings->testAttribute(QWebEngineSettings::ErrorPageEnabled));
-        Q_EMIT q->loadFinished(false);
+        QTimer::singleShot(0, q, [q](){
+            emit q->loadFinished(false);
+        });
         return;
     }
 
@@ -356,7 +358,9 @@ void QWebEnginePagePrivate::loadFinished(bool success, const QUrl &url, bool isE
     // Delay notifying failure until the error-page is done loading.
     // Error-pages are not loaded on failures due to abort.
     if (success || errorCode == -3 /* ERR_ABORTED*/ || !settings->testAttribute(QWebEngineSettings::ErrorPageEnabled)) {
-        Q_EMIT q->loadFinished(success);
+        QTimer::singleShot(0, q, [q, success](){
+            emit q->loadFinished(success);
+        });
     }
     updateNavigationActions();
 }
-- 
GitLab