It's a copy of http://codereview.chromium.org/155772, with the fix for valgrind test failure.

This CL implements the second TODO item of issue 12343:
2) We should send back an ACK to the second process.  If the second process doesn't get an ACK in the given timeout, it should kill the first process and go ahead and start.

The approach of this CL is to append process id to the singleton's socket filename, such as "SingletonSocket-12345", and creates a symbol link "SingletonSocket" to the real socket file. In ProcessSingleton::NotifyOtherProcess() if it's successfully connected to "SingletonSocket" but no ACK received, then the original process can be killed by its process id retrieved from the symbol link.

BUG=http://crbug.com/12343 ProcessSingleton Linux cleanups
TEST=In one terminal, launch chrome and stop the process by pressing ctrl-z, then launch chrome again in another terminal. The second chrome shall be started in 5 seconds, and the first one shall be killed.

Review URL: http://codereview.chromium.org/160436

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22144 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 437 insertions, 94 deletions

diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h
index 3ef620a..ed572fe 100644
--- a/chrome/browser/process_singleton.h
+++ b/chrome/browser/process_singleton.h
@@ -68,6 +68,12 @@ 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_;
 
@@ -92,9 +98,6 @@ 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 eebbd39..3302cf2 100644
--- a/chrome/browser/process_singleton_linux.cc
+++ b/chrome/browser/process_singleton_linux.cc
@@ -7,15 +7,39 @@
 // 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 <errno.h>
 #include <fcntl.h>
-#include <sys/types.h>
+#include <signal.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/types.h>
 #include <sys/un.h>
+#include <unistd.h>
+#include <cstring>
 #include <set>
+#include <string>
 
 #include "base/base_paths.h"
 #include "base/basictypes.h"
@@ -24,6 +48,7 @@
 #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"
@@ -36,12 +61,16 @@
 #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) {
@@ -53,6 +82,181 @@ 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<base::ProcessHandle>(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
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -65,37 +269,7 @@ class ProcessSingleton::LinuxWatcher
       public MessageLoop::DestructionObserver,
       public base::RefCountedThreadSafe<ProcessSingleton::LinuxWatcher> {
  public:
-  // 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<std::string> 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:
+  // A helper class to read message from an established socket.
   class SocketReader : public MessageLoopForIO::Watcher {
    public:
     SocketReader(ProcessSingleton::LinuxWatcher* parent,
@@ -108,13 +282,12 @@ 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() {
-      int rv = HANDLE_EINTR(close(fd_));
-      DCHECK_EQ(0, rv) << "Error closing socket: " << strerror(errno);
+      CloseSocket(fd_);
     }
 
     // MessageLoopForIO::Watcher impl.
@@ -124,6 +297,10 @@ 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() {
@@ -154,6 +331,40 @@ 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<std::string>& 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);
 
@@ -181,6 +392,8 @@ 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);
@@ -197,13 +410,18 @@ void ProcessSingleton::LinuxWatcher::StartListening(int socket) {
                           &fd_watcher_, this);
 }
 
-void ProcessSingleton::LinuxWatcher::HandleMessage(std::string current_dir,
-    std::vector<std::string> argv) {
+void ProcessSingleton::LinuxWatcher::HandleMessage(
+    const std::string& current_dir, const std::vector<std::string>& argv,
+    SocketReader* reader) {
   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;
   }
 
@@ -211,6 +429,8 @@ void ProcessSingleton::LinuxWatcher::HandleMessage(std::string current_dir,
   // 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;
   }
 
@@ -229,14 +449,15 @@ void ProcessSingleton::LinuxWatcher::HandleMessage(std::string current_dir,
     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) {
@@ -258,8 +479,7 @@ void ProcessSingleton::LinuxWatcher::SocketReader::OnFileCanReadWithoutBlocking(
     if (rv < 0) {
       if (errno != EAGAIN && errno != EWOULDBLOCK) {
         LOG(ERROR) << "read() failed: " << strerror(errno);
-        rv = HANDLE_EINTR(close(fd));
-        DCHECK_EQ(0, rv) << "Error closing socket: " << strerror(errno);
+        CloseSocket(fd);
         return;
       } else {
         // It would block, so we just return and continue to watch for the next
@@ -291,6 +511,10 @@ 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.
@@ -302,9 +526,24 @@ void ProcessSingleton::LinuxWatcher::SocketReader::OnFileCanReadWithoutBlocking(
       parent_,
       &ProcessSingleton::LinuxWatcher::HandleMessage,
       current_dir,
-      tokens));
+      tokens,
+      this));
   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.
 }
@@ -325,7 +564,10 @@ ProcessSingleton::~ProcessSingleton() {
 bool ProcessSingleton::NotifyOtherProcess() {
   int socket;
   sockaddr_un addr;
-  SetupSocket(&socket, &addr);
+  SetupSocket(socket_path_.value(), &socket, &addr);
+
+  // It'll close the socket automatically when exiting this method.
+  SocketCloser socket_closer(socket);
 
   // Connecting to the socket
   int ret = HANDLE_EINTR(connect(socket,
@@ -356,46 +598,59 @@ bool ProcessSingleton::NotifyOtherProcess() {
   }
 
   // Send the message
-  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 (!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 rv = shutdown(socket, SHUT_WR);
-  if (rv < 0) {
+  if (shutdown(socket, SHUT_WR) < 0)
     LOG(ERROR) << "shutdown() failed: " << strerror(errno);
-  } 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.
+
+  // 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;
   }
 
-  rv = HANDLE_EINTR(close(socket));
-  DCHECK_EQ(0, rv) << strerror(errno);
+  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;
+  }
 
-  // Assume the other process is handling the request.
+  NOTREACHED() << "The other process returned unknown message: " << buf;
   return true;
 }
 
 void ProcessSingleton::Create() {
   int sock;
   sockaddr_un addr;
-  SetupSocket(&sock, &addr);
 
-  if (unlink(socket_path_.value().c_str()) < 0)
-    DCHECK_EQ(errno, ENOENT);
+  // 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 (bind(sock, reinterpret_cast<sockaddr*>(&addr), sizeof(addr)) < 0) {
     LOG(ERROR) << "bind() failed: " << strerror(errno);
@@ -403,7 +658,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.";
-    close(sock);
+    CloseSocket(sock);
     return;
   }
 
@@ -420,18 +675,3 @@ 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
new file mode 100644
index 0000000..6baf3ec
--- /dev/null
+++ b/chrome/browser/process_singleton_linux_uitest.cc
@@ -0,0 +1,102 @@
+// 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 <sys/types.h>
+#include <sys/wait.h>
+#include <signal.h>
+#include <unistd.h>
+#include <vector>
+#include <string>
+
+#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<std::string> old_argv =
+        CommandLine::ForCurrentProcess()->argv();
+    std::vector<std::string> 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 a02becc..74e1d51 100644
--- a/chrome/browser/process_singleton_win.cc
+++ b/chrome/browser/process_singleton_win.cc
@@ -78,8 +78,6 @@ 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<DWORD>((to_send.length() + 1) * sizeof(wchar_t));
@@ -90,7 +88,7 @@ bool ProcessSingleton::NotifyOtherProcess() {
                          NULL,
                          reinterpret_cast<LPARAM>(&cds),
                          SMTO_ABORTIFHUNG,
-                         kTimeout,
+                         kTimeoutInSeconds * 1000,
                          &result)) {
     // It is possible that the process owning this window may have died by now.
     if (!result) {