Lock profile mutex during interceptRequest

This makes setRequestInterceptor take effect atomically, so it's always safe to
delete the interceptor after unsetting it from the profile. On the other hand,
we run into a deadlock if the user code tries to wait for a profile operation on
the UI thread to finish. It seems quite unlikely though that user code would do
that and it would probably still be easier for users to understand and debug
than the current non-deterministic use-after-free problems.

Task-number: QTBUG-72260
Change-Id: I1784a9d9f00029d440508f0bb076d1081a326758
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
Reviewed-by: Leena Miettinen <riitta-leena.miettinen@qt.io>

4 files changed, 18 insertions, 7 deletions

diff --git a/src/core/api/qwebengineurlrequestinfo.cpp b/src/core/api/qwebengineurlrequestinfo.cpp
index 2bb870071..ea9081fc1 100644
--- a/src/core/api/qwebengineurlrequestinfo.cpp
+++ b/src/core/api/qwebengineurlrequestinfo.cpp
@@ -120,6 +120,9 @@ ASSERT_ENUMS_MATCH(QtWebEngineCore::WebContentsAdapterClient::OtherNavigation, Q
 
     \a info contains the information about the URL request and will track internally
     whether its members have been altered.
+
+    \warning All method calls to the profile on the main thread will block until
+    execution of this function is finished.
 */
 
 
diff --git a/src/core/net/network_delegate_qt.cpp b/src/core/net/network_delegate_qt.cpp
index 551302291..91e3fa138 100644
--- a/src/core/net/network_delegate_qt.cpp
+++ b/src/core/net/network_delegate_qt.cpp
@@ -225,7 +225,7 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, net::Complet
 
     const QUrl qUrl = toQt(request->url());
 
-    QWebEngineUrlRequestInterceptor* interceptor = m_profileIOData->requestInterceptor();
+    QWebEngineUrlRequestInterceptor* interceptor = m_profileIOData->acquireInterceptor();
     if (interceptor) {
         QWebEngineUrlRequestInfoPrivate *infoPrivate = new QWebEngineUrlRequestInfoPrivate(toQt(resourceType),
                                                                                            toQt(navigationType),
@@ -234,6 +234,7 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, net::Complet
                                                                                            QByteArray::fromStdString(request->method()));
         QWebEngineUrlRequestInfo requestInfo(infoPrivate);
         interceptor->interceptRequest(requestInfo);
+        m_profileIOData->releaseInterceptor();
         if (requestInfo.changed()) {
             int result = infoPrivate->shouldBlockRequest ? net::ERR_BLOCKED_BY_CLIENT : net::OK;
 
@@ -249,7 +250,8 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, net::Complet
             if (result != net::OK)
                 return result;
         }
-    }
+    } else
+        m_profileIOData->releaseInterceptor();
 
     if (!resourceInfo)
         return net::OK;
diff --git a/src/core/profile_io_data_qt.cpp b/src/core/profile_io_data_qt.cpp
index 0a0d242a3..7783f1ae7 100644
--- a/src/core/profile_io_data_qt.cpp
+++ b/src/core/profile_io_data_qt.cpp
@@ -690,14 +690,17 @@ void ProfileIODataQt::updateRequestInterceptor()
     // We in this case do not need to regenerate any Chromium classes.
 }
 
-QWebEngineUrlRequestInterceptor *ProfileIODataQt::requestInterceptor()
+QWebEngineUrlRequestInterceptor *ProfileIODataQt::acquireInterceptor()
 {
-    // used in NetworkDelegateQt::OnBeforeURLRequest
-    Q_ASSERT(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
-    QMutexLocker lock(&m_mutex);
+    m_mutex.lock();
     return m_requestInterceptor;
 }
 
+void ProfileIODataQt::releaseInterceptor()
+{
+    m_mutex.unlock();
+}
+
 bool ProfileIODataQt::canSetCookie(const QUrl &firstPartyUrl, const QByteArray &cookieLine, const QUrl &url) const
 {
     return m_cookieDelegate->canSetCookie(firstPartyUrl,cookieLine, url);
diff --git a/src/core/profile_io_data_qt.h b/src/core/profile_io_data_qt.h
index 6961e2ad2..5b416861c 100644
--- a/src/core/profile_io_data_qt.h
+++ b/src/core/profile_io_data_qt.h
@@ -89,10 +89,13 @@ public:
     void generateUserAgent();
     void generateJobFactory();
     void regenerateJobFactory();
-    QWebEngineUrlRequestInterceptor *requestInterceptor();
     bool canSetCookie(const QUrl &firstPartyUrl, const QByteArray &cookieLine, const QUrl &url) const;
     bool canGetCookies(const QUrl &firstPartyUrl, const QUrl &url) const;
 
+    // Used in NetworkDelegateQt::OnBeforeURLRequest.
+    QWebEngineUrlRequestInterceptor *acquireInterceptor();
+    void releaseInterceptor();
+
     void setRequestContextData(content::ProtocolHandlerMap *protocolHandlers,
                                content::URLRequestInterceptorScopedVector request_interceptors);
     void setFullConfiguration(); // runs on ui thread