Skip to content

Commit

Permalink
{M103 PICK} [Fix UserAgentReduction Policy] (1) Make ForceMajorVersio…
Browse files Browse the repository at this point in the history 
…nToMinorPosition a class enum

Before we fix the issue with UserAgentReduction we should make the first
argument to GetProductAndVersion safer so that it won't be confused with
the new second argument.

This CL is part of a series:
(1) Make ForceMajorVersionToMinorPosition a class enum
(2) Fix GetProductAndVersion

(cherry picked from commit 55d0f40)

Bug: 1330588
Change-Id: Ic6e43a67410a030b4bb12f5eb646907bc199303f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3681901
Auto-Submit: Ari Chivukula <arichiv@chromium.org>
Reviewed-by: Victor Tan <victortan@chromium.org>
Commit-Queue: Ari Chivukula <arichiv@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1009892}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3686542
Cr-Commit-Position: refs/branch-heads/5060@{#633}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
@arichiv
arichiv authored and Chromium LUCI CQ committed Jun 7, 2022
1 parent cf16869 commit b329dae
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 34 deletions.
3 changes: 2 additions & 1 deletion chrome/browser/chrome_content_browser_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,8 @@ void ChromeContentBrowserClient::RegisterProfilePrefs(
registry->RegisterBooleanPref(prefs::kOriginAgentClusterDefaultEnabled, true);
registry->RegisterIntegerPref(
prefs::kForceMajorVersionToMinorPositionInUserAgent,
embedder_support::ForceMajorVersionToMinorPosition::kDefault);
static_cast<int>(
embedder_support::ForceMajorVersionToMinorPosition::kDefault));
registry->RegisterBooleanPref(
policy::policy_prefs::kIsolatedAppsDeveloperModeAllowed, true);
}
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/chrome_content_browser_client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -892,9 +892,6 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
UserAgentReductionEnterprisePolicyState
GetUserAgentReductionEnterprisePolicyState(content::BrowserContext* context);

embedder_support::ForceMajorVersionToMinorPosition
GetForceMajorVersionToMinorPosition(content::BrowserContext* context);

// Vector of additional ChromeContentBrowserClientParts.
// Parts are deleted in the reverse order they are added.
std::vector<ChromeContentBrowserClientParts*> extra_parts_;
Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/user_agent/user_agent_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,11 @@ class UserAgentBrowserTest : public InProcessBrowserTest,
prefs::kUserAgentReduction);
}

void set_force_major_version_to_minor_policy(int policy) {
void set_force_major_version_to_minor_policy(
ForceMajorVersionToMinorPolicyState policy) {
browser()->profile()->GetPrefs()->SetInteger(
prefs::kForceMajorVersionToMinorPositionInUserAgent, policy);
prefs::kForceMajorVersionToMinorPositionInUserAgent,
static_cast<int>(policy));
}

std::string observed_user_agent() { return observered_user_agent_; }
Expand Down
19 changes: 10 additions & 9 deletions components/embedder_support/user_agent_utils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,13 @@ const std::string& GetWindowsPlatformVersion() {
// the minor position.
// TODO(crbug.com/1290820): Remove this method along with policy.
bool ShouldForceMajorVersionToMinorPosition(
ForceMajorVersionToMinorPosition force_major_to_minor = kDefault) {
ForceMajorVersionToMinorPosition force_major_to_minor) {
return (
(force_major_to_minor != kForceDisabled &&
(force_major_to_minor !=
ForceMajorVersionToMinorPosition::kForceDisabled &&
base::FeatureList::IsEnabled(
blink::features::kForceMajorVersionInMinorPositionInUserAgent)) ||
force_major_to_minor == kForceEnabled);
force_major_to_minor == ForceMajorVersionToMinorPosition::kForceEnabled);
}

const std::string& GetMajorInMinorVersionNumber() {
Expand Down Expand Up @@ -527,7 +528,7 @@ blink::UserAgentMetadata GetUserAgentMetadata() {
return GetUserAgentMetadata(nullptr);
}

blink::UserAgentMetadata GetUserAgentMetadata(PrefService* pref_service) {
blink::UserAgentMetadata GetUserAgentMetadata(const PrefService* pref_service) {
blink::UserAgentMetadata metadata;
bool enable_updated_grease_by_policy = true;
UserAgentOptions ua_options;
Expand Down Expand Up @@ -600,17 +601,17 @@ int GetHighestKnownUniversalApiContractVersionForTesting() {

// TODO(crbug.com/1290820): Remove this function with policy.
embedder_support::ForceMajorVersionToMinorPosition GetMajorToMinorFromPrefs(
PrefService* pref_service) {
const PrefService* pref_service) {
if (!pref_service->HasPrefPath(kForceMajorVersionToMinorPosition))
return kDefault;
return ForceMajorVersionToMinorPosition::kDefault;
switch (pref_service->GetInteger(kForceMajorVersionToMinorPosition)) {
case 1:
return kForceDisabled;
return ForceMajorVersionToMinorPosition::kForceDisabled;
case 2:
return kForceEnabled;
return ForceMajorVersionToMinorPosition::kForceEnabled;
case 0:
default:
return kDefault;
return ForceMajorVersionToMinorPosition::kDefault;
}
}

Expand Down
31 changes: 18 additions & 13 deletions components/embedder_support/user_agent_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,41 +22,46 @@ class WebContents;

namespace embedder_support {

// TODO(crbug.com/1291612): Move this enum definition to
// chrome/browser/chrome_content_browser_client.h
// TODO(crbug.com/1290820): Remove this enum along with policy.
enum ForceMajorVersionToMinorPosition {
enum class ForceMajorVersionToMinorPosition {
kDefault = 0,
kForceDisabled = 1,
kForceEnabled = 2,
};

struct UserAgentOptions {
bool force_major_version_100 = false;
ForceMajorVersionToMinorPosition force_major_to_minor = kDefault;
ForceMajorVersionToMinorPosition force_major_to_minor =
ForceMajorVersionToMinorPosition::kDefault;
};

// Returns the product & version string. Examples:
// "Chrome/101.0.0.0" - if UA reduction is enabled
// "Chrome/101.0.4698.0" - if UA reduction is not enabled
// "Chrome/101.0.0.0" - if UA reduction is enabled w/o major to minor
// "Chrome/101.0.4698.0" - if UA reduction isn't enabled w/o major to minor
// "Chrome/99.101.0.0" - if UA reduction is enabled w/ major to minor
// "Chrome/99.101.0.4698.0" - if UA reduction isn'n enabled w/ major to minor
// TODO(crbug.com/1291612): modify to accept an optional PrefService*.
std::string GetProductAndVersion(
ForceMajorVersionToMinorPosition force_major_to_minor = kDefault);
ForceMajorVersionToMinorPosition force_major_to_minor =
ForceMajorVersionToMinorPosition::kDefault);

// Returns the user agent string for Chrome.
// TODO(crbug.com/1291612): modify to accept an optional PrefService*.
std::string GetFullUserAgent(
ForceMajorVersionToMinorPosition force_major_to_minor = kDefault);
ForceMajorVersionToMinorPosition force_major_to_minor =
ForceMajorVersionToMinorPosition::kDefault);

// Returns the reduced user agent string for Chrome.
// TODO(crbug.com/1291612): modify to accept an optional PrefService*.
std::string GetReducedUserAgent(
ForceMajorVersionToMinorPosition force_major_to_minor = kDefault);
ForceMajorVersionToMinorPosition force_major_to_minor =
ForceMajorVersionToMinorPosition::kDefault);

// Returns the full or "reduced" user agent string, depending on the
// UserAgentReduction enterprise policy and blink::features::kReduceUserAgent
// TODO(crbug.com/1291612): modify to accept an optional PrefService*.
std::string GetUserAgent(
ForceMajorVersionToMinorPosition force_major_to_minor = kDefault);
std::string GetUserAgent(ForceMajorVersionToMinorPosition force_major_to_minor =
ForceMajorVersionToMinorPosition::kDefault);

// Returns UserAgentMetadata per the default policy.
// This override is currently used in fuchsia, where the enterprise policy
Expand All @@ -66,7 +71,7 @@ blink::UserAgentMetadata GetUserAgentMetadata();
// Return UserAgentMetadata, potentially overridden by policy.
// Note that this override is likely to be removed once an enterprise
// escape hatch is no longer needed. See https://crbug.com/1261908.
blink::UserAgentMetadata GetUserAgentMetadata(PrefService* local_state);
blink::UserAgentMetadata GetUserAgentMetadata(const PrefService* local_state);

// Return UserAgentBrandList based on the expected output version type.
blink::UserAgentBrandList GenerateBrandVersionList(
Expand Down Expand Up @@ -107,7 +112,7 @@ int GetHighestKnownUniversalApiContractVersionForTesting();
// the provided integer policy value for ForceMajorVersionToMinorPosition.
// TODO(crbug.com/1290820): Remove this function with policy.
embedder_support::ForceMajorVersionToMinorPosition GetMajorToMinorFromPrefs(
PrefService* pref_service);
const PrefService* pref_service);

} // namespace embedder_support

Expand Down
19 changes: 13 additions & 6 deletions components/embedder_support/user_agent_utils_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,8 @@ TEST_F(UserAgentUtilsTest, UserAgentStringReduced) {
blink::features::kForceMajorVersionInMinorPositionInUserAgent},
{});
{
std::string buffer = GetReducedUserAgent(kForceDisabled);
std::string buffer =
GetReducedUserAgent(ForceMajorVersionToMinorPosition::kForceDisabled);
std::string device_compat = "Mobile ";
EXPECT_EQ(buffer,
base::StringPrintf(content::frozen_user_agent_strings::kAndroid,
Expand Down Expand Up @@ -465,7 +466,8 @@ TEST_F(UserAgentUtilsTest, UserAgentStringReduced) {
blink::features::kForceMajorVersionInMinorPositionInUserAgent},
{});
{
std::string buffer = GetReducedUserAgent(kForceDisabled);
std::string buffer =
GetReducedUserAgent(ForceMajorVersionToMinorPosition::kForceDisabled);
EXPECT_EQ(buffer, base::StringPrintf(
content::frozen_user_agent_strings::kDesktop,
content::GetUnifiedPlatform().c_str(),
Expand Down Expand Up @@ -547,7 +549,8 @@ TEST_F(UserAgentUtilsTest, UserAgentMetadata) {

// ForceMajorVersionToMinorPosition: kForceDisabled
pref_service.registry()->RegisterIntegerPref(
kForceMajorVersionToMinorPosition, kForceDisabled);
kForceMajorVersionToMinorPosition,
static_cast<int>(ForceMajorVersionToMinorPosition::kForceDisabled));
metadata = GetUserAgentMetadata(&pref_service);
EXPECT_EQ(metadata.full_version, full_version);
EXPECT_TRUE(ContainsBrandVersion(metadata.brand_version_list,
Expand All @@ -569,7 +572,9 @@ TEST_F(UserAgentUtilsTest, UserAgentMetadata) {
major_to_minor_product_brand_full_version));

// ForceMajorVersionToMinorPosition: kForceEnabled
pref_service.SetInteger(kForceMajorVersionToMinorPosition, kForceEnabled);
pref_service.SetInteger(
kForceMajorVersionToMinorPosition,
static_cast<int>(ForceMajorVersionToMinorPosition::kForceEnabled));
metadata = GetUserAgentMetadata(&pref_service);
EXPECT_EQ(metadata.full_version, major_to_minor_full_version);
EXPECT_TRUE(ContainsBrandVersion(metadata.brand_version_list,
Expand Down Expand Up @@ -1007,7 +1012,8 @@ TEST_F(UserAgentUtilsTest, GetProductAndVersion) {
EXPECT_EQ(minor_version, "0");

// Ensure policy is respected if ForceMajorToMinor is force enabled
product = GetProductAndVersion(/*force_major_to_minor=*/kForceEnabled);
product =
GetProductAndVersion(ForceMajorVersionToMinorPosition::kForceEnabled);
EXPECT_TRUE(re2::RE2::FullMatch(product, kChromeProductVersionRegex,
&major_version, &minor_version));
EXPECT_EQ(major_version, "99");
Expand Down Expand Up @@ -1038,7 +1044,8 @@ TEST_F(UserAgentUtilsTest, GetProductAndVersion) {
/*enabled_features=*/{blink::features::
kForceMajorVersionInMinorPositionInUserAgent},
/*disabled_features=*/{blink::features::kReduceUserAgentMinorVersion});
product = GetProductAndVersion(/*force_major_to_minor=*/kForceDisabled);
product =
GetProductAndVersion(ForceMajorVersionToMinorPosition::kForceDisabled);
EXPECT_TRUE(re2::RE2::FullMatch(product, kChromeProductVersionRegex,
&major_version, &minor_version,
&build_version));
Expand Down

0 comments on commit b329dae

Please sign in to comment.