Skip to content

Commit

Permalink
[Merge to M103] Revert "Remove the AcceptCHFrame base::Feature"
Browse files Browse the repository at this point in the history
This reverts commit 03382ac.

Reason for revert: We ran into a bug related to AcceptCHFrame, would like to bring the feature flag back.

Bug: 1328370

Original change's description:
> Remove the AcceptCHFrame base::Feature
>
> The feature has been enabled by default for some time, the experiment
> launched at 100%, and the Finch config was recently deleted.  Therefore,
> we are just cleaning up this no-longer-needed feature.
>
> Bug: 1312988
> Change-Id: Idb01ccb6d7554830bbe8093022e95d88acbbeb97
> Fixed: 1312988
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3569123
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Commit-Queue: Ali Beyad <abeyad@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#989657}

Bug: 1312988
Change-Id: Ie6e330c4d12c88d9616a8c88a189c6baca026f1d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3660961
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Victor Tan <victortan@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1006639}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3666517
Cr-Commit-Position: refs/branch-heads/5060@{#241}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
Tanych authored and Chromium LUCI CQ committed May 25, 2022
1 parent 8bf9a16 commit fa5feb5
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 2 deletions.
5 changes: 5 additions & 0 deletions content/browser/loader/navigation_url_loader_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,11 @@ void NavigationURLLoaderImpl::OnAcceptCHFrameReceived(
const std::vector<network::mojom::WebClientHintsType>& accept_ch_frame,
OnAcceptCHFrameReceivedCallback callback) {
received_accept_ch_frame_ = true;
if (!base::FeatureList::IsEnabled(network::features::kAcceptCHFrame)) {
std::move(callback).Run(net::OK);
return;
}

LogAcceptCHFrameStatus(AcceptCHFrameRestart::kFramePresent);

// Given that this is happening in the middle of navigation, there should
Expand Down
6 changes: 6 additions & 0 deletions services/network/public/cpp/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ const base::FeatureParam<bool> kPlatformProvidedTrustTokenIssuance{
const base::Feature kWebSocketReassembleShortMessages{
"WebSocketReassembleShortMessages", base::FEATURE_ENABLED_BY_DEFAULT};

// Enable support for ACCEPT_CH H2/3 frame as part of Client Hint Reliability.
// See:
// https://tools.ietf.org/html/draft-davidben-http-client-hint-reliability-02#section-4.3
const base::Feature kAcceptCHFrame{"AcceptCHFrame",
base::FEATURE_ENABLED_BY_DEFAULT};

const base::Feature kSCTAuditingRetryReports{"SCTAuditingRetryReports",
base::FEATURE_ENABLED_BY_DEFAULT};

Expand Down
3 changes: 3 additions & 0 deletions services/network/public/cpp/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ extern const base::FeatureParam<bool> kPlatformProvidedTrustTokenIssuance;
COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature kWebSocketReassembleShortMessages;

COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature kAcceptCHFrame;

COMPONENT_EXPORT(NETWORK_CPP)
extern const base::Feature kSCTAuditingRetryReports;

Expand Down
3 changes: 2 additions & 1 deletion services/network/url_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,8 @@ int URLLoader::OnConnected(net::URLRequest* url_request,
return net::ERR_FAILED;
}

if (!accept_ch_frame_observer_ || info.accept_ch_frame.empty()) {
if (!accept_ch_frame_observer_ || info.accept_ch_frame.empty() ||
!base::FeatureList::IsEnabled(features::kAcceptCHFrame)) {
return net::OK;
}

Expand Down
3 changes: 2 additions & 1 deletion services/network/url_loader_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,8 @@ class URLLoaderTest : public testing::Test {
net::URLRequestFailedJob::AddUrlHandler();

scoped_feature_list_.InitWithFeatures(
/*enabled_features=*/{net::features::kRecordRadioWakeupTrigger},
/*enabled_features=*/{features::kAcceptCHFrame,
net::features::kRecordRadioWakeupTrigger},
/*disabled_features=*/{});
}
~URLLoaderTest() override {
Expand Down

0 comments on commit fa5feb5

Please sign in to comment.