OnReceiveResponse taking metadata

Chromium change:
https://chromium.googlesource.com/chromium/src/+/154cddb1a0e16d9b10f8b9c16c91e20909d71425

commit 154cddb1a0e16d9b10f8b9c16c91e20909d71425
Author: Leszek Swirski <leszeks@chromium.org>
Date:   Mon Aug 29 17:57:19 2022 +0000

    [loader] Send cached metadata as part of OnReceiveResponse

    CachedMetadata is an opaque data blob optionally stored alongside
    responses in the response cache. It stores data specific to the
    response -- in particular, for JavaScript files it will store the V8
    code cache. Before this patch, CachedMetadata was sent to the renderer
    using a separate OnReceiveCachedMetadata message, after
    OnReceiveResponse.

    This mechanism is only used by the Service Worker installed resources; in the HTTP cache case, there is a separate "isolated code cache"
    which is queried at the same time as the HTTP cache and the two
    requests are synchronised (cf. https://crbug.com/812168). This means
    that, before this patch, HTTP code caches were received at the same
    time as the body (thanks to the synchronisation), but Service Worker code caches were received _after_ the body (which is sent as part of
    OnReceiveResponse). The consequence was that Service Worker code
    caches were dropped in favour of (re)parsing the script response body,
    as that parse was started before the CachedMetadata was received.

    Now, rather than a separate OnReceiveCachedMetadata, send an optional
    CachedMetadata alongside the headers and body in OnReceiveResponse.
    This allows the receiver of the response to more carefully interleave
    dealing with the metadata and body.

    Review note: Since the Service Worker is the only user of this API, the
    interesting parts of this patch are the handling of service worker
    and the changes in the Blink UrlLoaderClient. Unfortunately, there are
    other implementers of the UrlLoaderClient interface which either
    ignore or trivially pass through the cached metadata, and the
    signature change of the OnReceiveResponse method makes this CL
    impossible to split by subdirectory for these trivial changes.

    Bug: 1350077
This commit is contained in:
Claudio DeSouza
2022-10-13 16:19:57 -04:00
committed by Emerick Rogul
parent 1dab038e49
commit f794107dea
5 changed files with 20 additions and 20 deletions
@@ -221,9 +221,11 @@ void BraveProxyingURLLoaderFactory::InProgressRequest::OnReceiveEarlyHints(
void BraveProxyingURLLoaderFactory::InProgressRequest::OnReceiveResponse(
network::mojom::URLResponseHeadPtr head,
mojo::ScopedDataPipeConsumerHandle body) {
mojo::ScopedDataPipeConsumerHandle body,
absl::optional<mojo_base::BigBuffer> cached_metadata) {
current_response_head_ = std::move(head);
current_response_body_ = std::move(body);
cached_metadata_ = std::move(cached_metadata);
ctx_->internal_redirect = false;
HandleResponseOrRedirectHeaders(
base::BindRepeating(&InProgressRequest::ContinueToResponseStarted,
@@ -249,11 +251,6 @@ void BraveProxyingURLLoaderFactory::InProgressRequest::OnUploadProgress(
std::move(callback));
}
void BraveProxyingURLLoaderFactory::InProgressRequest::OnReceiveCachedMetadata(
mojo_base::BigBuffer data) {
target_client_->OnReceiveCachedMetadata(std::move(data));
}
void BraveProxyingURLLoaderFactory::InProgressRequest::OnTransferSizeUpdated(
int32_t transfer_size_diff) {
target_client_->OnTransferSizeUpdated(transfer_size_diff);
@@ -367,7 +364,8 @@ void BraveProxyingURLLoaderFactory::InProgressRequest::
}
// Craft the response.
target_client_->OnReceiveResponse(std::move(response), std::move(consumer));
target_client_->OnReceiveResponse(std::move(response), std::move(consumer),
std::move(cached_metadata_));
auto write_data = std::make_unique<WriteData>();
write_data->client = weak_factory_.GetWeakPtr();
@@ -527,7 +525,8 @@ void BraveProxyingURLLoaderFactory::InProgressRequest::
proxied_client_receiver_.Resume();
target_client_->OnReceiveResponse(std::move(current_response_head_),
std::move(current_response_body_));
std::move(current_response_body_),
std::move(cached_metadata_));
}
void BraveProxyingURLLoaderFactory::InProgressRequest::ContinueToBeforeRedirect(
@@ -85,15 +85,16 @@ class BraveProxyingURLLoaderFactory
// network::mojom::URLLoaderClient:
void OnReceiveEarlyHints(
network::mojom::EarlyHintsPtr early_hints) override;
void OnReceiveResponse(network::mojom::URLResponseHeadPtr response_head,
mojo::ScopedDataPipeConsumerHandle body) override;
void OnReceiveResponse(
network::mojom::URLResponseHeadPtr response_head,
mojo::ScopedDataPipeConsumerHandle body,
absl::optional<mojo_base::BigBuffer> cached_metadata) override;
void OnReceiveRedirect(
const net::RedirectInfo& redirect_info,
network::mojom::URLResponseHeadPtr response_head) override;
void OnUploadProgress(int64_t current_position,
int64_t total_size,
OnUploadProgressCallback callback) override;
void OnReceiveCachedMetadata(mojo_base::BigBuffer data) override;
void OnTransferSizeUpdated(int32_t transfer_size_diff) override;
void OnComplete(const network::URLLoaderCompletionStatus& status) override;
@@ -147,6 +148,7 @@ class BraveProxyingURLLoaderFactory
// ExtensionWebRequestEventRouter) through much of the request's lifetime.
// That code supports both Network Service and non-Network Service behavior,
// which is why this weirdness exists here.
absl::optional<mojo_base::BigBuffer> cached_metadata_;
network::mojom::URLResponseHeadPtr current_response_head_;
mojo::ScopedDataPipeConsumerHandle current_response_body_;
scoped_refptr<net::HttpResponseHeaders> override_headers_;
@@ -77,7 +77,8 @@ class SecurityIndicatorTest
mojo::CreateDataPipe(nullptr, producer_handle, consumer_handle);
params->client->OnReceiveResponse(std::move(resource_response),
std::move(consumer_handle));
std::move(consumer_handle),
absl::nullopt);
network::URLLoaderCompletionStatus completion_status;
completion_status.ssl_info = ssl_info;
@@ -66,7 +66,8 @@ void BodySnifferURLLoader::OnReceiveEarlyHints(
void BodySnifferURLLoader::OnReceiveResponse(
network::mojom::URLResponseHeadPtr response_head,
mojo::ScopedDataPipeConsumerHandle body) {
mojo::ScopedDataPipeConsumerHandle body,
absl::optional<mojo_base::BigBuffer> cached_metadata) {
// OnReceiveResponse() shouldn't be called because BodySnifferURLLoader is
// created by WillProcessResponse(), which is equivalent
// to OnReceiveResponse().
@@ -90,10 +91,6 @@ void BodySnifferURLLoader::OnUploadProgress(
std::move(ack_callback));
}
void BodySnifferURLLoader::OnReceiveCachedMetadata(mojo_base::BigBuffer data) {
destination_url_loader_client_->OnReceiveCachedMetadata(std::move(data));
}
void BodySnifferURLLoader::OnTransferSizeUpdated(int32_t transfer_size_diff) {
destination_url_loader_client_->OnTransferSizeUpdated(transfer_size_diff);
}
@@ -68,15 +68,16 @@ class BodySnifferURLLoader : public network::mojom::URLLoaderClient,
// network::mojom::URLLoaderClient implementation (called from the source of
// the response):
void OnReceiveEarlyHints(network::mojom::EarlyHintsPtr early_hints) override;
void OnReceiveResponse(network::mojom::URLResponseHeadPtr response_head,
mojo::ScopedDataPipeConsumerHandle body) override;
void OnReceiveResponse(
network::mojom::URLResponseHeadPtr response_head,
mojo::ScopedDataPipeConsumerHandle body,
absl::optional<mojo_base::BigBuffer> cached_metadata) override;
void OnReceiveRedirect(
const net::RedirectInfo& redirect_info,
network::mojom::URLResponseHeadPtr response_head) override;
void OnUploadProgress(int64_t current_position,
int64_t total_size,
OnUploadProgressCallback ack_callback) override;
void OnReceiveCachedMetadata(mojo_base::BigBuffer data) override;
void OnTransferSizeUpdated(int32_t transfer_size_diff) override;
void OnComplete(const network::URLLoaderCompletionStatus& status) override;