Skip to content

Commit

Permalink
[BestEffortServiceWorker] Add RaceNetworkRequest token
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. (This CL) Generates a token of RaceNetworkRequest, and pass it to the
blink side to make the corresponding request in the fetch handler
discoverable.
2. 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.

This CL introduce a token of RaceNetworkRequest as a new param to
FetchAPIRequest mojom. A token is passed when RaceNetworkRequest is
dispatched, thus outside of ServiceWorker fetch handler. In this CL
ServiceWorkerMainResourceLoader adds the token. We don't support
subreousrce at this moment. The token is `base::UnguessableToken`.

To identify the same fetch request in the ServiceWorker,
`platform/loader/fetch/resource_request` will have the token as a
member. This member will be used in the child CL.

This CL doesn't change the default behavior.

Change-Id: I54280de1923512d6cd292e82b66c52df0f342790
Bug: 1420517
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4393865
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Minoru Chikamune <chikamune@chromium.org>
Commit-Queue: Shunya Shishido <sisidovski@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1150212}
  • Loading branch information
sisidovski authored and Chromium LUCI CQ committed May 29, 2023
1 parent cdabd3a commit ab60569
Show file tree
Hide file tree
Showing 12 changed files with 85 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,10 @@ void ServiceWorkerFetchDispatcher::DispatchFetchEvent() {
params->preload_url_loader_client_receiver =
std::move(preload_url_loader_client_receiver_);
params->is_offline_capability_check = is_offline_capability_check_;
if (race_network_request_token_) {
params->request->service_worker_race_network_request_token =
race_network_request_token_;
}

// |endpoint()| is owned by |version_|. So it is safe to pass the
// unretained raw pointer of |version_| to OnFetchEventFinished callback.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ class CONTENT_EXPORT ServiceWorkerFetchDispatcher {
scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
int frame_tree_node_id);

void set_race_network_request_token(base::UnguessableToken token) {
race_network_request_token_ = token;
}

private:
class ResponseCallback;
class URLLoaderAssets;
Expand Down Expand Up @@ -141,6 +145,8 @@ class CONTENT_EXPORT ServiceWorkerFetchDispatcher {
mojo::PendingReceiver<network::mojom::URLLoaderClient>
preload_url_loader_client_receiver_;

base::UnguessableToken race_network_request_token_;

// Whether to dispatch an offline-capability-check fetch event.
const bool is_offline_capability_check_ = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ bool ServiceWorkerMainResourceLoader::MaybeStartRaceNetworkRequest(

// Create URLLoader related assets to handle the request triggered by
// RaceNetworkRequset.
fetch_dispatcher_->set_race_network_request_token(
base::UnguessableToken::Create());
auto race_network_request_url_loader_client =
std::make_unique<ServiceWorkerRaceNetworkRequestURLLoaderClient>(
resource_request_, AsWeakPtr());
Expand Down
3 changes: 2 additions & 1 deletion content/common/background_fetch/background_fetch_types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ blink::mojom::FetchAPIRequestPtr BackgroundFetchSettledFetch::CloneRequest(
request->fetch_window_id, request->keepalive, request->is_reload,
request->is_history_navigation, request->devtools_stack_id,
request->trust_token_params.Clone(), request->target_address_space,
request->attribution_reporting_eligibility);
request->attribution_reporting_eligibility,
/*service_worker_race_network_request_token=*/absl::nullopt);
}

} // namespace content
6 changes: 6 additions & 0 deletions third_party/blink/public/mojom/fetch/fetch_api_request.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -211,4 +211,10 @@ struct FetchAPIRequest {

network.mojom.AttributionReportingEligibility attribution_reporting_eligibility =
network.mojom.AttributionReportingEligibility.kUnset;

// Used for BestEffortServiceWorker(crbug.com/1420517). Specifies the ID of
// the RaceNetworkRequest 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.
mojo_base.mojom.UnguessableToken? service_worker_race_network_request_token;
};
3 changes: 3 additions & 0 deletions third_party/blink/renderer/core/fetch/fetch_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,9 @@ void FetchManager::Loader::PerformHTTPFetch() {

request.SetOriginalDestination(fetch_request_data_->OriginalDestination());

request.SetServiceWorkerRaceNetworkRequestToken(
fetch_request_data_->ServiceWorkerRaceNetworkRequestToken());

// "3. Append `Host`, ..."
// FIXME: Implement this when the spec is fixed.

Expand Down
12 changes: 12 additions & 0 deletions third_party/blink/renderer/core/fetch/fetch_request_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "third_party/blink/renderer/core/fetch/fetch_request_data.h"

#include "base/unguessable_token.h"
#include "net/base/request_priority.h"
#include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink.h"
#include "third_party/blink/public/platform/platform.h"
Expand Down Expand Up @@ -205,6 +206,11 @@ FetchRequestData* FetchRequestData::Create(
request->SetAttributionReportingEligibility(
fetch_api_request->attribution_reporting_eligibility);

if (fetch_api_request->service_worker_race_network_request_token) {
request->SetServiceWorkerRaceNetworkRequestToken(
fetch_api_request->service_worker_race_network_request_token.value());
}

return request;
}

Expand Down Expand Up @@ -238,12 +244,18 @@ FetchRequestData* FetchRequestData::CloneExceptBody() {
request->trust_token_params_ = trust_token_params_;
request->attribution_reporting_eligibility_ =
attribution_reporting_eligibility_;
request->service_worker_race_network_request_token_ =
service_worker_race_network_request_token_;
return request;
}

FetchRequestData* FetchRequestData::Clone(ScriptState* script_state,
ExceptionState& exception_state) {
FetchRequestData* request = FetchRequestData::CloneExceptBody();
if (request->service_worker_race_network_request_token_) {
request->service_worker_race_network_request_token_ =
base::UnguessableToken::Null();
}
if (buffer_) {
BodyStreamBuffer* new1 = nullptr;
BodyStreamBuffer* new2 = nullptr;
Expand Down
13 changes: 13 additions & 0 deletions third_party/blink/renderer/core/fetch/fetch_request_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,14 @@ class CORE_EXPORT FetchRequestData final
attribution_reporting_eligibility_ = eligibility;
}

base::UnguessableToken ServiceWorkerRaceNetworkRequestToken() const {
return service_worker_race_network_request_token_;
}
void SetServiceWorkerRaceNetworkRequestToken(
const base::UnguessableToken& token) {
service_worker_race_network_request_token_ = token;
}

void Trace(Visitor*) const;

private:
Expand Down Expand Up @@ -242,6 +250,11 @@ class CORE_EXPORT FetchRequestData final
HeapMojoRemote<network::mojom::blink::URLLoaderFactory> url_loader_factory_;
base::UnguessableToken window_id_;
Member<ExecutionContext> execution_context_;

// A token set only when the fetch request is initiated with ServiceWorker
// RaceNetworkRequest(crbug.com/1420517). When the request is cloned, this
// member shouldn't be copied to the new request.
base::UnguessableToken service_worker_race_network_request_token_;
};

} // namespace blink
Expand Down
22 changes: 22 additions & 0 deletions third_party/blink/renderer/core/fetch/fetch_request_data_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink.h"
#include "third_party/blink/renderer/core/fetch/fetch_header_list.h"
#include "third_party/blink/renderer/platform/bindings/exception_context.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"

