Skip to content

Commit

Permalink
Move servable_ etc. to PrefetchResponseReader
Browse files Browse the repository at this point in the history 
This CL further moves members needed for serving
from `PrefetchStreamingURLLoader` to
`PrefetchResponseReader`, as preparation to make
PrefetchContainer servable multiple times as long as
`PrefetchResponseReader` is alive.

This CL slightly changes the ordering
between setting `head_` and calling
`on_prefetch_response_started_callback_`
in `PrefetchStreamingURLLoader::OnReceiveResponse()`,
but this shouldn't affect the behavior because
the dependency between those two were removed by
https://chromium-review.googlesource.com/c/chromium/src/+/4671133.

Bug: 1449360
Change-Id: I4675153f240da0704aafe09eb807e443e41995bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4671134
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Max Curran <curranmax@chromium.org>
Reviewed-by: Lingqi Chi <lingqi@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184454}
  • Loading branch information
@hiroshige-g
hiroshige-g authored and Chromium LUCI CQ committed Aug 16, 2023
1 parent f2bff59 commit 334d0fa
Show file tree
Hide file tree
Showing 8 changed files with 171 additions and 121 deletions.
49 changes: 36 additions & 13 deletions content/browser/preloading/prefetch/prefetch_container.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,19 @@ PrefetchStreamingURLLoader* PrefetchContainer::GetLastStreamingURLLoader()
return streaming_loaders_.back().get();
}

const PrefetchResponseReader* PrefetchContainer::GetNonRedirectResponseReader()
const {
if (redirect_chain_.empty()) {
return nullptr;
}
if (!redirect_chain_.back()->response_reader_->GetHead()) {
// Either the last PrefetchResponseReader is for a redirect response, or for
// a final response not yet receiving its header.
return nullptr;
}
return redirect_chain_.back()->response_reader_.get();
}

