From b63216e9f06bf596deda96cb26c5bee92b7b09f8 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Tue, 21 Jun 2022 12:42:28 +0200 Subject: Cleanup item delegate logic Remove two indirect values we can calculate without tracking, and clean up naming in webenginequick. Change-Id: Ibfab7013f314b428dca707036fe5f027558dff72 Reviewed-by: Peter Varga (cherry picked from commit 4495d477e0093933d711978da08b5a90afef08ff) Reviewed-by: Qt Cherry-pick Bot --- .../render_widget_host_view_qt_delegate_item.h | 4 -- src/webenginequick/api/qquickwebengineview.cpp | 74 ++++++++++------------ src/webenginequick/api/qquickwebengineview_p_p.h | 8 +-- ...er_widget_host_view_qt_delegate_quickwindow.cpp | 6 +- src/webenginewidgets/api/qwebengineview.cpp | 44 ++++++------- src/webenginewidgets/api/qwebengineview_p.h | 2 +- 6 files changed, 63 insertions(+), 75 deletions(-) diff --git a/src/core/render_widget_host_view_qt_delegate_item.h b/src/core/render_widget_host_view_qt_delegate_item.h index 515f2b28c..431ecd064 100644 --- a/src/core/render_widget_host_view_qt_delegate_item.h +++ b/src/core/render_widget_host_view_qt_delegate_item.h @@ -46,9 +46,7 @@ #include QT_BEGIN_NAMESPACE -class QQuickWebEngineView; class QQuickWebEngineViewPrivate; -class QWebEnginePage; class QWebEngineViewPrivate; QT_END_NAMESPACE @@ -151,8 +149,6 @@ private: Qt::InputMethodHints m_inputMethodHints = {}; QList m_windowConnections; WebContentsAdapterClient *m_adapterClient = nullptr; - QWebEnginePage *m_page = nullptr; - QQuickWebEngineView *m_view = nullptr; WidgetDelegate *m_widgetDelegate = nullptr; }; diff --git a/src/webenginequick/api/qquickwebengineview.cpp b/src/webenginequick/api/qquickwebengineview.cpp index 015f916b3..5c411fd1e 100644 --- a/src/webenginequick/api/qquickwebengineview.cpp +++ b/src/webenginequick/api/qquickwebengineview.cpp @@ -164,13 +164,13 @@ public: void Bind(WebContentsAdapterClient *client) override { - QQuickWebEngineViewPrivate::bindViewAndWidget( - static_cast(client)->q_func(), m_contentItem); + QQuickWebEngineViewPrivate::bindViewAndDelegateItem( + static_cast(client), m_contentItem); } void Unbind() override { - QQuickWebEngineViewPrivate::bindViewAndWidget(nullptr, m_contentItem); + QQuickWebEngineViewPrivate::bindViewAndDelegateItem(nullptr, m_contentItem); } void Destroy() override @@ -234,9 +234,7 @@ QQuickWebEngineViewPrivate::~QQuickWebEngineViewPrivate() m_profile->d_ptr->removeWebContentsAdapterClient(this); if (m_faviconProvider) m_faviconProvider->detach(q_ptr); - // q_ptr->d_ptr might be null due to destroy() - if (q_ptr->d_ptr) - bindViewAndWidget(q_ptr, nullptr); + bindViewAndDelegateItem(this, nullptr); } void QQuickWebEngineViewPrivate::initializeProfile() @@ -269,7 +267,7 @@ void QQuickWebEngineViewPrivate::releaseProfile() // The profile for this web contents is about to be // garbage collected, delete WebContents first and // let the QQuickWebEngineView be collected later by gc. - bindViewAndWidget(q_ptr, nullptr); + bindViewAndDelegateItem(this, nullptr); q_ptr->d_ptr.reset(); } @@ -909,59 +907,57 @@ void QQuickWebEngineViewPrivate::setFullScreenMode(bool fullscreen) } } -void QQuickWebEngineViewPrivate::bindViewAndWidget(QQuickWebEngineView *view, - RenderWidgetHostViewQtDelegateItem *widget) +// static +void QQuickWebEngineViewPrivate::bindViewAndDelegateItem(QQuickWebEngineViewPrivate *viewPrivate, + RenderWidgetHostViewQtDelegateItem *delegateItem) { - auto oldWidget = view ? view->d_func()->widget : nullptr; - auto oldView = widget ? widget->m_view : nullptr; + auto oldDelegateItem = viewPrivate ? viewPrivate->delegateItem : nullptr; + auto oldAdapterClient = delegateItem ? delegateItem->m_adapterClient : nullptr; + + auto *oldViewPrivate = static_cast(oldAdapterClient); // Change pointers first. - if (widget && oldView != view) { - if (oldView) - oldView->d_func()->widget = nullptr; - widget->m_view = view; - } + if (oldViewPrivate && oldViewPrivate != viewPrivate) + oldViewPrivate->delegateItem = nullptr; - if (view && oldWidget != widget) { - if (oldWidget) - oldWidget->m_view = nullptr; - view->d_func()->widget = widget; - } + if (viewPrivate && oldDelegateItem != delegateItem) + viewPrivate->delegateItem = delegateItem; // Then notify. - if (widget && oldView != view && oldView) - oldView->d_func()->widgetChanged(widget, nullptr); + if (oldViewPrivate && oldViewPrivate != viewPrivate) + oldViewPrivate->delegateItemChanged(delegateItem, nullptr); - if (view && oldWidget != widget) - view->d_func()->widgetChanged(oldWidget, widget); + if (viewPrivate && oldDelegateItem != delegateItem) + viewPrivate->delegateItemChanged(oldDelegateItem, delegateItem); } -void QQuickWebEngineViewPrivate::widgetChanged(QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *oldWidget, - QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *newWidget) +void QQuickWebEngineViewPrivate::delegateItemChanged(QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *oldDelegateItem, + QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *newDelegateItem) { Q_Q(QQuickWebEngineView); - if (oldWidget) { - oldWidget->setParentItem(nullptr); + if (oldDelegateItem) { + oldDelegateItem->setParentItem(nullptr); #if QT_CONFIG(accessibility) - if (!QtWebEngineCore::closingDown()) - QAccessible::deleteAccessibleInterface( - QAccessible::uniqueId(QAccessible::queryAccessibleInterface(oldWidget))); + if (!QtWebEngineCore::closingDown()) { + if (auto iface = QAccessible::queryAccessibleInterface(oldDelegateItem)) + QAccessible::deleteAccessibleInterface(QAccessible::uniqueId(iface)); + } #endif } - if (newWidget) { + if (newDelegateItem) { Q_ASSERT(!QtWebEngineCore::closingDown()); #if QT_CONFIG(accessibility) - QAccessible::registerAccessibleInterface(new QtWebEngineCore::RenderWidgetHostViewQtDelegateQuickAccessible(newWidget, q)); + QAccessible::registerAccessibleInterface(new QtWebEngineCore::RenderWidgetHostViewQtDelegateQuickAccessible(newDelegateItem, q)); #endif - newWidget->setParentItem(q); - newWidget->setSize(q->boundingRect().size()); + newDelegateItem->setParentItem(q); + newDelegateItem->setSize(q->boundingRect().size()); // Focus on creation if the view accepts it if (q->activeFocusOnPress()) - newWidget->setFocus(true); + newDelegateItem->setFocus(true); } } @@ -1696,8 +1692,8 @@ void QQuickWebEngineView::geometryChange(const QRectF &newGeometry, const QRectF { QQuickItem::geometryChange(newGeometry, oldGeometry); Q_D(QQuickWebEngineView); - if (d->widget) - d->widget->setSize(newGeometry.size()); + if (d->delegateItem) + d->delegateItem->setSize(newGeometry.size()); } void QQuickWebEngineView::itemChange(ItemChange change, const ItemChangeData &value) diff --git a/src/webenginequick/api/qquickwebengineview_p_p.h b/src/webenginequick/api/qquickwebengineview_p_p.h index 9f066af90..f4d5c1a92 100644 --- a/src/webenginequick/api/qquickwebengineview_p_p.h +++ b/src/webenginequick/api/qquickwebengineview_p_p.h @@ -178,9 +178,9 @@ public: void ensureContentsAdapter(); void setFullScreenMode(bool); - static void bindViewAndWidget(QQuickWebEngineView *view, QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *widget); - void widgetChanged(QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *oldWidget, - QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *newWidget); + static void bindViewAndDelegateItem(QQuickWebEngineViewPrivate *viewPrivate, QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *delegateItem); + void delegateItemChanged(QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *oldDelegateItem, + QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *newDelegateItem); QQuickWebEngineProfile *m_profile; QSharedPointer adapter; @@ -204,7 +204,7 @@ public: bool m_defaultAudioMuted; bool m_isBeingAdopted; mutable QQuickWebEngineAction *actions[QQuickWebEngineView::WebActionCount]; - QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *widget = nullptr; + QtWebEngineCore::RenderWidgetHostViewQtDelegateItem *delegateItem = nullptr; bool profileInitialized() const; QQuickWebEngineScriptCollection *getUserScripts(); diff --git a/src/webenginequick/render_widget_host_view_qt_delegate_quickwindow.cpp b/src/webenginequick/render_widget_host_view_qt_delegate_quickwindow.cpp index 911d10c8a..259e3c406 100644 --- a/src/webenginequick/render_widget_host_view_qt_delegate_quickwindow.cpp +++ b/src/webenginequick/render_widget_host_view_qt_delegate_quickwindow.cpp @@ -151,13 +151,13 @@ bool RenderWidgetHostViewQtDelegateQuickWindow::ActiveFocusOnPress() void RenderWidgetHostViewQtDelegateQuickWindow::Bind(QtWebEngineCore::WebContentsAdapterClient *client) { - QQuickWebEngineViewPrivate::bindViewAndWidget( - static_cast(client)->q_func(), m_realDelegate.data()); + QQuickWebEngineViewPrivate::bindViewAndDelegateItem( + static_cast(client), m_realDelegate.data()); } void RenderWidgetHostViewQtDelegateQuickWindow::Unbind() { - QQuickWebEngineViewPrivate::bindViewAndWidget(nullptr, m_realDelegate.data()); + QQuickWebEngineViewPrivate::bindViewAndDelegateItem(nullptr, m_realDelegate.data()); } void RenderWidgetHostViewQtDelegateQuickWindow::Destroy() diff --git a/src/webenginewidgets/api/qwebengineview.cpp b/src/webenginewidgets/api/qwebengineview.cpp index 9b8e72ce1..54a1ba0b1 100644 --- a/src/webenginewidgets/api/qwebengineview.cpp +++ b/src/webenginewidgets/api/qwebengineview.cpp @@ -142,15 +142,15 @@ public: void Bind(WebContentsAdapterClient *client) override { - BindPage(static_cast(client)->q_func()); + BindPage(static_cast(client)); } - void BindPage(QWebEnginePage *page) + void BindPage(QWebEnginePagePrivate *page) { if (m_pageDestroyedConnection) QObject::disconnect(m_pageDestroyedConnection); QWebEngineViewPrivate::bindPageAndWidget(page, this); - m_pageDestroyedConnection = QObject::connect(page, &QObject::destroyed, this, &WebEngineQuickWidget::UnbindPage); + m_pageDestroyedConnection = QObject::connect(page->q_ptr, &QObject::destroyed, this, &WebEngineQuickWidget::UnbindPage); } void UnbindPage() @@ -723,7 +723,7 @@ void QWebEngineViewPrivate::bindPageAndView(QWebEnginePage *page, QWebEngineView view->d_func()->pageChanged(nullptr, page); if (!widget && page && page->d_func()->item) { widget = new QtWebEngineCore::WebEngineQuickWidget(page->d_func()->item, nullptr); - widget->BindPage(page); + widget->BindPage(page->d_func()); } if (oldWidget != widget) view->d_func()->widgetChanged(oldWidget, widget); @@ -732,39 +732,35 @@ void QWebEngineViewPrivate::bindPageAndView(QWebEnginePage *page, QWebEngineView delete oldPage; } -void QWebEngineViewPrivate::bindPageAndWidget( - QWebEnginePage *page, QtWebEngineCore::WebEngineQuickWidget *widget) +void QWebEngineViewPrivate::bindPageAndWidget(QWebEnginePagePrivate *pagePrivate, + QtWebEngineCore::WebEngineQuickWidget *widget) { - auto oldPage = (widget && widget->m_contentItem) ? widget->m_contentItem->m_page : nullptr; - auto oldWidget = page ? static_cast(page->d_func()->widget) : nullptr; + auto oldAdapterClient = (widget && widget->m_contentItem) ? widget->m_contentItem->m_adapterClient : nullptr; + auto oldWidget = pagePrivate ? static_cast(pagePrivate->widget) : nullptr; + + auto *oldPagePrivate = static_cast(oldAdapterClient); // Change pointers first. - if (widget && oldPage != page) { - if (oldPage && oldPage->d_func()) { - oldPage->d_func()->widget = nullptr; - oldPage->d_func()->item = nullptr; - } - if (widget->m_contentItem) - widget->m_contentItem->m_page = page; + if (oldPagePrivate && oldPagePrivate != pagePrivate) { + oldPagePrivate->widget = nullptr; + oldPagePrivate->item = nullptr; } - if (page && oldWidget != widget) { - if (oldWidget && oldWidget->m_contentItem) - oldWidget->m_contentItem->m_page = nullptr; - page->d_func()->widget = widget; - page->d_func()->item = widget->m_contentItem; + if (pagePrivate && oldWidget != widget) { + pagePrivate->widget = widget; + pagePrivate->item = widget->m_contentItem; } // Then notify. - if (widget && oldPage != page && oldPage && oldPage->d_func()) { - if (auto oldView = oldPage->d_func()->view) + if (oldPagePrivate && oldPagePrivate != pagePrivate) { + if (auto oldView = oldPagePrivate->view) static_cast(oldView)->widgetChanged(widget, nullptr); } - if (page && oldWidget != widget) { - if (auto view = page->d_func()->view) + if (pagePrivate && oldWidget != widget) { + if (auto view = pagePrivate->view) static_cast(view)->widgetChanged(oldWidget, widget); } } diff --git a/src/webenginewidgets/api/qwebengineview_p.h b/src/webenginewidgets/api/qwebengineview_p.h index 44eb32203..15644926f 100644 --- a/src/webenginewidgets/api/qwebengineview_p.h +++ b/src/webenginewidgets/api/qwebengineview_p.h @@ -111,7 +111,7 @@ public: QWebEngineViewPrivate(); virtual ~QWebEngineViewPrivate(); static void bindPageAndView(QWebEnginePage *page, QWebEngineView *view); - static void bindPageAndWidget(QWebEnginePage *page, + static void bindPageAndWidget(QWebEnginePagePrivate *pagePrivate, QtWebEngineCore::WebEngineQuickWidget *widget); QIcon webActionIcon(QWebEnginePage::WebAction action); void unhandledKeyEvent(QKeyEvent *event) override; -- cgit v1.2.3