From 6c7148af789af401b39f0283f8060624432c5b56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCri=20Valdmann?= Date: Thu, 23 Apr 2020 12:55:58 +0200 Subject: Restore behavior of OpenURLFromTab if createWindow returns this Instead of using QSharedPointer's reference count to communicate adoption/non-adoption, change adoptNewWindow to return a adapter pointer, with null meaning non-adoption. Then change QWebEnginePage's implementation to reuse already existing adapters if possible, restoring previous behavior of OpenURLFromTab when createWindow returns this. Task-number: QTBUG-80596 Change-Id: I8ee7c31e4294aabd3207c504cba67d6171c66cb0 Reviewed-by: Allan Sandfeld Jensen --- src/core/web_contents_adapter_client.h | 5 ++++- src/core/web_contents_delegate_qt.cpp | 13 ++++++++----- src/core/web_contents_delegate_qt.h | 5 ++++- src/webengine/api/qquickwebengineview.cpp | 9 ++++++--- src/webengine/api/qquickwebengineview_p_p.h | 5 ++++- src/webenginewidgets/api/qwebenginepage.cpp | 12 ++++++++++-- src/webenginewidgets/api/qwebenginepage_p.h | 5 ++++- 7 files changed, 40 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h index 250801f51..04df99f0e 100644 --- a/src/core/web_contents_adapter_client.h +++ b/src/core/web_contents_adapter_client.h @@ -469,7 +469,10 @@ public: virtual void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) = 0; virtual void focusContainer() = 0; virtual void unhandledKeyEvent(QKeyEvent *event) = 0; - virtual void adoptNewWindow(QSharedPointer newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect & initialGeometry, const QUrl &targetUrl) = 0; + virtual QSharedPointer + adoptNewWindow(QSharedPointer newWebContents, + WindowOpenDisposition disposition, bool userGesture, + const QRect &initialGeometry, const QUrl &targetUrl) = 0; virtual bool isBeingAdopted() = 0; virtual void close() = 0; virtual void windowCloseRejected() = 0; diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp index 3c37286f8..216e4faf1 100644 --- a/src/core/web_contents_delegate_qt.cpp +++ b/src/core/web_contents_delegate_qt.cpp @@ -663,14 +663,17 @@ void WebContentsDelegateQt::overrideWebPreferences(content::WebContents *webCont m_viewClient->webEngineSettings()->overrideWebPreferences(webContents, webPreferences); } -QWeakPointer WebContentsDelegateQt::createWindow(std::unique_ptr new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture) +QSharedPointer +WebContentsDelegateQt::createWindow(std::unique_ptr new_contents, + WindowOpenDisposition disposition, const gfx::Rect &initial_pos, + bool user_gesture) { QSharedPointer newAdapter = QSharedPointer::create(std::move(new_contents)); - m_viewClient->adoptNewWindow(newAdapter, static_cast(disposition), user_gesture, toQt(initial_pos), m_initialTargetUrl); - - // If the client didn't reference the adapter, it will be deleted now, and the weak pointer zeroed. - return newAdapter; + return m_viewClient->adoptNewWindow( + std::move(newAdapter), + static_cast(disposition), user_gesture, + toQt(initial_pos), m_initialTargetUrl); } void WebContentsDelegateQt::allowCertificateError(const QSharedPointer &errorController) diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h index a8c9561ea..bfef9a1df 100644 --- a/src/core/web_contents_delegate_qt.h +++ b/src/core/web_contents_delegate_qt.h @@ -201,7 +201,10 @@ public: base::WeakPtr AsWeakPtr() { return m_weakPtrFactory.GetWeakPtr(); } private: - QWeakPointer createWindow(std::unique_ptr new_contents, WindowOpenDisposition disposition, const gfx::Rect& initial_pos, bool user_gesture); + QSharedPointer + createWindow(std::unique_ptr new_contents, + WindowOpenDisposition disposition, const gfx::Rect &initial_pos, + bool user_gesture); void EmitLoadStarted(const QUrl &url, bool isErrorPage = false); void EmitLoadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()); void EmitLoadCommitted(); diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp index 7052afe42..4096eb7f6 100644 --- a/src/webengine/api/qquickwebengineview.cpp +++ b/src/webengine/api/qquickwebengineview.cpp @@ -547,12 +547,13 @@ void QQuickWebEngineViewPrivate::unhandledKeyEvent(QKeyEvent *event) QCoreApplication::sendEvent(q->parentItem(), event); } -void QQuickWebEngineViewPrivate::adoptNewWindow(QSharedPointer newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &, const QUrl &targetUrl) +QSharedPointer +QQuickWebEngineViewPrivate::adoptNewWindow(QSharedPointer newWebContents, + WindowOpenDisposition disposition, bool userGesture, + const QRect &, const QUrl &targetUrl) { Q_Q(QQuickWebEngineView); QQuickWebEngineNewViewRequest request; - // This increases the ref-count of newWebContents and will tell Chromium - // to start loading it and possibly return it to its parent page window.open(). request.m_adapter = newWebContents; request.m_isUserInitiated = userGesture; request.m_requestedUrl = targetUrl; @@ -575,6 +576,8 @@ void QQuickWebEngineViewPrivate::adoptNewWindow(QSharedPointernewViewRequested(&request); + + return newWebContents; } bool QQuickWebEngineViewPrivate::isBeingAdopted() diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h index 5c884e36e..e696f6a0c 100644 --- a/src/webengine/api/qquickwebengineview_p_p.h +++ b/src/webengine/api/qquickwebengineview_p_p.h @@ -121,7 +121,10 @@ public: void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) override; void focusContainer() override; void unhandledKeyEvent(QKeyEvent *event) override; - void adoptNewWindow(QSharedPointer newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &, const QUrl &targetUrl) override; + QSharedPointer + adoptNewWindow(QSharedPointer newWebContents, + WindowOpenDisposition disposition, bool userGesture, const QRect &, + const QUrl &targetUrl) override; bool isBeingAdopted() override; void close() override; void windowCloseRejected() override; diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index 6f0009183..8e6f402ff 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -348,7 +348,10 @@ void QWebEnginePagePrivate::unhandledKeyEvent(QKeyEvent *event) QGuiApplication::sendEvent(view->parentWidget(), event); } -void QWebEnginePagePrivate::adoptNewWindow(QSharedPointer newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry, const QUrl &targetUrl) +QSharedPointer +QWebEnginePagePrivate::adoptNewWindow(QSharedPointer newWebContents, + WindowOpenDisposition disposition, bool userGesture, + const QRect &initialGeometry, const QUrl &targetUrl) { Q_Q(QWebEnginePage); Q_UNUSED(userGesture); @@ -356,7 +359,10 @@ void QWebEnginePagePrivate::adoptNewWindow(QSharedPointer ne QWebEnginePage *newPage = q->createWindow(toWindowType(disposition)); if (!newPage) - return; + return nullptr; + + if (!newWebContents->webContents()) + return newPage->d_func()->adapter; // Reuse existing adapter // Mark the new page as being in the process of being adopted, so that a second mouse move event // sent by newWebContents->initialize() gets filtered in RenderWidgetHostViewQt::forwardEvent. @@ -375,6 +381,8 @@ void QWebEnginePagePrivate::adoptNewWindow(QSharedPointer ne if (!initialGeometry.isEmpty()) emit newPage->geometryChangeRequested(initialGeometry); + + return newWebContents; } bool QWebEnginePagePrivate::isBeingAdopted() diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h index ae845c530..b4424ec4b 100644 --- a/src/webenginewidgets/api/qwebenginepage_p.h +++ b/src/webenginewidgets/api/qwebenginepage_p.h @@ -112,7 +112,10 @@ public: void loadFinished(bool success, const QUrl &url, bool isErrorPage = false, int errorCode = 0, const QString &errorDescription = QString()) override; void focusContainer() override; void unhandledKeyEvent(QKeyEvent *event) override; - void adoptNewWindow(QSharedPointer newWebContents, WindowOpenDisposition disposition, bool userGesture, const QRect &initialGeometry, const QUrl &targetUrl) override; + QSharedPointer + adoptNewWindow(QSharedPointer newWebContents, + WindowOpenDisposition disposition, bool userGesture, + const QRect &initialGeometry, const QUrl &targetUrl) override; bool isBeingAdopted() override; void close() override; void windowCloseRejected() override; -- cgit v1.2.3