Skip to content

Commit

Permalink
[BestEffortServiceWorker] Use custom URLLoaderFactory in fetch handler
Browse files Browse the repository at this point in the history
This CL is a part of efforts that achieve the dedupe function to
BestEffortServiceWorker.

At a high-level, the dedupe is achieved by reusing the
RaceNetworkRequest response as a response of the corresponding request
in the fetch handler, and do not send a new fetch request.

We can split into some smaller steps to implement this.
1. Generates a token of RaceNetworkRequest, and pass it to the
blink side to make the corresponding request in the fetch handler
discoverable.
2. (This CL) Adds a map of the token and URLLoaderFactory in
ServiceWorkerGlobalScope, and replaces the default URLLoader when the
fetch request is same as the one called in RaceNetworkRequest which is
outside of the fetch handler.
3. Fuses two message pipes into one in a custom URLLoaderClient so that
we can reuse the RaceNetworkRequest result and don't dispatch duplicated
requests in the fetch handler.

In this CL, `network::mojom::URLLoaderFactory` pending remote is added
to DispatchFetchEventParams. In ServiceWorkerGlobalScope, this factory
mojo remote is stored in the map along with the RaceNetworkRequest
token.

When a fetch event happens from worker,
`LoaderFactoryForWorker::CreateURLLoader` is called to dispatch fetch
request. In order to avoid making an additional requset, this CL
replaces the default URLLoader with the one created from the custom
URLLoaderFactory.

This CL adds the mechanism to replace the URLLoader in blink but doesn't
actually pass the custom URLLoaderClient via mojo to keep the CL small,
this mechanism will be used in the descendant CL.

This CL doesn't change default behavior.

Change-Id: Ib3c2ecf850ead1637ef98b9098e83d71ab5a0497
Bug: 1420517
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4561004
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Shunya Shishido <sisidovski@chromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Minoru Chikamune <chikamune@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1150213}
  • Loading branch information
sisidovski authored and Chromium LUCI CQ committed May 29, 2023
1 parent ab60569 commit 08607a9
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@
module blink.mojom;

import "services/network/public/mojom/url_loader.mojom";
import "services/network/public/mojom/url_loader_factory.mojom";
import "third_party/blink/public/mojom/blob/blob.mojom";
import "third_party/blink/public/mojom/fetch/fetch_api_request.mojom";

// Parameters used for dispatching a FetchEvent.
// This struct is used by ServiceWorker main resource or subreosurce loader,
// those are located in both browser and renderer processes. This is used to
// pass parameters to ServiceWorker global scope.
struct DispatchFetchEventParams {
// FetchEvent#request.
FetchAPIRequest request;
Expand All @@ -26,4 +30,10 @@ struct DispatchFetchEventParams {
// Indicates if the RaceNetworkRequest is dispatched outside of the service
// worker. crbug.com/1420517 for more details.
bool did_start_race_network_request = false;

// Used for BestEffortServiceWorker(crbug.com/1420517). Specifies the cusotm
// URLLoaderFactory if the RaceNetworkRequest is triggered. This value is
// referred by the fetch process in blink, in order to detect and dedupe
// the corresponding fetch event in ServiceWorker.
pending_remote<network.mojom.URLLoaderFactory>? race_network_request_loader_factory;
};
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,21 @@ std::unique_ptr<URLLoader> LoaderFactoryForWorker::CreateURLLoader(
std::move(keep_alive_handle), back_forward_cache_loader_helper);
}
}
// URLLoader for RaceNetworkRequest
absl::optional<mojo::PendingRemote<network::mojom::blink::URLLoaderFactory>>
race_network_request_url_loader_factory =
global_scope_->FindRaceNetworkRequestURLLoaderFactory(
request.GetServiceWorkerRaceNetworkRequestToken());
if (race_network_request_url_loader_factory) {
return web_context_
->WrapURLLoaderFactory(
std::move(race_network_request_url_loader_factory.value()))
->CreateURLLoader(
wrapped, freezable_task_runner, unfreezable_task_runner,
std::move(keep_alive_handle), back_forward_cache_loader_helper);
}
} else {
DCHECK(!web_context_->GetScriptLoaderFactory());
CHECK(!web_context_->GetScriptLoaderFactory());
}

