From 1e11575a32685f7ac01f8b0cd425fa8d3ce3de33 Mon Sep 17 00:00:00 2001
From: Allan Sandfeld Jensen <allan.jensen@qt.io>
Date: Wed, 22 Aug 2018 16:12:38 +0200
Subject: [PATCH] Avoid reattach of page before deleting it
Also fixes potential double delete if you do delete a view's implied
page.
Change-Id: Ib74128c0801f992694f4a5d8c148974039a6c7b2
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
Reviewed-by: Zakor Tamas <ztamas@inf.u-szeged.hu>
---
src/webenginewidgets/api/qwebenginepage.cpp | 2 +-
src/webenginewidgets/api/qwebengineview.cpp | 15 ++++++++-------
src/webenginewidgets/api/qwebengineview_p.h | 2 +-
.../widgets/qwebengineview/tst_qwebengineview.cpp | 15 +++++++++++++++
4 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 96468f173..a86681191 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -902,7 +902,7 @@ QWebEnginePage::~QWebEnginePage()
Q_D(QWebEnginePage);
setDevToolsPage(nullptr);
d->adapter->stopFinding();
- QWebEngineViewPrivate::bind(d->view, 0);
+ QWebEngineViewPrivate::bind(nullptr, this, true);
}
QWebEngineHistory *QWebEnginePage::history() const
diff --git a/src/webenginewidgets/api/qwebengineview.cpp b/src/webenginewidgets/api/qwebengineview.cpp
index a207af392..aa51e5b0e 100644
--- a/src/webenginewidgets/api/qwebengineview.cpp
+++ b/src/webenginewidgets/api/qwebengineview.cpp
@@ -55,7 +55,7 @@
QT_BEGIN_NAMESPACE
-void QWebEngineViewPrivate::bind(QWebEngineView *view, QWebEnginePage *page)
+void QWebEngineViewPrivate::bind(QWebEngineView *view, QWebEnginePage *page, bool pageBeingDeleted)
{
if (view && page == view->d_func()->page)
return;
@@ -64,20 +64,22 @@ void QWebEngineViewPrivate::bind(QWebEngineView *view, QWebEnginePage *page)
// Un-bind page from its current view.
if (QWebEngineView *oldView = page->d_func()->view) {
page->disconnect(oldView);
- oldView->d_func()->page = 0;
+ oldView->d_func()->page = nullptr;
}
page->d_func()->view = view;
- page->d_func()->adapter->reattachRWHV();
+ if (!pageBeingDeleted)
+ page->d_func()->adapter->reattachRWHV();
}
if (view) {
// Un-bind view from its current page.
if (QWebEnginePage *oldPage = view->d_func()->page) {
oldPage->disconnect(view);
- oldPage->d_func()->view = 0;
- oldPage->d_func()->adapter->reattachRWHV();
+ oldPage->d_func()->view = nullptr;
if (oldPage->parent() == view)
delete oldPage;
+ else
+ oldPage->d_func()->adapter->reattachRWHV();
}
view->d_func()->page = page;
}
@@ -147,8 +149,7 @@ QWebEngineView::QWebEngineView(QWidget *parent)
QWebEngineView::~QWebEngineView()
{
- Q_D(QWebEngineView);
- QWebEngineViewPrivate::bind(0, d->page);
+ QWebEngineViewPrivate::bind(this, nullptr);
}
QWebEnginePage* QWebEngineView::page() const
diff --git a/src/webenginewidgets/api/qwebengineview_p.h b/src/webenginewidgets/api/qwebengineview_p.h
index 7f0cdac45..bfb44bec5 100644
--- a/src/webenginewidgets/api/qwebengineview_p.h
+++ b/src/webenginewidgets/api/qwebengineview_p.h
@@ -65,7 +65,7 @@ public:
Q_DECLARE_PUBLIC(QWebEngineView)
QWebEngineView *q_ptr;
- static void bind(QWebEngineView *view, QWebEnginePage *page);
+ static void bind(QWebEngineView *view, QWebEnginePage *page, bool pageBeingDeleted = false);
QWebEngineViewPrivate();
diff --git a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
index 5f30fa6b9..1499a5f05 100644
--- a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
+++ b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
@@ -188,6 +188,7 @@ private Q_SLOTS:
void webUIURLs();
void visibilityState();
void jsKeyboardEvent();
+ void deletePage();
};
// This will be called before the first test function is executed.
@@ -2793,5 +2794,19 @@ void tst_QWebEngineView::jsKeyboardEvent()
QCOMPARE(evaluateJavaScriptSync(view.page(), "log"), expected);
}
+void tst_QWebEngineView::deletePage()
+{
+ QWebEngineView view;
+ QWebEnginePage *page = view.page();
+ QVERIFY(page);
+ QCOMPARE(page->parent(), &view);
+ delete page;
+ // Test that a new page is created and that it is useful:
+ QVERIFY(view.page());
+ QSignalSpy spy(view.page(), &QWebEnginePage::loadFinished);
+ view.page()->load(QStringLiteral("about:blank"));
+ QTRY_VERIFY(spy.count());
+}
+
QTEST_MAIN(tst_QWebEngineView)
#include "tst_qwebengineview.moc"
--
GitLab