Move page's view and widget bindings to view

Change-Id: Ia60a1b864bb173bfd12a94ac4791477a88ea0842
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

7 files changed, 102 insertions, 97 deletions

diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index c18da80b6..7c3edcee1 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -651,7 +651,8 @@ const QObject *QWebEnginePagePrivate::holdingQObject() const
 void QWebEnginePagePrivate::widgetChanged(RenderWidgetHostViewQtDelegate *newWidgetBase)
 {
     Q_Q(QWebEnginePage);
-    bindPageAndWidget(q, static_cast<RenderWidgetHostViewQtDelegateWidget *>(newWidgetBase));
+    QWebEngineViewPrivate::bindPageAndWidget(
+            q, static_cast<RenderWidgetHostViewQtDelegateWidget *>(newWidgetBase));
 }
 
 void QWebEnginePagePrivate::findTextFinished(const QWebEngineFindTextResult &result)
@@ -666,90 +667,6 @@ void QWebEnginePagePrivate::ensureInitialized() const
         adapter->loadDefault();
 }
 
-void QWebEnginePagePrivate::bindPageAndView(QWebEnginePage *page, QWebEngineView *view)
-{
-    auto oldView = page ? page->d_func()->view : nullptr;
-    auto oldPage = view ? view->d_func()->page : nullptr;
-
-    bool ownNewPage = false;
-    bool deleteOldPage = false;
-
-    // Change pointers first.
-
-    if (page && oldView != view) {
-        if (oldView) {
-            ownNewPage = oldView->d_func()->m_ownsPage;
-            oldView->d_func()->page = nullptr;
-            oldView->d_func()->m_ownsPage = false;
-        }
-        page->d_func()->view = view;
-    }
-
-    if (view && oldPage != page) {
-        if (oldPage) {
-            if (oldPage->d_func())
-                oldPage->d_func()->view = nullptr;
-            deleteOldPage = view->d_func()->m_ownsPage;
-        }
-        view->d_func()->m_ownsPage = ownNewPage;
-        view->d_func()->page = page;
-    }
-
-    // Then notify.
-
-    auto widget = page ? page->d_func()->widget : nullptr;
-    auto oldWidget = (oldPage && oldPage->d_func()) ? oldPage->d_func()->widget : nullptr;
-
-    if (page && oldView != view && oldView) {
-        oldView->d_func()->pageChanged(page, nullptr);
-        if (widget)
-            oldView->d_func()->widgetChanged(widget, nullptr);
-    }
-
-    if (view && oldPage != page) {
-        if (oldPage && oldPage->d_func())
-            view->d_func()->pageChanged(oldPage, page);
-        else
-            view->d_func()->pageChanged(nullptr, page);
-        if (oldWidget != widget)
-            view->d_func()->widgetChanged(oldWidget, widget);
-    }
-    if (deleteOldPage)
-        delete oldPage;
-}
-
-void QWebEnginePagePrivate::bindPageAndWidget(QWebEnginePage *page, RenderWidgetHostViewQtDelegateWidget *widget)
-{
-    auto oldPage = widget ? widget->m_page : nullptr;
-    auto oldWidget = page ? page->d_func()->widget : nullptr;
-
-    // Change pointers first.
-
-    if (widget && oldPage != page) {
-        if (oldPage && oldPage->d_func())
-            oldPage->d_func()->widget = nullptr;
-        widget->m_page = page;
-    }
-
-    if (page && oldWidget != widget) {
-        if (oldWidget)
-            oldWidget->m_page = nullptr;
-        page->d_func()->widget = widget;
-    }
-
-    // Then notify.
-
-    if (widget && oldPage != page && oldPage && oldPage->d_func()) {
-        if (auto oldView = oldPage->d_func()->view)
-            oldView->d_func()->widgetChanged(widget, nullptr);
-    }
-
-    if (page && oldWidget != widget) {
-        if (auto view = page->d_func()->view)
-            view->d_func()->widgetChanged(oldWidget, widget);
-    }
-}
-
 QWebEnginePage::QWebEnginePage(QObject* parent)
     : QObject(parent)
     , d_ptr(new QWebEnginePagePrivate())
@@ -938,8 +855,8 @@ QWebEnginePage::~QWebEnginePage()
     if (d_ptr) {
         // d_ptr might be exceptionally null if profile adapter got deleted first
         setDevToolsPage(nullptr);
-        QWebEnginePagePrivate::bindPageAndView(this, nullptr);
-        QWebEnginePagePrivate::bindPageAndWidget(this, nullptr);
+        emit _q_aboutToDelete();
+        QWebEngineViewPrivate::bindPageAndWidget(this, nullptr);
     }
 }
 
