Skip to content

Commit

Permalink
Allow PrefetchService to block until the head of a prefetch is received
Browse files Browse the repository at this point in the history
when trying to serve the prefetch.

This CL allows for a prefetch that has been started but waiting on the
head of the response to be served. In this case, PrefetchService will
block until the head of the prefetch is received, then pass it to
PrefetchURLLoaderInterceptor it is servable.

(cherry picked from commit 2e83b5d)

Bug: 1375852
Change-Id: I32a99bfae3fe739020ad1b5313c0a1a8b6c6a82a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4121433
Reviewed-by: Sreeja Kamishetty <sreejakshetty@chromium.org>
Commit-Queue: Max Curran <curranmax@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1087624}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4134704
Auto-Submit: Max Curran <curranmax@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5481@{#126}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Max Curran authored and Chromium LUCI CQ committed Jan 4, 2023
1 parent b4f1f23 commit 68bad88
Show file tree
Hide file tree
Showing 19 changed files with 774 additions and 239 deletions.
51 changes: 44 additions & 7 deletions content/browser/preloading/prefetch/prefetch_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,12 @@ PreloadingFailureReason ToPreloadingFailureReason(PrefetchStatus status) {

// Please follow go/preloading-dashboard-updates if a new outcome enum or a
// failure reason enum is added.
void SetTriggeringOutcomeAndFailureReasonFromStatus(PreloadingAttempt* attempt,
PrefetchStatus status) {
void SetTriggeringOutcomeAndFailureReasonFromStatus(
PreloadingAttempt* attempt,
absl::optional<PrefetchStatus> old_prefetch_status,
PrefetchStatus new_prefetch_status) {
if (attempt) {
switch (status) {
switch (new_prefetch_status) {
case PrefetchStatus::kPrefetchNotFinishedInTime:
attempt->SetTriggeringOutcome(PreloadingTriggeringOutcome::kRunning);
break;
Expand All @@ -121,6 +123,19 @@ void SetTriggeringOutcomeAndFailureReasonFromStatus(PreloadingAttempt* attempt,
attempt->SetTriggeringOutcome(PreloadingTriggeringOutcome::kReady);
break;
case PrefetchStatus::kPrefetchResponseUsed:
if (old_prefetch_status &&
old_prefetch_status.value() !=
PrefetchStatus::kPrefetchSuccessful &&
old_prefetch_status.value() !=
PrefetchStatus::kPrefetchUsedNoProbe) {
// If the new prefetch status is |kPrefetchResponseUsed| but the
// previous status is not |kPrefetchSuccessful|, then temporarily
// update the triggering outcome to |kReady| to ensure valid
// triggering outcome state transitions. This can occur in cases
// where the prefetch is served before the body is fully received.
attempt->SetTriggeringOutcome(PreloadingTriggeringOutcome::kReady);
}

attempt->SetTriggeringOutcome(PreloadingTriggeringOutcome::kSuccess);
break;
// A decoy is considered eligible because a network request is made for
Expand All @@ -131,7 +146,8 @@ void SetTriggeringOutcomeAndFailureReasonFromStatus(PreloadingAttempt* attempt,
case PrefetchStatus::kPrefetchFailedNetError:
case PrefetchStatus::kPrefetchFailedNon2XX:
case PrefetchStatus::kPrefetchFailedMIMENotSupported:
attempt->SetFailureReason(ToPreloadingFailureReason(status));
attempt->SetFailureReason(
ToPreloadingFailureReason(new_prefetch_status));
break;
case PrefetchStatus::kPrefetchHeldback:
// `kPrefetchAllowed` will soon transition into `kPrefetchNotStarted`.
Expand Down Expand Up @@ -230,10 +246,12 @@ PrefetchContainer::~PrefetchContainer() {
}

void PrefetchContainer::SetPrefetchStatus(PrefetchStatus prefetch_status) {
prefetch_status_ = prefetch_status;
SetHoldbackFromStatus(attempt_.get(), prefetch_status);
SetTriggeringOutcomeAndFailureReasonFromStatus(attempt_.get(),
prefetch_status);
SetTriggeringOutcomeAndFailureReasonFromStatus(
attempt_.get(),
/*old_prefetch_status=*/prefetch_status_,
/*new_prefetch_status=*/prefetch_status);
prefetch_status_ = prefetch_status;
}

PrefetchStatus PrefetchContainer::GetPrefetchStatus() const {
Expand Down Expand Up @@ -299,6 +317,16 @@ bool PrefetchContainer::HaveDefaultContextCookiesChanged() const {
return false;
}

bool PrefetchContainer::HasIsolatedCookieCopyStarted() const {
switch (cookie_copy_status_) {
case CookieCopyStatus::kNotStarted:
return false;
case CookieCopyStatus::kInProgress:
case CookieCopyStatus::kCompleted:
return true;
}
}

bool PrefetchContainer::IsIsolatedCookieCopyInProgress() const {
switch (cookie_copy_status_) {
case CookieCopyStatus::kNotStarted:
Expand Down Expand Up @@ -463,6 +491,15 @@ void PrefetchContainer::UpdatePrefetchRequestMetrics(
completion_status->completion_time - head->load_timing.request_start;
}

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_loader_ || streaming_loader_->GetHead()) {
return false;
}
return PrefetchShouldBlockUntilHead(prefetch_type_.GetEagerness());
}

bool PrefetchContainer::IsPrefetchServable(
base::TimeDelta cacheable_duration) const {
// Whether or not the response (either full or partial) from the streaming URL
Expand Down
5 changes: 5 additions & 0 deletions content/browser/preloading/prefetch/prefetch_container.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ class CONTENT_EXPORT PrefetchContainer {
// context must be copied over to the default network context. These functions
// are used to check and update the status of this process, as well as record
// metrics about how long this process takes.
bool HasIsolatedCookieCopyStarted() const;
bool IsIsolatedCookieCopyInProgress() const;
void OnIsolatedCookieCopyStart();
void OnIsolatedCookiesReadCompleteAndWriteStart();
Expand Down Expand Up @@ -172,6 +173,10 @@ class CONTENT_EXPORT PrefetchContainer {
// resource.
void OnPrefetchComplete();

// Whether or not |PrefetchService| should block until the head of |this| is
// received on a navigation to a matching URL.
bool ShouldBlockUntilHeadReceived() const;

// Whether or not |this| is servable.
bool IsPrefetchServable(base::TimeDelta cacheable_duration) const;

Expand Down
27 changes: 18 additions & 9 deletions content/browser/preloading/prefetch/prefetch_container_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,17 @@ TEST_F(PrefetchContainerTest, CreatePrefetchContainer) {
PrefetchContainer prefetch_container(
GlobalRenderFrameHostId(1234, 5678), GURL("https://test.com"),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true),
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager),
blink::mojom::Referrer(), nullptr);

EXPECT_EQ(prefetch_container.GetReferringRenderFrameHostId(),
GlobalRenderFrameHostId(1234, 5678));
EXPECT_EQ(prefetch_container.GetURL(), GURL("https://test.com"));
EXPECT_EQ(prefetch_container.GetPrefetchType(),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true));
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager));

EXPECT_EQ(prefetch_container.GetPrefetchContainerKey(),
std::make_pair(GlobalRenderFrameHostId(1234, 5678),
Expand All @@ -114,7 +116,8 @@ TEST_F(PrefetchContainerTest, PrefetchStatus) {
PrefetchContainer prefetch_container(
GlobalRenderFrameHostId(1234, 5678), GURL("https://test.com"),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true),
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager),
blink::mojom::Referrer(), nullptr);

EXPECT_FALSE(prefetch_container.HasPrefetchStatus());
Expand All @@ -130,7 +133,8 @@ TEST_F(PrefetchContainerTest, IsDecoy) {
PrefetchContainer prefetch_container(
GlobalRenderFrameHostId(1234, 5678), GURL("https://test.com"),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true),
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager),
blink::mojom::Referrer(), nullptr);

EXPECT_FALSE(prefetch_container.IsDecoy());
Expand All @@ -143,7 +147,8 @@ TEST_F(PrefetchContainerTest, Servable) {
PrefetchContainer prefetch_container(
GlobalRenderFrameHostId(1234, 5678), GURL("https://test.com"),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true),
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager),
blink::mojom::Referrer(), nullptr);

prefetch_container.TakePrefetchedResponse(
Expand All @@ -162,7 +167,8 @@ TEST_F(PrefetchContainerTest, CookieListener) {
PrefetchContainer prefetch_container(
GlobalRenderFrameHostId(1234, 5678), GURL("https://test.com"),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true),
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager),
blink::mojom::Referrer(), nullptr);

EXPECT_FALSE(prefetch_container.HaveDefaultContextCookiesChanged());
Expand All @@ -182,7 +188,8 @@ TEST_F(PrefetchContainerTest, CookieCopy) {
PrefetchContainer prefetch_container(
GlobalRenderFrameHostId(1234, 5678), GURL("https://test.com"),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true),
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager),
blink::mojom::Referrer(), nullptr);
prefetch_container.RegisterCookieListener(cookie_manager());

Expand Down Expand Up @@ -238,7 +245,8 @@ TEST_F(PrefetchContainerTest, PrefetchProxyPrefetchedResourceUkm) {
std::make_unique<PrefetchContainer>(
GlobalRenderFrameHostId(1234, 5678), GURL("https://test.com"),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true),
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager),
blink::mojom::Referrer(), nullptr);

network::URLLoaderCompletionStatus completion_status;
Expand Down Expand Up @@ -357,7 +365,8 @@ TEST_F(PrefetchContainerTest, PrefetchProxyPrefetchedResourceUkm_NothingSet) {
std::make_unique<PrefetchContainer>(
GlobalRenderFrameHostId(1234, 5678), GURL("https://test.com"),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true),
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager),
blink::mojom::Referrer(), nullptr);
prefetch_container.reset();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ void PrefetchDocumentManager::ProcessCandidates(
bool use_prefetch_proxy = !is_same_origin && private_prefetch;
prefetches.emplace_back(
candidate->url,
PrefetchType(use_isolated_network_context, use_prefetch_proxy),
PrefetchType(use_isolated_network_context, use_prefetch_proxy,
candidate->eagerness),
*candidate->referrer);
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ TEST_F(PrefetchDocumentManagerTest, ProcessSpeculationCandidates) {
candidate1->requires_anonymous_client_ip_when_cross_origin = true;
candidate1->url = GetCrossOriginUrl("/candidate1.html");
candidate1->referrer = blink::mojom::Referrer::New();
candidate1->eagerness = blink::mojom::SpeculationEagerness::kEager;
candidates.push_back(std::move(candidate1));

// Create candidate for non-private cross-origin prefetch. This candidate
Expand All @@ -268,6 +269,7 @@ TEST_F(PrefetchDocumentManagerTest, ProcessSpeculationCandidates) {
candidate2->requires_anonymous_client_ip_when_cross_origin = false;
candidate2->url = GetCrossOriginUrl("/candidate2.html");
candidate2->referrer = blink::mojom::Referrer::New();
candidate2->eagerness = blink::mojom::SpeculationEagerness::kEager;
candidates.push_back(std::move(candidate2));

// Create candidate for non-private cross-origin prefetch. This candidate
Expand All @@ -277,6 +279,7 @@ TEST_F(PrefetchDocumentManagerTest, ProcessSpeculationCandidates) {
candidate3->requires_anonymous_client_ip_when_cross_origin = false;
candidate3->url = GetSameOriginUrl("/candidate3.html");
candidate3->referrer = blink::mojom::Referrer::New();
candidate3->eagerness = blink::mojom::SpeculationEagerness::kEager;
candidates.push_back(std::move(candidate3));

// Create candidate for private cross-origin prefetch with subresources. This
Expand All @@ -287,6 +290,7 @@ TEST_F(PrefetchDocumentManagerTest, ProcessSpeculationCandidates) {
candidate4->requires_anonymous_client_ip_when_cross_origin = true;
candidate4->url = GetCrossOriginUrl("/candidate4.html");
candidate4->referrer = blink::mojom::Referrer::New();
candidate4->eagerness = blink::mojom::SpeculationEagerness::kEager;
candidates.push_back(std::move(candidate4));

// Create candidate for prerender. This candidate should not be prefetched by
Expand All @@ -296,8 +300,19 @@ TEST_F(PrefetchDocumentManagerTest, ProcessSpeculationCandidates) {
candidate5->requires_anonymous_client_ip_when_cross_origin = false;
candidate5->url = GetCrossOriginUrl("/candidate5.html");
candidate5->referrer = blink::mojom::Referrer::New();
candidate5->eagerness = blink::mojom::SpeculationEagerness::kEager;
candidates.push_back(std::move(candidate5));

// Create candidate for private cross-origin prefetch with default eagerness.
// This candidate should be prefetched by |PrefetchDocumentManager|.
auto candidate6 = blink::mojom::SpeculationCandidate::New();
candidate6->action = blink::mojom::SpeculationAction::kPrefetch;
candidate6->requires_anonymous_client_ip_when_cross_origin = true;
candidate6->url = GetCrossOriginUrl("/candidate6.html");
candidate6->referrer = blink::mojom::Referrer::New();
candidate6->eagerness = blink::mojom::SpeculationEagerness::kDefault;
candidates.push_back(std::move(candidate6));

// Process the candidates with the |PrefetchDocumentManager| for the current
// document.
auto* prefetch_document_manager =
Expand All @@ -309,19 +324,27 @@ TEST_F(PrefetchDocumentManagerTest, ProcessSpeculationCandidates) {
// Check that the candidates that should be prefetched were sent to
// |PrefetchService|.
const auto& prefetch_urls = GetPrefetches();
ASSERT_EQ(prefetch_urls.size(), 3U);
ASSERT_EQ(prefetch_urls.size(), 4U);
EXPECT_EQ(prefetch_urls[0]->GetURL(), GetCrossOriginUrl("/candidate1.html"));
EXPECT_EQ(prefetch_urls[0]->GetPrefetchType(),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true));
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kEager));
EXPECT_EQ(prefetch_urls[1]->GetURL(), GetCrossOriginUrl("/candidate2.html"));
EXPECT_EQ(prefetch_urls[1]->GetPrefetchType(),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/false));
/*use_prefetch_proxy=*/false,
blink::mojom::SpeculationEagerness::kEager));
EXPECT_EQ(prefetch_urls[2]->GetURL(), GetSameOriginUrl("/candidate3.html"));
EXPECT_EQ(prefetch_urls[2]->GetPrefetchType(),
PrefetchType(/*use_isolated_network_context=*/false,
/*use_prefetch_proxy=*/false));
/*use_prefetch_proxy=*/false,
blink::mojom::SpeculationEagerness::kEager));
EXPECT_EQ(prefetch_urls[3]->GetURL(), GetCrossOriginUrl("/candidate6.html"));
EXPECT_EQ(prefetch_urls[3]->GetPrefetchType(),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/true,
blink::mojom::SpeculationEagerness::kDefault));

// Check that the only remaining entries in candidates are those that
// shouldn't be prefetched by |PrefetchService|.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ TEST_F(PrefetchNetworkContextTest, CreateIsolatedURLLoaderFactory) {
std::make_unique<PrefetchNetworkContext>(
prefetch_service(),
PrefetchType(/*use_isolated_network_context=*/true,
/*use_prefetch_proxy=*/false),
/*use_prefetch_proxy=*/false,
blink::mojom::SpeculationEagerness::kEager),
referring_origin, main_rfh()->GetGlobalId());

prefetch_network_context->GetURLLoaderFactory();
Expand Down Expand Up @@ -139,7 +140,8 @@ TEST_F(PrefetchNetworkContextTest,
std::make_unique<PrefetchNetworkContext>(
prefetch_service(),
PrefetchType(/*use_isolated_network_context=*/false,
/*use_prefetch_proxy=*/false),
/*use_prefetch_proxy=*/false,
blink::mojom::SpeculationEagerness::kEager),
referring_origin, main_rfh()->GetGlobalId());

prefetch_network_context->GetURLLoaderFactory();
Expand Down
15 changes: 15 additions & 0 deletions content/browser/preloading/prefetch/prefetch_params.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/rand_util.h"
#include "content/browser/preloading/prefetch/prefetch_features.h"
#include "content/public/browser/prefetch_service_delegate.h"
#include "third_party/blink/public/mojom/speculation_rules/speculation_rules.mojom.h"

namespace content {

Expand Down Expand Up @@ -222,6 +223,20 @@ bool PrefetchUseStreamingURLLoader() {
features::kPrefetchUseContentRefactor, "use_streaming_url_loader", true);
}

bool PrefetchShouldBlockUntilHead(
blink::mojom::SpeculationEagerness prefetch_eagerness) {
switch (prefetch_eagerness) {
case blink::mojom::SpeculationEagerness::kEager:
return base::GetFieldTrialParamByFeatureAsBool(
features::kPrefetchUseContentRefactor,
"block_until_head_eager_prefetch", false);
case blink::mojom::SpeculationEagerness::kDefault:
return base::GetFieldTrialParamByFeatureAsBool(
features::kPrefetchUseContentRefactor,
"block_until_head_default_prefetch", true);
}
}

bool IsContentPrefetchHoldback() {
return base::GetFieldTrialParamByFeatureAsBool(
features::kPrefetchUseContentRefactor, "prefetch_holdback", false);
Expand Down
7 changes: 7 additions & 0 deletions content/browser/preloading/prefetch/prefetch_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include "base/time/time.h"
#include "content/common/content_export.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/mojom/speculation_rules/speculation_rules.mojom.h"

#include "url/gurl.h"

namespace content {
Expand Down Expand Up @@ -111,6 +113,11 @@ int PrefetchCanaryCheckRetries();
// prefetches.
bool PrefetchUseStreamingURLLoader();

// Whether or not |PrefetchService| should block until the head of a prefetch
// request is received when considering to serve a prefetch for a navigation.
bool PrefetchShouldBlockUntilHead(
blink::mojom::SpeculationEagerness prefetch_eagerness);

// Returns whether the client is involved in the Holdback Finch
// experiment group.
bool IsContentPrefetchHoldback();
Expand Down

0 comments on commit 68bad88

Please sign in to comment.