Skip to content

Commit

Permalink
appcache: Remove AppCacheFactory from url loader factory bundle
Browse files Browse the repository at this point in the history
Bug: 582750
Change-Id: I30f60dce343e1a702f314bf2b10290f64734c1cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3208947
Auto-Submit: enne <enne@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/main@{#930644}
  • Loading branch information
quisquous authored and Chromium LUCI CQ committed Oct 12, 2021
1 parent 19a3082 commit be0664d
Show file tree
Hide file tree
Showing 18 changed files with 37 additions and 214 deletions.
14 changes: 2 additions & 12 deletions content/browser/appcache/appcache_request_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -645,18 +645,8 @@ bool AppCacheRequestHandler::MaybeCreateLoaderForResponse(

absl::optional<SubresourceLoaderParams>
AppCacheRequestHandler::MaybeCreateSubresourceLoaderParams() {
if (!should_create_subresource_loader_)
return absl::nullopt;

// The factory is destroyed when the renderer drops the connection.
mojo::PendingRemote<network::mojom::URLLoaderFactory> factory_remote;

AppCacheSubresourceURLFactory::CreateURLLoaderFactory(
appcache_host_, factory_remote.InitWithNewPipeAndPassReceiver());

SubresourceLoaderParams params;
params.pending_appcache_loader_factory = std::move(factory_remote);
return absl::optional<SubresourceLoaderParams>(std::move(params));
// TODO(enne): remove the rest of this file.
return absl::nullopt;
}

void AppCacheRequestHandler::MaybeCreateSubresourceLoader(
Expand Down
2 changes: 0 additions & 2 deletions content/browser/navigation_subresource_loader_params.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ SubresourceLoaderParams::SubresourceLoaderParams(

SubresourceLoaderParams& SubresourceLoaderParams::operator=(
SubresourceLoaderParams&& other) {
pending_appcache_loader_factory =
std::move(other.pending_appcache_loader_factory);
controller_service_worker_info =
std::move(other.controller_service_worker_info);
controller_service_worker_object_host =
Expand Down
6 changes: 0 additions & 6 deletions content/browser/navigation_subresource_loader_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ struct CONTENT_EXPORT SubresourceLoaderParams {
SubresourceLoaderParams(SubresourceLoaderParams&& other);
SubresourceLoaderParams& operator=(SubresourceLoaderParams&& other);

// For AppCache.
// Subresource loader factory info for appcache, that is to be used to
// create a subresource loader in the renderer.
mojo::PendingRemote<network::mojom::URLLoaderFactory>
pending_appcache_loader_factory;

// For ServiceWorkers.
// The controller service worker, non-null if the frame is to be
// controlled by the service worker.
Expand Down
34 changes: 0 additions & 34 deletions content/browser/renderer_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7783,40 +7783,6 @@ void RenderFrameHostImpl::CommitNavigation(
.GetTupleOrPrecursorTupleIfOpaque()
.scheme();

// NOTE: On Network Service navigations, we want to ensure that a frame is
// given everything it will need to load any accessible subresources. We
// however only do this for cross-document navigations, because the
// alternative would be redundant effort.
if (subresource_loader_params &&
subresource_loader_params->pending_appcache_loader_factory.is_valid()) {
// If the caller has supplied a factory for AppCache, use it.
mojo::Remote<network::mojom::URLLoaderFactory> appcache_remote(std::move(
subresource_loader_params->pending_appcache_loader_factory));

mojo::PendingRemote<network::mojom::URLLoaderFactory>
appcache_proxied_remote;
auto appcache_proxied_receiver =
appcache_proxied_remote.InitWithNewPipeAndPassReceiver();
bool use_proxy =
GetContentClient()->browser()->WillCreateURLLoaderFactory(
browser_context, this, GetProcess()->GetID(),
ContentBrowserClient::URLLoaderFactoryType::kDocumentSubResource,
subresource_loader_factories_config.origin(),
/*navigation_id=*/absl::nullopt,
subresource_loader_factories_config.ukm_source_id(),
&appcache_proxied_receiver, /*header_client=*/nullptr,
/*bypass_redirect_checks=*/nullptr,
/*disable_secure_dns=*/nullptr, /*factory_override=*/nullptr);
if (use_proxy) {
appcache_remote->Clone(std::move(appcache_proxied_receiver));
appcache_remote.reset();
appcache_remote.Bind(std::move(appcache_proxied_remote));
}

subresource_loader_factories->pending_appcache_factory() =
appcache_remote.Unbind();
}

ContentBrowserClient::NonNetworkURLLoaderFactoryMap non_network_factories;

// Set up the default factory.
Expand Down
2 changes: 0 additions & 2 deletions content/browser/worker_host/shared_worker_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ class SharedWorkerHostTest : public testing::Test {
mojo::PendingRemote<network::mojom::URLLoaderFactory>
loader_factory_remote =
network::NotImplementedURLLoaderFactory::Create();
subresource_loader_params->pending_appcache_loader_factory =
std::move(loader_factory_remote);

// Set up for service worker.
auto service_worker_handle =
Expand Down
7 changes: 0 additions & 7 deletions content/browser/worker_host/worker_script_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,6 @@ void DidCreateScriptLoader(
bool success) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

// If a URLLoaderFactory for AppCache is supplied, use that.
if (subresource_loader_params &&
subresource_loader_params->pending_appcache_loader_factory) {
subresource_loader_factories->pending_appcache_factory() =
std::move(subresource_loader_params->pending_appcache_loader_factory);
}

// Prepare the controller service worker info to pass to the renderer.
blink::mojom::ControllerServiceWorkerInfoPtr controller;
base::WeakPtr<ServiceWorkerObjectHost> controller_service_worker_object_host;
Expand Down
4 changes: 2 additions & 2 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2887,7 +2887,7 @@ void RenderFrameImpl::CommitNavigationWithParams(
this, std::move(container_info),
std::move(controller_service_worker_info),
network::SharedURLLoaderFactory::Create(
new_loader_factories->CloneWithoutAppCacheFactory()));
new_loader_factories->Clone()));
}

WebString subresource_filter = navigation_params->response.HttpHeaderField(
Expand Down Expand Up @@ -3363,7 +3363,7 @@ RenderFrameImpl::CreateWorkerFetchContext() {
blink::WebDedicatedOrSharedWorkerFetchContext::Create(
provider->context(), GetWebView()->GetRendererPreferences(),
std::move(watcher_receiver), GetLoaderFactoryBundle()->Clone(),
GetLoaderFactoryBundle()->CloneWithoutAppCacheFactory(),
GetLoaderFactoryBundle()->Clone(),
/*pending_subresource_loader_updater=*/mojo::NullReceiver(),
web_cors_exempt_header_list,
std::move(pending_resource_load_info_notifier));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ DedicatedWorkerHostFactoryClient::CloneWorkerFetchContext(
->CloneForNestedWorker(
service_worker_provider_context_.get(),
subresource_loader_factory_bundle_->Clone(),
subresource_loader_factory_bundle_
->CloneWithoutAppCacheFactory(),
subresource_loader_factory_bundle_->Clone(),
std::move(pending_subresource_loader_updater_),
std::move(task_runner));
} else {
Expand Down Expand Up @@ -119,7 +118,7 @@ DedicatedWorkerHostFactoryClient::CreateWorkerFetchContext(
service_worker_provider_context_.get(), renderer_preference,
std::move(watcher_receiver),
subresource_loader_factory_bundle_->Clone(),
subresource_loader_factory_bundle_->CloneWithoutAppCacheFactory(),
subresource_loader_factory_bundle_->Clone(),
std::move(pending_subresource_loader_updater_),
web_cors_exempt_header_list,
std::move(pending_resource_load_info_notifier));
Expand Down
5 changes: 2 additions & 3 deletions content/renderer/worker/embedded_shared_worker_stub.cc
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,9 @@ EmbeddedSharedWorkerStub::CreateWorkerFetchContext(
mojo::PendingReceiver<blink::mojom::RendererPreferenceWatcher>
preference_watcher_receiver,
const std::vector<std::string>& cors_exempt_header_list) {
// Make the factory used for service worker network fallback (that should
// skip AppCache if it is provided).
// Make the factory used for service worker network fallback.
std::unique_ptr<network::PendingSharedURLLoaderFactory> fallback_factory =
subresource_loader_factory_bundle_->CloneWithoutAppCacheFactory();
subresource_loader_factory_bundle_->Clone();

blink::WebVector<blink::WebString> web_cors_exempt_header_list(
cors_exempt_header_list.size());
Expand Down
15 changes: 0 additions & 15 deletions third_party/blink/common/loader/url_loader_factory_bundle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ scoped_refptr<network::SharedURLLoaderFactory>
PendingURLLoaderFactoryBundle::CreateFactory() {
auto other = std::make_unique<PendingURLLoaderFactoryBundle>();
other->pending_default_factory_ = std::move(pending_default_factory_);
other->pending_appcache_factory_ = std::move(pending_appcache_factory_);
other->pending_scheme_specific_factories_ =
std::move(pending_scheme_specific_factories_);
other->pending_isolated_world_factories_ =
Expand Down Expand Up @@ -93,10 +92,6 @@ network::mojom::URLLoaderFactory* URLLoaderFactoryBundle::GetFactory(
return it2->second.get();
}

// AppCache factory must be used if it's given.
if (appcache_factory_)
return appcache_factory_.get();

// Hitting the DCHECK below means that a subresource load has unexpectedly
// happened in a speculative frame (or in a test frame created via
// RenderViewTest). This most likely indicates a bug somewhere else.
Expand Down Expand Up @@ -137,11 +132,6 @@ URLLoaderFactoryBundle::Clone() {
CloneRemoteMapToPendingRemoteMap(isolated_world_factories_),
bypass_redirect_checks_);

if (appcache_factory_) {
appcache_factory_->Clone(pending_factories->pending_appcache_factory()
.InitWithNewPipeAndPassReceiver());
}

return pending_factories;
}

Expand All @@ -156,11 +146,6 @@ void URLLoaderFactoryBundle::Update(
default_factory_.Bind(
std::move(pending_factories->pending_default_factory()));
}
if (pending_factories->pending_appcache_factory()) {
appcache_factory_.reset();
appcache_factory_.Bind(
std::move(pending_factories->pending_appcache_factory()));
}
BindPendingRemoteMapToRemoteMap(
&scheme_specific_factories_,
std::move(pending_factories->pending_scheme_specific_factories()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,6 @@ mojo::PendingRemote<network::mojom::URLLoaderFactory> Traits::default_factory(
return std::move(bundle->pending_default_factory());
}

// static
mojo::PendingRemote<network::mojom::URLLoaderFactory> Traits::appcache_factory(
BundleInfoType& bundle) {
return std::move(bundle->pending_appcache_factory());
}

// static
blink::PendingURLLoaderFactoryBundle::SchemeMap
Traits::scheme_specific_factories(BundleInfoType& bundle) {
Expand All @@ -51,8 +45,6 @@ bool Traits::Read(blink::mojom::URLLoaderFactoryBundleDataView data,

(*out_bundle)->pending_default_factory() = data.TakeDefaultFactory<
mojo::PendingRemote<network::mojom::URLLoaderFactory>>();
(*out_bundle)->pending_appcache_factory() = data.TakeAppcacheFactory<
mojo::PendingRemote<network::mojom::URLLoaderFactory>>();
if (!data.ReadSchemeSpecificFactories(
&(*out_bundle)->pending_scheme_specific_factories())) {
return false;
Expand Down
12 changes: 0 additions & 12 deletions third_party/blink/public/common/loader/url_loader_factory_bundle.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,6 @@ class BLINK_COMMON_EXPORT PendingURLLoaderFactoryBundle
return pending_default_factory_;
}

mojo::PendingRemote<network::mojom::URLLoaderFactory>&
pending_appcache_factory() {
return pending_appcache_factory_;
}

SchemeMap& pending_scheme_specific_factories() {
return pending_scheme_specific_factories_;
}
Expand All @@ -85,8 +80,6 @@ class BLINK_COMMON_EXPORT PendingURLLoaderFactoryBundle

mojo::PendingRemote<network::mojom::URLLoaderFactory>
pending_default_factory_;
mojo::PendingRemote<network::mojom::URLLoaderFactory>
pending_appcache_factory_;
SchemeMap pending_scheme_specific_factories_;

// TODO(https://crbug.com/1098410): Remove the
Expand Down Expand Up @@ -156,11 +149,6 @@ class BLINK_COMMON_EXPORT URLLoaderFactoryBundle
// context should not be given access to the network.
mojo::Remote<network::mojom::URLLoaderFactory> default_factory_;

// |appcache_factory_| is a special loader factory that intercepts
// requests when the context has AppCache. See also
// AppCacheSubresourceURLFactory.
mojo::Remote<network::mojom::URLLoaderFactory> appcache_factory_;

// Map from URL scheme to Remote<URLLoaderFactory> for handling URL requests
// for schemes not handled by the |default_factory_|. See also
// PendingURLLoaderFactoryBundle::SchemeMap and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ struct URLLoaderFactoryBundle {
map<url.mojom.Origin, pending_remote<network.mojom.URLLoaderFactory>>
isolated_world_factories;

// A special factory that is used for AppCache.
// TODO(https://crbug.com/582750): Drop this when AppCache is deprecated.
pending_remote<network.mojom.URLLoaderFactory>? appcache_factory;

// Whether redirect checks should be bypassed, since they are happening in the
// browser.
bool bypass_redirect_checks = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ class BLINK_PLATFORM_EXPORT ChildPendingURLLoaderFactoryBundle
ChildPendingURLLoaderFactoryBundle(
mojo::PendingRemote<network::mojom::URLLoaderFactory>
pending_default_factory,
mojo::PendingRemote<network::mojom::URLLoaderFactory>
pending_default_network_factory,
SchemeMap pending_scheme_specific_factories,
OriginMap pending_isolated_world_factories,
mojo::PendingRemote<network::mojom::URLLoaderFactory>
Expand All @@ -58,7 +56,6 @@ class BLINK_PLATFORM_EXPORT ChildPendingURLLoaderFactoryBundle
std::unique_ptr<ChildPendingURLLoaderFactoryBundle> pending_bundle(
new ChildPendingURLLoaderFactoryBundle(
std::move(pending_default_factory),
{}, // pending_default_network_factory
{}, // pending_scheme_specific_factories
{}, // pending_isolated_world_factories
{}, // pending_prefetch_loader_factory
Expand Down Expand Up @@ -100,13 +97,6 @@ class BLINK_PLATFORM_EXPORT ChildURLLoaderFactoryBundle
override;
std::unique_ptr<network::PendingSharedURLLoaderFactory> Clone() override;

// Does the same as Clone(), but without cloning the appcache_factory_.
// This is used for creating a bundle for network fallback loading with
// Service Workers (where AppCache must be skipped), and only when
// claim() is called.
virtual std::unique_ptr<network::PendingSharedURLLoaderFactory>
CloneWithoutAppCacheFactory();

std::unique_ptr<ChildPendingURLLoaderFactoryBundle> PassInterface();

void Update(
Expand All @@ -124,9 +114,6 @@ class BLINK_PLATFORM_EXPORT ChildURLLoaderFactoryBundle
~ChildURLLoaderFactoryBundle() override;

private:
std::unique_ptr<network::PendingSharedURLLoaderFactory> CloneInternal(
bool include_appcache);

mojo::Remote<network::mojom::URLLoaderFactory> prefetch_loader_factory_;

std::map<GURL, mojom::TransferrableURLLoaderPtr> subresource_overrides_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ class BLINK_PLATFORM_EXPORT TrackedChildPendingURLLoaderFactoryBundle
TrackedChildPendingURLLoaderFactoryBundle(
mojo::PendingRemote<network::mojom::URLLoaderFactory>
pending_default_factory,
mojo::PendingRemote<network::mojom::URLLoaderFactory>
pending_appcache_factory,
SchemeMap pending_scheme_specific_factories,
OriginMap pending_isolated_world_factories,
mojo::PendingRemote<network::mojom::URLLoaderFactory>
Expand Down Expand Up @@ -135,8 +133,6 @@ class BLINK_PLATFORM_EXPORT HostChildURLLoaderFactoryBundle
// ChildURLLoaderFactoryBundle overrides.
// Returns |std::unique_ptr<TrackedChildPendingURLLoaderFactoryBundle>|.
std::unique_ptr<network::PendingSharedURLLoaderFactory> Clone() override;
std::unique_ptr<network::PendingSharedURLLoaderFactory>
CloneWithoutAppCacheFactory() override;
bool IsHostChildURLLoaderFactoryBundle() const override;

// Update this bundle with |info|, and post cloned |info| to tracked bundles.
Expand Down
Loading

0 comments on commit be0664d

Please sign in to comment.