PrefetchResponseReader::RequestHandler
PrefetchContainer::Reader::CreateRequestHandler() {
return GetPrefetchContainer()->CreateRequestHandlerInternal(*this);
Expand Down Expand Up @@ -744,7 +757,6 @@ bool PrefetchContainer::HasStreamingURLLoadersForTest() const {
}

void PrefetchContainer::ResetAllStreamingURLLoaders() {
CHECK(!streaming_loaders_.empty());
for (auto& streaming_loader : streaming_loaders_) {
// The PrefetchStreamingURLLoader and PrefetchResponseReader can be deleted
// in one of its callbacks, so instead of deleting it immediately, it is
Expand Down Expand Up @@ -807,13 +819,13 @@ void PrefetchContainer::OnPrefetchComplete() {
UMA_HISTOGRAM_COUNTS_100("PrefetchProxy.Prefetch.RedirectChainSize",
redirect_chain_.size());

if (streaming_loaders_.empty()) {
if (!GetNonRedirectResponseReader()) {
return;
}

UpdatePrefetchRequestMetrics(
GetLastStreamingURLLoader()->GetCompletionStatus(),
GetLastStreamingURLLoader()->GetHead());
GetNonRedirectResponseReader()->GetCompletionStatus(),
GetNonRedirectResponseReader()->GetHead());
UpdateServingPageMetrics();
}

Expand All @@ -839,9 +851,12 @@ void PrefetchContainer::UpdatePrefetchRequestMetrics(

bool PrefetchContainer::ShouldBlockUntilHeadReceived() const {
// Can only block until head if the request has been started using a streaming
// URL loader and head hasn't been received yet.
if (streaming_loaders_.empty() || GetLastStreamingURLLoader()->GetHead() ||
GetLastStreamingURLLoader()->Failed()) {
// URL loader...
if (streaming_loaders_.empty() || GetLastStreamingURLLoader()->Failed()) {
return false;
}
// ... and head hasn't been received yet.
if (GetNonRedirectResponseReader()) {
return false;
}
return PrefetchShouldBlockUntilHead(prefetch_type_.GetEagerness());
Expand All @@ -861,10 +876,17 @@ void PrefetchContainer::ResetBlockUntilHeadTimer() {

bool PrefetchContainer::IsPrefetchServable(
base::TimeDelta cacheable_duration) const {
// Whether or not the response (either full or partial) from the streaming URL
// loader is servable.
return !streaming_loaders_.empty() &&
GetLastStreamingURLLoader()->Servable(cacheable_duration);
// Currently `CreateRequestHandler()` requires the corresponding streaming
// loader.
// TODO(crbug.com/1449360): Remove this requirement.
if (streaming_loaders_.empty()) {
return false;
}

// Whether or not the non-redirect response (either fully or partially
// received body) is servable.
return GetNonRedirectResponseReader() &&
GetNonRedirectResponseReader()->Servable(cacheable_duration);
}

bool PrefetchContainer::Reader::DoesCurrentURLToServeMatch(
Expand Down Expand Up @@ -905,8 +927,9 @@ const GURL& PrefetchContainer::Reader::GetCurrentURLToServe() const {
}

const network::mojom::URLResponseHead* PrefetchContainer::GetHead() {
PrefetchStreamingURLLoader* streaming_loader = GetLastStreamingURLLoader();
return streaming_loader ? streaming_loader->GetHead() : nullptr;
return GetNonRedirectResponseReader()
? GetNonRedirectResponseReader()->GetHead()
: nullptr;
}

void PrefetchContainer::SetServingPageMetrics(
Expand Down
13 changes: 11 additions & 2 deletions content/browser/preloading/prefetch/prefetch_container.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,19 @@ class CONTENT_EXPORT PrefetchContainer {

bool HasStreamingURLLoadersForTest() const;

// Returns the last |PrefetchStreamingURLLoader| from |streaming_loaders_|.
// This URL loader should be used when fetching the prefetch.
// Returns the last |PrefetchStreamingURLLoader| from |streaming_loaders_|,
// i.e. the URL loader being used for prefetching the current redirect hop.
// This method should be used during prefetching and shouldn't be called for
// serving purpose.
//
// TODO(https://crbug.com/1449360): Migrate callers (e.g. to
// GetNonRedirectResponseReader()) that don't meet this criteria.
PrefetchStreamingURLLoader* GetLastStreamingURLLoader() const;

// Returns the PrefetchResponseReader corresponding to the last non-redirect
// response, if already received its head, or otherwise nullptr.
const PrefetchResponseReader* GetNonRedirectResponseReader() const;

// Clears all |PrefetchStreamingURLLoader|s and |PrefetchResponseReader|s from
// |streaming_loaders_|.
void ResetAllStreamingURLLoaders();
Expand Down
2 changes: 1 addition & 1 deletion content/browser/preloading/prefetch/prefetch_container_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ TEST_F(PrefetchContainerTest, MultipleStreamingURLLoaders) {
EXPECT_EQ(prefetch_container->GetLastStreamingURLLoader(), nullptr);

EXPECT_FALSE(prefetch_container->IsPrefetchServable(base::TimeDelta::Max()));
EXPECT_FALSE(prefetch_container->GetHead());
EXPECT_TRUE(prefetch_container->GetHead());

EXPECT_TRUE(streaming_loaders[1]);
base::RunLoop().RunUntilIdle();
Expand Down
18 changes: 7 additions & 11 deletions content/browser/preloading/prefetch/prefetch_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1332,19 +1332,15 @@ void PrefetchService::OnPrefetchResponseCompleted(
RecordPrefetchProxyPrefetchMainframeBodyLength(body_length);
}

if (prefetch_container->GetLastStreamingURLLoader()) {
if (!prefetch_container->IsPrefetchServable(PrefetchCacheableDuration())) {
// If the prefetch from the streaming URL loader cannot be served at this
// point, then it can be discarded.
if (!prefetch_container->GetLastStreamingURLLoader()->Servable(
PrefetchCacheableDuration())) {
prefetch_container->ResetAllStreamingURLLoaders();
} else {
PrefetchDocumentManager* prefetch_document_manager =
prefetch_container->GetPrefetchDocumentManager();
if (prefetch_document_manager) {
prefetch_document_manager->OnPrefetchSuccessful(
prefetch_container.get());
}
prefetch_container->ResetAllStreamingURLLoaders();
} else {
PrefetchDocumentManager* prefetch_document_manager =
prefetch_container->GetPrefetchDocumentManager();
if (prefetch_document_manager) {
prefetch_document_manager->OnPrefetchSuccessful(prefetch_container.get());
}
}

Expand Down
101 changes: 56 additions & 45 deletions content/browser/preloading/prefetch/prefetch_streaming_url_loader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void PrefetchStreamingURLLoader::SetResponseReader(
}
}

bool PrefetchStreamingURLLoader::Servable(
bool PrefetchResponseReader::Servable(
base::TimeDelta cacheable_duration) const {
// If the response hasn't been received yet (meaning response_complete_time_
// is absl::nullopt), we can still serve the prefetch (depending on |head_|).
Expand Down Expand Up @@ -151,24 +151,22 @@ void PrefetchStreamingURLLoader::OnReceiveResponse(
// Cached metadata is not supported for prefetch.
cached_metadata.reset();

head_ = std::move(head);
head_->was_in_prefetch_cache = true;
head->was_in_prefetch_cache = true;

// Checks head to determine if the prefetch can be served.
DCHECK(on_prefetch_response_started_callback_);
status_ = std::move(on_prefetch_response_started_callback_).Run(head_.get());
status_ = std::move(on_prefetch_response_started_callback_).Run(head.get());

// Update servable_ based on the returned status_
bool servable = false;
switch (status_) {
case PrefetchStreamingURLLoaderStatus::kHeadReceivedWaitingOnBody:
servable_ = true;
servable = true;
break;
case PrefetchStreamingURLLoaderStatus::kPrefetchWasDecoy:
case PrefetchStreamingURLLoaderStatus::kFailedInvalidHead:
case PrefetchStreamingURLLoaderStatus::kFailedInvalidHeaders:
case PrefetchStreamingURLLoaderStatus::kFailedNon2XX:
case PrefetchStreamingURLLoaderStatus::kFailedMIMENotSupported:
servable_ = false;
break;
case PrefetchStreamingURLLoaderStatus::kWaitingOnHead:
case PrefetchStreamingURLLoaderStatus::kRedirected_DEPRECATED:
Expand All @@ -189,19 +187,21 @@ void PrefetchStreamingURLLoader::OnReceiveResponse(
break;
}

if (!servable_) {
if (on_received_head_callback_) {
std::move(on_received_head_callback_).Run();
}

return;
if (servable) {
head->navigation_delivery_type =
network::mojom::NavigationDeliveryType::kNavigationalPrefetch;
} else {
// Discard `body` for non-servable cases, to keep the existing behavior and
// also because `body` is not used.
body.reset();
}

head_->navigation_delivery_type =
network::mojom::NavigationDeliveryType::kNavigationalPrefetch;

// `head` and `body` are discarded if `response_reader_` is `nullptr`, because
// it means the `PrefetchResponseReader` is deleted and thus we no longer
// serve the prefetched result.
if (response_reader_) {
response_reader_->OnReceiveResponse(head_->Clone(), std::move(body));
response_reader_->OnReceiveResponse(servable, std::move(head),
std::move(body));
}

if (on_received_head_callback_) {
Expand Down Expand Up @@ -256,7 +256,6 @@ void PrefetchStreamingURLLoader::HandleRedirect(
}
break;
case PrefetchStreamingURLLoaderStatus::kFailedInvalidRedirect:
servable_ = false;
if (on_received_head_callback_) {
std::move(on_received_head_callback_).Run();
}
Expand Down Expand Up @@ -303,34 +302,30 @@ void PrefetchStreamingURLLoader::OnComplete(
DisconnectPrefetchURLLoaderMojo();
timeout_timer_.AbandonAndStop();

completion_status_ = completion_status;
response_complete_time_ = base::TimeTicks::Now();

if (status_ == PrefetchStreamingURLLoaderStatus::kWaitingOnHead ||
status_ == PrefetchStreamingURLLoaderStatus::kHeadReceivedWaitingOnBody) {
status_ = completion_status_->error_code == net::OK
status_ = completion_status.error_code == net::OK
? PrefetchStreamingURLLoaderStatus::kSuccessfulNotServed
: PrefetchStreamingURLLoaderStatus::kFailedNetError;
} else if (status_ == PrefetchStreamingURLLoaderStatus::
kSuccessfulServedBeforeCompletion &&
completion_status_->error_code != net::OK) {
completion_status.error_code != net::OK) {
status_ = PrefetchStreamingURLLoaderStatus::kFailedNetErrorButServed;
}

if (completion_status_->error_code != net::OK) {
if (response_reader_) {
response_reader_->OnComplete(completion_status);
}

if (completion_status.error_code != net::OK) {
// Note that we may have already started serving the prefetch if it was
// marked as servable in |OnReceiveResponse|.
servable_ = false;
if (on_received_head_callback_) {
std::move(on_received_head_callback_).Run();
}
}

std::move(on_prefetch_response_completed_callback_)
.Run(completion_status_.value());
if (response_reader_) {
response_reader_->OnComplete(completion_status_.value());
}
std::move(on_prefetch_response_completed_callback_).Run(completion_status);
}

void PrefetchStreamingURLLoader::OnStartServing() {
Expand All @@ -341,12 +336,13 @@ void PrefetchStreamingURLLoader::OnStartServing() {
kStopSwitchInNetworkContextForRedirect) {
status_ = PrefetchStreamingURLLoaderStatus::
kServedSwitchInNetworkContextForRedirect;
} else if (response_reader_ &&
response_reader_->GetCompletionStatus().has_value()) {
status_ =
PrefetchStreamingURLLoaderStatus::kSuccessfulServedAfterCompletion;
} else {
status_ =
completion_status_.has_value()
? PrefetchStreamingURLLoaderStatus::kSuccessfulServedAfterCompletion
: PrefetchStreamingURLLoaderStatus::
kSuccessfulServedBeforeCompletion;
PrefetchStreamingURLLoaderStatus::kSuccessfulServedBeforeCompletion;
}
}

Expand Down Expand Up @@ -449,15 +445,25 @@ void PrefetchResponseReader::RunEventQueue() {
void PrefetchResponseReader::OnComplete(
network::URLLoaderCompletionStatus completion_status) {
DCHECK(!last_event_added_);
DCHECK(!response_complete_time_);
DCHECK(!completion_status_);

last_event_added_ = true;
response_complete_time_ = base::TimeTicks::Now();
completion_status_ = completion_status;

if (completion_status.error_code != net::OK) {
servable_ = false;
}

if (serving_url_loader_client_ &&
event_queue_status_ == EventQueueStatus::kFinished) {
ForwardCompletionStatus(completion_status);
ForwardCompletionStatus();
return;
}
AddEventToQueue(
base::BindOnce(&PrefetchResponseReader::ForwardCompletionStatus,
base::Unretained(this), completion_status));
base::Unretained(this)));
}

void PrefetchResponseReader::OnReceiveEarlyHints(
Expand Down Expand Up @@ -502,19 +508,25 @@ void PrefetchResponseReader::HandleRedirect(
}

void PrefetchResponseReader::OnReceiveResponse(
bool servable,
network::mojom::URLResponseHeadPtr head,
mojo::ScopedDataPipeConsumerHandle body) {
DCHECK(!last_event_added_);
DCHECK(event_queue_status_ == EventQueueStatus::kNotStarted);
DCHECK(!servable_);
DCHECK(!head_);
DCHECK(head);

servable_ = servable;
head_ = std::move(head);
AddEventToQueue(base::BindOnce(&PrefetchResponseReader::ForwardResponse,
base::Unretained(this), std::move(head),
std::move(body)));
base::Unretained(this), std::move(body)));
}

void PrefetchResponseReader::ForwardCompletionStatus(
network::URLLoaderCompletionStatus completion_status) {
void PrefetchResponseReader::ForwardCompletionStatus() {
DCHECK(serving_url_loader_client_);
serving_url_loader_client_->OnComplete(completion_status);
DCHECK(completion_status_);
serving_url_loader_client_->OnComplete(completion_status_.value());
}

void PrefetchResponseReader::ForwardEarlyHints(
Expand All @@ -537,13 +549,12 @@ void PrefetchResponseReader::ForwardRedirect(
}

void PrefetchResponseReader::ForwardResponse(
network::mojom::URLResponseHeadPtr head,
mojo::ScopedDataPipeConsumerHandle body) {
DCHECK(serving_url_loader_client_);
DCHECK(head);
DCHECK(head_);
DCHECK(body);
serving_url_loader_client_->OnReceiveResponse(std::move(head),
std::move(body), absl::nullopt);
serving_url_loader_client_->OnReceiveResponse(head_->Clone(), std::move(body),
absl::nullopt);
}

void PrefetchResponseReader::FollowRedirect(
Expand Down

0 comments on commit 334d0fa

Please sign in to comment.