From 8637807161aa6a5046e3374750b7f24ae497a823 Mon Sep 17 00:00:00 2001 From: "suzhe@chromium.org" Date: Fri, 31 Jul 2009 07:09:34 +0000 Subject: [PATCH] Reverting 22144. still broke valgrind ui_tests. Review URL: http://codereview.chromium.org/159694 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22145 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/process_singleton.h | 9 +- chrome/browser/process_singleton_linux.cc | 416 ++++-------------- .../browser/process_singleton_linux_uitest.cc | 102 ----- chrome/browser/process_singleton_win.cc | 4 +- chrome/chrome.gyp | 5 - 5 files changed, 94 insertions(+), 442 deletions(-) delete mode 100644 chrome/browser/process_singleton_linux_uitest.cc diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h index ed572fe46841ea..3ef620a6bcde4a 100644 --- a/chrome/browser/process_singleton.h +++ b/chrome/browser/process_singleton.h @@ -68,12 +68,6 @@ class ProcessSingleton : public NonThreadSafe { } private: -#if defined(OS_WIN) || defined(OS_LINUX) - // Timeout for the current browser process to respond. 20 seconds should be - // enough. It's only used in Windows and Linux implementations. - static const int kTimeoutInSeconds = 20; -#endif - bool locked_; gfx::NativeWindow foreground_window_; @@ -98,6 +92,9 @@ class ProcessSingleton : public NonThreadSafe { HWND remote_window_; // The HWND_MESSAGE of another browser. HWND window_; // The HWND_MESSAGE window. #elif defined(OS_LINUX) + // Set up a socket and sockaddr appropriate for messaging. + void SetupSocket(int* sock, struct sockaddr_un* addr); + // Path in file system to the socket. FilePath socket_path_; diff --git a/chrome/browser/process_singleton_linux.cc b/chrome/browser/process_singleton_linux.cc index 3302cf2eeba324..eebbd39ae63d66 100644 --- a/chrome/browser/process_singleton_linux.cc +++ b/chrome/browser/process_singleton_linux.cc @@ -7,39 +7,15 @@ // send a message to the first chrome browser process with the current // directory and second process command line flags. The second process then // exits. -// -// The socket file's name contains the process id of chrome's browser process, -// eg. "SingletonSocket-9156". A symbol link named "SingletonSocket" will be -// created and pointed to the real socket file, so they would look like: -// -// SingletonSocket -> SingletonSocket-9156 -// SingletonSocket-9156 -// -// So that the socket file can be connected through "SingletonSocket" and the -// process id can also be retrieved from it by calling readlink(). -// -// When the second process sends the current directory and command line flags to -// the first process, it waits for an ACK message back from the first process -// for a certain time. If there is no ACK message back in time, then the first -// process will be considered as hung for some reason. The second process then -// retrieves the process id from the symbol link and kills it by sending -// SIGKILL. Then the second process starts as normal. -// -// TODO(james.su@gmail.com): Add unittest for this class. #include "chrome/browser/process_singleton.h" #include #include -#include -#include -#include #include +#include #include -#include -#include #include -#include #include "base/base_paths.h" #include "base/basictypes.h" @@ -48,7 +24,6 @@ #include "base/logging.h" #include "base/message_loop.h" #include "base/path_service.h" -#include "base/process_util.h" #include "base/stl_util-inl.h" #include "base/string_util.h" #include "base/time.h" @@ -61,16 +36,12 @@ #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_paths.h" -const int ProcessSingleton::kTimeoutInSeconds; - namespace { const char kStartToken[] = "START"; -const char kACKToken[] = "ACK"; -const char kShutdownToken[] = "SHUTDOWN"; const char kTokenDelimiter = '\0'; +const int kTimeOutInSeconds = 5; const int kMaxMessageLength = 32 * 1024; -const int kMaxACKMessageLength = arraysize(kShutdownToken) - 1; // Return 0 on success, -1 on failure. int SetNonBlocking(int fd) { @@ -82,181 +53,6 @@ int SetNonBlocking(int fd) { return fcntl(fd, F_SETFL, flags | O_NONBLOCK); } -// Close a socket and check return value. -void CloseSocket(int fd) { - int rv = HANDLE_EINTR(close(fd)); - DCHECK_EQ(0, rv) << "Error closing socket: " << strerror(errno); -} - -// Write a message to a socket fd. -bool WriteToSocket(int fd, const char *message, size_t length) { - DCHECK(message); - DCHECK(length); - size_t bytes_written = 0; - do { - ssize_t rv = HANDLE_EINTR( - write(fd, message + bytes_written, length - bytes_written)); - if (rv < 0) { - if (errno == EAGAIN || errno == EWOULDBLOCK) { - // The socket shouldn't block, we're sending so little data. Just give - // up here, since NotifyOtherProcess() doesn't have an asynchronous api. - LOG(ERROR) << "ProcessSingleton would block on write(), so it gave up."; - return false; - } - LOG(ERROR) << "write() failed: " << strerror(errno); - return false; - } - bytes_written += rv; - } while (bytes_written < length); - - return true; -} - -// Wait a socket for read for a certain timeout in seconds. -// Returns -1 if error occurred, 0 if timeout reached, > 0 if the socket is -// ready for read. -int WaitSocketForRead(int fd, int timeout) { - fd_set read_fds; - struct timeval tv; - - FD_ZERO(&read_fds); - FD_SET(fd, &read_fds); - tv.tv_sec = timeout; - tv.tv_usec = 0; - - return HANDLE_EINTR(select(fd + 1, &read_fds, NULL, NULL, &tv)); -} - -// Read a message from a socket fd, with an optional timeout in seconds. -// If |timeout| <= 0 then read immediately. -// Return number of bytes actually read, or -1 on error. -ssize_t ReadFromSocket(int fd, char *buf, size_t bufsize, int timeout) { - if (timeout > 0) { - int rv = WaitSocketForRead(fd, timeout); - if (rv <= 0) - return rv; - } - - size_t bytes_read = 0; - do { - ssize_t rv = HANDLE_EINTR(read(fd, buf + bytes_read, bufsize - bytes_read)); - if (rv < 0) { - if (errno != EAGAIN && errno != EWOULDBLOCK) { - LOG(ERROR) << "read() failed: " << strerror(errno); - return rv; - } else { - // It would block, so we just return what has been read. - return bytes_read; - } - } else if (!rv) { - // No more data to read. - return bytes_read; - } else { - bytes_read += rv; - } - } while (bytes_read < bufsize); - - return bytes_read; -} - -// Set up a socket and sockaddr appropriate for messaging. -void SetupSocket(const std::string& path, int* sock, struct sockaddr_un* addr) { - *sock = socket(PF_UNIX, SOCK_STREAM, 0); - CHECK(*sock >= 0) << "socket() failed: " << strerror(errno); - - int rv = SetNonBlocking(*sock); - DCHECK_EQ(0, rv) << "Failed to make non-blocking socket."; - - addr->sun_family = AF_UNIX; - CHECK(path.length() < arraysize(addr->sun_path)) - << "Socket path too long: " << path; - base::strlcpy(addr->sun_path, path.c_str(), arraysize(addr->sun_path)); -} - -// Read a symbol link, return empty string if given path is not a symbol link. -std::string ReadLink(const std::string& path) { - struct stat statbuf; - - if (lstat(path.c_str(), &statbuf) < 0) { - DCHECK_EQ(errno, ENOENT); - return std::string(); - } - - if (S_ISLNK(statbuf.st_mode)) { - char buf[PATH_MAX + 1]; - ssize_t len = readlink(path.c_str(), buf, PATH_MAX); - if (len > 0) { - buf[len] = '\0'; - FilePath real_path(buf); - // If it's not an absolute path, then it's necessary to prepend the - // original path's dirname. - if (!real_path.IsAbsolute()) { - real_path = FilePath(path).DirName().Append(real_path); - } - return real_path.value(); - } else { - LOG(ERROR) << "readlink(" << path << ") failed: " << strerror(errno); - } - } - - return std::string(); -} - -// Unlink a socket path. If the path is a symbol link, then the symbol link -// and the real path referenced by the symbol link will be unlinked together. -bool UnlinkSocketPath(const std::string& path) { - std::string real_path = ReadLink(path); - - bool ret = true; - if (real_path.length()) - ret = UnlinkSocketPath(real_path); - - int rv = unlink(path.c_str()); - if (rv < 0) - DCHECK_EQ(errno, ENOENT); - - return rv == 0 && ret; -} - -// Extract the process's pid from a symbol link path and kill it. -// The pid will be appended to the end of path with a preceding dash, such as: -// .../SingletonSocket-1234 -void KillProcessBySocketPath(const std::string& path) { - std::string real_path = ReadLink(path); - - // If the path is not a symbol link, try to extract pid from the path itself. - if (real_path.empty()) - real_path = path; - - // Only extract pid from the base name, to avoid finding wrong value from its - // parent path name. - std::string base_name = FilePath(real_path).BaseName().value(); - DCHECK(base_name.length()); - - std::string::size_type pos = base_name.rfind('-'); - if (pos != std::string::npos) { - std::string pid_str = base_name.substr(pos + 1); - int pid; - if (StringToInt(pid_str, &pid)) { - // TODO(james.su@gmail.com): Is SIGKILL ok? - int rv = kill(static_cast(pid), SIGKILL); - DCHECK_EQ(0, rv) << "Error killing process:" << strerror(errno); - return; - } - } - - LOG(ERROR) << "Failed to extract pid from path: " << real_path; -} - -// A helper class to close a socket automatically. -class SocketCloser { - public: - explicit SocketCloser(int fd) : fd_(fd) { } - ~SocketCloser() { CloseSocket(fd_); } - private: - int fd_; -}; - } // namespace /////////////////////////////////////////////////////////////////////////////// @@ -269,7 +65,37 @@ class ProcessSingleton::LinuxWatcher public MessageLoop::DestructionObserver, public base::RefCountedThreadSafe { public: - // A helper class to read message from an established socket. + // We expect to only be constructed on the UI thread. + explicit LinuxWatcher(ProcessSingleton* parent) + : ui_message_loop_(MessageLoop::current()), + parent_(parent) { + } + + virtual ~LinuxWatcher() { + STLDeleteElements(&readers_); + } + + // Start listening for connections on the socket. This method should be + // called from the IO thread. + void StartListening(int socket); + + // This method determines if we should use the same process and if we should, + // opens a new browser tab. This runs on the UI thread. + void HandleMessage(std::string current_dir, std::vector argv); + + // MessageLoopForIO::Watcher impl. These run on the IO thread. + virtual void OnFileCanReadWithoutBlocking(int fd); + virtual void OnFileCanWriteWithoutBlocking(int fd) { + // ProcessSingleton only watches for accept (read) events. + NOTREACHED(); + } + + // MessageLoop::DestructionObserver + virtual void WillDestroyCurrentMessageLoop() { + fd_watcher_.StopWatchingFileDescriptor(); + } + + private: class SocketReader : public MessageLoopForIO::Watcher { public: SocketReader(ProcessSingleton::LinuxWatcher* parent, @@ -282,12 +108,13 @@ class ProcessSingleton::LinuxWatcher // Wait for reads. MessageLoopForIO::current()->WatchFileDescriptor( fd, true, MessageLoopForIO::WATCH_READ, &fd_reader_, this); - timer_.Start(base::TimeDelta::FromSeconds(kTimeoutInSeconds), + timer_.Start(base::TimeDelta::FromSeconds(kTimeOutInSeconds), this, &SocketReader::OnTimerExpiry); } virtual ~SocketReader() { - CloseSocket(fd_); + int rv = HANDLE_EINTR(close(fd_)); + DCHECK_EQ(0, rv) << "Error closing socket: " << strerror(errno); } // MessageLoopForIO::Watcher impl. @@ -297,10 +124,6 @@ class ProcessSingleton::LinuxWatcher NOTREACHED(); } - // Finish handling the incoming message by optionally sending back an ACK - // message and removing this SocketReader. - void FinishWithACK(const char *message, size_t length); - private: // If we haven't completed in a reasonable amount of time, give up. void OnTimerExpiry() { @@ -331,40 +154,6 @@ class ProcessSingleton::LinuxWatcher DISALLOW_COPY_AND_ASSIGN(SocketReader); }; - // We expect to only be constructed on the UI thread. - explicit LinuxWatcher(ProcessSingleton* parent) - : ui_message_loop_(MessageLoop::current()), - parent_(parent) { - } - - virtual ~LinuxWatcher() { - STLDeleteElements(&readers_); - } - - // Start listening for connections on the socket. This method should be - // called from the IO thread. - void StartListening(int socket); - - // This method determines if we should use the same process and if we should, - // opens a new browser tab. This runs on the UI thread. - // |reader| is for sending back ACK message. - void HandleMessage(const std::string& current_dir, - const std::vector& argv, - SocketReader *reader); - - // MessageLoopForIO::Watcher impl. These run on the IO thread. - virtual void OnFileCanReadWithoutBlocking(int fd); - virtual void OnFileCanWriteWithoutBlocking(int fd) { - // ProcessSingleton only watches for accept (read) events. - NOTREACHED(); - } - - // MessageLoop::DestructionObserver - virtual void WillDestroyCurrentMessageLoop() { - fd_watcher_.StopWatchingFileDescriptor(); - } - - private: // Removes and deletes the SocketReader. void RemoveSocketReader(SocketReader* reader); @@ -392,8 +181,6 @@ void ProcessSingleton::LinuxWatcher::OnFileCanReadWithoutBlocking(int fd) { LOG(ERROR) << "accept() failed: " << strerror(errno); return; } - int rv = SetNonBlocking(connection_socket); - DCHECK_EQ(0, rv) << "Failed to make non-blocking socket."; SocketReader* reader = new SocketReader(this, ui_message_loop_, connection_socket); @@ -410,18 +197,13 @@ void ProcessSingleton::LinuxWatcher::StartListening(int socket) { &fd_watcher_, this); } -void ProcessSingleton::LinuxWatcher::HandleMessage( - const std::string& current_dir, const std::vector& argv, - SocketReader* reader) { +void ProcessSingleton::LinuxWatcher::HandleMessage(std::string current_dir, + std::vector argv) { DCHECK(ui_message_loop_ == MessageLoop::current()); - DCHECK(reader); // Ignore the request if the browser process is already in shutdown path. if (!g_browser_process || g_browser_process->IsShuttingDown()) { LOG(WARNING) << "Not handling interprocess notification as browser" " is shutting down"; - // Send back "SHUTDOWN" message, so that the client process can start up - // without killing this process. - reader->FinishWithACK(kShutdownToken, arraysize(kShutdownToken) - 1); return; } @@ -429,8 +211,6 @@ void ProcessSingleton::LinuxWatcher::HandleMessage( // we are probably in a first run critical phase. if (parent_->locked()) { DLOG(WARNING) << "Browser is locked"; - // Send back "ACK" message to prevent the client process from starting up. - reader->FinishWithACK(kACKToken, arraysize(kACKToken) - 1); return; } @@ -449,15 +229,14 @@ void ProcessSingleton::LinuxWatcher::HandleMessage( return; } + // TODO(tc): Send an ACK response on success. + // Run the browser startup sequence again, with the command line of the // signalling process. FilePath current_dir_file_path(current_dir); BrowserInit::ProcessCommandLine(parsed_command_line, current_dir_file_path.ToWStringHack(), false, profile, NULL); - - // Send back "ACK" message to prevent the client process from starting up. - reader->FinishWithACK(kACKToken, arraysize(kACKToken) - 1); } void ProcessSingleton::LinuxWatcher::RemoveSocketReader(SocketReader* reader) { @@ -479,7 +258,8 @@ void ProcessSingleton::LinuxWatcher::SocketReader::OnFileCanReadWithoutBlocking( if (rv < 0) { if (errno != EAGAIN && errno != EWOULDBLOCK) { LOG(ERROR) << "read() failed: " << strerror(errno); - CloseSocket(fd); + rv = HANDLE_EINTR(close(fd)); + DCHECK_EQ(0, rv) << "Error closing socket: " << strerror(errno); return; } else { // It would block, so we just return and continue to watch for the next @@ -511,10 +291,6 @@ void ProcessSingleton::LinuxWatcher::SocketReader::OnFileCanReadWithoutBlocking( return; } - // Stop the expiration timer to prevent this SocketReader object from being - // terminated unexpectly. - timer_.Stop(); - std::string current_dir = tokens[1]; // Remove the first two tokens. The remaining tokens should be the command // line argv array. @@ -526,24 +302,9 @@ void ProcessSingleton::LinuxWatcher::SocketReader::OnFileCanReadWithoutBlocking( parent_, &ProcessSingleton::LinuxWatcher::HandleMessage, current_dir, - tokens, - this)); + tokens)); fd_reader_.StopWatchingFileDescriptor(); - // LinuxWatcher::HandleMessage() is in charge of destroying this SocketReader - // object by invoking SocketReader::FinishWithACK(). -} - -void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK( - const char *message, size_t length) { - if (message && length) { - // Not necessary to care about the return value. - WriteToSocket(fd_, message, length); - } - - if (shutdown(fd_, SHUT_WR) < 0) - LOG(ERROR) << "shutdown() failed: " << strerror(errno); - parent_->RemoveSocketReader(this); // We are deleted beyond this point. } @@ -564,10 +325,7 @@ ProcessSingleton::~ProcessSingleton() { bool ProcessSingleton::NotifyOtherProcess() { int socket; sockaddr_un addr; - SetupSocket(socket_path_.value(), &socket, &addr); - - // It'll close the socket automatically when exiting this method. - SocketCloser socket_closer(socket); + SetupSocket(&socket, &addr); // Connecting to the socket int ret = HANDLE_EINTR(connect(socket, @@ -598,59 +356,46 @@ bool ProcessSingleton::NotifyOtherProcess() { } // Send the message - if (!WriteToSocket(socket, to_send.data(), to_send.length())) { - // Try to kill the other process, because it might have been dead. - KillProcessBySocketPath(socket_path_.value()); - return false; - } + int bytes_written = 0; + const int buf_len = to_send.length(); + do { + int rv = HANDLE_EINTR( + write(socket, to_send.data() + bytes_written, buf_len - bytes_written)); + if (rv < 0) { + if (errno == EAGAIN || errno == EWOULDBLOCK) { + // The socket shouldn't block, we're sending so little data. Just give + // up here, since NotifyOtherProcess() doesn't have an asynchronous api. + LOG(ERROR) << "ProcessSingleton would block on write(), so it gave up."; + return false; + } + LOG(ERROR) << "write() failed: " << strerror(errno); + return false; + } + bytes_written += rv; + } while (bytes_written < buf_len); - if (shutdown(socket, SHUT_WR) < 0) + int rv = shutdown(socket, SHUT_WR); + if (rv < 0) { LOG(ERROR) << "shutdown() failed: " << strerror(errno); - - // Read ACK message from the other process. It might be blocked for a certain - // timeout, to make sure the other process has enough time to return ACK. - char buf[kMaxACKMessageLength + 1]; - ssize_t len = - ReadFromSocket(socket, buf, kMaxACKMessageLength, kTimeoutInSeconds); - - // Failed to read ACK, the other process might have been frozen. - if (len <= 0) { - KillProcessBySocketPath(socket_path_.value()); - return false; + } else { + // TODO(tc): We should wait for an ACK, and if we don't get it in a certain + // time period, kill the other process. } - buf[len] = '\0'; - if (strncmp(buf, kShutdownToken, arraysize(kShutdownToken) - 1) == 0) { - // The other process is shutting down, it's safe to start a new process. - return false; - } else if (strncmp(buf, kACKToken, arraysize(kACKToken) - 1) == 0) { - // Assume the other process is handling the request. - return true; - } + rv = HANDLE_EINTR(close(socket)); + DCHECK_EQ(0, rv) << strerror(errno); - NOTREACHED() << "The other process returned unknown message: " << buf; + // Assume the other process is handling the request. return true; } void ProcessSingleton::Create() { int sock; sockaddr_un addr; + SetupSocket(&sock, &addr); - // Append the process id to the socket path, so that other process can find it - // out. - std::string path = StringPrintf( - "%s-%u", socket_path_.value().c_str(), base::GetCurrentProcId()); - SetupSocket(path, &sock, &addr); - - UnlinkSocketPath(socket_path_.value()); - - // Create symbol link before binding the socket, so that the socket file can - // always be reached and removed by another process. - // The symbol link only contains the filename part of the socket file, so that - // the whole config directory can be moved without breaking the symbol link. - std::string symlink_content = FilePath(path).BaseName().value(); - if (symlink(symlink_content.c_str(), socket_path_.value().c_str()) < 0) - NOTREACHED() << "Failed to create symbol link: " << strerror(errno); + if (unlink(socket_path_.value().c_str()) < 0) + DCHECK_EQ(errno, ENOENT); if (bind(sock, reinterpret_cast(&addr), sizeof(addr)) < 0) { LOG(ERROR) << "bind() failed: " << strerror(errno); @@ -658,7 +403,7 @@ void ProcessSingleton::Create() { "directory. This means that running multiple instances of " "the Chrome binary will start multiple browser process " "rather than opening a new window in the existing process."; - CloseSocket(sock); + close(sock); return; } @@ -675,3 +420,18 @@ void ProcessSingleton::Create() { &ProcessSingleton::LinuxWatcher::StartListening, sock)); } + +void ProcessSingleton::SetupSocket(int* sock, struct sockaddr_un* addr) { + *sock = socket(PF_UNIX, SOCK_STREAM, 0); + if (*sock < 0) + LOG(FATAL) << "socket() failed: " << strerror(errno); + + int rv = SetNonBlocking(*sock); + DCHECK_EQ(0, rv) << "Failed to make non-blocking socket."; + + addr->sun_family = AF_UNIX; + if (socket_path_.value().length() > sizeof(addr->sun_path) - 1) + LOG(FATAL) << "Socket path too long: " << socket_path_.value(); + base::strlcpy(addr->sun_path, socket_path_.value().c_str(), + sizeof(addr->sun_path)); +} diff --git a/chrome/browser/process_singleton_linux_uitest.cc b/chrome/browser/process_singleton_linux_uitest.cc deleted file mode 100644 index 6baf3ec4527318..00000000000000 --- a/chrome/browser/process_singleton_linux_uitest.cc +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/process_singleton.h" - -#include -#include -#include -#include -#include -#include - -#include "base/path_service.h" -#include "base/string_util.h" -#include "base/thread.h" -#include "chrome/browser/browser.h" -#include "chrome/browser/browser_process.h" -#include "chrome/common/chrome_constants.h" -#include "chrome/common/chrome_paths.h" -#include "chrome/test/ui/ui_test.h" -#include "testing/gtest/include/gtest/gtest.h" - -class ProcessSingletonLinuxTest : public UITest { - protected: - // A helper method to call ProcessSingleton::NotifyOtherProcess(). - // |url| will be added to CommandLine for current process, so that it can be - // sent to browser process by ProcessSingleton::NotifyOtherProcess(). - void NotifyOtherProcess(const std::string& url, bool expect_result) { - FilePath user_data_dir; - PathService::Get(chrome::DIR_USER_DATA, &user_data_dir); - - std::vector old_argv = - CommandLine::ForCurrentProcess()->argv(); - std::vector argv; - argv.push_back(old_argv[0]); - argv.push_back(url); - - CommandLine::Reset(); - CommandLine::Init(argv); - - ProcessSingleton process_singleton(user_data_dir); - - if (expect_result) - EXPECT_TRUE(process_singleton.NotifyOtherProcess()); - else - EXPECT_FALSE(process_singleton.NotifyOtherProcess()); - } -}; - -// Test if the socket file and symbol link created by ProcessSingletonLinux -// are valid. When running this test, the ProcessSingleton object is already -// initiated by UITest. So we just test against this existing object. -TEST_F(ProcessSingletonLinuxTest, CheckSocketFile) { - FilePath user_data_dir; - FilePath path; - PathService::Get(chrome::DIR_USER_DATA, &user_data_dir); - - path = user_data_dir.Append(chrome::kSingletonSocketFilename); - - struct stat statbuf; - ASSERT_EQ(0, lstat(path.value().c_str(), &statbuf)); - ASSERT_TRUE(S_ISLNK(statbuf.st_mode)); - char buf[PATH_MAX + 1]; - ssize_t len = readlink(path.value().c_str(), buf, PATH_MAX); - ASSERT_GT(len, 0); - buf[len] = '\0'; - - path = user_data_dir.Append(buf); - ASSERT_EQ(0, lstat(path.value().c_str(), &statbuf)); - ASSERT_TRUE(S_ISSOCK(statbuf.st_mode)); -} - -// TODO(james.su@gmail.com): port following tests to Windows. -// Test success case of NotifyOtherProcess(). -TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessSuccess) { - std::string url("about:blank"); - int original_tab_count = GetTabCount(); - - NotifyOtherProcess(url, true); - EXPECT_EQ(original_tab_count + 1, GetTabCount()); - EXPECT_EQ(url, GetActiveTabURL().spec()); -} - -// Test failure case of NotifyOtherProcess(). -TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessFailure) { - // Block the browser process, then it'll be killed by - // ProcessSingleton::NotifyOtherProcess(). - kill(process(), SIGSTOP); - - // Wait for a while to make sure the browser process is actually stopped. - // It's necessary when running with valgrind. - sleep(1); - - std::string url("about:blank"); - NotifyOtherProcess(url, false); - - // Wait for a while to make sure the browser process is actually killed. - sleep(1); - - EXPECT_FALSE(IsBrowserRunning()); -} diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc index 74e1d51ccdbfae..a02becc28e16ee 100644 --- a/chrome/browser/process_singleton_win.cc +++ b/chrome/browser/process_singleton_win.cc @@ -78,6 +78,8 @@ bool ProcessSingleton::NotifyOtherProcess() { AllowSetForegroundWindow(process_id); + // Gives 20 seconds timeout for the current browser process to respond. + const int kTimeout = 20000; COPYDATASTRUCT cds; cds.dwData = 0; cds.cbData = static_cast((to_send.length() + 1) * sizeof(wchar_t)); @@ -88,7 +90,7 @@ bool ProcessSingleton::NotifyOtherProcess() { NULL, reinterpret_cast(&cds), SMTO_ABORTIFHUNG, - kTimeoutInSeconds * 1000, + kTimeout, &result)) { // It is possible that the process owning this window may have died by now. if (!result) { diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index bf7276d52df79c..d20f6f6d14d7ed 100644 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -3559,7 +3559,6 @@ 'browser/media_uitest.cc', 'browser/metrics/metrics_service_uitest.cc', 'browser/printing/printing_layout_uitest.cc', - 'browser/process_singleton_linux_uitest.cc', 'browser/renderer_host/resource_dispatcher_host_uitest.cc', 'browser/sanity_uitest.cc', 'browser/session_history_uitest.cc', @@ -3599,10 +3598,6 @@ 'test/reliability/page_load_test.cc', 'test/ui/layout_plugin_uitest.cc', ], - }, { # else: OS != "linux" - 'sources!': [ - 'browser/process_singleton_linux_uitest.cc', - ], }], ['OS=="linux" and toolkit_views==1', { 'dependencies': [