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>

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"