diff options
Diffstat (limited to 'chrome/browser')
| -rw-r--r-- | chrome/browser/chrome_browser_main.cc | 10 | ||||
| -rw-r--r-- | chrome/browser/process_singleton.h | 15 | ||||
| -rw-r--r-- | chrome/browser/process_singleton_linux.cc | 18 | ||||
| -rw-r--r-- | chrome/browser/process_singleton_linux_unittest.cc | 49 | ||||
| -rw-r--r-- | chrome/browser/process_singleton_mac.cc | 12 | ||||
| -rw-r--r-- | chrome/browser/process_singleton_mac_unittest.cc | 38 | ||||
| -rw-r--r-- | chrome/browser/process_singleton_win.cc | 18 |
7 files changed, 82 insertions, 78 deletions
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index 15900ce..93042e2 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -480,8 +480,8 @@ bool ProcessSingletonNotificationCallback( g_browser_process->PlatformSpecificCommandLineProcessing(command_line); // TODO(erikwright): Consider removing this - AFAIK it is no longer used. - // Handle the --uninstall-extension startup action. This needs to done here - // in the process that is running with the target profile, otherwise the + // Handle the --uninstall-extension startup action. This needs to done here in + // the process that is running with the target profile, otherwise the // uninstall will fail to unload and remove all components. if (command_line.HasSwitch(switches::kUninstallExtension)) { // The uninstall extension switch can't be combined with the profile @@ -797,7 +797,8 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() { // Android's first run is done in Java instead of native. #if !defined(OS_ANDROID) - process_singleton_.reset(new ProcessSingleton(user_data_dir_)); + process_singleton_.reset(new ProcessSingleton( + user_data_dir_, base::Bind(&ProcessSingletonNotificationCallback))); // Ensure ProcessSingleton won't process messages too early. It will be // unlocked in PostBrowserStart(). process_singleton_->Lock(NULL); @@ -1157,8 +1158,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // 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_ = process_singleton_->NotifyOtherProcessOrCreate( - base::Bind(&ProcessSingletonNotificationCallback)); + notify_result_ = process_singleton_->NotifyOtherProcessOrCreate(); UMA_HISTOGRAM_ENUMERATION("NotifyOtherProcessOrCreate.Result", notify_result_, ProcessSingleton::NUM_NOTIFY_RESULTS); diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h index 0cded79..439e170 100644 --- a/chrome/browser/process_singleton.h +++ b/chrome/browser/process_singleton.h @@ -60,28 +60,26 @@ class ProcessSingleton : public base::NonThreadSafe { const CommandLine& command_line, const base::FilePath& current_directory)> NotificationCallback; - explicit ProcessSingleton(const base::FilePath& user_data_dir); + ProcessSingleton(const base::FilePath& user_data_dir, + const NotificationCallback& notification_callback); ~ProcessSingleton(); // Notify another process, if available. Otherwise sets ourselves as the - // singleton instance and stores the provided callback for notification from - // future processes. Returns PROCESS_NONE if we became the singleton + // singleton instance. Returns PROCESS_NONE if we became the singleton // instance. Callers are guaranteed to either have notified an existing // process or have grabbed the singleton (unless the profile is locked by an // unreachable process). // TODO(brettw): Make the implementation of this method non-platform-specific // by making Linux re-use the Windows implementation. - NotifyResult NotifyOtherProcessOrCreate( - const NotificationCallback& notification_callback); + NotifyResult NotifyOtherProcessOrCreate(); // Sets ourself up as the singleton instance. Returns true on success. If // false is returned, we are not the singleton instance and the caller must - // exit. Otherwise, stores the provided callback for notification from - // future processes. + // exit. // NOTE: Most callers should generally prefer NotifyOtherProcessOrCreate() to // this method, only callers for whom failure is prefered to notifying another // process should call this directly. - bool Create(const NotificationCallback& notification_callback); + bool Create(); // Clear any lock state during shutdown. void Cleanup(); @@ -133,7 +131,6 @@ class ProcessSingleton : public base::NonThreadSafe { bool kill_unresponsive); NotifyResult NotifyOtherProcessWithTimeoutOrCreate( const CommandLine& command_line, - const NotificationCallback& notification_callback, int timeout_seconds); void OverrideCurrentPidForTesting(base::ProcessId pid); void OverrideKillCallbackForTesting( diff --git a/chrome/browser/process_singleton_linux.cc b/chrome/browser/process_singleton_linux.cc index ccdb637..20f7db7 100644 --- a/chrome/browser/process_singleton_linux.cc +++ b/chrome/browser/process_singleton_linux.cc @@ -692,9 +692,12 @@ void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK( /////////////////////////////////////////////////////////////////////////////// // ProcessSingleton // -ProcessSingleton::ProcessSingleton(const base::FilePath& user_data_dir) +ProcessSingleton::ProcessSingleton( + const base::FilePath& user_data_dir, + const NotificationCallback& notification_callback) : locked_(false), foreground_window_(NULL), + notification_callback_(notification_callback), current_pid_(base::GetCurrentProcId()), ALLOW_THIS_IN_INITIALIZER_LIST(watcher_(new LinuxWatcher(this))) { socket_path_ = user_data_dir.Append(chrome::kSingletonSocketFilename); @@ -835,24 +838,21 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( return PROCESS_NOTIFIED; } -ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate( - const NotificationCallback& notification_callback) { +ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() { return NotifyOtherProcessWithTimeoutOrCreate( *CommandLine::ForCurrentProcess(), - notification_callback, kTimeoutInSeconds); } ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeoutOrCreate( const CommandLine& command_line, - const NotificationCallback& notification_callback, int timeout_seconds) { NotifyResult result = NotifyOtherProcessWithTimeout(command_line, timeout_seconds, true); if (result != PROCESS_NONE) return result; - if (Create(notification_callback)) + if (Create()) return PROCESS_NONE; // If the Create() failed, try again to notify. (It could be that another // instance was starting at the same time and managed to grab the lock before @@ -879,8 +879,7 @@ void ProcessSingleton::DisablePromptForTesting() { g_disable_prompt = true; } -bool ProcessSingleton::Create( - const NotificationCallback& notification_callback) { +bool ProcessSingleton::Create() { int sock; sockaddr_un addr; @@ -937,8 +936,6 @@ bool ProcessSingleton::Create( if (listen(sock, 5) < 0) NOTREACHED() << "listen failed: " << safe_strerror(errno); - notification_callback_ = notification_callback; - DCHECK(BrowserThread::IsMessageLoopValid(BrowserThread::IO)); BrowserThread::PostTask( BrowserThread::IO, @@ -999,4 +996,3 @@ void ProcessSingleton::KillProcess(int pid) { DCHECK(rv == 0 || errno == ESRCH) << "Error killing process: " << safe_strerror(errno); } - diff --git a/chrome/browser/process_singleton_linux_unittest.cc b/chrome/browser/process_singleton_linux_unittest.cc index 33102ab..358739d 100644 --- a/chrome/browser/process_singleton_linux_unittest.cc +++ b/chrome/browser/process_singleton_linux_unittest.cc @@ -43,11 +43,26 @@ class ProcessSingletonLinuxTest : public testing::Test { class TestableProcessSingleton : public ProcessSingleton { public: explicit TestableProcessSingleton(const base::FilePath& user_data_dir) - : ProcessSingleton(user_data_dir) {} + : ProcessSingleton( + user_data_dir, + base::Bind( + &TestableProcessSingleton::NotificationCallback, + ALLOW_THIS_IN_INITIALIZER_LIST(base::Unretained(this)))) {} + + + std::vector<CommandLine::StringVector> callback_command_lines_; + using ProcessSingleton::NotifyOtherProcessWithTimeout; using ProcessSingleton::NotifyOtherProcessWithTimeoutOrCreate; using ProcessSingleton::OverrideCurrentPidForTesting; using ProcessSingleton::OverrideKillCallbackForTesting; + + private: + bool NotificationCallback(const CommandLine& command_line, + const base::FilePath& current_directory) { + callback_command_lines_.push_back(command_line.argv()); + return true; + } }; ProcessSingletonLinuxTest() @@ -122,7 +137,8 @@ class ProcessSingletonLinuxTest : public testing::Test { CommandLine command_line(CommandLine::ForCurrentProcess()->GetProgram()); command_line.AppendArg("about:blank"); if (override_kill) { - process_singleton->OverrideCurrentPidForTesting(base::GetCurrentProcId() + 1); + process_singleton->OverrideCurrentPidForTesting( + base::GetCurrentProcId() + 1); process_singleton->OverrideKillCallbackForTesting( base::Bind(&ProcessSingletonLinuxTest::KillCallback, base::Unretained(this))); @@ -141,14 +157,18 @@ class ProcessSingletonLinuxTest : public testing::Test { CommandLine command_line(CommandLine::ForCurrentProcess()->GetProgram()); command_line.AppendArg(url); return process_singleton->NotifyOtherProcessWithTimeoutOrCreate( - command_line, base::Bind(&NotificationCallback), timeout.InSeconds()); + command_line, timeout.InSeconds()); } void CheckNotified() { - ASSERT_EQ(1u, callback_command_lines_.size()); + ASSERT_TRUE(process_singleton_on_thread_ != NULL); + ASSERT_EQ(1u, process_singleton_on_thread_->callback_command_lines_.size()); bool found = false; - for (size_t i = 0; i < callback_command_lines_[0].size(); ++i) { - if (callback_command_lines_[0][i] == "about:blank") { + for (size_t i = 0; + i < process_singleton_on_thread_->callback_command_lines_[0].size(); + ++i) { + if (process_singleton_on_thread_->callback_command_lines_[0][i] == + "about:blank") { found = true; break; } @@ -184,9 +204,7 @@ class ProcessSingletonLinuxTest : public testing::Test { ASSERT_TRUE(!process_singleton_on_thread_); process_singleton_on_thread_ = CreateProcessSingleton(); ASSERT_EQ(ProcessSingleton::PROCESS_NONE, - process_singleton_on_thread_->NotifyOtherProcessOrCreate( - base::Bind(&ProcessSingletonLinuxTest::InternalCallback, - base::Unretained(this)))); + process_singleton_on_thread_->NotifyOtherProcessOrCreate()); } void DestructProcessSingleton() { @@ -194,12 +212,6 @@ class ProcessSingletonLinuxTest : public testing::Test { delete process_singleton_on_thread_; } - bool InternalCallback(const CommandLine& command_line, - const base::FilePath& current_directory) { - callback_command_lines_.push_back(command_line.argv()); - return true; - } - void KillCallback(int pid) { kill_callbacks_++; } @@ -211,8 +223,6 @@ class ProcessSingletonLinuxTest : public testing::Test { scoped_ptr<base::Thread> worker_thread_; TestableProcessSingleton* process_singleton_on_thread_; - - std::vector<CommandLine::StringVector> callback_command_lines_; }; } // namespace @@ -348,8 +358,7 @@ TEST_F(ProcessSingletonLinuxTest, CreateFailsWithExistingBrowser) { scoped_ptr<TestableProcessSingleton> process_singleton( CreateProcessSingleton()); process_singleton->OverrideCurrentPidForTesting(base::GetCurrentProcId() + 1); - EXPECT_FALSE(process_singleton->Create( - base::Bind(&NotificationCallback))); + EXPECT_FALSE(process_singleton->Create()); } // Test that Create fails when another browser is using the profile directory @@ -370,7 +379,7 @@ TEST_F(ProcessSingletonLinuxTest, CreateChecksCompatibilitySocket) { socket_path_.value().c_str())); ASSERT_EQ(0, unlink(cookie_path_.value().c_str())); - EXPECT_FALSE(process_singleton->Create(base::Bind(&NotificationCallback))); + EXPECT_FALSE(process_singleton->Create()); } // Test that we fail when lock says process is on another host and we can't diff --git a/chrome/browser/process_singleton_mac.cc b/chrome/browser/process_singleton_mac.cc index 6f26a32..bc03c53 100644 --- a/chrome/browser/process_singleton_mac.cc +++ b/chrome/browser/process_singleton_mac.cc @@ -38,7 +38,9 @@ const int kMaxErrno = 102; // ourselves. An exclusive lock is used to flush out anyone making incorrect // assumptions. -ProcessSingleton::ProcessSingleton(const base::FilePath& user_data_dir) +ProcessSingleton::ProcessSingleton( + const base::FilePath& user_data_dir, + const NotificationCallback& /* notification_callback */) : locked_(false), foreground_window_(NULL), lock_path_(user_data_dir.Append(chrome::kSingletonLockFilename)), @@ -56,10 +58,9 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { return PROCESS_NONE; } -ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate( - const NotificationCallback& notification_callback) { +ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() { // Windows tries NotifyOtherProcess() first. - return Create(notification_callback) ? PROCESS_NONE : PROFILE_IN_USE; + return Create() ? PROCESS_NONE : PROFILE_IN_USE; } // Attempt to acquire an exclusive lock on an empty file in the @@ -71,8 +72,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate( // that for now because it would require confidence that this code is // never called in a situation where an alert wouldn't work. // http://crbug.com/59061 -bool ProcessSingleton::Create( - const NotificationCallback& notification_callback) { +bool ProcessSingleton::Create() { DCHECK_EQ(-1, lock_fd_) << "lock_path_ is already open."; lock_fd_ = HANDLE_EINTR(open(lock_path_.value().c_str(), diff --git a/chrome/browser/process_singleton_mac_unittest.cc b/chrome/browser/process_singleton_mac_unittest.cc index e35c7f1..4246c27 100644 --- a/chrome/browser/process_singleton_mac_unittest.cc +++ b/chrome/browser/process_singleton_mac_unittest.cc @@ -67,9 +67,10 @@ class ProcessSingletonMacTest : public PlatformTest { // Test that the base case doesn't blow up. TEST_F(ProcessSingletonMacTest, Basic) { - ProcessSingleton ps(temp_dir_.path()); + ProcessSingleton ps(temp_dir_.path(), + ProcessSingleton::NotificationCallback()); EXPECT_FALSE(IsLocked()); - EXPECT_TRUE(ps.Create(ProcessSingleton::NotificationCallback())); + EXPECT_TRUE(ps.Create()); EXPECT_TRUE(IsLocked()); ps.Cleanup(); EXPECT_FALSE(IsLocked()); @@ -79,8 +80,9 @@ TEST_F(ProcessSingletonMacTest, Basic) { TEST_F(ProcessSingletonMacTest, DestructorReleases) { EXPECT_FALSE(IsLocked()); { - ProcessSingleton ps(temp_dir_.path()); - EXPECT_TRUE(ps.Create(ProcessSingleton::NotificationCallback())); + ProcessSingleton ps(temp_dir_.path(), + ProcessSingleton::NotificationCallback()); + EXPECT_TRUE(ps.Create()); EXPECT_TRUE(IsLocked()); } EXPECT_FALSE(IsLocked()); @@ -88,8 +90,10 @@ TEST_F(ProcessSingletonMacTest, DestructorReleases) { // Multiple singletons should interlock appropriately. TEST_F(ProcessSingletonMacTest, Interlock) { - ProcessSingleton ps1(temp_dir_.path()); - ProcessSingleton ps2(temp_dir_.path()); + ProcessSingleton ps1(temp_dir_.path(), + ProcessSingleton::NotificationCallback()); + ProcessSingleton ps2(temp_dir_.path(), + ProcessSingleton::NotificationCallback()); // Windows and Linux use a command-line flag to suppress this, but // it is on a sub-process so the scope is contained. Rather than @@ -99,24 +103,26 @@ TEST_F(ProcessSingletonMacTest, Interlock) { // When |ps1| has the lock, |ps2| cannot get it. EXPECT_FALSE(IsLocked()); - EXPECT_TRUE(ps1.Create(ProcessSingleton::NotificationCallback())); + EXPECT_TRUE(ps1.Create()); EXPECT_TRUE(IsLocked()); - EXPECT_FALSE(ps2.Create(ProcessSingleton::NotificationCallback())); + EXPECT_FALSE(ps2.Create()); ps1.Cleanup(); // And when |ps2| has the lock, |ps1| cannot get it. EXPECT_FALSE(IsLocked()); - EXPECT_TRUE(ps2.Create(ProcessSingleton::NotificationCallback())); + EXPECT_TRUE(ps2.Create()); EXPECT_TRUE(IsLocked()); - EXPECT_FALSE(ps1.Create(ProcessSingleton::NotificationCallback())); + EXPECT_FALSE(ps1.Create()); ps2.Cleanup(); EXPECT_FALSE(IsLocked()); } // Like |Interlock| test, but via |NotifyOtherProcessOrCreate()|. TEST_F(ProcessSingletonMacTest, NotifyOtherProcessOrCreate) { - ProcessSingleton ps1(temp_dir_.path()); - ProcessSingleton ps2(temp_dir_.path()); + ProcessSingleton ps1(temp_dir_.path(), + ProcessSingleton::NotificationCallback()); + ProcessSingleton ps2(temp_dir_.path(), + ProcessSingleton::NotificationCallback()); // Windows and Linux use a command-line flag to suppress this, but // it is on a sub-process so the scope is contained. Rather than @@ -128,22 +134,22 @@ TEST_F(ProcessSingletonMacTest, NotifyOtherProcessOrCreate) { EXPECT_FALSE(IsLocked()); EXPECT_EQ( ProcessSingleton::PROCESS_NONE, - ps1.NotifyOtherProcessOrCreate(ProcessSingleton::NotificationCallback())); + ps1.NotifyOtherProcessOrCreate()); EXPECT_TRUE(IsLocked()); EXPECT_EQ( ProcessSingleton::PROFILE_IN_USE, - ps2.NotifyOtherProcessOrCreate(ProcessSingleton::NotificationCallback())); + ps2.NotifyOtherProcessOrCreate()); ps1.Cleanup(); // And when |ps2| has the lock, |ps1| cannot get it. EXPECT_FALSE(IsLocked()); EXPECT_EQ( ProcessSingleton::PROCESS_NONE, - ps2.NotifyOtherProcessOrCreate(ProcessSingleton::NotificationCallback())); + ps2.NotifyOtherProcessOrCreate()); EXPECT_TRUE(IsLocked()); EXPECT_EQ( ProcessSingleton::PROFILE_IN_USE, - ps1.NotifyOtherProcessOrCreate(ProcessSingleton::NotificationCallback())); + ps1.NotifyOtherProcessOrCreate()); ps2.Cleanup(); EXPECT_FALSE(IsLocked()); } diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc index 5ad4c0f..5abe2e3 100644 --- a/chrome/browser/process_singleton_win.cc +++ b/chrome/browser/process_singleton_win.cc @@ -252,8 +252,11 @@ bool ProcessSingleton::EscapeVirtualization( return false; } -ProcessSingleton::ProcessSingleton(const base::FilePath& user_data_dir) +ProcessSingleton::ProcessSingleton( + const base::FilePath& user_data_dir, + const NotificationCallback& notification_callback) : window_(NULL), locked_(false), foreground_window_(NULL), + notification_callback_(notification_callback), is_virtualized_(false), lock_file_(INVALID_HANDLE_VALUE), user_data_dir_(user_data_dir) { } @@ -384,10 +387,9 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { return PROCESS_NONE; } -ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate( - const NotificationCallback& notification_callback) { +ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() { ProcessSingleton::NotifyResult result = PROCESS_NONE; - if (!Create(notification_callback)) { + if (!Create()) { result = NotifyOtherProcess(); if (result == PROCESS_NONE) result = PROFILE_IN_USE; @@ -401,10 +403,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate( // Look for a Chrome instance that uses the same profile directory. If there // isn't one, create a message window with its title set to the profile // directory path. -bool ProcessSingleton::Create( - const NotificationCallback& notification_callback) { - DCHECK(notification_callback_.is_null()); - +bool ProcessSingleton::Create() { static const wchar_t kMutexName[] = L"Local\\ChromeProcessSingletonStartup!"; static const wchar_t kMetroActivationEventName[] = L"Local\\ChromeProcessSingletonStartupMetroActivation!"; @@ -531,9 +530,6 @@ bool ProcessSingleton::Create( } } - if (window_ != NULL) - notification_callback_ = notification_callback; - return window_ != NULL; } |