Commit 9ff89ace authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Make CastExtensionURLLoaderFactory always owned by its |receivers_|.

This CL introduces CastExtensionURLLoaderFactory::Create static method
that allows creating an CastExtensionURLLoaderFactory 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<CastExtensionURLLoaderFactory>, 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 CastExtensionURLLoaderFactory 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: I6802fe556698bcba84e8332bcaae4665410c2f82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410579
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarSean Topping <seantopping@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810927}
parent aa3fc85f
......@@ -877,10 +877,9 @@ void CastContentBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
extensions::CreateExtensionNavigationURLLoaderFactory(
browser_context, ukm_source_id,
!!extensions::WebViewGuest::FromWebContents(web_contents));
uniquely_owned_factories->emplace(
extensions::kExtensionScheme,
std::make_unique<CastExtensionURLLoaderFactory>(
browser_context, std::move(extension_factory)));
factories->emplace(extensions::kExtensionScheme,
CastExtensionURLLoaderFactory::Create(
browser_context, std::move(extension_factory)));
#endif
}
......@@ -900,10 +899,9 @@ void CastContentBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories(
auto* browser_context = frame_host->GetProcess()->GetBrowserContext();
auto extension_factory = extensions::CreateExtensionURLLoaderFactory(
render_process_id, render_frame_id);
uniquely_owned_factories->emplace(
extensions::kExtensionScheme,
std::make_unique<CastExtensionURLLoaderFactory>(
browser_context, std::move(extension_factory)));
factories->emplace(extensions::kExtensionScheme,
CastExtensionURLLoaderFactory::Create(
browser_context, std::move(extension_factory)));
#endif
factories->emplace(
......
......@@ -172,8 +172,10 @@ class CastExtensionURLLoader : public network::mojom::URLLoader,
CastExtensionURLLoaderFactory::CastExtensionURLLoaderFactory(
content::BrowserContext* browser_context,
mojo::PendingRemote<network::mojom::URLLoaderFactory> extension_factory)
: extension_registry_(extensions::ExtensionRegistry::Get(browser_context)),
mojo::PendingRemote<network::mojom::URLLoaderFactory> extension_factory,
mojo::PendingReceiver<network::mojom::URLLoaderFactory> factory_receiver)
: content::NonNetworkURLLoaderFactoryBase(std::move(factory_receiver)),
extension_registry_(extensions::ExtensionRegistry::Get(browser_context)),
extension_factory_(std::move(extension_factory)),
network_factory_(
content::BrowserContext::GetDefaultStoragePartition(browser_context)
......@@ -232,9 +234,20 @@ void CastExtensionURLLoaderFactory::CreateLoaderAndStart(
network_factory_);
}
void CastExtensionURLLoaderFactory::Clone(
mojo::PendingReceiver<network::mojom::URLLoaderFactory> factory_receiver) {
receivers_.Add(this, std::move(factory_receiver));
// static
mojo::PendingRemote<network::mojom::URLLoaderFactory>
CastExtensionURLLoaderFactory::Create(
content::BrowserContext* browser_context,
mojo::PendingRemote<network::mojom::URLLoaderFactory> extension_factory) {
mojo::PendingRemote<network::mojom::URLLoaderFactory> pending_remote;
// The CastExtensionURLLoaderFactory will delete itself when there are no more
// receivers - see the NonNetworkURLLoaderFactoryBase::OnDisconnect method.
new CastExtensionURLLoaderFactory(
browser_context, std::move(extension_factory),
pending_remote.InitWithNewPipeAndPassReceiver());
return pending_remote;
}
} // namespace shell
......
......@@ -6,6 +6,7 @@
#define CHROMECAST_BROWSER_CAST_EXTENSION_URL_LOADER_FACTORY_H_
#include "base/macros.h"
#include "content/public/browser/non_network_url_loader_factory_base.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
......@@ -28,16 +29,31 @@ namespace shell {
// extension scheme. Cast uses its own factory that resues the extensions
// URLLoader implementation because Cast sometimes loads extension resources
// from the web.
class CastExtensionURLLoaderFactory : public network::mojom::URLLoaderFactory {
class CastExtensionURLLoaderFactory
: public content::NonNetworkURLLoaderFactoryBase {
public:
// Returns mojo::PendingRemote to a newly constructed
// CastExtensionURLLoaderFactory. 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).
//
// |extension_factory| is the default extension factory that will be used if
// the request isn't fetched from the web.
CastExtensionURLLoaderFactory(
static mojo::PendingRemote<network::mojom::URLLoaderFactory> Create(
content::BrowserContext* browser_context,
mojo::PendingRemote<network::mojom::URLLoaderFactory> extension_factory);
~CastExtensionURLLoaderFactory() override;
private:
~CastExtensionURLLoaderFactory() override;
// |extension_factory| is the default extension factory that will be used if
// the request isn't fetched from the web.
CastExtensionURLLoaderFactory(
content::BrowserContext* browser_context,
mojo::PendingRemote<network::mojom::URLLoaderFactory> extension_factory,
mojo::PendingReceiver<network::mojom::URLLoaderFactory> factory_receiver);
// network::mojom::URLLoaderFactory:
void CreateLoaderAndStart(
mojo::PendingReceiver<network::mojom::URLLoader> loader_receiver,
......@@ -48,10 +64,7 @@ class CastExtensionURLLoaderFactory : public network::mojom::URLLoaderFactory {
mojo::PendingRemote<network::mojom::URLLoaderClient> client,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation)
override;
void Clone(mojo::PendingReceiver<network::mojom::URLLoaderFactory>
factory_receiver) override;
mojo::ReceiverSet<network::mojom::URLLoaderFactory> receivers_;
extensions::ExtensionRegistry* extension_registry_;
mojo::Remote<network::mojom::URLLoaderFactory> extension_factory_;
scoped_refptr<network::SharedURLLoaderFactory> network_factory_;
......
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