Skip to content

Commit

Permalink
Revert "Fully ship EarlyProcessSingleton experiment"
Browse files Browse the repository at this point in the history
This reverts commit e010fb9.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=1475609

Original change's description:
> Fully ship EarlyProcessSingleton experiment
>
> This CL is shipping by default the early process singleton
> experiment.
>
> Previously, the process singleton was taken after the
> creation of the BrowserProcessImpl and was not covering all the
> accesses to Chrome user-data-dir (a.k.a prefs).
>
> The new early implementation is lifting the scope of the singleton
> into ChromeMainDelegate class. This is simplifying the logic around
> what is covered by the singleton.
>
> There will be no BrowserProcessImpl(...) unless the process own the
> singleton.
>
> Bug: 1340599
> Change-Id: I4e87059f9d218f0bd178f0c93cf97d33bdeea44a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4779488
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1184324}

Bug: 1340599, 1475609
Change-Id: I79806d2bfc37cb73151aa28d539639fc6a1b9e01
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4812735
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1188243}
  • Loading branch information
bergeret authored and Chromium LUCI CQ committed Aug 25, 2023
1 parent 0d63ed5 commit 4614246
Show file tree
Hide file tree
Showing 5 changed files with 261 additions and 44 deletions.
95 changes: 52 additions & 43 deletions chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -679,55 +679,63 @@ absl::optional<int> ChromeMainDelegate::PostEarlyInitialization(
}

#if BUILDFLAG(ENABLE_PROCESS_SINGLETON)
// Take the Chrome process singleton lock. The process can become the
// Browser process if it succeeds to take the lock. Otherwise, the
// command-line is sent to the actual Browser process and the current
// process exits.

// The User Data dir is guaranteed to be valid as per InitializeUserDataDir.
base::FilePath user_data_dir =
base::PathService::CheckedGet(chrome::DIR_USER_DATA);
// Configure the early process singleton experiment.
const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();
ChromeProcessSingleton::SetupEarlySingletonFeature(command_line);

ChromeProcessSingleton::CreateInstance(user_data_dir);
if (ChromeProcessSingleton::IsEarlySingletonFeatureEnabled()) {
// Take the Chrome process singleton lock. The process can become the
// Browser process if it succeed to take the lock. Otherwise, the
// command-line is sent to the actual Browser process and the current
// process can be exited.
base::FilePath user_data_dir;
if (!base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
return chrome::RESULT_CODE_MISSING_DATA;
}

ProcessSingleton::NotifyResult notify_result =
ChromeProcessSingleton::GetInstance()->NotifyOtherProcessOrCreate();
UMA_HISTOGRAM_ENUMERATION("Chrome.ProcessSingleton.NotifyResult",
notify_result, ProcessSingleton::kNumNotifyResults);
ChromeProcessSingleton::CreateInstance(user_data_dir);

// If |notify_result| is not PROCESS_NONE, this process will exit. To ensure
// that the histograms emitted in this process are reported, report the
// metrics accumulated this session with a future session's metrics.
if (notify_result != ProcessSingleton::PROCESS_NONE) {
DeferBrowserMetrics(user_data_dir);
}
ProcessSingleton::NotifyResult notify_result =
ChromeProcessSingleton::GetInstance()->NotifyOtherProcessOrCreate();
UMA_HISTOGRAM_ENUMERATION("Chrome.ProcessSingleton.NotifyResult",
notify_result,
ProcessSingleton::kNumNotifyResults);

switch (notify_result) {
case ProcessSingleton::PROCESS_NONE:
// No process already running, continue on to starting a new one.
break;

case ProcessSingleton::PROCESS_NOTIFIED: {
// Ensure there is an instance of ResourceBundle that is initialized for
// localized string resource accesses.
ui::ScopedStartupResourceBundle startup_resource_bundle;
printf("%s\n", base::SysWideToNativeMB(
base::UTF16ToWide(l10n_util::GetStringUTF16(
IDS_USED_EXISTING_BROWSER)))
.c_str());
return chrome::RESULT_CODE_NORMAL_EXIT_PROCESS_NOTIFIED;
// If |notify_result| is not PROCESS_NONE, this process will exit. To ensure
// that the histograms emitted in this process are reported, report the
// metrics accumulated this session with a future session's metrics.
if (notify_result != ProcessSingleton::PROCESS_NONE) {
DeferBrowserMetrics(user_data_dir);
}

case ProcessSingleton::PROFILE_IN_USE:
return chrome::RESULT_CODE_PROFILE_IN_USE;
switch (notify_result) {
case ProcessSingleton::PROCESS_NONE:
// No process already running, continue on to starting a new one.
break;

case ProcessSingleton::PROCESS_NOTIFIED: {
// Ensure there is an instance of ResourceBundle that is initialized for
// localized string resource accesses.
ui::ScopedStartupResourceBundle startup_resource_bundle;
printf("%s\n", base::SysWideToNativeMB(
base::UTF16ToWide(l10n_util::GetStringUTF16(
IDS_USED_EXISTING_BROWSER)))
.c_str());
return chrome::RESULT_CODE_NORMAL_EXIT_PROCESS_NOTIFIED;
}

case ProcessSingleton::LOCK_ERROR:
LOG(ERROR) << "Failed to create a ProcessSingleton for your profile "
"directory. This means that running multiple instances "
"would start multiple browser processes rather than "
"opening a new window in the existing process. Aborting "
"now to avoid profile corruption.";
return chrome::RESULT_CODE_PROFILE_IN_USE;
case ProcessSingleton::PROFILE_IN_USE:
return chrome::RESULT_CODE_PROFILE_IN_USE;

case ProcessSingleton::LOCK_ERROR:
LOG(ERROR) << "Failed to create a ProcessSingleton for your profile "
"directory. This means that running multiple instances "
"would start multiple browser processes rather than "
"opening a new window in the existing process. Aborting "
"now to avoid profile corruption.";
return chrome::RESULT_CODE_PROFILE_IN_USE;
}
}
#endif

Expand Down Expand Up @@ -1648,7 +1656,8 @@ void ChromeMainDelegate::ProcessExiting(const std::string& process_type) {
}

#if BUILDFLAG(ENABLE_PROCESS_SINGLETON)
ChromeProcessSingleton::DeleteInstance();
if (ChromeProcessSingleton::IsEarlySingletonFeatureEnabled())
ChromeProcessSingleton::DeleteInstance();
#endif // BUILDFLAG(ENABLE_PROCESS_SINGLETON)

if (SubprocessNeedsResourceBundle(process_type))
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/chrome_browser_field_trials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include "base/time/time.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/buildflags.h"
#include "chrome/browser/chrome_process_singleton.h"
#include "chrome/browser/metrics/chrome_browser_sampling_trials.h"
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
#include "chrome/browser/metrics/chrome_metrics_service_client.h"
Expand Down Expand Up @@ -109,6 +111,9 @@ void ChromeBrowserFieldTrials::SetUpClientSideFieldTrials(
}

void ChromeBrowserFieldTrials::RegisterSyntheticTrials() {
#if BUILDFLAG(ENABLE_PROCESS_SINGLETON)
ChromeProcessSingleton::RegisterEarlySingletonFeature();
#endif // BUILDFLAG(ENABLE_PROCESS_SINGLETON)
#if BUILDFLAG(IS_ANDROID)
static constexpr char kReachedCodeProfilerTrial[] =
"ReachedCodeProfilerSynthetic2";
Expand Down
69 changes: 68 additions & 1 deletion chrome/browser/chrome_browser_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,11 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() {
// Force MediaCaptureDevicesDispatcher to be created on UI thread.
MediaCaptureDevicesDispatcher::GetInstance();

#if BUILDFLAG(ENABLE_PROCESS_SINGLETON)
if (!ChromeProcessSingleton::IsEarlySingletonFeatureEnabled())
ChromeProcessSingleton::CreateInstance(user_data_dir_);
#endif // BUILDFLAG(ENABLE_PROCESS_SINGLETON)

// Android's first run is done in Java instead of native.
#if !BUILDFLAG(IS_ANDROID)
// Cache first run state early.
Expand Down Expand Up @@ -1169,7 +1174,8 @@ void ChromeBrowserMainParts::PostCreateThreads() {
#endif

#if BUILDFLAG(ENABLE_PROCESS_SINGLETON)
ChromeProcessSingleton::GetInstance()->StartWatching();
if (ChromeProcessSingleton::IsEarlySingletonFeatureEnabled())
ChromeProcessSingleton::GetInstance()->StartWatching();
#endif

tracing::SetupBackgroundTracingFieldTrial();
Expand Down Expand Up @@ -1451,6 +1457,62 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
CHECK(aura::Env::GetInstance());
#endif // defined(USE_AURA)

#if BUILDFLAG(ENABLE_PROCESS_SINGLETON)
if (!ChromeProcessSingleton::IsEarlySingletonFeatureEnabled()) {
// When another process is running, use that process instead of starting a
// new one. NotifyOtherProcess will currently give the other process up to
// 20 seconds to respond. Note that this needs to be done before we attempt
// to read the profile.
notify_result_ =
ChromeProcessSingleton::GetInstance()->NotifyOtherProcessOrCreate();
UMA_HISTOGRAM_ENUMERATION("Chrome.ProcessSingleton.NotifyResult",
notify_result_,
ProcessSingleton::kNumNotifyResults);

// If `notify_result_` is not PROCESS_NONE, this process will exit.
// Conditionally defer browser metrics (which is how metrics are reported
// when the early singleton feature is enabled) to verify whether the
// metrics reporting mechanism has an impact on the metrics. If
// ShouldMergeMetrics() returns false, the metrics will instead be sent in
// an independent log in some future session.
if (ChromeProcessSingleton::ShouldMergeMetrics() &&
notify_result_ != ProcessSingleton::PROCESS_NONE) {
base::FilePath user_data_dir;
if (base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
DeferBrowserMetrics(user_data_dir);
}
}

switch (notify_result_) {
case ProcessSingleton::PROCESS_NONE:
// No process already running, fall through to starting a new one.
ChromeProcessSingleton::GetInstance()->StartWatching();
g_browser_process->platform_part()
->PlatformSpecificCommandLineProcessing(
*base::CommandLine::ForCurrentProcess());
break;

case ProcessSingleton::PROCESS_NOTIFIED:
printf("%s\n", base::SysWideToNativeMB(
base::UTF16ToWide(l10n_util::GetStringUTF16(
IDS_USED_EXISTING_BROWSER)))
.c_str());
return chrome::RESULT_CODE_NORMAL_EXIT_PROCESS_NOTIFIED;

case ProcessSingleton::PROFILE_IN_USE:
return chrome::RESULT_CODE_PROFILE_IN_USE;

case ProcessSingleton::LOCK_ERROR:
LOG(ERROR) << "Failed to create a ProcessSingleton for your profile "
"directory. This means that running multiple instances "
"would start multiple browser processes rather than "
"opening a new window in the existing process. Aborting "
"now to avoid profile corruption.";
return chrome::RESULT_CODE_PROFILE_IN_USE;
}
}
#endif // BUILDFLAG(ENABLE_PROCESS_SINGLETON)

#if BUILDFLAG(IS_WIN)
// We must call DoUpgradeTasks now that we own the browser singleton to
// finish upgrade tasks (swap) and relaunch if necessary.
Expand Down Expand Up @@ -1979,6 +2041,11 @@ void ChromeBrowserMainParts::PostDestroyThreads() {
master_prefs_.reset();
#endif // !BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(ENABLE_PROCESS_SINGLETON)
if (!ChromeProcessSingleton::IsEarlySingletonFeatureEnabled())
ChromeProcessSingleton::DeleteInstance();
#endif // BUILDFLAG(ENABLE_PROCESS_SINGLETON)

device_event_log::Shutdown();
#endif // BUILDFLAG(IS_ANDROID)
}
Expand Down
129 changes: 129 additions & 0 deletions chrome/browser/chrome_process_singleton.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,99 @@
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
#include "chrome/common/chrome_switches.h"

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC)
#include "base/hash/hash.h"
#include "chrome/common/channel_info.h"
#include "components/version_info/channel.h"
#endif

#if BUILDFLAG(IS_WIN)
#include "base/strings/utf_string_conversions.h"
#include "base/win/registry.h"
#endif

#if BUILDFLAG(IS_LINUX)
#include "base/files/file_util.h"
#endif

#if BUILDFLAG(IS_MAC)
#include "base/mac/mac_util.h"
#endif

namespace {

constexpr char kEarlySingletonForceEnabledGroup[] = "Enabled_Forced3";
constexpr char kEarlySingletonEnabledGroup[] = "Enabled3";
constexpr char kEarlySingletonDisabledMergeGroup[] = "Disabled_Merge3";
constexpr char kEarlySingletonDefaultGroup[] = "Default3";

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC)
constexpr char kEarlySingletonDisabledGroup[] = "Disabled3";
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC)

const char* g_early_singleton_feature_group_ = nullptr;
ChromeProcessSingleton* g_chrome_process_singleton_ = nullptr;

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC)

std::string GetMachineGUID() {
std::string machine_guid;
#if BUILDFLAG(IS_WIN)
base::win::RegKey key;
std::wstring value;
if (key.Open(HKEY_LOCAL_MACHINE, L"SOFTWARE\\Microsoft\\Cryptography",
KEY_QUERY_VALUE | KEY_WOW64_64KEY) != ERROR_SUCCESS ||
key.ReadValue(L"MachineGuid", &value) != ERROR_SUCCESS || value.empty()) {
return std::string();
}

if (!base::WideToUTF8(value.c_str(), value.length(), &machine_guid))
return std::string();
#endif // BUILDFLAG(IS_WIN)

#if BUILDFLAG(IS_LINUX)
if (!base::ReadFileToString(base::FilePath("/etc/machine-id"),
&machine_guid)) {
return std::string();
}
#endif // BUILDFLAG(IS_LINUX)

#if BUILDFLAG(IS_MAC)
machine_guid = base::mac::GetPlatformSerialNumber();
#endif // BUILDFLAG(IS_MAC)

return machine_guid;
}

const char* EnrollMachineInEarlySingletonFeature() {
// Run experiment on early channels only.
const version_info::Channel channel = chrome::GetChannel();
if (channel != version_info::Channel::CANARY &&
channel != version_info::Channel::DEV &&
channel != version_info::Channel::BETA &&
channel != version_info::Channel::UNKNOWN) {
return kEarlySingletonDefaultGroup;
}

const std::string machine_guid = GetMachineGUID();
if (machine_guid.empty()) {
return kEarlySingletonDefaultGroup;
}

switch (base::Hash(machine_guid + "EarlyProcessSingleton") % 3) {
case 0:
return kEarlySingletonEnabledGroup;
case 1:
return kEarlySingletonDisabledGroup;
case 2:
return kEarlySingletonDisabledMergeGroup;
default:
NOTREACHED();
return kEarlySingletonDefaultGroup;
}
}
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC)

} // namespace

ChromeProcessSingleton::ChromeProcessSingleton(
Expand Down Expand Up @@ -82,6 +171,46 @@ ChromeProcessSingleton* ChromeProcessSingleton::GetInstance() {
return g_chrome_process_singleton_;
}

// static
void ChromeProcessSingleton::SetupEarlySingletonFeature(
const base::CommandLine& command_line) {
DCHECK(!g_early_singleton_feature_group_);
if (command_line.HasSwitch(switches::kEnableEarlyProcessSingleton)) {
g_early_singleton_feature_group_ = kEarlySingletonForceEnabledGroup;
return;
}

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC)
g_early_singleton_feature_group_ = EnrollMachineInEarlySingletonFeature();
#else
g_early_singleton_feature_group_ = kEarlySingletonDefaultGroup;
#endif
}

// static
void ChromeProcessSingleton::RegisterEarlySingletonFeature() {
DCHECK(g_early_singleton_feature_group_);
// The synthetic trial needs to use kCurrentLog to ensure that UMA report will
// be generated from the metrics log that is open at the time of registration.
ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(
"EarlyProcessSingleton", g_early_singleton_feature_group_,
variations::SyntheticTrialAnnotationMode::kCurrentLog);
}

// static
bool ChromeProcessSingleton::IsEarlySingletonFeatureEnabled() {
return g_early_singleton_feature_group_ == kEarlySingletonEnabledGroup ||
g_early_singleton_feature_group_ == kEarlySingletonForceEnabledGroup;
}

// static
bool ChromeProcessSingleton::ShouldMergeMetrics() {
// This should not be called when the early singleton feature is enabled.
DCHECK(g_early_singleton_feature_group_ && !IsEarlySingletonFeatureEnabled());

return g_early_singleton_feature_group_ == kEarlySingletonDisabledMergeGroup;
}

bool ChromeProcessSingleton::NotificationCallback(
const base::CommandLine& command_line,
const base::FilePath& current_directory) {
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/chrome_process_singleton.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ class ChromeProcessSingleton {
// Retrieve the chrome process singleton instance for the current process.
static ChromeProcessSingleton* GetInstance();

// Setup the experiment for the early process singleton. Remove this code
// when the experiment is over (http://www.crbug.com/1340599).
static void SetupEarlySingletonFeature(const base::CommandLine& command_line);
static void RegisterEarlySingletonFeature();
static bool IsEarlySingletonFeatureEnabled();
static bool ShouldMergeMetrics();

private:
bool NotificationCallback(const base::CommandLine& command_line,
const base::FilePath& current_directory);
Expand Down

0 comments on commit 4614246

Please sign in to comment.