diff options
author | Michal Klocek <michal.klocek@qt.io> | 2019-02-01 12:01:26 +0100 |
---|---|---|
committer | Michal Klocek <michal.klocek@qt.io> | 2019-02-04 15:34:00 +0000 |
commit | 5e92adf5f85c4ec8340d5b7e92166a5e4f8e9883 (patch) | |
tree | 350d28ccf5de9a85cc26f520bd0dda2222bc1021 /src | |
parent | a8b48040243c95316a9dad92360f086cf6e537af (diff) |
Move QWebEngineUrlRequestInterceptor::intercept to ui thread
Currently interceptor sufferers thread safety issues, when
custom profiles are deleted, interceptor is set to be nullptr,
however it can be still referenced in IO thread. Since
profile was split to ui and io part, where io part can outlive
the ui part, this can boost thread safety issues.
Since QWebEngineUrlRequestInterceptor is living on ui thread
simplify the logic move intercept call to ui thread.
This fixes the issue of referencing interceptor in io thread.
Add new method to install interceptor setUrlRequestInterceptor,
and deprecate old one. Update interceptor install method name
on page to match the profile one.
Task-number: QTBUG-69844
Change-Id: I5dd2b6b734fd91906cccc6c1408ffbe7b1b4250c
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'src')
-rw-r--r-- | src/core/api/qwebengineurlrequestinfo.cpp | 10 | ||||
-rw-r--r-- | src/core/api/qwebengineurlrequestinterceptor.h | 2 | ||||
-rw-r--r-- | src/core/net/network_delegate_qt.cpp | 53 | ||||
-rw-r--r-- | src/core/net/url_request_notification.cpp | 34 | ||||
-rw-r--r-- | src/core/net/url_request_notification.h | 8 | ||||
-rw-r--r-- | src/core/profile_io_data_qt.cpp | 10 | ||||
-rw-r--r-- | src/core/profile_io_data_qt.h | 2 | ||||
-rw-r--r-- | src/webengine/api/qquickwebengineprofile.cpp | 27 | ||||
-rw-r--r-- | src/webengine/api/qquickwebengineprofile.h | 3 | ||||
-rw-r--r-- | src/webenginewidgets/api/qwebenginepage.cpp | 2 | ||||
-rw-r--r-- | src/webenginewidgets/api/qwebenginepage.h | 2 | ||||
-rw-r--r-- | src/webenginewidgets/api/qwebengineprofile.cpp | 28 | ||||
-rw-r--r-- | src/webenginewidgets/api/qwebengineprofile.h | 3 |
13 files changed, 137 insertions, 47 deletions
diff --git a/src/core/api/qwebengineurlrequestinfo.cpp b/src/core/api/qwebengineurlrequestinfo.cpp index c3e5b5445..dc2d07740 100644 --- a/src/core/api/qwebengineurlrequestinfo.cpp +++ b/src/core/api/qwebengineurlrequestinfo.cpp @@ -96,8 +96,8 @@ ASSERT_ENUMS_MATCH(QtWebEngineCore::WebContentsAdapterClient::OtherNavigation, Q interceptor on the profile enables intercepting, blocking, and modifying URL requests before they reach the networking stack of Chromium. - You can install the interceptor on a profile via QWebEngineProfile::setRequestInterceptor() - or QQuickWebEngineProfile::setRequestInterceptor(). + You can install the interceptor on a profile via QWebEngineProfile::setUrlRequestInterceptor() + or QQuickWebEngineProfile::setUrlRequestInterceptor(). When using the \l{Qt WebEngine Widgets Module}, \l{QWebEnginePage::acceptNavigationRequest()} offers further options to accept or block requests. @@ -115,11 +115,7 @@ ASSERT_ENUMS_MATCH(QtWebEngineCore::WebContentsAdapterClient::OtherNavigation, Q \fn void QWebEngineUrlRequestInterceptor::interceptRequest(QWebEngineUrlRequestInfo &info) Reimplementing this virtual function makes it possible to intercept URL - requests. For interceptors installed on a QWebEngineProfile, the function is executed - on the I/O thread, and thus it may not be thread-safe to interact with pages. If the - interceptor was installed on a QWebEnginePage, the function is executed on the main - application thread, and can safely interact with other user classes. Both versions will - be stalling the URL request until handled. + requests. This method will be stalling the URL request until handled. \a info contains the information about the URL request and will track internally whether its members have been altered. diff --git a/src/core/api/qwebengineurlrequestinterceptor.h b/src/core/api/qwebengineurlrequestinterceptor.h index dc2a15ee3..2b07681ca 100644 --- a/src/core/api/qwebengineurlrequestinterceptor.h +++ b/src/core/api/qwebengineurlrequestinterceptor.h @@ -55,7 +55,7 @@ class QWEBENGINECORE_EXPORT QWebEngineUrlRequestInterceptor : public QObject Q_OBJECT Q_DISABLE_COPY(QWebEngineUrlRequestInterceptor) public: - explicit QWebEngineUrlRequestInterceptor(QObject *p = Q_NULLPTR) + explicit QWebEngineUrlRequestInterceptor(QObject *p = nullptr) : QObject (p) { } diff --git a/src/core/net/network_delegate_qt.cpp b/src/core/net/network_delegate_qt.cpp index d2b9f06b3..3641cb845 100644 --- a/src/core/net/network_delegate_qt.cpp +++ b/src/core/net/network_delegate_qt.cpp @@ -131,33 +131,41 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, net::Complet QByteArray::fromStdString(request->method())); QWebEngineUrlRequestInfo requestInfo(infoPrivate); - if (QWebEngineUrlRequestInterceptor* interceptor = m_profileIOData->acquireInterceptor()) { - interceptor->interceptRequest(requestInfo); - m_profileIOData->releaseInterceptor(); - if (requestInfo.changed()) { - int result = infoPrivate->shouldBlockRequest ? net::ERR_BLOCKED_BY_CLIENT : net::OK; - - if (qUrl != infoPrivate->url) - *newUrl = toGurl(infoPrivate->url); - - if (!infoPrivate->extraHeaders.isEmpty()) { - auto end = infoPrivate->extraHeaders.constEnd(); - for (auto header = infoPrivate->extraHeaders.constBegin(); header != end; ++header) - request->SetExtraRequestHeaderByName(header.key().toStdString(), header.value().toStdString(), /* overwrite */ true); + // Deprecated =begin + // quick peek if deprecated + QWebEngineUrlRequestInterceptor* profileInterceptor = m_profileIOData->requestInterceptor(); + if (profileInterceptor && profileInterceptor->property("deprecated").toBool()) { + profileInterceptor = nullptr; + if (QWebEngineUrlRequestInterceptor* interceptor = m_profileIOData->acquireInterceptor()) { + interceptor->interceptRequest(requestInfo); + m_profileIOData->releaseInterceptor(); + if (requestInfo.changed()) { + int result = infoPrivate->shouldBlockRequest ? net::ERR_BLOCKED_BY_CLIENT : net::OK; + + if (qUrl != infoPrivate->url) + *newUrl = toGurl(infoPrivate->url); + + if (!infoPrivate->extraHeaders.isEmpty()) { + auto end = infoPrivate->extraHeaders.constEnd(); + for (auto header = infoPrivate->extraHeaders.constBegin(); header != end; ++header) + request->SetExtraRequestHeaderByName(header.key().toStdString(), header.value().toStdString(), /* overwrite */ true); + } + + if (result != net::OK) + return result; + + requestInfo.resetChanged(); } - - if (result != net::OK) - return result; - - requestInfo.resetChanged(); + } else { + m_profileIOData->releaseInterceptor(); } - } else - m_profileIOData->releaseInterceptor(); + } + // Deprecated =cut if (!resourceInfo) return net::OK; - if (!m_profileIOData->hasPageInterceptors() && !content::IsResourceTypeFrame(resourceType)) + if (!m_profileIOData->hasPageInterceptors() && !profileInterceptor && !content::IsResourceTypeFrame(resourceType)) return net::OK; auto webContentsGetter = resourceInfo->GetWebContentsGetterForRequest(); @@ -167,7 +175,8 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, net::Complet newUrl, std::move(requestInfo), webContentsGetter, - std::move(callback) + std::move(callback), + profileInterceptor ? m_profileIOData->profileAdapter() : nullptr ); // We'll run the callback after we notified the UI thread. diff --git a/src/core/net/url_request_notification.cpp b/src/core/net/url_request_notification.cpp index 325474bc4..6da661cff 100644 --- a/src/core/net/url_request_notification.cpp +++ b/src/core/net/url_request_notification.cpp @@ -46,6 +46,7 @@ #include "net/url_request/url_request.h" #include "web_contents_adapter_client.h" #include "web_contents_view_qt.h" +#include "profile_io_data_qt.h" #include "qwebengineurlrequestinfo_p.h" #include "type_conversion.h" @@ -73,7 +74,8 @@ URLRequestNotification::URLRequestNotification(net::URLRequest *request, GURL *newUrl, QWebEngineUrlRequestInfo &&requestInfo, content::ResourceRequestInfo::WebContentsGetter webContentsGetter, - net::CompletionOnceCallback callback) + net::CompletionOnceCallback callback, + QPointer<ProfileAdapter> adapter) : m_request(request) , m_isMainFrameRequest(isMainFrameRequest) , m_newUrl(newUrl) @@ -81,6 +83,7 @@ URLRequestNotification::URLRequestNotification(net::URLRequest *request, , m_requestInfo(std::move(requestInfo)) , m_webContentsGetter(webContentsGetter) , m_callback(std::move(callback)) + , m_profileAdapter(adapter) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); @@ -99,36 +102,45 @@ void URLRequestNotification::notify() // May run concurrently with cancel() so no peeking at m_request here. - int error = net::OK; + int result = net::OK; content::WebContents *webContents = m_webContentsGetter.Run(); if (webContents) { + + if (m_profileAdapter) { + QWebEngineUrlRequestInterceptor* interceptor = m_profileAdapter->requestInterceptor(); + interceptor->interceptRequest(m_requestInfo); + } + WebContentsAdapterClient *client = - WebContentsViewQt::from(static_cast<content::WebContentsImpl*>(webContents)->GetView())->client(); + WebContentsViewQt::from(static_cast<content::WebContentsImpl*>(webContents)->GetView())->client(); + + if (!m_requestInfo.changed()) { + client->interceptRequest(m_requestInfo); + } - client->interceptRequest(m_requestInfo); if (m_requestInfo.changed()) { - error = m_requestInfo.d_ptr->shouldBlockRequest ? net::ERR_BLOCKED_BY_CLIENT : net::OK; + result = m_requestInfo.d_ptr->shouldBlockRequest ? net::ERR_BLOCKED_BY_CLIENT : net::OK; // We handle the rest of the changes later when we are back in I/O thread } // Only do navigationRequested on MAIN_FRAME and SUB_FRAME resources - if (error == net::OK && content::IsResourceTypeFrame(fromQt(m_requestInfo.resourceType()))) { + if (result == net::OK && content::IsResourceTypeFrame(fromQt(m_requestInfo.resourceType()))) { int navigationRequestAction = WebContentsAdapterClient::AcceptRequest; client->navigationRequested(m_requestInfo.navigationType(), m_requestInfo.requestUrl(), navigationRequestAction, m_isMainFrameRequest); - error = net::ERR_FAILED; + result = net::ERR_FAILED; switch (static_cast<WebContentsAdapterClient::NavigationRequestAction>(navigationRequestAction)) { case WebContentsAdapterClient::AcceptRequest: - error = net::OK; + result = net::OK; break; case WebContentsAdapterClient::IgnoreRequest: - error = net::ERR_ABORTED; + result = net::ERR_ABORTED; break; } - DCHECK(error != net::ERR_FAILED); + DCHECK(result != net::ERR_FAILED); } } @@ -136,7 +148,7 @@ void URLRequestNotification::notify() base::PostTaskWithTraits( FROM_HERE, {content::BrowserThread::IO}, - base::BindOnce(&URLRequestNotification::complete, base::Unretained(this), error)); + base::BindOnce(&URLRequestNotification::complete, base::Unretained(this), result)); } void URLRequestNotification::cancel() diff --git a/src/core/net/url_request_notification.h b/src/core/net/url_request_notification.h index 172e034a3..1d9acf12f 100644 --- a/src/core/net/url_request_notification.h +++ b/src/core/net/url_request_notification.h @@ -43,6 +43,7 @@ #include "content/public/browser/resource_request_info.h" #include "net/base/completion_once_callback.h" #include "qwebengineurlrequestinfo.h" +#include <QPointer> class GURL; @@ -52,6 +53,9 @@ class URLRequest; namespace QtWebEngineCore { +class ProfileAdapter; +class ProfileIoDataQt; + // Notifies WebContentsAdapterClient of a new URLRequest. class URLRequestNotification { public: @@ -60,7 +64,8 @@ public: GURL *newUrl, QWebEngineUrlRequestInfo &&requestInfo, content::ResourceRequestInfo::WebContentsGetter webContentsGetter, - net::CompletionOnceCallback callback); + net::CompletionOnceCallback callback, + QPointer<ProfileAdapter> adapter); ~URLRequestNotification() = default; void cancel(); void notify(); @@ -74,6 +79,7 @@ private: QWebEngineUrlRequestInfo m_requestInfo; content::ResourceRequestInfo::WebContentsGetter m_webContentsGetter; net::CompletionOnceCallback m_callback; + QPointer<ProfileAdapter> m_profileAdapter; }; } #endif diff --git a/src/core/profile_io_data_qt.cpp b/src/core/profile_io_data_qt.cpp index 55d3bba5a..dc5b6624e 100644 --- a/src/core/profile_io_data_qt.cpp +++ b/src/core/profile_io_data_qt.cpp @@ -195,6 +195,11 @@ ProfileIODataQt::~ProfileIODataQt() delete m_proxyConfigService.fetchAndStoreAcquire(0); } +QPointer<ProfileAdapter> ProfileIODataQt::profileAdapter() +{ + return m_profileAdapter; +} + void ProfileIODataQt::shutdownOnUIThread() { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); @@ -741,6 +746,11 @@ QWebEngineUrlRequestInterceptor *ProfileIODataQt::acquireInterceptor() return m_requestInterceptor; } +QWebEngineUrlRequestInterceptor *ProfileIODataQt::requestInterceptor() +{ + return m_requestInterceptor; +} + bool ProfileIODataQt::hasPageInterceptors() { // used in NetworkDelegateQt::OnBeforeURLRequest diff --git a/src/core/profile_io_data_qt.h b/src/core/profile_io_data_qt.h index bcf49e22b..f3460935a 100644 --- a/src/core/profile_io_data_qt.h +++ b/src/core/profile_io_data_qt.h @@ -75,6 +75,7 @@ public: ProfileIODataQt(ProfileQt *profile); // runs on ui thread virtual ~ProfileIODataQt(); + QPointer<ProfileAdapter> profileAdapter(); content::ResourceContext *resourceContext(); net::URLRequestContext *urlRequestContext(); void initializeOnIOThread(); @@ -96,6 +97,7 @@ public: // Used in NetworkDelegateQt::OnBeforeURLRequest. QWebEngineUrlRequestInterceptor *acquireInterceptor(); void releaseInterceptor(); + QWebEngineUrlRequestInterceptor *requestInterceptor(); void setRequestContextData(content::ProtocolHandlerMap *protocolHandlers, content::URLRequestInterceptorScopedVector request_interceptors); diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp index 4448d44d1..8a6c20f67 100644 --- a/src/webengine/api/qquickwebengineprofile.cpp +++ b/src/webengine/api/qquickwebengineprofile.cpp @@ -922,20 +922,45 @@ void QQuickWebEngineProfile::clearHttpCache() d->profileAdapter()->clearHttpCache(); } - +#if QT_DEPRECATED_SINCE(5, 13) /*! Registers a request interceptor singleton \a interceptor to intercept URL requests. The profile does not take ownership of the pointer. + \obsolete + + Interceptors installed with this method will call + QWebEngineUrlRequestInterceptor::interceptRequest on the I/O thread. Therefore + the user has to provide thread-safe interaction with the other user classes. + Use setUrlRequestInterceptor instead. + \sa QWebEngineUrlRequestInterceptor + */ void QQuickWebEngineProfile::setRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor) { Q_D(QQuickWebEngineProfile); + interceptor->setProperty("deprecated", true); + d->profileAdapter()->setRequestInterceptor(interceptor); + qWarning("Use of deprecated not tread-safe setter, use setUrlRequestInterceptor instead."); +} +#endif + +/*! + Registers a request interceptor singleton \a interceptor to intercept URL requests. + + The profile does not take ownership of the pointer. + + \sa QWebEngineUrlRequestInfo QWebEngineUrlRequestInterceptor +*/ +void QQuickWebEngineProfile::setUrlRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor) +{ + Q_D(QQuickWebEngineProfile); d->profileAdapter()->setRequestInterceptor(interceptor); } + /*! Returns the custom URL scheme handler register for the URL scheme \a scheme. */ diff --git a/src/webengine/api/qquickwebengineprofile.h b/src/webengine/api/qquickwebengineprofile.h index 1e2e3e030..f4460ba18 100644 --- a/src/webengine/api/qquickwebengineprofile.h +++ b/src/webengine/api/qquickwebengineprofile.h @@ -126,7 +126,10 @@ public: QWebEngineCookieStore *cookieStore() const; +#if QT_DEPRECATED_SINCE(5, 13) void setRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor); +#endif + void setUrlRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor); const QWebEngineUrlSchemeHandler *urlSchemeHandler(const QByteArray &) const; void installUrlSchemeHandler(const QByteArray &scheme, QWebEngineUrlSchemeHandler *); diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index 06a708672..5940319c3 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -1802,7 +1802,7 @@ void QWebEnginePagePrivate::printRequested() \sa QWebEngineUrlRequestInfo, QWebEngineProfile::setRequestInterceptor() */ -void QWebEnginePage::setRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor) +void QWebEnginePage::setUrlRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor) { Q_D(QWebEnginePage); bool hadInterceptorChanged = bool(d->requestInterceptor) != bool(interceptor); diff --git a/src/webenginewidgets/api/qwebenginepage.h b/src/webenginewidgets/api/qwebenginepage.h index 5eda8c72f..55450e438 100644 --- a/src/webenginewidgets/api/qwebenginepage.h +++ b/src/webenginewidgets/api/qwebenginepage.h @@ -304,7 +304,7 @@ public: void setDevToolsPage(QWebEnginePage *page); QWebEnginePage *devToolsPage() const; - void setRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor); + void setUrlRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor); const QWebEngineContextMenuData &contextMenuData() const; diff --git a/src/webenginewidgets/api/qwebengineprofile.cpp b/src/webenginewidgets/api/qwebengineprofile.cpp index 7e80f9720..e9703ffe8 100644 --- a/src/webenginewidgets/api/qwebengineprofile.cpp +++ b/src/webenginewidgets/api/qwebengineprofile.cpp @@ -550,19 +550,43 @@ QWebEngineCookieStore* QWebEngineProfile::cookieStore() return d->profileAdapter()->cookieStore(); } - +#if QT_DEPRECATED_SINCE(5, 13) /*! Registers a request interceptor singleton \a interceptor to intercept URL requests. The profile does not take ownership of the pointer. + \obsolete + + Interceptors installed with this method will call + QWebEngineUrlRequestInterceptor::interceptRequest on the I/O thread. Therefore + the user has to provide thread-safe interaction with the other user classes. + Use setUrlRequestInterceptor instead. + \since 5.6 \sa QWebEngineUrlRequestInfo -*/ +*/ void QWebEngineProfile::setRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor) { Q_D(QWebEngineProfile); + interceptor->setProperty("deprecated", true); + d->profileAdapter()->setRequestInterceptor(interceptor); + qWarning("Use of deprecated not tread-safe setter, use setUrlRequestInterceptor instead."); +} +#endif +/*! + Registers a request interceptor singleton \a interceptor to intercept URL requests. + + The profile does not take ownership of the pointer. + + \since 5.13 + \sa QWebEngineUrlRequestInfo QWebEngineUrlRequestInterceptor +*/ + +void QWebEngineProfile::setUrlRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor) +{ + Q_D(QWebEngineProfile); d->profileAdapter()->setRequestInterceptor(interceptor); } diff --git a/src/webenginewidgets/api/qwebengineprofile.h b/src/webenginewidgets/api/qwebengineprofile.h index 9fc509851..79e83c377 100644 --- a/src/webenginewidgets/api/qwebengineprofile.h +++ b/src/webenginewidgets/api/qwebengineprofile.h @@ -106,7 +106,10 @@ public: void setHttpCacheMaximumSize(int maxSize); QWebEngineCookieStore* cookieStore(); +#if QT_DEPRECATED_SINCE(5, 13) void setRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor); +#endif + void setUrlRequestInterceptor(QWebEngineUrlRequestInterceptor *interceptor); void clearAllVisitedLinks(); void clearVisitedLinks(const QList<QUrl> &urls); |