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>

4 files changed, 56 insertions, 0 deletions

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"