Skip to content

Commit

Permalink
Extended stable channel detection for Google Chrome on Windows.
Browse files Browse the repository at this point in the history
- install_static::DetermineChannel now handles a channel override value
  of "extended". In this case, the channel is "" (for stable) and a new
  bool is returned indicating that it's really extended stable.
- install_static::InstallDetails retains the
  `is_extended_stable_channel` bool determined above for the process and
  install_static::IsExtendedStableChannel() now returns its value.
- install_static::InstallDetails now retains the true value of the
  channel override from which the channel was determined when the
  channel originates from an override. For an install following the
  extended stable update channel, this will be "extended".
- setup.exe now writes the override value above into the "channel" value
  of Chrome's Clients key so that the browser's channel detection can
  differentiate regular from extended stable in the same way as the
  installer.

With these changes, "setup.exe --channel=extended" now results in a
Google Chrome browser for which chrome::IsExtendedStableChannel()
returns true and chrome::GetChannelName() returns "" or "extended" based
on the chrome::WithExtendedStable value provided by the caller.

Bug: 1185621
Change-Id: Iab59ba1b32fabf6db14af0d623acfb2ba9efa7ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2752236
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Yann Dago <ydago@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#862127}
  • Loading branch information
GregTho authored and Chromium LUCI CQ committed Mar 11, 2021
1 parent 4fd35ea commit 918f67c
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 54 deletions.
8 changes: 4 additions & 4 deletions chrome/common/channel_info_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ namespace chrome {

std::string GetChannelName(WithExtendedStable with_extended_stable) {
#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
// TODO(https://crbug.com/1185621): Detect extended stable.
std::wstring channel(install_static::GetChromeChannelName());
std::wstring channel((with_extended_stable && IsExtendedStableChannel())
? L"extended"
: install_static::GetChromeChannelName());
#if defined(DCHECK_IS_CONFIGURABLE)
// Adorn the channel when DCHECKs are baked into the build, as there will be
// a performance hit. See https://crbug.com/812058 for details.
Expand All @@ -32,8 +33,7 @@ version_info::Channel GetChannel() {
}

bool IsExtendedStableChannel() {
// TODO(https://crbug.com/1185621): Detect extended stable.
return false;
return install_static::IsExtendedStableChannel();
}

} // namespace chrome
2 changes: 2 additions & 0 deletions chrome/install_static/install_details.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,6 @@ PrimaryInstallDetails::PrimaryInstallDetails() : InstallDetails(&payload_) {
payload_.product_version = &kProductVersion[0];
}

PrimaryInstallDetails::~PrimaryInstallDetails() = default;

} // namespace install_static
32 changes: 32 additions & 0 deletions chrome/install_static/install_details.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ class InstallDetails {
// dictated by the mode itself (kInstallMode).
ChannelOrigin channel_origin;

// The value that was used to select |channel| if |channel_origin| is
// kPolicy. This is the value provided to the installer via the --channel=
// command line switch or the value provided to the browser via the
// "channel" value in its Clients key.
const wchar_t* channel_override;

// The "ap" (additional parameters) value read from Chrome's ClientState key
// during process startup.
const wchar_t* update_ap;
Expand All @@ -76,6 +82,9 @@ class InstallDetails {

// True if installed in C:\Program Files{, {x86)}; otherwise, false.
bool system_level;

// True if |channel| is an empty string for the extended stable channel.
bool is_extended_stable_channel;
};

InstallDetails(const InstallDetails&) = delete;
Expand Down Expand Up @@ -162,6 +171,20 @@ class InstallDetails {
// channel, or kInstallMode.
ChannelOrigin channel_origin() const { return payload_->channel_origin; }

// Returns the value that was used to select the channel if |channel_origin()|
// returns kPolicy. This is the value provided to the installer via the
// --channel= command line switch or the value provided to the browser via the
// "channel" value in its Clients key.
std::wstring channel_override() const {
return payload_->channel_override ? std::wstring(payload_->channel_override)
: std::wstring();
}

// Returns true if channel is an empty string for the extended stable channel.
bool is_extended_stable_channel() const {
return payload_->is_extended_stable_channel;
}

// Returns the "ap" (additional parameters) value read from Chrome's
// ClientState key during process startup.
std::wstring update_ap() const {
Expand Down Expand Up @@ -233,6 +256,7 @@ class PrimaryInstallDetails : public InstallDetails {
PrimaryInstallDetails(const PrimaryInstallDetails&) = delete;
PrimaryInstallDetails(PrimaryInstallDetails&&) = delete;
PrimaryInstallDetails& operator=(const PrimaryInstallDetails&) = delete;
~PrimaryInstallDetails() override;

void set_mode(const InstallConstants* mode) { payload_.mode = mode; }
void set_channel(const std::wstring& channel) {
Expand All @@ -243,6 +267,13 @@ class PrimaryInstallDetails : public InstallDetails {
void set_channel_origin(ChannelOrigin origin) {
payload_.channel_origin = origin;
}
void set_channel_override(const std::wstring& channel_override) {
channel_override_ = channel_override;
payload_.channel_override = channel_override_.c_str();
}
void set_is_extended_stable_channel(bool is_extended_stable_channel) {
payload_.is_extended_stable_channel = is_extended_stable_channel;
}
void set_update_ap(const std::wstring& update_ap) {
update_ap_ = update_ap;
payload_.update_ap = update_ap_.c_str();
Expand All @@ -257,6 +288,7 @@ class PrimaryInstallDetails : public InstallDetails {

private:
std::wstring channel_;
std::wstring channel_override_;
std::wstring update_ap_;
std::wstring update_cohort_name_;
Payload payload_ = Payload();
Expand Down
44 changes: 30 additions & 14 deletions chrome/install_static/install_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ namespace {
// Chrome channel display names.
constexpr wchar_t kChromeChannelDev[] = L"dev";
constexpr wchar_t kChromeChannelBeta[] = L"beta";
constexpr wchar_t kChromeChannelExtended[] = L"extended";
constexpr wchar_t kChromeChannelStableExplicit[] = L"stable";
#endif

Expand Down Expand Up @@ -309,17 +310,26 @@ std::wstring ChannelFromAdditionalParameters(const InstallConstants& mode,
}

bool GetChromeChannelNameFromString(const wchar_t* channel_test,
std::wstring& channel) {
std::wstring& channel,
bool& is_extended_stable) {
if (!channel_test)
return false;
if (!*channel_test || !lstrcmpiW(channel_test, kChromeChannelStableExplicit))
if (!*channel_test ||
!lstrcmpiW(channel_test, kChromeChannelStableExplicit)) {
channel = std::wstring();
else if (!lstrcmpiW(channel_test, kChromeChannelBeta))
is_extended_stable = false;
} else if (!lstrcmpiW(channel_test, kChromeChannelExtended)) {
channel = std::wstring();
is_extended_stable = true;
} else if (!lstrcmpiW(channel_test, kChromeChannelBeta)) {
channel = kChromeChannelBeta;
else if (!lstrcmpiW(channel_test, kChromeChannelDev))
is_extended_stable = false;
} else if (!lstrcmpiW(channel_test, kChromeChannelDev)) {
channel = kChromeChannelDev;
else
is_extended_stable = false;
} else {
return false;
}
return true;
}

Expand Down Expand Up @@ -706,8 +716,7 @@ std::wstring GetChromeChannelName() {
}

bool IsExtendedStableChannel() {
// TODO(https://crbug.com/1185621): Detect extended stable.
return false;
return InstallDetails::Get().is_extended_stable_channel();
}

bool MatchPattern(const std::wstring& source, const std::wstring& pattern) {
Expand Down Expand Up @@ -939,7 +948,8 @@ DetermineChannelResult DetermineChannel(const InstallConstants& mode,
std::wstring* update_ap,
std::wstring* update_cohort_name) {
#if !BUILDFLAG(USE_GOOGLE_UPDATE_INTEGRATION)
return {std::wstring(), ChannelOrigin::kInstallMode};
return {std::wstring(), ChannelOrigin::kInstallMode,
/*is_extended_stable=*/false};
#else
// Read the "ap" value and cache it if requested.
std::wstring client_state(GetClientStateKeyPath(mode.app_guid));
Expand All @@ -963,17 +973,23 @@ DetermineChannelResult DetermineChannel(const InstallConstants& mode,
break;
case ChannelStrategy::ADDITIONAL_PARAMETERS: {
std::wstring channel_override_value;
if (channel_override && GetChromeChannelNameFromString(
channel_override, channel_override_value)) {
return {std::move(channel_override_value), ChannelOrigin::kPolicy};
bool is_extended_stable = false;
if (channel_override &&
GetChromeChannelNameFromString(
channel_override, channel_override_value, is_extended_stable)) {
return {std::move(channel_override_value), ChannelOrigin::kPolicy,
is_extended_stable};
}
return {ChannelFromAdditionalParameters(mode, ap_value),
ChannelOrigin::kAdditionalParameters};
ChannelOrigin::kAdditionalParameters,
/*is_extended_stable=*/false};
}
case ChannelStrategy::FIXED:
return {mode.default_channel_name, ChannelOrigin::kInstallMode};
return {mode.default_channel_name, ChannelOrigin::kInstallMode,
/*is_extended_stable=*/false};
}
return {std::wstring(), ChannelOrigin::kInstallMode};
return {std::wstring(), ChannelOrigin::kInstallMode,
/*is_extended_stable=*/false};
#endif
}

Expand Down
17 changes: 11 additions & 6 deletions chrome/install_static/install_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -293,14 +293,19 @@ bool RecursiveDirectoryCreate(const std::wstring& full_path);
struct DetermineChannelResult {
std::wstring channel_name;
ChannelOrigin origin;

// True if this client follows the extended stable update channel. May only be
// true if `channel_name` is "" and `origin` is kPolicy.
bool is_extended_stable;
};

// Returns the unadorned channel name and its origin based on the channel
// strategy for the install mode. |channel_override|, if not empty is the
// channel to return if |mode| supports non-fixed channels. |update_ap|, if not
// null, is set to the raw "ap" value read from Chrome's ClientState key in the
// registry. |update_cohort_name|, if not null, is set to the raw "cohort\name"
// value read from Chrome's ClientState key in the registry.
// Returns the unadorned channel name, its origin, and an indication of whether
// or not a stable ("") channel is truly the extended stable channel based on
// the channel strategy for the install mode. |channel_override|, if not empty
// is the channel to return if |mode| supports non-fixed channels. |update_ap|,
// if not null, is set to the raw "ap" value read from Chrome's ClientState key
// in the registry. |update_cohort_name|, if not null, is set to the raw
// "cohort\name" value read from Chrome's ClientState key in the registry.
DetermineChannelResult DetermineChannel(const InstallConstants& mode,
bool system_level,
const wchar_t* channel_override,
Expand Down
3 changes: 3 additions & 0 deletions chrome/install_static/product_install_details.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ std::unique_ptr<PrimaryInstallDetails> MakeProductDetails(
&update_ap, &update_cohort_name);
details->set_channel(channel.channel_name);
details->set_channel_origin(channel.origin);
if (channel.origin == ChannelOrigin::kPolicy)
details->set_channel_override(channel_from_registry);
details->set_is_extended_stable_channel(channel.is_extended_stable);
details->set_update_ap(update_ap);
details->set_update_cohort_name(update_cohort_name);

Expand Down
20 changes: 13 additions & 7 deletions chrome/install_static/product_install_details_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -307,20 +307,23 @@ TEST_P(MakeProductDetailsTest, DefaultChannel) {
// Test that the default channel is sniffed properly based on the channel
// override.
TEST_P(MakeProductDetailsTest, PolicyOverrideChannel) {
static constexpr std::tuple<const wchar_t*, const wchar_t*, const wchar_t*>
static constexpr std::tuple<const wchar_t*, const wchar_t*, const wchar_t*,
bool>
kChannelOverrides[] = {
{nullptr, L"", L""}, {nullptr, L"1.1-beta", L"beta"},
{L"", L"", L""}, {L"", L"1.1-beta", L""},
{L"stable", L"", L""}, {L"stable", L"1.1-beta", L""},
{L"dev", L"", L"dev"}, {L"dev", L"1.1-beta", L"dev"},
{L"beta", L"", L"beta"}, {L"beta", L"2.0-dev", L"beta"},
{L"", L"", L"", false}, {L"", L"1.1-beta", L"", false},
{L"stable", L"", L"", false}, {L"stable", L"1.1-beta", L"", false},
{L"extended", L"", L"", true}, {L"extended", L"1.1-beta", L"", true},
{L"dev", L"", L"dev", false}, {L"dev", L"1.1-beta", L"dev", false},
{L"beta", L"", L"beta", false}, {L"beta", L"2.0-dev", L"beta", false},
};
for (const auto& override_ap_channel : kChannelOverrides) {
const wchar_t* channel_override;
const wchar_t* ap;
const wchar_t* expected_channel;
bool extended_stable;

std::tie(channel_override, ap, expected_channel) = override_ap_channel;
std::tie(channel_override, ap, expected_channel, extended_stable) =
override_ap_channel;
if (ap)
SetAp(ap);
if (channel_override)
Expand All @@ -331,6 +334,9 @@ TEST_P(MakeProductDetailsTest, PolicyOverrideChannel) {
if (kInstallModes[test_data().index].channel_strategy ==
ChannelStrategy::ADDITIONAL_PARAMETERS) {
EXPECT_THAT(details->channel(), StrEq(expected_channel));
EXPECT_THAT(details->channel_origin(), Eq(ChannelOrigin::kPolicy));
EXPECT_THAT(details->channel_override(), StrEq(channel_override));
EXPECT_THAT(details->is_extended_stable_channel(), Eq(extended_stable));
} else {
// "ap" and override are ignored for this mode.
EXPECT_THAT(details->channel(), StrEq(test_data().channel));
Expand Down
34 changes: 22 additions & 12 deletions chrome/installer/setup/install_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -714,18 +714,10 @@ bool AppendPostInstallTasks(const InstallParams& install_params,
new Not(new ConditionRunIfFileExists(new_chrome_exe))));
regular_update_work_items->set_log_message("RegularUpdateWorkItemList");

// Convey the channel name to the browser if this installer instance's
// channel is enforced by policy. Otherwise, delete the registry value.
const auto& install_details = install_static::InstallDetails::Get();
if (install_details.channel_origin() ==
install_static::ChannelOrigin::kPolicy) {
post_install_task_list->AddSetRegValueWorkItem(
root, clients_key, KEY_WOW64_32KEY, google_update::kRegChannelField,
install_details.channel(), true);
} else {
regular_update_work_items->AddDeleteRegValueWorkItem(
root, clients_key, KEY_WOW64_32KEY, google_update::kRegChannelField);
}
// If a channel was specified by policy, update the "channel" registry value
// with it so that the browser knows which channel to use, otherwise delete
// whatever value that key holds.
AddChannelWorkItems(root, clients_key, regular_update_work_items.get());

// Since this was not an in-use-update, delete 'opv', 'cpv',
// and 'cmd' keys.
Expand Down Expand Up @@ -1048,4 +1040,22 @@ void AddOsUpgradeWorkItems(const InstallerState& installer_state,
}
}

void AddChannelWorkItems(HKEY root,
const std::wstring& clients_key,
WorkItemList* list) {
const auto& install_details = install_static::InstallDetails::Get();
if (install_details.channel_origin() ==
install_static::ChannelOrigin::kPolicy) {
// Use channel_override rather than simply channel so that extended stable
// is differentiated from regular.
list->AddSetRegValueWorkItem(root, clients_key, KEY_WOW64_32KEY,
google_update::kRegChannelField,
install_details.channel_override(),
/*overwrite=*/true);
} else {
list->AddDeleteRegValueWorkItem(root, clients_key, KEY_WOW64_32KEY,
google_update::kRegChannelField);
}
}

} // namespace installer
7 changes: 7 additions & 0 deletions chrome/installer/setup/install_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ void AddOsUpgradeWorkItems(const InstallerState& installer_state,
const base::Version& new_version,
WorkItemList* install_list);

// Adds work items to set or delete the "channel" value in `clients_key`. The
// value is set if a channel was provided to the installer via the --channel
// command line switch and deleted otherwise.
void AddChannelWorkItems(HKEY root,
const std::wstring& clients_key,
WorkItemList* list);

} // namespace installer

#endif // CHROME_INSTALLER_SETUP_INSTALL_WORKER_H_
3 changes: 3 additions & 0 deletions chrome/installer/setup/setup_install_details.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ std::unique_ptr<install_static::PrimaryInstallDetails> MakeInstallDetails(
&update_ap, &update_cohort_name);
details->set_channel(channel.channel_name);
details->set_channel_origin(channel.origin);
if (channel.origin == install_static::ChannelOrigin::kPolicy)
details->set_channel_override(channel_from_cmd_line);
details->set_is_extended_stable_channel(channel.is_extended_stable);
details->set_update_ap(update_ap);
details->set_update_cohort_name(update_cohort_name);

Expand Down
12 changes: 1 addition & 11 deletions chrome/installer/setup/setup_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -470,17 +470,7 @@ installer::InstallStatus RenameChromeExecutables(
// If a channel was specified by policy, update the "channel" registry value
// with it so that the browser knows which channel to use, otherwise delete
// whatever value that key holds.
const auto& install_details = install_static::InstallDetails::Get();
if (install_details.channel_origin() ==
install_static::ChannelOrigin::kPolicy) {
install_list->AddSetRegValueWorkItem(reg_root, clients_key, KEY_WOW64_32KEY,
google_update::kRegChannelField,
install_details.channel(), true);
} else {
install_list->AddDeleteRegValueWorkItem(reg_root, clients_key,
KEY_WOW64_32KEY,
google_update::kRegChannelField);
}
installer::AddChannelWorkItems(reg_root, clients_key, install_list.get());

// old_chrome.exe is still in use in most cases, so ignore failures here.
install_list->AddDeleteTreeWorkItem(chrome_old_exe, temp_path.path())
Expand Down

0 comments on commit 918f67c

Please sign in to comment.