namespace blink {

Expand Down Expand Up @@ -79,4 +81,24 @@ TEST(FetchRequestDataTest, CheckTrustTokenParamsAreCopiedWithCreate) {
EXPECT_EQ(*(request_data->TrustTokenParams()), *(trust_token_params_copy));
}

TEST(FetchRequestDataTest, CheckServiceworkerRaceNetworkRequestToken) {
// create a fetch API request instance
auto request = PrepareFetchAPIRequest();
const base::UnguessableToken token = base::UnguessableToken::Create();
request->service_worker_race_network_request_token = token;

// Create FetchRequestData
FetchRequestData* request_data = FetchRequestData::Create(
/*script_state=*/nullptr, std::move(request),
FetchRequestData::ForServiceWorkerFetchEvent::kTrue);
EXPECT_EQ(token, request_data->ServiceWorkerRaceNetworkRequestToken());

// Token is not cloned.
auto exception_state = ExceptionState(
nullptr, ExceptionContext(ExceptionContext::Context::kUnknown, nullptr));
auto* cloned_request_data = request_data->Clone(nullptr, exception_state);
EXPECT_TRUE(
cloned_request_data->ServiceWorkerRaceNetworkRequestToken().is_empty());
}

} // namespace blink
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/fetch/request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ FetchRequestData* CreateCopyOfFetchRequestDataForFetch(
request->SetTrustTokenParams(original->TrustTokenParams());
request->SetAttributionReportingEligibility(
original->AttributionReportingEligibility());
request->SetServiceWorkerRaceNetworkRequestToken(
original->ServiceWorkerRaceNetworkRequestToken());

// When a new request is created from another the destination is always reset
// to be `kEmpty`. In order to facilitate some later checks when a service
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ class ResponsesAccumulator : public RefCounted<ResponsesAccumulator> {
request->fetch_window_id, request->keepalive, request->is_reload,
request->is_history_navigation, request->devtools_stack_id,
request->trust_token_params.Clone(), request->target_address_space,
request->attribution_reporting_eligibility);
request->attribution_reporting_eligibility,
/*service_worker_race_network_request_token=*/absl::nullopt);
cache_remote_->Match(
std::move(request), mojom::blink::CacheQueryOptions::New(),
/*in_related_fetch_event=*/false, /*in_range_fetch_event=*/false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,15 @@ class PLATFORM_EXPORT ResourceRequestHead {
shared_dictionary_writer_enabled_ = shared_dictionary_writer_enabled;
}

base::UnguessableToken GetServiceWorkerRaceNetworkRequestToken() const {
return service_worker_race_network_request_token_;
}

void SetServiceWorkerRaceNetworkRequestToken(
const base::UnguessableToken& token) {
service_worker_race_network_request_token_ = token;
}

private:
const CacheControlHeader& GetCacheControlHeader() const;

Expand Down Expand Up @@ -745,6 +754,8 @@ class PLATFORM_EXPORT ResourceRequestHead {
// TODO(crbug.com/1413922): Remove this flag when we launch
// CompressionDictionaryTransport feature.
bool shared_dictionary_writer_enabled_ = false;

base::UnguessableToken service_worker_race_network_request_token_;
};

class PLATFORM_EXPORT ResourceRequestBody {
Expand Down

0 comments on commit ab60569

Please sign in to comment.