Fix life cycle issue of QWebEngineUrlRequestInterceptor

Since 5e92adf intercept() is called on ui thread, however
this patch does not consider deletion of interceptor while
io thread still process the request and for examples
checks interceptor's deprecated property. Fix it.

Note this only fixes issue for interceptors which are
not deprecated therefore run on ui thread.

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

4 files changed, 22 insertions, 9 deletions

diff --git a/src/core/net/network_delegate_qt.cpp b/src/core/net/network_delegate_qt.cpp
index 7f278fd92..2cca152e6 100644
--- a/src/core/net/network_delegate_qt.cpp
+++ b/src/core/net/network_delegate_qt.cpp
@@ -136,11 +136,11 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, net::Complet
 
     // 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);
+
+    if (m_profileIOData->isInterceptorDeprecated()) {
+        QWebEngineUrlRequestInterceptor* profileInterceptor = m_profileIOData->acquireInterceptor();
+        if (profileInterceptor && m_profileIOData->isInterceptorDeprecated()) {
+            profileInterceptor->interceptRequest(requestInfo);
             m_profileIOData->releaseInterceptor();
             if (requestInfo.changed()) {
                 int result = infoPrivate->shouldBlockRequest ? net::ERR_BLOCKED_BY_CLIENT : net::OK;
@@ -174,7 +174,9 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, net::Complet
     if (!resourceInfo)
         return net::OK;
 
-    if (!m_profileIOData->hasPageInterceptors() && !profileInterceptor && !content::IsResourceTypeFrame(resourceType))
+    // try to bail out
+    if (!m_profileIOData->hasPageInterceptors() && (!m_profileIOData->requestInterceptor() || m_profileIOData->isInterceptorDeprecated()) &&
+            !content::IsResourceTypeFrame(resourceType))
         return net::OK;
 
     auto webContentsGetter = resourceInfo->GetWebContentsGetterForRequest();
@@ -185,7 +187,7 @@ int NetworkDelegateQt::OnBeforeURLRequest(net::URLRequest *request, net::Complet
         std::move(requestInfo),
         webContentsGetter,
         std::move(callback),
-        profileInterceptor ? m_profileIOData->profileAdapter() : nullptr
+        m_profileIOData->profileAdapter()
     );
 
     // 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 e37ad35bc..9d309e314 100644
--- a/src/core/net/url_request_notification.cpp
+++ b/src/core/net/url_request_notification.cpp
@@ -107,7 +107,7 @@ void URLRequestNotification::notify()
 
     if (webContents) {
 
-        if (m_profileAdapter) {
+        if (m_profileAdapter && m_profileAdapter->requestInterceptor()) {
             QWebEngineUrlRequestInterceptor* interceptor = m_profileAdapter->requestInterceptor();
             if (!interceptor->property("deprecated").toBool())
                 interceptor->interceptRequest(m_requestInfo);
diff --git a/src/core/profile_io_data_qt.cpp b/src/core/profile_io_data_qt.cpp
index 27c97a986..6ed172335 100644
--- a/src/core/profile_io_data_qt.cpp
+++ b/src/core/profile_io_data_qt.cpp
@@ -741,9 +741,18 @@ void ProfileIODataQt::updateRequestInterceptor()
     QMutexLocker lock(&m_mutex);
     m_requestInterceptor = m_profileAdapter->requestInterceptor();
     m_hasPageInterceptors = m_profileAdapter->hasPageRequestInterceptor();
+    if (m_requestInterceptor)
+        m_isInterceptorDeprecated = m_requestInterceptor->property("deprecated").toBool();
+    else
+        m_isInterceptorDeprecated = false;
     // We in this case do not need to regenerate any Chromium classes.
 }
 
+bool ProfileIODataQt::isInterceptorDeprecated() const
+{
+    return m_isInterceptorDeprecated;
+}
+
 QWebEngineUrlRequestInterceptor *ProfileIODataQt::acquireInterceptor()
 {
     m_mutex.lock();
diff --git a/src/core/profile_io_data_qt.h b/src/core/profile_io_data_qt.h
index 570365085..a1b123771 100644
--- a/src/core/profile_io_data_qt.h
+++ b/src/core/profile_io_data_qt.h
@@ -120,7 +120,8 @@ public:
     void setGlobalCertificateVerification();
 
     // Used in NetworkDelegateQt::OnBeforeURLRequest.
-    QWebEngineUrlRequestInterceptor *acquireInterceptor();
+    bool isInterceptorDeprecated() const; // Remove for Qt6
+    QWebEngineUrlRequestInterceptor *acquireInterceptor(); // Remove for Qt6
     void releaseInterceptor();
     QWebEngineUrlRequestInterceptor *requestInterceptor();
 
@@ -191,6 +192,7 @@ private:
     base::WeakPtrFactory<ProfileIODataQt> m_weakPtrFactory; // this should be always the last member
     QString m_dataPath;
     bool m_pendingStorageRequestGeneration = false;
+    volatile bool m_isInterceptorDeprecated = false; // Remove for Qt6
     DISALLOW_COPY_AND_ASSIGN(ProfileIODataQt);
 
     friend class BrowsingDataRemoverObserverQt;