Commit 4bcb0ce7 authored by Alexander Cooper's avatar Alexander Cooper Committed by Commit Bot

Convert chrome/browser/*process_singleton* Callbacks/Binds

Converts Callbacks used by the *process_singleton* classes to Repeating
Callbacks. Most callbacks are obviously (at least potentially) called
multiple times; however, the ShouldKillRemoteProcessCallback appears to
only be called once, as a result of PreMainMessageLoopRun. Despite this,
this change opts to make it a RepeatingCallback as nothing in its usage
would prevent it from being called multiple times and it appears to only
be a callback (instead of a direct method call), so that the behavior
can be overridden by tests.

Additionally, converts base::Binds used by the *process_singleton*
classes where the consuming API has been updated to take a particular
type.

After this change the *process_singleton* files should be clean of any
base::Bind(, base::Callback<, and base::Closure calls.

Bug: 1007635
Change-Id: Ifbf9fc165e063440f4f6ac3212f0cd403f70dfa3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2296637Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarGreg Thompson <grt@chromium.org>
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Auto-Submit: Alexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789258}
parent 42e8173c
...@@ -28,7 +28,7 @@ void ChromeProcessSingleton::Cleanup() { ...@@ -28,7 +28,7 @@ void ChromeProcessSingleton::Cleanup() {
} }
void ChromeProcessSingleton::SetModalDialogNotificationHandler( void ChromeProcessSingleton::SetModalDialogNotificationHandler(
base::Closure notification_handler) { base::RepeatingClosure notification_handler) {
modal_dialog_lock_.SetModalDialogNotificationHandler( modal_dialog_lock_.SetModalDialogNotificationHandler(
std::move(notification_handler)); std::move(notification_handler));
} }
......
...@@ -44,7 +44,8 @@ class ChromeProcessSingleton { ...@@ -44,7 +44,8 @@ class ChromeProcessSingleton {
// Receives a callback to be run to close the active modal dialog, or an empty // Receives a callback to be run to close the active modal dialog, or an empty
// closure if the active dialog is dismissed. // closure if the active dialog is dismissed.
void SetModalDialogNotificationHandler(base::Closure notification_handler); void SetModalDialogNotificationHandler(
base::RepeatingClosure notification_handler);
// Executes previously queued command-line invocations and allows future // Executes previously queued command-line invocations and allows future
// invocations to be executed immediately. // invocations to be executed immediately.
......
...@@ -37,11 +37,11 @@ TEST(ChromeProcessSingletonTest, Basic) { ...@@ -37,11 +37,11 @@ TEST(ChromeProcessSingletonTest, Basic) {
ChromeProcessSingleton ps1( ChromeProcessSingleton ps1(
profile_dir.GetPath(), profile_dir.GetPath(),
base::Bind(&ServerCallback, base::Unretained(&callback_count))); base::BindRepeating(&ServerCallback, base::Unretained(&callback_count)));
ps1.Unlock(); ps1.Unlock();
ChromeProcessSingleton ps2(profile_dir.GetPath(), ChromeProcessSingleton ps2(profile_dir.GetPath(),
base::Bind(&ClientCallback)); base::BindRepeating(&ClientCallback));
ps2.Unlock(); ps2.Unlock();
ProcessSingleton::NotifyResult result = ps1.NotifyOtherProcessOrCreate(); ProcessSingleton::NotifyResult result = ps1.NotifyOtherProcessOrCreate();
...@@ -63,10 +63,10 @@ TEST(ChromeProcessSingletonTest, Lock) { ...@@ -63,10 +63,10 @@ TEST(ChromeProcessSingletonTest, Lock) {
ChromeProcessSingleton ps1( ChromeProcessSingleton ps1(
profile_dir.GetPath(), profile_dir.GetPath(),
base::Bind(&ServerCallback, base::Unretained(&callback_count))); base::BindRepeating(&ServerCallback, base::Unretained(&callback_count)));
ChromeProcessSingleton ps2(profile_dir.GetPath(), ChromeProcessSingleton ps2(profile_dir.GetPath(),
base::Bind(&ClientCallback)); base::BindRepeating(&ClientCallback));
ps2.Unlock(); ps2.Unlock();
ProcessSingleton::NotifyResult result = ps1.NotifyOtherProcessOrCreate(); ProcessSingleton::NotifyResult result = ps1.NotifyOtherProcessOrCreate();
...@@ -100,13 +100,13 @@ TEST(ChromeProcessSingletonTest, LockWithModalDialog) { ...@@ -100,13 +100,13 @@ TEST(ChromeProcessSingletonTest, LockWithModalDialog) {
ChromeProcessSingleton ps1( ChromeProcessSingleton ps1(
profile_dir.GetPath(), profile_dir.GetPath(),
base::Bind(&ServerCallback, base::Unretained(&callback_count))); base::BindRepeating(&ServerCallback, base::Unretained(&callback_count)));
ps1.SetModalDialogNotificationHandler( ps1.SetModalDialogNotificationHandler(base::BindRepeating(
base::Bind(&ModalNotificationHandler, &ModalNotificationHandler,
base::Unretained(&called_modal_notification_handler))); base::Unretained(&called_modal_notification_handler)));
ChromeProcessSingleton ps2(profile_dir.GetPath(), ChromeProcessSingleton ps2(profile_dir.GetPath(),
base::Bind(&ClientCallback)); base::BindRepeating(&ClientCallback));
ps2.Unlock(); ps2.Unlock();
ProcessSingleton::NotifyResult result = ps1.NotifyOtherProcessOrCreate(); ProcessSingleton::NotifyResult result = ps1.NotifyOtherProcessOrCreate();
...@@ -120,7 +120,7 @@ TEST(ChromeProcessSingletonTest, LockWithModalDialog) { ...@@ -120,7 +120,7 @@ TEST(ChromeProcessSingletonTest, LockWithModalDialog) {
ASSERT_TRUE(called_modal_notification_handler); ASSERT_TRUE(called_modal_notification_handler);
ASSERT_EQ(0, callback_count); ASSERT_EQ(0, callback_count);
ps1.SetModalDialogNotificationHandler(base::Closure()); ps1.SetModalDialogNotificationHandler(base::RepeatingClosure());
ps1.Unlock(); ps1.Unlock();
// The notifications sent while a modal dialog was open were processed after // The notifications sent while a modal dialog was open were processed after
// unlock. // unlock.
......
...@@ -103,8 +103,8 @@ class ProcessSingleton { ...@@ -103,8 +103,8 @@ class ProcessSingleton {
// handled within the current browser instance or false if the remote process // handled within the current browser instance or false if the remote process
// should handle it (i.e., because the current process is shutting down). // should handle it (i.e., because the current process is shutting down).
using NotificationCallback = using NotificationCallback =
base::Callback<bool(const base::CommandLine& command_line, base::RepeatingCallback<bool(const base::CommandLine& command_line,
const base::FilePath& current_directory)>; const base::FilePath& current_directory)>;
ProcessSingleton(const base::FilePath& user_data_dir, ProcessSingleton(const base::FilePath& user_data_dir,
const NotificationCallback& notification_callback); const NotificationCallback& notification_callback);
...@@ -138,7 +138,7 @@ class ProcessSingleton { ...@@ -138,7 +138,7 @@ class ProcessSingleton {
#if defined(OS_WIN) #if defined(OS_WIN)
// Called to query whether to kill a hung browser process that has visible // Called to query whether to kill a hung browser process that has visible
// windows. Return true to allow killing the hung process. // windows. Return true to allow killing the hung process.
using ShouldKillRemoteProcessCallback = base::Callback<bool()>; using ShouldKillRemoteProcessCallback = base::RepeatingCallback<bool()>;
void OverrideShouldKillRemoteProcessCallbackForTesting( void OverrideShouldKillRemoteProcessCallbackForTesting(
const ShouldKillRemoteProcessCallback& display_dialog_callback); const ShouldKillRemoteProcessCallback& display_dialog_callback);
#endif #endif
...@@ -164,7 +164,7 @@ class ProcessSingleton { ...@@ -164,7 +164,7 @@ class ProcessSingleton {
const base::TimeDelta& timeout); const base::TimeDelta& timeout);
void OverrideCurrentPidForTesting(base::ProcessId pid); void OverrideCurrentPidForTesting(base::ProcessId pid);
void OverrideKillCallbackForTesting( void OverrideKillCallbackForTesting(
const base::Callback<void(int)>& callback); const base::RepeatingCallback<void(int)>& callback);
#endif #endif
private: private:
...@@ -204,7 +204,7 @@ class ProcessSingleton { ...@@ -204,7 +204,7 @@ class ProcessSingleton {
// Function to call when the other process is hung and needs to be killed. // Function to call when the other process is hung and needs to be killed.
// Allows overriding for tests. // Allows overriding for tests.
base::Callback<void(int)> kill_callback_; base::RepeatingCallback<void(int)> kill_callback_;
// Path in file system to the socket. // Path in file system to the socket.
base::FilePath socket_path_; base::FilePath socket_path_;
......
...@@ -15,14 +15,15 @@ ProcessSingletonModalDialogLock::ProcessSingletonModalDialogLock( ...@@ -15,14 +15,15 @@ ProcessSingletonModalDialogLock::ProcessSingletonModalDialogLock(
ProcessSingletonModalDialogLock::~ProcessSingletonModalDialogLock() {} ProcessSingletonModalDialogLock::~ProcessSingletonModalDialogLock() {}
void ProcessSingletonModalDialogLock::SetModalDialogNotificationHandler( void ProcessSingletonModalDialogLock::SetModalDialogNotificationHandler(
base::Closure notification_handler) { base::RepeatingClosure notification_handler) {
notification_handler_ = std::move(notification_handler); notification_handler_ = std::move(notification_handler);
} }
ProcessSingleton::NotificationCallback ProcessSingleton::NotificationCallback
ProcessSingletonModalDialogLock::AsNotificationCallback() { ProcessSingletonModalDialogLock::AsNotificationCallback() {
return base::Bind(&ProcessSingletonModalDialogLock::NotificationCallbackImpl, return base::BindRepeating(
base::Unretained(this)); &ProcessSingletonModalDialogLock::NotificationCallbackImpl,
base::Unretained(this));
} }
bool ProcessSingletonModalDialogLock::NotificationCallbackImpl( bool ProcessSingletonModalDialogLock::NotificationCallbackImpl(
......
...@@ -31,7 +31,8 @@ class ProcessSingletonModalDialogLock { ...@@ -31,7 +31,8 @@ class ProcessSingletonModalDialogLock {
// Receives a callback to be run to close the active modal dialog, or an empty // Receives a callback to be run to close the active modal dialog, or an empty
// closure if the active dialog is dismissed. // closure if the active dialog is dismissed.
void SetModalDialogNotificationHandler(base::Closure notification_handler); void SetModalDialogNotificationHandler(
base::RepeatingClosure notification_handler);
// Returns the ProcessSingleton::NotificationCallback. // Returns the ProcessSingleton::NotificationCallback.
// The callback is only valid during the lifetime of the // The callback is only valid during the lifetime of the
...@@ -42,7 +43,7 @@ class ProcessSingletonModalDialogLock { ...@@ -42,7 +43,7 @@ class ProcessSingletonModalDialogLock {
bool NotificationCallbackImpl(const base::CommandLine& command_line, bool NotificationCallbackImpl(const base::CommandLine& command_line,
const base::FilePath& current_directory); const base::FilePath& current_directory);
base::Closure notification_handler_; base::RepeatingClosure notification_handler_;
ProcessSingleton::NotificationCallback original_callback_; ProcessSingleton::NotificationCallback original_callback_;
DISALLOW_COPY_AND_ASSIGN(ProcessSingletonModalDialogLock); DISALLOW_COPY_AND_ASSIGN(ProcessSingletonModalDialogLock);
......
...@@ -478,8 +478,8 @@ class ProcessSingleton::LinuxWatcher ...@@ -478,8 +478,8 @@ class ProcessSingleton::LinuxWatcher
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
// Wait for reads. // Wait for reads.
fd_watch_controller_ = base::FileDescriptorWatcher::WatchReadable( fd_watch_controller_ = base::FileDescriptorWatcher::WatchReadable(
fd, base::Bind(&SocketReader::OnSocketCanReadWithoutBlocking, fd, base::BindRepeating(&SocketReader::OnSocketCanReadWithoutBlocking,
base::Unretained(this))); base::Unretained(this)));
// If we haven't completed in a reasonable amount of time, give up. // If we haven't completed in a reasonable amount of time, give up.
timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(kTimeoutInSeconds), timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(kTimeoutInSeconds),
this, &SocketReader::CleanupAndDeleteSelf); this, &SocketReader::CleanupAndDeleteSelf);
...@@ -590,8 +590,8 @@ void ProcessSingleton::LinuxWatcher::StartListening(int socket) { ...@@ -590,8 +590,8 @@ void ProcessSingleton::LinuxWatcher::StartListening(int socket) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
// Watch for client connections on this socket. // Watch for client connections on this socket.
socket_watcher_ = base::FileDescriptorWatcher::WatchReadable( socket_watcher_ = base::FileDescriptorWatcher::WatchReadable(
socket, base::Bind(&LinuxWatcher::OnSocketCanReadWithoutBlocking, socket, base::BindRepeating(&LinuxWatcher::OnSocketCanReadWithoutBlocking,
base::Unretained(this), socket)); base::Unretained(this), socket));
} }
void ProcessSingleton::LinuxWatcher::HandleMessage( void ProcessSingleton::LinuxWatcher::HandleMessage(
...@@ -719,8 +719,8 @@ ProcessSingleton::ProcessSingleton( ...@@ -719,8 +719,8 @@ ProcessSingleton::ProcessSingleton(
lock_path_ = user_data_dir.Append(chrome::kSingletonLockFilename); lock_path_ = user_data_dir.Append(chrome::kSingletonLockFilename);
cookie_path_ = user_data_dir.Append(chrome::kSingletonCookieFilename); cookie_path_ = user_data_dir.Append(chrome::kSingletonCookieFilename);
kill_callback_ = base::Bind(&ProcessSingleton::KillProcess, kill_callback_ = base::BindRepeating(&ProcessSingleton::KillProcess,
base::Unretained(this)); base::Unretained(this));
} }
ProcessSingleton::~ProcessSingleton() { ProcessSingleton::~ProcessSingleton() {
...@@ -944,7 +944,7 @@ void ProcessSingleton::OverrideCurrentPidForTesting(base::ProcessId pid) { ...@@ -944,7 +944,7 @@ void ProcessSingleton::OverrideCurrentPidForTesting(base::ProcessId pid) {
} }
void ProcessSingleton::OverrideKillCallbackForTesting( void ProcessSingleton::OverrideKillCallbackForTesting(
const base::Callback<void(int)>& callback) { const base::RepeatingCallback<void(int)>& callback) {
kill_callback_ = callback; kill_callback_ = callback;
} }
......
...@@ -47,10 +47,10 @@ class ProcessSingletonPosixTest : public testing::Test { ...@@ -47,10 +47,10 @@ class ProcessSingletonPosixTest : public testing::Test {
class TestableProcessSingleton : public ProcessSingleton { class TestableProcessSingleton : public ProcessSingleton {
public: public:
explicit TestableProcessSingleton(const base::FilePath& user_data_dir) explicit TestableProcessSingleton(const base::FilePath& user_data_dir)
: ProcessSingleton( : ProcessSingleton(user_data_dir,
user_data_dir, base::BindRepeating(
base::Bind(&TestableProcessSingleton::NotificationCallback, &TestableProcessSingleton::NotificationCallback,
base::Unretained(this))) {} base::Unretained(this))) {}
std::vector<base::CommandLine::StringVector> callback_command_lines_; std::vector<base::CommandLine::StringVector> callback_command_lines_;
...@@ -175,9 +175,8 @@ class ProcessSingletonPosixTest : public testing::Test { ...@@ -175,9 +175,8 @@ class ProcessSingletonPosixTest : public testing::Test {
if (override_kill) { if (override_kill) {
process_singleton->OverrideCurrentPidForTesting( process_singleton->OverrideCurrentPidForTesting(
base::GetCurrentProcId() + 1); base::GetCurrentProcId() + 1);
process_singleton->OverrideKillCallbackForTesting( process_singleton->OverrideKillCallbackForTesting(base::BindRepeating(
base::Bind(&ProcessSingletonPosixTest::KillCallback, &ProcessSingletonPosixTest::KillCallback, base::Unretained(this)));
base::Unretained(this)));
} }
return process_singleton->NotifyOtherProcessWithTimeout( return process_singleton->NotifyOtherProcessWithTimeout(
......
...@@ -18,8 +18,9 @@ ProcessSingletonStartupLock::~ProcessSingletonStartupLock() { ...@@ -18,8 +18,9 @@ ProcessSingletonStartupLock::~ProcessSingletonStartupLock() {
ProcessSingleton::NotificationCallback ProcessSingleton::NotificationCallback
ProcessSingletonStartupLock::AsNotificationCallback() { ProcessSingletonStartupLock::AsNotificationCallback() {
return base::Bind(&ProcessSingletonStartupLock::NotificationCallbackImpl, return base::BindRepeating(
base::Unretained(this)); &ProcessSingletonStartupLock::NotificationCallbackImpl,
base::Unretained(this));
} }
void ProcessSingletonStartupLock::Unlock() { void ProcessSingletonStartupLock::Unlock() {
......
...@@ -267,8 +267,7 @@ ProcessSingleton::ProcessSingleton( ...@@ -267,8 +267,7 @@ ProcessSingleton::ProcessSingleton(
lock_file_(INVALID_HANDLE_VALUE), lock_file_(INVALID_HANDLE_VALUE),
user_data_dir_(user_data_dir), user_data_dir_(user_data_dir),
should_kill_remote_process_callback_( should_kill_remote_process_callback_(
base::Bind(&DisplayShouldKillMessageBox)) { base::BindRepeating(&DisplayShouldKillMessageBox)) {}
}
ProcessSingleton::~ProcessSingleton() { ProcessSingleton::~ProcessSingleton() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
...@@ -415,9 +414,10 @@ bool ProcessSingleton::Create() { ...@@ -415,9 +414,10 @@ bool ProcessSingleton::Create() {
if (lock_file_ != INVALID_HANDLE_VALUE) { if (lock_file_ != INVALID_HANDLE_VALUE) {
// Set the window's title to the path of our user data directory so // Set the window's title to the path of our user data directory so
// other Chrome instances can decide if they should forward to us. // other Chrome instances can decide if they should forward to us.
bool result = window_.CreateNamed( bool result =
base::Bind(&ProcessLaunchNotification, notification_callback_), window_.CreateNamed(base::BindRepeating(&ProcessLaunchNotification,
user_data_dir_.value()); notification_callback_),
user_data_dir_.value());
CHECK(result && window_.hwnd()); CHECK(result && window_.hwnd());
} }
} }
......
...@@ -126,8 +126,8 @@ MULTIPROCESS_TEST_MAIN(ProcessSingletonTestProcessMain) { ...@@ -126,8 +126,8 @@ MULTIPROCESS_TEST_MAIN(ProcessSingletonTestProcessMain) {
} }
// Instantiate the process singleton. // Instantiate the process singleton.
ProcessSingleton process_singleton(user_data_dir, ProcessSingleton process_singleton(
base::Bind(&NotificationCallback)); user_data_dir, base::BindRepeating(&NotificationCallback));
if (!process_singleton.Create()) if (!process_singleton.Create())
return kErrorResultCode; return kErrorResultCode;
...@@ -223,11 +223,11 @@ class ProcessSingletonTest : public base::MultiProcessTest { ...@@ -223,11 +223,11 @@ class ProcessSingletonTest : public base::MultiProcessTest {
// The ready event has been signalled - the process singleton is held by // The ready event has been signalled - the process singleton is held by
// the hung sub process. // the hung sub process.
test_singleton_.reset(new ProcessSingleton( test_singleton_.reset(new ProcessSingleton(
user_data_dir(), base::Bind(&NotificationCallback))); user_data_dir(), base::BindRepeating(&NotificationCallback)));
test_singleton_->OverrideShouldKillRemoteProcessCallbackForTesting( test_singleton_->OverrideShouldKillRemoteProcessCallbackForTesting(
base::Bind(&ProcessSingletonTest::MockShouldKillRemoteProcess, base::BindRepeating(&ProcessSingletonTest::MockShouldKillRemoteProcess,
base::Unretained(this), allow_kill)); base::Unretained(this), allow_kill));
} }
base::Process* browser_victim() { return &browser_victim_; } base::Process* browser_victim() { return &browser_victim_; }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment