Commit c38fa0f4 authored by Clark DuVall's avatar Clark DuVall Committed by Commit Bot

Potentially fix crash in WeakWrapperSharedURLLoaderFactory

This is a speculative fix for http://crbug.com/884638. What I believe is
happening is the RenderFrame is being destroyed while the
WeakWrapperSharedURLLoaderFactory is still alive, and a request attempts
to use it. This seems to be happening the most when the AdDelayThrottle
is enabled and is delaying ads requests, which may explain why the
request is finishing after the RenderFrame is destroyed.

Bug: 884638
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I66960e5c5150345e7fd59f5ab5795414ec482a71
Reviewed-on: https://chromium-review.googlesource.com/1241955Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594489}
parent 085c55b5
...@@ -1000,11 +1000,9 @@ class RenderFrameImpl::FrameURLLoaderFactory ...@@ -1000,11 +1000,9 @@ class RenderFrameImpl::FrameURLLoaderFactory
scoped_refptr<network::SharedURLLoaderFactory> loader_factory = scoped_refptr<network::SharedURLLoaderFactory> loader_factory =
frame_->GetLoaderFactoryBundle(); frame_->GetLoaderFactoryBundle();
if (request.GetRequestContext() == WebURLRequest::kRequestContextPrefetch && if (request.GetRequestContext() == WebURLRequest::kRequestContextPrefetch &&
frame_->prefetch_loader_factory_) { frame_->prefetch_shared_loader_factory_) {
// The frame should be alive when this factory is used. // The frame should be alive when this factory is used.
loader_factory = loader_factory = frame_->prefetch_shared_loader_factory_;
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
frame_->prefetch_loader_factory_.get());
} }
return std::make_unique<WebURLLoaderImpl>( return std::make_unique<WebURLLoaderImpl>(
RenderThreadImpl::current()->resource_dispatcher(), RenderThreadImpl::current()->resource_dispatcher(),
...@@ -1699,6 +1697,9 @@ mojom::FrameHost* RenderFrameImpl::GetFrameHost() { ...@@ -1699,6 +1697,9 @@ mojom::FrameHost* RenderFrameImpl::GetFrameHost() {
} }
RenderFrameImpl::~RenderFrameImpl() { RenderFrameImpl::~RenderFrameImpl() {
if (prefetch_shared_loader_factory_)
prefetch_shared_loader_factory_->Detach();
// If file chooser is still waiting for answer, dispatch empty answer. // If file chooser is still waiting for answer, dispatch empty answer.
if (file_chooser_completion_) { if (file_chooser_completion_) {
file_chooser_completion_->DidChooseFile(WebVector<WebString>()); file_chooser_completion_->DidChooseFile(WebVector<WebString>());
...@@ -3234,7 +3235,14 @@ void RenderFrameImpl::CommitNavigation( ...@@ -3234,7 +3235,14 @@ void RenderFrameImpl::CommitNavigation(
return; return;
} }
if (prefetch_shared_loader_factory_)
prefetch_shared_loader_factory_->Detach();
prefetch_loader_factory_ = std::move(prefetch_loader_factory); prefetch_loader_factory_ = std::move(prefetch_loader_factory);
if (prefetch_loader_factory_) {
prefetch_shared_loader_factory_ =
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
prefetch_loader_factory_.get());
}
// If the request was initiated in the context of a user gesture then make // If the request was initiated in the context of a user gesture then make
// sure that the navigation also executes in the context of a user gesture. // sure that the navigation also executes in the context of a user gesture.
......
...@@ -145,6 +145,7 @@ class MediaPermission; ...@@ -145,6 +145,7 @@ class MediaPermission;
} }
namespace network { namespace network {
class WeakWrapperSharedURLLoaderFactory;
struct ResourceResponseHead; struct ResourceResponseHead;
} }
...@@ -1620,6 +1621,8 @@ class CONTENT_EXPORT RenderFrameImpl ...@@ -1620,6 +1621,8 @@ class CONTENT_EXPORT RenderFrameImpl
// Set on CommitNavigation when Network Service is enabled, and used // Set on CommitNavigation when Network Service is enabled, and used
// by FrameURLLoaderFactory for prefetch requests. // by FrameURLLoaderFactory for prefetch requests.
network::mojom::URLLoaderFactoryPtr prefetch_loader_factory_; network::mojom::URLLoaderFactoryPtr prefetch_loader_factory_;
scoped_refptr<network::WeakWrapperSharedURLLoaderFactory>
prefetch_shared_loader_factory_;
// URLLoaderFactory instances used for subresource loading. // URLLoaderFactory instances used for subresource loading.
// Depending on how the frame was created, |loader_factories_| could be: // Depending on how the frame was created, |loader_factories_| could be:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment