Skip to content

Commit

Permalink
Add a build version param to the ReduceUserAgentMinorVersion feature
Browse files Browse the repository at this point in the history
The FeatureParam will allow setting the value of the frozen build number
in the Finch configuration.  If the param is set to X in the Finch
configuration, then the minor version would be 0.X.0.  The default value
is "0".

(cherry picked from commit 1356aec)

Bug: 1317577
Change-Id: I0bbd92031b916e6e372c36b9869634113b7fceff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3594880
Reviewed-by: Mike Taylor <miketaylr@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Ali Beyad <abeyad@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#994735}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3611532
Cr-Commit-Position: refs/branch-heads/5005@{#366}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
  • Loading branch information
abeyad authored and Chromium LUCI CQ committed May 3, 2022
1 parent d7f00be commit 3c77bd8
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 16 deletions.
15 changes: 12 additions & 3 deletions chrome/browser/chrome_worker_browsertest.cc
Expand Up @@ -257,14 +257,23 @@ class ChromeWorkerUserAgentBrowserTest
"Chrome/[0-9]+\\.([0-9]+\\.[0-9]+\\.[0-9]+)";
// The minor version in the reduced UA string is always "0.0.0".
static constexpr char kReducedMinorVersion[] = "0.0.0";
// The minor version in the ReduceUserAgentMinorVersion experiment is always
// "0.X.0", where X is the frozen build version.
const std::string kReduceUserAgentMinorVersion =
"0." +
std::string(
blink::features::kUserAgentFrozenBuildVersion.Get().data()) +
".0";

std::string minor_version;
EXPECT_TRUE(re2::RE2::PartialMatch(user_agent_value, kChromeVersionRegex,
&minor_version));
if (expected_user_agent_reduced ||
base::FeatureList::IsEnabled(
blink::features::kReduceUserAgentMinorVersion)) {

if (expected_user_agent_reduced) {
EXPECT_EQ(minor_version, kReducedMinorVersion);
} else if (base::FeatureList::IsEnabled(
blink::features::kReduceUserAgentMinorVersion)) {
EXPECT_EQ(minor_version, kReduceUserAgentMinorVersion);
} else {
EXPECT_NE(minor_version, kReducedMinorVersion);
}
Expand Down
14 changes: 11 additions & 3 deletions chrome/browser/client_hints/client_hints_browsertest.cc
Expand Up @@ -247,14 +247,22 @@ void CheckUserAgentMinorVersion(const std::string& user_agent_value,
"Chrome/[0-9]+\\.([0-9]+\\.[0-9]+\\.[0-9]+)";
// The minor version in the reduced UA string is always "0.0.0".
static constexpr char kReducedMinorVersion[] = "0.0.0";
// The minor version in the ReduceUserAgentMinorVersion experiment is always
// "0.X.0", where X is the frozen build version.
const std::string kReduceUserAgentMinorVersion =
"0." +
std::string(blink::features::kUserAgentFrozenBuildVersion.Get().data()) +
".0";

std::string minor_version;
EXPECT_TRUE(re2::RE2::PartialMatch(user_agent_value, kChromeVersionRegex,
&minor_version));
if (expected_user_agent_reduced ||
base::FeatureList::IsEnabled(
blink::features::kReduceUserAgentMinorVersion)) {

if (expected_user_agent_reduced) {
EXPECT_EQ(minor_version, kReducedMinorVersion);
} else if (base::FeatureList::IsEnabled(
blink::features::kReduceUserAgentMinorVersion)) {
EXPECT_EQ(minor_version, kReduceUserAgentMinorVersion);
} else {
EXPECT_NE(minor_version, kReducedMinorVersion);
}
Expand Down
3 changes: 2 additions & 1 deletion components/embedder_support/user_agent_utils.cc
Expand Up @@ -341,7 +341,8 @@ std::string GetProductAndVersion(
} else {
if (base::FeatureList::IsEnabled(
blink::features::kReduceUserAgentMinorVersion)) {
return version_info::GetProductNameAndVersionForReducedUserAgent();
return version_info::GetProductNameAndVersionForReducedUserAgent(
blink::features::kUserAgentFrozenBuildVersion.Get().data());
} else {
return version_info::GetProductNameAndVersionForUserAgent();
}
Expand Down
22 changes: 19 additions & 3 deletions components/embedder_support/user_agent_utils_unittest.cc
Expand Up @@ -919,21 +919,38 @@ TEST_F(UserAgentUtilsTest, GetProductAndVersion) {
EXPECT_EQ(major_version, "99");
EXPECT_EQ(minor_version, version_info::GetMajorVersionNumber());

// Ensure the build version FeatureParam is used when set.
scoped_feature_list.Reset();
scoped_feature_list.InitWithFeaturesAndParameters(
/*enabled_features=*/{{blink::features::kReduceUserAgentMinorVersion,
{{{"build_version", "5555"}}}}},
/*disabled_features=*/{
blink::features::kForceMajorVersionInMinorPositionInUserAgent});
product = GetProductAndVersion();
std::string build_version;
std::string patch_version;
EXPECT_TRUE(re2::RE2::FullMatch(product, kChromeProductVersionRegex,
&major_version, &minor_version,
&build_version, &patch_version));
EXPECT_EQ(major_version, version_info::GetMajorVersionNumber());
EXPECT_EQ(minor_version, "0");
EXPECT_EQ(build_version, "5555");
EXPECT_EQ(patch_version, "0");

// Ensure policy is respected if ForcemajorToMinor is force disabled, even if
// the respective Blink feature is enabled.
scoped_feature_list.Reset();
scoped_feature_list.InitWithFeatures(
/*enabled_features=*/{blink::features::
kForceMajorVersionInMinorPositionInUserAgent},
/*disabled_features=*/{blink::features::kReduceUserAgentMinorVersion});
std::string build_version;
product = GetProductAndVersion(/*force_major_to_minor=*/kForceDisabled);
EXPECT_TRUE(re2::RE2::FullMatch(product, kChromeProductVersionRegex,
&major_version, &minor_version,
&build_version));
EXPECT_EQ(major_version, version_info::GetMajorVersionNumber());
EXPECT_EQ(minor_version, "0");
EXPECT_NE(build_version, "0");
EXPECT_NE(build_version, "9999");

product = GetProductAndVersion();
EXPECT_TRUE(re2::RE2::FullMatch(product, kChromeProductVersionRegex,
Expand All @@ -949,7 +966,6 @@ TEST_F(UserAgentUtilsTest, GetProductAndVersion) {
/*disabled_features=*/{
blink::features::kForceMajorVersionInMinorPositionInUserAgent});
product = GetProductAndVersion();
std::string patch_version;
EXPECT_TRUE(re2::RE2::FullMatch(product, kChromeProductVersionRegex,
&major_version, &minor_version,
&build_version, &patch_version));
Expand Down
11 changes: 7 additions & 4 deletions components/version_info/version_info.cc
Expand Up @@ -8,6 +8,7 @@
#include "base/no_destructor.h"
#include "base/notreached.h"
#include "base/sanitizer_buildflags.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/version.h"
#include "build/branding_buildflags.h"
Expand All @@ -23,10 +24,12 @@ const std::string& GetProductNameAndVersionForUserAgent() {
return *product_and_version;
}

const std::string& GetProductNameAndVersionForReducedUserAgent() {
static const base::NoDestructor<std::string> product_and_version(
"Chrome/" + GetMajorVersionNumber() + ".0.0.0");
return *product_and_version;
const std::string GetProductNameAndVersionForReducedUserAgent(
const std::string& build_version) {
std::string product_and_version;
base::StrAppend(&product_and_version, {"Chrome/", GetMajorVersionNumber(),
".0.", build_version, ".0"});
return product_and_version;
}

std::string GetProductName() {
Expand Down
6 changes: 4 additions & 2 deletions components/version_info/version_info.h
Expand Up @@ -20,8 +20,10 @@ namespace version_info {
const std::string& GetProductNameAndVersionForUserAgent();

// Returns the product name and reduced version information for the User-Agent
// header, in the format: Chrome/<major_version>.0.0.0.
const std::string& GetProductNameAndVersionForReducedUserAgent();
// header, in the format: Chrome/<major_version>.0.build_version.0, where
// `build_version` is a frozen BUILD number.
const std::string GetProductNameAndVersionForReducedUserAgent(
const std::string& build_version);

// Returns the product name, e.g. "Chromium" or "Google Chrome".
std::string GetProductName();
Expand Down
3 changes: 3 additions & 0 deletions testing/variations/fieldtrial_testing_config.json
Expand Up @@ -6279,6 +6279,9 @@
"experiments": [
{
"name": "Enabled",
"params": {
"build_version": "9999"
},
"enable_features": [
"ReduceUserAgentMinorVersion"
]
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/common/features.cc
Expand Up @@ -1375,6 +1375,8 @@ const base::Feature kOptimizeViewportConstrainedPaintInvalidation{

const base::Feature kReduceUserAgentMinorVersion{
"ReduceUserAgentMinorVersion", base::FEATURE_DISABLED_BY_DEFAULT};
const base::FeatureParam<std::string> kUserAgentFrozenBuildVersion{
&kReduceUserAgentMinorVersion, "build_version", "0"};

const base::Feature kReportFCPOnlyOnSuccessfulCommit{
"ReportFCPOnlyOnSuccessfulCommit", base::FEATURE_DISABLED_BY_DEFAULT};
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/public/common/features.h
Expand Up @@ -677,6 +677,8 @@ BLINK_COMMON_EXPORT extern const base::Feature

// If enabled, the minor version of the User-Agent string will be reduced.
BLINK_COMMON_EXPORT extern const base::Feature kReduceUserAgentMinorVersion;
BLINK_COMMON_EXPORT extern const base::FeatureParam<std::string>
kUserAgentFrozenBuildVersion;

// If enabled, we only report FCP if there’s a successful commit to the
// compositor. Otherwise, FCP may be reported if first BeginMainFrame results in
Expand Down

0 comments on commit 3c77bd8

Please sign in to comment.