Skip to content

Commit

Permalink
Revert "Add Sec-CH-UA-Reduced client hint header when Origin Trial to…
Browse files Browse the repository at this point in the history
…ken present"

This reverts commit d9c057e.

Reason for revert: Browser tests is failing on linux-chromeos-chrome.
See [1].

[1] https://ci.chromium.org/ui/p/chrome/builders/ci/linux-chromeos-chrome/16242/overview

Original change's description:
> Add Sec-CH-UA-Reduced client hint header when Origin Trial token present
>
> In this CL, we add logic to parse and accept the Sec-CH-UA-Reduced
> client hint in the Accept-CH header and and only send the
> Sec-CH-UA-Reduced client hint only in the presence of a valid
> UserAgentReduction Origin Trial token.
>
> If the Origin Trial token is present and valid, and the origin
> contains Sec-CH-UA-Reduced in the Accept-CH cache, then on all
> subsequent requests to the origin, the Sec-CH-UA-Reduced request
> header will be set with a value of "?1" (sh-boolean).  If the
> Accept-CH cache does not contain Sec-CH-UA-Reduced, or the Origin
> Trial token is not valid, then Sec-CH-UA-Reduced will not be sent
> in the request headers.
>
> NB: A subsequent CL will change the User-Agent request header to
> the reduced UA string if the Sec-CH-UA-Reduced header is sent.
>
> Chrome Platform Status: https://chromestatus.com/feature/5704553745874944
> Design Doc: https://docs.google.com/document/d/1feIxK9S7oNgT2oGGebbxE9X0O-4wTKcsP_gRaY99tq4
>
> Bug: 1222742
> Change-Id: If855d4bb393540d49de3be80aa9bc7c80f861c50
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3042810
> Commit-Queue: Ali Beyad <abeyad@chromium.org>
> Reviewed-by: Aaron Tagliaboschi <aarontag@chromium.org>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#906810}

Bug: 1222742
Change-Id: I0d8aab1d56d78f8323d4a3e3ed88b6eef370e1b9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3061119
Auto-Submit: Daniel Hosseinian <dhoss@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Daniel Hosseinian <dhoss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#906884}
  • Loading branch information
Daniel Hosseinian authored and Chromium LUCI CQ committed Jul 29, 2021
1 parent 56d287c commit f6370c3
Show file tree
Hide file tree
Showing 23 changed files with 124 additions and 556 deletions.
200 changes: 2 additions & 198 deletions chrome/browser/client_hints/client_hints_browsertest.cc
Expand Up @@ -11,10 +11,8 @@
#include "base/containers/contains.h"
#include "base/metrics/field_trial_param_associator.h"
#include "base/run_loop.h"
#include "base/strings/strcat.h"
#include "base/strings/string_util.h"
#include "base/synchronization/lock.h"
#include "base/test/bind.h"
#include "base/test/metrics/histogram_tester.h"
#include "build/build_config.h"
#include "chrome/browser/content_settings/cookie_settings_factory.h"
Expand All @@ -30,7 +28,6 @@
#include "components/content_settings/core/browser/cookie_settings.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/content_settings/core/common/pref_names.h"
#include "components/embedder_support/switches.h"
#include "components/embedder_support/user_agent_utils.h"
#include "components/metrics/content/subprocess_metrics_provider.h"
#include "components/page_load_metrics/browser/page_load_metrics_test_waiter.h"
Expand All @@ -42,8 +39,6 @@
#include "content/public/common/content_switches.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "content/public/test/test_utils.h"
#include "content/public/test/url_loader_interceptor.h"
#include "net/dns/mock_host_resolver.h"
Expand All @@ -57,9 +52,7 @@
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/network_switches.h"
#include "services/network/public/mojom/web_client_hints_types.mojom-shared.h"
#include "testing/gmock/include/gmock/gmock-matchers.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/common/client_hints/client_hints.h"
#include "third_party/blink/public/common/web_preferences/web_preferences.h"

Expand Down Expand Up @@ -699,10 +692,8 @@ class ClientHintsBrowserTest : public InProcessBrowserTest,
continue;
}

// Skip over the `Sec-CH-UA-Reduced` client hint because it is only added
// in the presence of a valid "UserAgentReduction" Origin Trial token.
// `Sec-CH-UA-Reduced` is tested via UaReducedOriginTrialBrowserTest
// below.
// TODO(crbug.com/1226193): Remove this block when support for
// `Sec-CH-UA-Reduced` has been added.
if (std::string(blink::kClientHintsHeaderMapping[i]) ==
"sec-ch-ua-reduced") {
continue;
Expand Down Expand Up @@ -2484,190 +2475,3 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTestWithEmulatedMedia,
ui_test_utils::NavigateToURL(browser(), test_url());
EXPECT_EQ(prefers_color_scheme_observed(), "dark");
}

// Tests that the Sec-CH-UA-Reduced client hint is sent if and only if the
// UserAgentReduction Origin Trial token is present and valid in the response
// headers.
//
// The test Origin Trial token found in the test files was generated by running
// (in tools/origin_trials):
// generate_token.py https://127.0.0.1:44444 UserAgentReduction
// --expire-timestamp=2000000000
//
// The Origin Trial token expires in 2033. Generate a new token by then, or
// find a better way to re-generate a test trial token.
class UaReducedOriginTrialBrowserTest : public InProcessBrowserTest {
public:
UaReducedOriginTrialBrowserTest()
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
https_server_.ServeFilesFromSourceDirectory(
"chrome/test/data/client_hints");
https_server_.RegisterRequestMonitor(base::BindRepeating(
&UaReducedOriginTrialBrowserTest::MonitorResourceRequest,
base::Unretained(this)));
// Always start the server on the same port, because Origin Trial tokens are
// bound to an origin (schema/host/port).
EXPECT_TRUE(https_server_.Start(/*port=*/44444));
}

void SetUpCommandLine(base::CommandLine* command_line) override {
// The public key for the default privatey key used by the
// tools/origin_trials/generate_token.py tool.
static constexpr char kOriginTrialTestPublicKey[] =
"dRCs+TocuKkocNKa0AtZ4awrt9XKH2SQCI6o4FY6BNA=";
command_line->AppendSwitchASCII(embedder_support::kOriginTrialPublicKey,
kOriginTrialTestPublicKey);
}

void SetUpOnMainThread() override {
host_resolver()->AddRule("*", "127.0.0.1");
}

static absl::optional<std::string> GetHeaderValue(
const net::test_server::HttpRequest& request,
const std::string& header) {
if (request.headers.find(header) != request.headers.end()) {
return request.headers.find(header)->second;
} else {
return absl::nullopt;
}
}

// Called by `https_server_`, as a callback when navigating to a URL and a
// request is about to be sent.
void MonitorResourceRequest(const net::test_server::HttpRequest& request) {
absl::optional<std::string> ua_reduced_value =
GetHeaderValue(request, "sec-ch-ua-reduced");
if (ua_reduced_value.has_value()) {
SetUaReducedClientHintValue(*ua_reduced_value);
} else {
ClearUaReducedClientHintValue();
}
}

absl::optional<std::string> UaReducedClientHintValue() {
base::AutoLock lock(ua_reduced_client_hint_value_mutex_);
return ua_reduced_client_hint_value_;
}

void SetUaReducedClientHintValue(const std::string& value) {
base::AutoLock lock(ua_reduced_client_hint_value_mutex_);
ua_reduced_client_hint_value_ = value;
}

void ClearUaReducedClientHintValue() {
base::AutoLock lock(ua_reduced_client_hint_value_mutex_);
ua_reduced_client_hint_value_.reset();
}

GURL ua_reduced_with_valid_origin_trial_token_url() const {
return https_server_.GetURL(
"/accept_ch_ua_reduced_with_valid_origin_trial.html");
}

GURL ua_reduced_with_invalid_origin_trial_token_url() const {
return https_server_.GetURL(
"/accept_ch_ua_reduced_with_invalid_origin_trial.html");
}

GURL ua_reduced_with_no_origin_trial_token_url() const {
return https_server_.GetURL(
"/accept_ch_ua_reduced_with_no_origin_trial.html");
}

GURL ua_reduced_missing_with_valid_origin_trial_token_url() const {
return https_server_.GetURL(
"/accept_ch_ua_reduced_missing_valid_origin_trial.html");
}

GURL critical_ch_ua_reduced_with_valid_origin_trial_token_url() const {
return https_server_.GetURL(
"/critical_ch_ua_reduced_with_valid_origin_trial.html");
}

GURL critical_ch_ua_reduced_with_invalid_origin_trial_token_url() const {
return https_server_.GetURL(
"/critical_ch_ua_reduced_with_invalid_origin_trial.html");
}

void NavigateTwiceWithoutCriticalCH(const GURL& url) {
// The first navigation is in order to set the Accept-CH cache with the
// client hints in the Accept-CH response header.
ui_test_utils::NavigateToURL(browser(), url);
// In the first navigation, the client hints should not be present.
EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt));
// The next navigation is to inspect the request headers sent in subsequent
// requests.
ui_test_utils::NavigateToURL(browser(), url);
}

private:
net::EmbeddedTestServer https_server_;
base::Lock ua_reduced_client_hint_value_mutex_;
// Guarded by a mutex because it's written to by the IO sequence and read from
// the main test execution thread.
absl::optional<std::string> ua_reduced_client_hint_value_
GUARDED_BY(ua_reduced_client_hint_value_mutex_);
};

IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest,
AcceptChUaReducedWithValidOriginTrialToken) {
NavigateTwiceWithoutCriticalCH(
ua_reduced_with_valid_origin_trial_token_url());

EXPECT_THAT(UaReducedClientHintValue(), Optional(Eq("?1")));
}

IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest,
AcceptChUaReducedWithInvalidOriginTrialToken) {
NavigateTwiceWithoutCriticalCH(
ua_reduced_with_invalid_origin_trial_token_url());

// The response contained Sec-CH-UA-Reduced in the Accept-CH header, but the
// origin trial token is invalid.
EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt));
}

IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest,
AcceptChUaReducedWithNoOriginTrialToken) {
NavigateTwiceWithoutCriticalCH(ua_reduced_with_no_origin_trial_token_url());

// The response contained Sec-CH-UA-Reduced in the Accept-CH header, but the
// origin trial token is not present.
EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt));
}

IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest,
NoAcceptChUaReducedWithValidOriginTrialToken) {
NavigateTwiceWithoutCriticalCH(
ua_reduced_missing_with_valid_origin_trial_token_url());

// The response contained a valid Origin Trial token, but no Sec-CH-UA-Reduced
// in the Accept-CH header.
EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt));
}

IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest,
CriticalChUaReducedWithValidOriginTrialToken) {
ui_test_utils::NavigateToURL(
browser(), critical_ch_ua_reduced_with_valid_origin_trial_token_url());

// The initial navigation also contains the Critical-CH header, so the
// Sec-CH-UA-Reduced header should be set after the first navigation.
EXPECT_THAT(UaReducedClientHintValue(), Optional(Eq("?1")));
}

IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest,
CriticalChUaReducedWithInvalidOriginTrialToken) {
ui_test_utils::NavigateToURL(
browser(), critical_ch_ua_reduced_with_invalid_origin_trial_token_url());

// The Origin Trial token is invalid, so the Critical-CH should not have
// resulted in the Sec-CH-UA-Reduced header being sent.
EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt));
}

IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest,
ThirdPartyUaReducedWithValidOriginTrialToken) {
// TODO(crbug.com/1222742): Implement this test.
}

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

0 comments on commit f6370c3

Please sign in to comment.