From 98bbdd3330171332c8b73aa5329d24e7780c4661 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Thu, 12 Mar 2020 17:46:17 +0100 Subject: Fix crash in ProxyingURLLoaderFactoryQt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need to be on the UI thread to walk the frame-node tree. Task-number: QTBUG-82999 Change-Id: I8011a29e91d6af92da341cbdd01fc9403c587e23 Reviewed-by: Tamas Zakor Reviewed-by: Jüri Valdmann --- src/core/content_browser_client_qt.cpp | 1 - src/core/net/proxying_url_loader_factory_qt.cpp | 48 ++++++++++++++++--------- src/core/net/proxying_url_loader_factory_qt.h | 4 --- 3 files changed, 32 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/core/content_browser_client_qt.cpp b/src/core/content_browser_client_qt.cpp index a8d9e6ddc..d38a5916e 100644 --- a/src/core/content_browser_client_qt.cpp +++ b/src/core/content_browser_client_qt.cpp @@ -1200,7 +1200,6 @@ bool ContentBrowserClientQt::WillCreateURLLoaderFactory( base::PostTask(FROM_HERE, { content::BrowserThread::IO }, base::BindOnce(&ProxyingURLLoaderFactoryQt::CreateProxy, process_id, browser_context->GetResourceContext(), - static_cast(frame), std::move(proxied_receiver), std::move(target_factory_info))); return true; diff --git a/src/core/net/proxying_url_loader_factory_qt.cpp b/src/core/net/proxying_url_loader_factory_qt.cpp index a2cb268b0..4931bb3e3 100644 --- a/src/core/net/proxying_url_loader_factory_qt.cpp +++ b/src/core/net/proxying_url_loader_factory_qt.cpp @@ -94,13 +94,14 @@ class InterceptedRequest : public network::mojom::URLLoader { public: InterceptedRequest(int process_id, uint64_t request_id, int32_t routing_id, uint32_t options, - const network::ResourceRequest &request, const GURL &top_document_url, + const network::ResourceRequest &request, const net::MutableNetworkTrafficAnnotationTag &traffic_annotation, ProfileIODataQt *profileData, network::mojom::URLLoaderRequest loader_request, network::mojom::URLLoaderClientPtr client, network::mojom::URLLoaderFactoryPtr target_factory); ~InterceptedRequest() override; + void Start(); void Restart(); void InterceptOnUIThread(); @@ -170,7 +171,7 @@ private: }; InterceptedRequest::InterceptedRequest(int process_id, uint64_t request_id, int32_t routing_id, uint32_t options, - const network::ResourceRequest &request, const GURL &top_document_url, + const network::ResourceRequest &request, const net::MutableNetworkTrafficAnnotationTag &traffic_annotation, ProfileIODataQt *profileData, network::mojom::URLLoaderRequest loader_request, @@ -180,7 +181,6 @@ InterceptedRequest::InterceptedRequest(int process_id, uint64_t request_id, int3 , request_id_(request_id) , routing_id_(routing_id) , options_(options) - , m_topDocumentUrl(top_document_url) , request_(request) , traffic_annotation_(traffic_annotation) , m_profileData(profileData) @@ -203,6 +203,31 @@ InterceptedRequest::~InterceptedRequest() m_weakFactory.InvalidateWeakPtrs(); } +void InterceptedRequest::Start() +{ + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + content::FrameTreeNode *frameTreeNode = nullptr; + if (process_id_) { + content::RenderFrameHostImpl *renderFrameHost = content::RenderFrameHostImpl::FromID(process_id_, + request_.render_frame_id); + if (renderFrameHost) + frameTreeNode = renderFrameHost->frame_tree_node(); + } else { + frameTreeNode = content::FrameTreeNode::GloballyFindByID(request_.render_frame_id); + } + // Follows a similar path to the root as RenderFrameHostImpl::CalculateSiteForCookies() + if (frameTreeNode && frameTreeNode->frame_tree() + && frameTreeNode->frame_tree()->root() + && frameTreeNode->frame_tree()->root()->current_frame_host()) + m_topDocumentUrl = frameTreeNode->frame_tree()->root()->current_frame_host()->GetLastCommittedURL(); + else + LOG(INFO) << "InterceptedRequest::Start() - null frameTreeNode or path to top frame"; + + base::PostTask(FROM_HERE, {content::BrowserThread::IO}, + base::BindOnce(&InterceptedRequest::Restart, m_weakPtr)); +} + void InterceptedRequest::Restart() { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); @@ -466,10 +491,9 @@ void InterceptedRequest::SendErrorAndCompleteImmediately(int error_code) ProxyingURLLoaderFactoryQt::ProxyingURLLoaderFactoryQt(int process_id, content::ResourceContext *resourceContext, - content::RenderFrameHostImpl *host, mojo::PendingReceiver loader_receiver, network::mojom::URLLoaderFactoryPtrInfo target_factory_info) - : m_processId(process_id), m_resourceContext(resourceContext), m_renderFrameHost(host), m_weakFactory(this) + : m_processId(process_id), m_resourceContext(resourceContext), m_weakFactory(this) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); if (target_factory_info) { @@ -490,14 +514,13 @@ ProxyingURLLoaderFactoryQt::~ProxyingURLLoaderFactoryQt() // static void ProxyingURLLoaderFactoryQt::CreateProxy(int process_id, content::ResourceContext *resourceContext, - content::RenderFrameHostImpl *host, mojo::PendingReceiver loader_receiver, network::mojom::URLLoaderFactoryPtrInfo target_factory_info) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); // Will manage its own lifetime - new ProxyingURLLoaderFactoryQt(process_id, resourceContext, host, std::move(loader_receiver), std::move(target_factory_info)); + new ProxyingURLLoaderFactoryQt(process_id, resourceContext, std::move(loader_receiver), std::move(target_factory_info)); } void ProxyingURLLoaderFactoryQt::CreateLoaderAndStart(network::mojom::URLLoaderRequest loader, int32_t routing_id, @@ -522,20 +545,13 @@ void ProxyingURLLoaderFactoryQt::CreateLoaderAndStart(network::mojom::URLLoaderR if (m_targetFactory) m_targetFactory->Clone(mojo::MakeRequest(&target_factory_clone)); - // Follows a similar path to the root as RenderFrameHostImpl::CalculateSiteForCookies() - GURL top_document_url; - if (m_renderFrameHost) - top_document_url = m_renderFrameHost->frame_tree_node()->frame_tree()->root()->current_frame_host()->GetLastCommittedURL(); - else - LOG(INFO) << "ProxyingURLLoaderFactoryQt::CreateLoaderAndStart() - null m_renderFrameHost, shouldn't happen"; - // Will manage its own lifetime InterceptedRequest *req = new InterceptedRequest(m_processId, request_id, routing_id, options, request, - top_document_url, traffic_annotation, profileIOData, std::move(loader), std::move(client), std::move(target_factory_clone)); - req->Restart(); + base::PostTask(FROM_HERE, {content::BrowserThread::UI}, + base::BindOnce(&InterceptedRequest::Start, base::Unretained(req))); } void ProxyingURLLoaderFactoryQt::OnTargetFactoryError() diff --git a/src/core/net/proxying_url_loader_factory_qt.h b/src/core/net/proxying_url_loader_factory_qt.h index 4d913f545..0a436fd81 100644 --- a/src/core/net/proxying_url_loader_factory_qt.h +++ b/src/core/net/proxying_url_loader_factory_qt.h @@ -60,7 +60,6 @@ // found in the LICENSE file. namespace content { -class RenderFrameHostImpl; class ResourceContext; } @@ -70,14 +69,12 @@ class ProxyingURLLoaderFactoryQt : public network::mojom::URLLoaderFactory { public: ProxyingURLLoaderFactoryQt(int process_id, content::ResourceContext *resourceContext, - content::RenderFrameHostImpl *host, mojo::PendingReceiver loader_receiver, network::mojom::URLLoaderFactoryPtrInfo target_factory_info); ~ProxyingURLLoaderFactoryQt() override; static void CreateProxy(int process_id, content::ResourceContext *resourceContext, - content::RenderFrameHostImpl *host, mojo::PendingReceiver loader_receiver, network::mojom::URLLoaderFactoryPtrInfo target_factory_info); @@ -97,7 +94,6 @@ private: network::mojom::URLLoaderFactoryPtr m_targetFactory; content::ResourceContext *m_resourceContext; - content::RenderFrameHostImpl *m_renderFrameHost; base::WeakPtrFactory m_weakFactory; -- cgit v1.2.3