diff --git a/src/webenginewidgets/api/qwebenginepage.h b/src/webenginewidgets/api/qwebenginepage.h
index b67caf806..250b809bd 100644
--- a/src/webenginewidgets/api/qwebenginepage.h
+++ b/src/webenginewidgets/api/qwebenginepage.h
@@ -364,6 +364,9 @@ Q_SIGNALS:
 
     void findTextFinished(const QWebEngineFindTextResult &result);
 
+    // TODO: fixme / rewrite bindPageToView
+    void _q_aboutToDelete();
+
 protected:
     virtual QWebEnginePage *createWindow(WebWindowType type);
     virtual QStringList chooseFiles(FileSelectionMode mode, const QStringList &oldFiles, const QStringList &acceptedMimeTypes);
@@ -373,7 +376,6 @@ protected:
     virtual void javaScriptConsoleMessage(JavaScriptConsoleMessageLevel level, const QString& message, int lineNumber, const QString& sourceID);
     virtual void certificateError(QWebEngineCertificateError certificateError);
     virtual bool acceptNavigationRequest(const QUrl &url, NavigationType type, bool isMainFrame);
-
 private:
     Q_DISABLE_COPY(QWebEnginePage)
     Q_DECLARE_PRIVATE(QWebEnginePage)
diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h
index 0ecb10b73..342461c7a 100644
--- a/src/webenginewidgets/api/qwebenginepage_p.h
+++ b/src/webenginewidgets/api/qwebenginepage_p.h
@@ -171,10 +171,6 @@ public:
     void setFullScreenMode(bool);
     void ensureInitialized() const;
 
-    static void bindPageAndView(QWebEnginePage *page, QWebEngineView *view);
-    static void bindPageAndWidget(QWebEnginePage *page,
-                                  QtWebEngineCore::RenderWidgetHostViewQtDelegateWidget *widget);
-
     QSharedPointer<QtWebEngineCore::WebContentsAdapter> adapter;
     QWebEngineHistory *history;
     QWebEngineProfile *profile;
diff --git a/src/webenginewidgets/api/qwebengineview.cpp b/src/webenginewidgets/api/qwebengineview.cpp
index b646cee49..f0f8b586e 100644
--- a/src/webenginewidgets/api/qwebengineview.cpp
+++ b/src/webenginewidgets/api/qwebengineview.cpp
@@ -320,6 +320,91 @@ QWebEngineViewPrivate::QWebEngineViewPrivate()
 #endif // QT_NO_ACCESSIBILITY
 }
 
