Commit aa3116de authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Make AboutURLLoaderFactory always owned by its |receivers_|.

This CL introduces AboutURLLoaderFactory::Create static method that
allows creating an AboutURLLoaderFactory that is owned by its
|receivers_| and will self-delete when the last receiver disconnects.

This CL removes the ability to directly construct and own a
std::unique_ptr<AboutURLLoaderFactory>, because this ability means that
the factory can be destructed while receivers bound via the Clone method
are still alive (see the associated bug).  In particular, this CL stops
exposing AboutURLLoaderFactory constructor as a public member, which
forces construction to go via the new Create static method.

This CL mostly just follows the pattern established earlier by
https://crrev.com/c/2337411.

Bug: 1106995
Change-Id: Id93cc5b5078019d95dd19988ffdffb8e40d7ac31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2358053
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarŁukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805584}
parent ca01818e
...@@ -10,7 +10,10 @@ ...@@ -10,7 +10,10 @@
namespace content { namespace content {
AboutURLLoaderFactory::AboutURLLoaderFactory() = default; AboutURLLoaderFactory::AboutURLLoaderFactory(
mojo::PendingReceiver<network::mojom::URLLoaderFactory> factory_receiver)
: NonNetworkURLLoaderFactoryBase(std::move(factory_receiver)) {}
AboutURLLoaderFactory::~AboutURLLoaderFactory() = default; AboutURLLoaderFactory::~AboutURLLoaderFactory() = default;
void AboutURLLoaderFactory::CreateLoaderAndStart( void AboutURLLoaderFactory::CreateLoaderAndStart(
...@@ -41,9 +44,16 @@ void AboutURLLoaderFactory::CreateLoaderAndStart( ...@@ -41,9 +44,16 @@ void AboutURLLoaderFactory::CreateLoaderAndStart(
client_remote->OnComplete(network::URLLoaderCompletionStatus(net::OK)); client_remote->OnComplete(network::URLLoaderCompletionStatus(net::OK));
} }
void AboutURLLoaderFactory::Clone( // static
mojo::PendingReceiver<network::mojom::URLLoaderFactory> loader) { mojo::PendingRemote<network::mojom::URLLoaderFactory>
receivers_.Add(this, std::move(loader)); AboutURLLoaderFactory::Create() {
mojo::PendingRemote<network::mojom::URLLoaderFactory> pending_remote;
// The AboutURLLoaderFactory will delete itself when there are no more
// receivers - see the NonNetworkURLLoaderFactoryBase::OnDisconnect method.
new AboutURLLoaderFactory(pending_remote.InitWithNewPipeAndPassReceiver());
return pending_remote;
} }
} // namespace content } // namespace content
...@@ -6,23 +6,29 @@ ...@@ -6,23 +6,29 @@
#define CONTENT_BROWSER_ABOUT_URL_LOADER_FACTORY_H_ #define CONTENT_BROWSER_ABOUT_URL_LOADER_FACTORY_H_
#include "base/macros.h" #include "base/macros.h"
#include "content/browser/loader/non_network_url_loader_factory_base.h"
#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "services/network/public/mojom/url_loader_factory.mojom.h"
namespace content { namespace content {
// URLLoaderFactory for handling about: URLs. This treats everything as // URLLoaderFactory for handling about: URLs. This treats everything as
// about:blank since no other about: features should be available to web // about:blank since no other about: features should be available to web
// content. // content.
class AboutURLLoaderFactory : public network::mojom::URLLoaderFactory { class AboutURLLoaderFactory : public NonNetworkURLLoaderFactoryBase {
public: public:
AboutURLLoaderFactory(); // Returns mojo::PendingRemote to a newly constructed AboutURLLoadedFactory.
~AboutURLLoaderFactory() override; // The factory is self-owned - it will delete itself once there are no more
// receivers (including the receiver associated with the returned
// mojo::PendingRemote and the receivers bound by the Clone method).
static mojo::PendingRemote<network::mojom::URLLoaderFactory> Create();
private: private:
explicit AboutURLLoaderFactory(
mojo::PendingReceiver<network::mojom::URLLoaderFactory> factory_receiver);
// network::mojom::URLLoaderFactory: // network::mojom::URLLoaderFactory:
~AboutURLLoaderFactory() override;
void CreateLoaderAndStart( void CreateLoaderAndStart(
mojo::PendingReceiver<network::mojom::URLLoader> loader, mojo::PendingReceiver<network::mojom::URLLoader> loader,
int32_t routing_id, int32_t routing_id,
...@@ -32,10 +38,6 @@ class AboutURLLoaderFactory : public network::mojom::URLLoaderFactory { ...@@ -32,10 +38,6 @@ class AboutURLLoaderFactory : public network::mojom::URLLoaderFactory {
mojo::PendingRemote<network::mojom::URLLoaderClient> client, mojo::PendingRemote<network::mojom::URLLoaderClient> client,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) const net::MutableNetworkTrafficAnnotationTag& traffic_annotation)
override; override;
void Clone(
mojo::PendingReceiver<network::mojom::URLLoaderFactory> loader) override;
mojo::ReceiverSet<network::mojom::URLLoaderFactory> receivers_;
DISALLOW_COPY_AND_ASSIGN(AboutURLLoaderFactory); DISALLOW_COPY_AND_ASSIGN(AboutURLLoaderFactory);
}; };
......
...@@ -1209,8 +1209,8 @@ NavigationURLLoaderImpl::NavigationURLLoaderImpl( ...@@ -1209,8 +1209,8 @@ NavigationURLLoaderImpl::NavigationURLLoaderImpl(
storage_partition_->GetFileSystemContext(), storage_domain)); storage_partition_->GetFileSystemContext(), storage_domain));
} }
non_network_uniquely_owned_factories_.emplace( non_network_url_loader_factories_.emplace(url::kAboutScheme,
url::kAboutScheme, std::make_unique<AboutURLLoaderFactory>()); AboutURLLoaderFactory::Create());
non_network_uniquely_owned_factories_.emplace( non_network_uniquely_owned_factories_.emplace(
url::kDataScheme, std::make_unique<DataURLLoaderFactory>()); url::kDataScheme, std::make_unique<DataURLLoaderFactory>());
......
...@@ -6095,6 +6095,7 @@ void RenderFrameHostImpl::CommitNavigation( ...@@ -6095,6 +6095,7 @@ void RenderFrameHostImpl::CommitNavigation(
} }
non_network_uniquely_owned_factories_.clear(); non_network_uniquely_owned_factories_.clear();
ContentBrowserClient::NonNetworkURLLoaderFactoryMap non_network_factories;
// Set up the default factory. // Set up the default factory.
mojo::PendingRemote<network::mojom::URLLoaderFactory> mojo::PendingRemote<network::mojom::URLLoaderFactory>
...@@ -6127,8 +6128,8 @@ void RenderFrameHostImpl::CommitNavigation( ...@@ -6127,8 +6128,8 @@ void RenderFrameHostImpl::CommitNavigation(
// WebUIURLLoaderFactory will kill the renderer if it sees a request // WebUIURLLoaderFactory will kill the renderer if it sees a request
// with a non-chrome scheme. Register a URLLoaderFactory for the about // with a non-chrome scheme. Register a URLLoaderFactory for the about
// scheme so about:blank doesn't kill the renderer. // scheme so about:blank doesn't kill the renderer.
non_network_uniquely_owned_factories_[url::kAboutScheme] = non_network_factories[url::kAboutScheme] =
std::make_unique<AboutURLLoaderFactory>(); AboutURLLoaderFactory::Create();
} else { } else {
// This is a webui scheme that doesn't have webui bindings. Give it // This is a webui scheme that doesn't have webui bindings. Give it
// access to the network loader as it might require it. // access to the network loader as it might require it.
...@@ -6204,7 +6205,6 @@ void RenderFrameHostImpl::CommitNavigation( ...@@ -6204,7 +6205,6 @@ void RenderFrameHostImpl::CommitNavigation(
// //
// TODO(crbug.com/888079): In the future, use // TODO(crbug.com/888079): In the future, use
// GetOriginForURLLoaderFactory/GetOriginToCommit. // GetOriginForURLLoaderFactory/GetOriginToCommit.
ContentBrowserClient::NonNetworkURLLoaderFactoryMap non_network_factories;
if ((common_params->url.SchemeIsFile() || if ((common_params->url.SchemeIsFile() ||
(common_params->url.IsAboutBlank() && (common_params->url.IsAboutBlank() &&
common_params->initiator_origin && common_params->initiator_origin &&
......
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