From 96fb3a30457c8925431b67c0f35abb2833b7fb7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BCri=20Valdmann?= <juri.valdmann@qt.io>
Date: Fri, 12 Apr 2019 11:33:16 +0200
Subject: [PATCH] Fix QWebEngineView::setPage not deleting old page
Also fix QWebEnginePage::setView not deleting old page
Also fix wrong page being deleted if it's parented to the view.
Fixes: QTBUG-75131
Fixes: QTBUG-75175
Change-Id: Ie4dfb15b3182de7aa3a94cddcac54ea40a86121b
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
src/webenginewidgets/api/qwebenginepage.cpp | 7 +++
src/webenginewidgets/api/qwebengineview.cpp | 2 +
src/webenginewidgets/api/qwebengineview_p.h | 1 +
.../qwebengineview/tst_qwebengineview.cpp | 46 +++++++++++++++++++
4 files changed, 56 insertions(+)
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index dd720a41b..85a162383 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -775,6 +775,13 @@ void QWebEnginePagePrivate::bindPageAndView(QWebEnginePage *page, QWebEngineView
view->d_func()->pageChanged(oldPage, page);
if (oldWidget != widget)
view->d_func()->widgetChanged(oldWidget, widget);
+
+ // At this point m_ownsPage should still refer to oldPage,
+ // it is only set for the new page after binding.
+ if (view->d_func()->m_ownsPage) {
+ delete oldPage;
+ view->d_func()->m_ownsPage = false;
+ }
}
}
diff --git a/src/webenginewidgets/api/qwebengineview.cpp b/src/webenginewidgets/api/qwebengineview.cpp
index 966b30100..381f1385c 100644
--- a/src/webenginewidgets/api/qwebengineview.cpp
+++ b/src/webenginewidgets/api/qwebengineview.cpp
@@ -126,6 +126,7 @@ static QAccessibleInterface *webAccessibleFactory(const QString &, QObject *obje
QWebEngineViewPrivate::QWebEngineViewPrivate()
: page(0)
, m_dragEntered(false)
+ , m_ownsPage(false)
{
#ifndef QT_NO_ACCESSIBILITY
QAccessible::installFactory(&webAccessibleFactory);
@@ -176,6 +177,7 @@ QWebEnginePage* QWebEngineView::page() const
if (!d->page) {
QWebEngineView *that = const_cast<QWebEngineView*>(this);
that->setPage(new QWebEnginePage(that));
+ d->m_ownsPage = true;
}
return d->page;
}
diff --git a/src/webenginewidgets/api/qwebengineview_p.h b/src/webenginewidgets/api/qwebengineview_p.h
index 28fb883aa..7848e0cf3 100644
--- a/src/webenginewidgets/api/qwebengineview_p.h
+++ b/src/webenginewidgets/api/qwebengineview_p.h
@@ -77,6 +77,7 @@ public:
QWebEnginePage *page;
bool m_dragEntered;
+ mutable bool m_ownsPage;
};
#ifndef QT_NO_ACCESSIBILITY
diff --git a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
index 5f05f70ab..6ab63b54e 100644
--- a/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
+++ b/tests/auto/widgets/qwebengineview/tst_qwebengineview.cpp
@@ -195,6 +195,10 @@ private Q_SLOTS:
void deletePage();
void closeOpenerTab();
void switchPage();
+ void setPageDeletesImplicitPage();
+ void setViewDeletesImplicitPage();
+ void setPagePreservesExplicitPage();
+ void setViewPreservesExplicitPage();
};
// This will be called before the first test function is executed.
@@ -3196,5 +3200,47 @@ void tst_QWebEngineView::switchPage()
QTRY_COMPARE(webView.grab().toImage().pixelColor(QPoint(150,150)), Qt::black);
}
+void tst_QWebEngineView::setPageDeletesImplicitPage()
+{
+ QWebEngineView view;
+ QPointer<QWebEnginePage> implicitPage = view.page();
+ QWebEnginePage explicitPage;
+ view.setPage(&explicitPage);
+ QCOMPARE(view.page(), &explicitPage);
+ QVERIFY(!implicitPage); // should be deleted
+}
+
+void tst_QWebEngineView::setViewDeletesImplicitPage()
+{
+ QWebEngineView view;
+ QPointer<QWebEnginePage> implicitPage = view.page();
+ QWebEnginePage explicitPage;
+ explicitPage.setView(&view);
+ QCOMPARE(view.page(), &explicitPage);
+ QVERIFY(!implicitPage); // should be deleted
+}
+
+void tst_QWebEngineView::setPagePreservesExplicitPage()
+{
+ QWebEngineView view;
+ QPointer<QWebEnginePage> explicitPage1 = new QWebEnginePage(&view);
+ QPointer<QWebEnginePage> explicitPage2 = new QWebEnginePage(&view);
+ view.setPage(explicitPage1.data());
+ view.setPage(explicitPage2.data());
+ QCOMPARE(view.page(), explicitPage2.data());
+ QVERIFY(explicitPage1); // should not be deleted
+}
+
+void tst_QWebEngineView::setViewPreservesExplicitPage()
+{
+ QWebEngineView view;
+ QPointer<QWebEnginePage> explicitPage1 = new QWebEnginePage(&view);
+ QPointer<QWebEnginePage> explicitPage2 = new QWebEnginePage(&view);
+ explicitPage1->setView(&view);
+ explicitPage2->setView(&view);
+ QCOMPARE(view.page(), explicitPage2.data());
+ QVERIFY(explicitPage1); // should not be deleted
+}
+
QTEST_MAIN(tst_QWebEngineView)
#include "tst_qwebengineview.moc"
--
GitLab