return web_context_->GetURLLoaderFactory()->CreateURLLoader(
Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/workers/worker_global_scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ class CORE_EXPORT WorkerGlobalScope
const base::UnguessableToken& GetDevToolsToken() const override;
bool IsInitialized() const final { return !url_.IsNull(); }
CodeCacheHost* GetCodeCacheHost() override;
absl::optional<mojo::PendingRemote<network::mojom::blink::URLLoaderFactory>>
FindRaceNetworkRequestURLLoaderFactory(
const base::UnguessableToken& token) override {
return absl::nullopt;
}

void ExceptionUnhandled(int exception_id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,12 @@ class CORE_EXPORT WorkerOrWorkletGlobalScope
const String& message,
SourceLocation* location);

// Called when BestEffortServiceWorker(crbug.com/1420517) is enabled.
virtual absl::optional<
mojo::PendingRemote<network::mojom::blink::URLLoaderFactory>>
FindRaceNetworkRequestURLLoaderFactory(
const base::UnguessableToken& token) = 0;

protected:
// Sets outside's CSP used for off-main-thread top-level worker script
// fetch.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ class CORE_EXPORT WorkletGlobalScope
const base::UnguessableToken& GetDevToolsToken() const override;
bool IsInitialized() const final { return true; }
CodeCacheHost* GetCodeCacheHost() override;
absl::optional<mojo::PendingRemote<network::mojom::blink::URLLoaderFactory>>
FindRaceNetworkRequestURLLoaderFactory(
const base::UnguessableToken& token) override {
return absl::nullopt;
}

// Returns `blob_url_store_pending_remote_` for use when instantiating the
// PublicURLManager in threaded worklet contexts. This method should only be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1526,6 +1526,14 @@ void ServiceWorkerGlobalScope::StartFetchEvent(

NoteNewFetchEvent(fetch_request);

if (params->race_network_request_loader_factory &&
params->request->service_worker_race_network_request_token) {
race_network_request_loader_factories_.insert(
String(params->request->service_worker_race_network_request_token
->ToString()),
std::move(params->race_network_request_loader_factory));
}

Request* request = Request::Create(
ScriptController()->GetScriptState(), std::move(params->request),
Request::ForServiceWorkerFetchEvent::kTrue);
Expand Down Expand Up @@ -2686,4 +2694,15 @@ bool ServiceWorkerGlobalScope::SetAttributeEventListener(
return WorkerGlobalScope::SetAttributeEventListener(event_type, listener);
}

absl::optional<mojo::PendingRemote<network::mojom::blink::URLLoaderFactory>>
ServiceWorkerGlobalScope::FindRaceNetworkRequestURLLoaderFactory(
const base::UnguessableToken& token) {
mojo::PendingRemote<network::mojom::blink::URLLoaderFactory> result =
race_network_request_loader_factories_.Take(String(token.ToString()));
if (result) {
return result;
}
return absl::nullopt;
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,10 @@ class MODULES_EXPORT ServiceWorkerGlobalScope final
bool SetAttributeEventListener(const AtomicString& event_type,
EventListener* listener) override;

absl::optional<mojo::PendingRemote<network::mojom::blink::URLLoaderFactory>>
FindRaceNetworkRequestURLLoaderFactory(
const base::UnguessableToken& token) final;

protected:
// EventTarget
bool AddEventListenerInternal(
Expand Down Expand Up @@ -736,6 +740,11 @@ class MODULES_EXPORT ServiceWorkerGlobalScope final
mojom::blink::AncestorFrameType ancestor_frame_type_;

blink::BlinkStorageKey storage_key_;

// TODO(crbug.com/918702) WTF::HashMap cannot use base::UnguessableToken as a
// key. As a workaround uses WTF::String as a key instead.
HashMap<String, mojo::PendingRemote<network::mojom::blink::URLLoaderFactory>>
race_network_request_loader_factories_;
};

template <>
Expand Down

0 comments on commit 08607a9

Please sign in to comment.