+void QWebEngineViewPrivate::bindPageAndView(QWebEnginePage *page, QWebEngineView *view)
+{
+    auto oldView = page ? page->d_func()->view : nullptr;
+    auto oldPage = view ? view->d_func()->page : nullptr;
+
+    bool ownNewPage = false;
+    bool deleteOldPage = false;
+
+    // Change pointers first.
+
+    if (page && oldView != view) {
+        if (oldView) {
+            ownNewPage = oldView->d_func()->m_ownsPage;
+            oldView->d_func()->page = nullptr;
+            oldView->d_func()->m_ownsPage = false;
+        }
+        page->d_func()->view = view;
+    }
+
+    if (view && oldPage != page) {
+        if (oldPage) {
+            if (oldPage->d_func())
+                oldPage->d_func()->view = nullptr;
+            deleteOldPage = view->d_func()->m_ownsPage;
+        }
+        view->d_func()->m_ownsPage = ownNewPage;
+        view->d_func()->page = page;
+    }
+
+    // Then notify.
+
+    auto widget = page ? page->d_func()->widget : nullptr;
+    auto oldWidget = (oldPage && oldPage->d_func()) ? oldPage->d_func()->widget : nullptr;
+
+    if (page && oldView != view && oldView) {
+        oldView->d_func()->pageChanged(page, nullptr);
+        if (widget)
+            oldView->d_func()->widgetChanged(widget, nullptr);
+    }
+
+    if (view && oldPage != page) {
+        if (oldPage && oldPage->d_func())
+            view->d_func()->pageChanged(oldPage, page);
+        else
+            view->d_func()->pageChanged(nullptr, page);
+        if (oldWidget != widget)
+            view->d_func()->widgetChanged(oldWidget, widget);
+    }
+    if (deleteOldPage)
+        delete oldPage;
+}
+
+void QWebEngineViewPrivate::bindPageAndWidget(
+        QWebEnginePage *page, QtWebEngineCore::RenderWidgetHostViewQtDelegateWidget *widget)
+{
+    auto oldPage = widget ? widget->m_page : nullptr;
+    auto oldWidget = page ? page->d_func()->widget : nullptr;
+
+    // Change pointers first.
+
+    if (widget && oldPage != page) {
+        if (oldPage && oldPage->d_func())
+            oldPage->d_func()->widget = nullptr;
+        widget->m_page = page;
+    }
+
+    if (page && oldWidget != widget) {
+        if (oldWidget)
+            oldWidget->m_page = nullptr;
+        page->d_func()->widget = widget;
+    }
+
+    // Then notify.
+
+    if (widget && oldPage != page && oldPage && oldPage->d_func()) {
+        if (auto oldView = oldPage->d_func()->view)
+            oldView->d_func()->widgetChanged(widget, nullptr);
+    }
+
+    if (page && oldWidget != widget) {
+        if (auto view = page->d_func()->view)
+            view->d_func()->widgetChanged(oldWidget, widget);
+    }
+}
+
 /*!
     \fn QWebEngineView::renderProcessTerminated(QWebEnginePage::RenderProcessTerminationStatus terminationStatus, int exitCode)
     \since 5.6
@@ -355,7 +440,7 @@ QWebEngineView::QWebEngineView(QWidget *parent)
 QWebEngineView::~QWebEngineView()
 {
     blockSignals(true);
-    QWebEnginePagePrivate::bindPageAndView(nullptr, this);
+    QWebEngineViewPrivate::bindPageAndView(nullptr, this);
 }
 
 QWebEnginePage* QWebEngineView::page() const
@@ -371,7 +456,9 @@ QWebEnginePage* QWebEngineView::page() const
 
 void QWebEngineView::setPage(QWebEnginePage *newPage)
 {
-    QWebEnginePagePrivate::bindPageAndView(newPage, this);
+    QWebEngineViewPrivate::bindPageAndView(newPage, this);
+    connect(newPage, &QWebEnginePage::_q_aboutToDelete, this,
+            [newPage]() { QWebEngineViewPrivate::bindPageAndView(newPage, nullptr); });
 }
 
 void QWebEngineView::load(const QUrl& url)
diff --git a/src/webenginewidgets/api/qwebengineview_p.h b/src/webenginewidgets/api/qwebengineview_p.h
index 2efb97e13..ddebebdf9 100644
--- a/src/webenginewidgets/api/qwebengineview_p.h
+++ b/src/webenginewidgets/api/qwebengineview_p.h
@@ -85,6 +85,9 @@ public:
                           QString *result);
     void setToolTip(const QString &toolTipText);
     QWebEngineViewPrivate();
+    static void bindPageAndView(QWebEnginePage *page, QWebEngineView *view);
+    static void bindPageAndWidget(QWebEnginePage *page,
+                                  QtWebEngineCore::RenderWidgetHostViewQtDelegateWidget *widget);
 
     QWebEnginePage *page;
     bool m_dragEntered;
diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
index db8a236e2..319883d2c 100644
--- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
+++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp
@@ -180,7 +180,7 @@ RenderWidgetHostViewQtDelegateWidget::RenderWidgetHostViewQtDelegateWidget(Rende
 
 RenderWidgetHostViewQtDelegateWidget::~RenderWidgetHostViewQtDelegateWidget()
 {
-    QWebEnginePagePrivate::bindPageAndWidget(nullptr, this);
+    QWebEngineViewPrivate::bindPageAndWidget(nullptr, this);
 }
 
 void RenderWidgetHostViewQtDelegateWidget::connectRemoveParentBeforeParentDelete()
diff --git a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h
index 0dd5f1cc3..ce1110014 100644
--- a/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h
+++ b/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.h
@@ -51,7 +51,7 @@ QT_BEGIN_NAMESPACE
 class QWebEnginePage;
 class QWebEngineView;
 class QWebEngineViewAccessible;
-class QWebEnginePagePrivate;
+class QWebEngineViewPrivate;
 QT_END_NAMESPACE
 
 namespace QtWebEngineCore {
@@ -102,7 +102,7 @@ private slots:
     void removeParentBeforeParentDelete();
 
 private:
-    friend QWebEnginePagePrivate;
+    friend QWebEngineViewPrivate;
 
     RenderWidgetHostViewQtDelegateClient *m_client;
     QScopedPointer<QQuickItem> m_rootItem;