Commit 6e5bd03f authored by Tsuyoshi Horo's avatar Tsuyoshi Horo Committed by Commit Bot

Reland "Send SXG accept header of navigation only to limited origins for Origin Trial."

This reverts commit 4762a830.

The difference from the original patch is that in this patch
SignedExchangeAcceptHeaderBrowserTest uses SetUp() instead of
SetUpOnMainThread() to avoid the failures in TSAN bots.

Bug: 887201
TBR=kinuko@, kouhei@, ksakamoto@

Original change's description:
> Send SXG accept header of navigation only to limited origins for Origin Trial.
>
> Before this patch:
>   The SXG accept header is sent to all origins when SignedHTTPExchange feature
>   or SignedHTTPExchangeOriginTrial feature is enabled.
>
> After this patch:
>   The SXG accept header is sent when
>     - SignedHTTPExchange feature is enabled.
>     or
>     - SignedHTTPExchangeOriginTrial feature and SignedHTTPExchangeAcceptHeader
>       feature are enabled and the origin of the URL is in the OriginList of
>       SignedHTTPExchangeAcceptHeader field trial.
>
> Bug: 887201
> Change-Id: Ic67a8bc0644b1b6172030ff75162c04663dd33f0
> Reviewed-on: https://chromium-review.googlesource.com/1242768
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
> Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
> Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#595002}

Change-Id: Iff188ffe2096c35d2c9fb4eee3723e78f5901b26
Reviewed-on: https://chromium-review.googlesource.com/1251144Reviewed-by: default avatarTsuyoshi Horo <horo@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595065}
parent f9147ab6
......@@ -221,10 +221,8 @@ std::unique_ptr<network::ResourceRequest> CreateResourceRequest(
request_info->begin_params->headers);
std::string accept_value = network::kFrameAcceptHeader;
// TODO(https://crbug.com/840704): Decide whether the Accept header should
// advertise the state of kSignedHTTPExchangeOriginTrial before starting the
// Origin-Trial.
if (signed_exchange_utils::IsSignedExchangeHandlingEnabled()) {
if (signed_exchange_utils::ShouldAdvertiseAcceptHeader(
url::Origin::Create(request_info->common_params.url))) {
DCHECK(!accept_value.empty());
accept_value.append(kAcceptHeaderSignedExchangeSuffix);
}
......@@ -988,6 +986,27 @@ class NavigationURLLoaderImpl::URLLoaderRequestController
// |resource_request_| during redirect.
url_loader_modified_request_headers_ = modified_request_headers;
if (signed_exchange_utils::NeedToCheckRedirectedURLForAcceptHeader()) {
// Currently we send the SignedExchange accept header only for the limited
// origins when SignedHTTPExchangeOriginTrial feature is enabled without
// SignedHTTPExchange feature. We need to put the SignedExchange accept
// header on when redirecting to the origins in the OriginList of
// SignedHTTPExchangeAcceptHeader field trial, and need to remove it when
// redirecting to out of the OriginList.
if (!url_loader_modified_request_headers_)
url_loader_modified_request_headers_ = net::HttpRequestHeaders();
std::string accept_value = network::kFrameAcceptHeader;
if (signed_exchange_utils::ShouldAdvertiseAcceptHeader(
url::Origin::Create(resource_request_->url))) {
DCHECK(!accept_value.empty());
accept_value.append(kAcceptHeaderSignedExchangeSuffix);
}
url_loader_modified_request_headers_->SetHeader(network::kAcceptHeader,
accept_value);
resource_request_->headers.SetHeader(network::kAcceptHeader,
accept_value);
}
Restart();
}
......
......@@ -1505,10 +1505,8 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
headers.AddHeadersFromString(info.begin_params->headers);
std::string accept_value = network::kFrameAcceptHeader;
// TODO(https://crbug.com/840704): Decide whether the Accept header should
// advertise the state of kSignedHTTPExchangeOriginTrial before starting the
// Origin-Trial.
if (signed_exchange_utils::IsSignedExchangeHandlingEnabled()) {
if (signed_exchange_utils::ShouldAdvertiseAcceptHeader(
url::Origin::Create(info.common_params.url))) {
DCHECK(!accept_value.empty());
accept_value.append(kAcceptHeaderSignedExchangeSuffix);
}
......
......@@ -48,9 +48,29 @@ OriginsList CreateAdvertiseAcceptHeaderOriginsList() {
} // namespace
bool NeedToCheckRedirectedURLForAcceptHeader() {
// When SignedHTTPExchange is enabled, the SignedExchange accept header must
// be sent to all origins. So we don't need to check the redirected URL.
return !base::FeatureList::IsEnabled(features::kSignedHTTPExchange) &&
base::FeatureList::IsEnabled(
features::kSignedHTTPExchangeOriginTrial) &&
base::FeatureList::IsEnabled(
features::kSignedHTTPExchangeAcceptHeader);
}
bool ShouldAdvertiseAcceptHeader(const url::Origin& origin) {
if (!base::FeatureList::IsEnabled(features::kSignedHTTPExchangeAcceptHeader))
// When SignedHTTPExchange is enabled, we must send the SignedExchange accept
// header to all origins.
if (base::FeatureList::IsEnabled(features::kSignedHTTPExchange))
return true;
// When SignedHTTPExchangeOriginTrial is not enabled or
// SignedHTTPExchangeAcceptHeader is not enabled, we must not send the
// SignedExchange accept header.
if (!base::FeatureList::IsEnabled(features::kSignedHTTPExchangeOriginTrial) ||
!base::FeatureList::IsEnabled(
features::kSignedHTTPExchangeAcceptHeader)) {
return false;
}
// |origins_list| is initialized in a thread-safe manner.
// Querying OriginsList::Match() should be safe since it's read-only access.
......
......@@ -38,9 +38,14 @@ void ReportErrorAndTraceEvent(
base::Optional<SignedExchangeError::FieldIndexPair> error_field =
base::nullopt);
// Returns true when SignedHTTPExchange feature is NOT enabled and
// SignedHTTPExchangeOriginTrial and SignedHTTPExchangeAcceptHeader features are
// enabled.
bool NeedToCheckRedirectedURLForAcceptHeader();
// Returns true if Accept headers should be sent with
// "application/signed-exchange".
bool ShouldAdvertiseAcceptHeader(const url::Origin& origin);
CONTENT_EXPORT bool ShouldAdvertiseAcceptHeader(const url::Origin& origin);
// Returns true when SignedHTTPExchange feature or SignedHTTPExchangeOriginTrial
// feature is enabled.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment