Split singleton creation and notifications watching
This CL is decoupling the process singleton Create(...) and
start Watching(...). The current CL should be a no-op.
* The Create(...) phase will take the locks and ensures the browser
is unique. After that phase, the notifications are not yet
processed.
* The call to StartWatching(...) will ensure there are some code
pumping the notifications from other processes.
The purpose of this split is to make it easier to lift the process singleton into earlier startup phases. The Create(...) function can
be called before the creation of the IO-Thread while the StartWatching(...) must happen after.
Bug: 1340599
Change-Id: I11b95c38137736bfaf5edbae86379e124e2990a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3781028
Reviewed-by: Scott Violet <[email protected]>
Commit-Queue: Etienne Bergeron <[email protected]>
Reviewed-by: Gabriel Charette <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1030549}
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index e43b0d8..aca2f19f 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -1424,6 +1424,7 @@
switch (notify_result_) {
case ProcessSingleton::PROCESS_NONE:
// No process already running, fall through to starting a new one.
+ process_singleton_->StartWatching();
g_browser_process->platform_part()->PlatformSpecificCommandLineProcessing(
*base::CommandLine::ForCurrentProcess());
break;
diff --git a/chrome/browser/chrome_process_singleton.cc b/chrome/browser/chrome_process_singleton.cc
index 0919af8..d97fa8a 100644
--- a/chrome/browser/chrome_process_singleton.cc
+++ b/chrome/browser/chrome_process_singleton.cc
@@ -31,6 +31,10 @@
return process_singleton_.NotifyOtherProcessOrCreate();
}
+void ChromeProcessSingleton::StartWatching() {
+ process_singleton_.StartWatching();
+}
+
void ChromeProcessSingleton::Cleanup() {
process_singleton_.Cleanup();
}
diff --git a/chrome/browser/chrome_process_singleton.h b/chrome/browser/chrome_process_singleton.h
index 1cb2ce2..14f047d 100644
--- a/chrome/browser/chrome_process_singleton.h
+++ b/chrome/browser/chrome_process_singleton.h
@@ -42,6 +42,12 @@
// PROFILE_IN_USE if it happens to use the same profile directory.
ProcessSingleton::NotifyResult NotifyOtherProcessOrCreate();
+ // Start watching for notifications from other processes. After this call,
+ // the notifications sent by other process can be processed. This call
+ // requires the browser threads (UI / IO) to be created. Requests that occur
+ // before calling StartWatching(...) will be blocked and may timeout.
+ void StartWatching();
+
// Clear any lock state during shutdown.
void Cleanup();
diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h
index 16bb3aa..7cd82d27 100644
--- a/chrome/browser/process_singleton.h
+++ b/chrome/browser/process_singleton.h
@@ -127,6 +127,9 @@
// another process should call this directly.
bool Create();
+ // Start watching for notifications from other processes.
+ void StartWatching();
+
// Clear any lock state during shutdown.
void Cleanup();
@@ -217,6 +220,7 @@
// Temporary directory to hold the socket.
base::ScopedTempDir socket_dir_;
+ int sock_ = -1;
// Helper class for linux specific messages. LinuxWatcher is ref counted
// because it posts messages between threads.
diff --git a/chrome/browser/process_singleton_fuchsia.cc b/chrome/browser/process_singleton_fuchsia.cc
index 9ab1745..254842a3 100644
--- a/chrome/browser/process_singleton_fuchsia.cc
+++ b/chrome/browser/process_singleton_fuchsia.cc
@@ -38,6 +38,11 @@
return true;
}
+void ProcessSingleton::StartWatching() {
+ // TODO(crbug.com/1235293)
+ NOTIMPLEMENTED_LOG_ONCE();
+}
+
void ProcessSingleton::Cleanup() {
// TODO(crbug.com/1235293)
NOTIMPLEMENTED_LOG_ONCE();
diff --git a/chrome/browser/process_singleton_posix.cc b/chrome/browser/process_singleton_posix.cc
index fa7ae481..7e4cc9c 100644
--- a/chrome/browser/process_singleton_posix.cc
+++ b/chrome/browser/process_singleton_posix.cc
@@ -757,8 +757,7 @@
const base::FilePath& user_data_dir,
const NotificationCallback& notification_callback)
: notification_callback_(notification_callback),
- current_pid_(base::GetCurrentProcId()),
- watcher_(new LinuxWatcher(this)) {
+ current_pid_(base::GetCurrentProcId()) {
socket_path_ = user_data_dir.Append(chrome::kSingletonSocketFilename);
lock_path_ = user_data_dir.Append(chrome::kSingletonLockFilename);
cookie_path_ = user_data_dir.Append(chrome::kSingletonCookieFilename);
@@ -1051,10 +1050,9 @@
// leaving a dangling symlink.
base::FilePath socket_target_path =
socket_dir_.GetPath().Append(chrome::kSingletonSocketFilename);
- int sock;
SockaddrUn addr;
socklen_t socklen;
- SetupSocket(socket_target_path.value(), &sock, &addr, &socklen);
+ SetupSocket(socket_target_path.value(), &sock_, &addr, &socklen);
// Setup the socket symlink and the two cookies.
base::FilePath cookie(GenerateCookie());
@@ -1073,21 +1071,26 @@
return false;
}
- if (bind(sock, reinterpret_cast<sockaddr*>(&addr), socklen) < 0) {
+ if (bind(sock_, reinterpret_cast<sockaddr*>(&addr), socklen) < 0) {
PLOG(ERROR) << "Failed to bind() " << socket_target_path.value();
- CloseSocket(sock);
+ CloseSocket(sock_);
return false;
}
- if (listen(sock, 5) < 0)
+ if (listen(sock_, 5) < 0)
NOTREACHED() << "listen failed: " << base::safe_strerror(errno);
+ return true;
+}
+
+void ProcessSingleton::StartWatching() {
+ DCHECK_GE(sock_, 0);
+ DCHECK(!watcher_);
+ watcher_ = new LinuxWatcher(this);
DCHECK(BrowserThread::IsThreadInitialized(BrowserThread::IO));
content::GetIOThreadTaskRunner({})->PostTask(
FROM_HERE, base::BindOnce(&ProcessSingleton::LinuxWatcher::StartListening,
- watcher_, sock));
-
- return true;
+ watcher_, sock_));
}
void ProcessSingleton::Cleanup() {
diff --git a/chrome/browser/process_singleton_posix_unittest.cc b/chrome/browser/process_singleton_posix_unittest.cc
index 5be7f4ed..0918b12 100644
--- a/chrome/browser/process_singleton_posix_unittest.cc
+++ b/chrome/browser/process_singleton_posix_unittest.cc
@@ -60,6 +60,7 @@
using ProcessSingleton::NotifyOtherProcessWithTimeoutOrCreate;
using ProcessSingleton::OverrideCurrentPidForTesting;
using ProcessSingleton::OverrideKillCallbackForTesting;
+ using ProcessSingleton::StartWatching;
private:
bool NotificationCallback(const base::CommandLine& command_line,
@@ -248,6 +249,7 @@
process_singleton_on_thread_ = CreateProcessSingleton();
ASSERT_EQ(ProcessSingleton::PROCESS_NONE,
process_singleton_on_thread_->NotifyOtherProcessOrCreate());
+ process_singleton_on_thread_->StartWatching();
}
void DestructProcessSingleton() {
diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc
index 363c18e..41bc176 100644
--- a/chrome/browser/process_singleton_win.cc
+++ b/chrome/browser/process_singleton_win.cc
@@ -430,6 +430,8 @@
return window_.hwnd() != NULL;
}
+void ProcessSingleton::StartWatching() {}
+
void ProcessSingleton::Cleanup() {
}
