Use process_singleton_linux on Mac.

chrome/browser/process_singleton_posix.cc

 // Copyright (c) 2012 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.
 
 // On Linux, when the user tries to launch a second copy of chrome, we check
 // for a socket in the user's profile directory.  If the socket file is open we
 // send a message to the first chrome browser process with the current
 // directory and second process command line flags.  The second process then
 // exits.
 //
@@ skipping 56 matching lines @@
 #include "base/sequenced_task_runner_helpers.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
-#include "chrome/browser/ui/process_singleton_dialog_linux.h"
 #include "chrome/common/chrome_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "net/base/net_util.h"
 #include "ui/base/l10n/l10n_util.h"
 
+#if defined(OS_LINUX)
+#include "chrome/browser/ui/process_singleton_dialog_linux.h"
+#endif
+
 #if defined(TOOLKIT_VIEWS) && !defined(OS_CHROMEOS)
 #include "ui/views/linux_ui/linux_ui.h"
 #endif
 
 using content::BrowserThread;
 
 const int ProcessSingleton::kTimeoutInSeconds;
 
 namespace {
 
@@ skipping 196 matching lines @@
     *pid = -1;
 
   return true;
 }
 
 // Returns true if the user opted to unlock the profile.
 bool DisplayProfileInUseError(const base::FilePath& lock_path,
                               const std::string& hostname,
                               int pid) {
   base::string16 error = l10n_util::GetStringFUTF16(
-      IDS_PROFILE_IN_USE_LINUX,
+      IDS_PROFILE_IN_USE_POSIX,
       base::IntToString16(pid),
       base::ASCIIToUTF16(hostname));
+  LOG(ERROR) << base::SysWideToNativeMB(base::UTF16ToWide(error)).c_str();
+  if (!g_disable_prompt) {
+#if defined(OS_LINUX)
   base::string16 relaunch_button_text = l10n_util::GetStringUTF16(
       IDS_PROFILE_IN_USE_LINUX_RELAUNCH);
-  LOG(ERROR) << base::SysWideToNativeMB(base::UTF16ToWide(error)).c_str();
-  if (!g_disable_prompt)
-    return ShowProcessSingletonDialog(error, relaunch_button_text);
+  return ShowProcessSingletonDialog(error, relaunch_button_text);

[Scott Hess - ex-Googler, 2014/04/18 20:51:33]

Indentation is wrong in here.  I think having the #if/#endif outside the if()
block entirely would be cleaner.

Actually - it would be even cleaner to have:

if (g_disable_prompt)
  return false;

Then put the other cases below that, ending with:

NOTREACHED()
return false;

[jackhou1, 2014/04/24 02:34:54]

Done.

+#endif
+#if defined(OS_MACOSX)

[Scott Hess - ex-Googler, 2014/04/18 20:51:33]

I think #elif defined(OS_MACOSX) would make sense, here, to clarify that these
are mutually-exclusive.

[jackhou1, 2014/04/24 02:34:54]

Done.

+  // On Mac, always usurp the lock.
+  return true;
+#endif
+  }

[tapted, 2014/04/17 08:36:14]

should this closing paren be before the #if?

[jackhou1, 2014/04/24 02:34:54]

It matches with the "if (!g_disable_prompt) {", but that's changed now.

   return false;
 }
 
 bool IsChromeProcess(pid_t pid) {
   base::FilePath other_chrome_path(base::GetProcessExecutablePath(pid));
   return (!other_chrome_path.empty() &&
           other_chrome_path.BaseName() ==
           base::FilePath(chrome::kBrowserProcessExecutableName));
 }
 
@@ skipping 569 matching lines @@
       return false;
   }
 
   // Create the socket file somewhere in /tmp which is usually mounted as a
   // normal filesystem. Some network filesystems (notably AFS) are screwy and
   // do not support Unix domain sockets.
   if (!socket_dir_.CreateUniqueTempDir()) {
     LOG(ERROR) << "Failed to create socket directory.";
     return false;
   }
+
+  // Check that the directory was created with the correct permissions.
+  int dir_mode = 0;
+  if (!base::GetPosixFilePermissions(socket_dir_.path(), &dir_mode) ||
+      dir_mode != base::FILE_PERMISSION_USER_MASK) {

[mattm, 2014/04/17 21:44:55]

Should log an error here?

[Scott Hess - ex-Googler, 2014/04/18 20:51:33]

As long as it doesn't cruft up buildbot or other logs.  I'm not really sure what
good any logging in here will do, as the users who's systems are messed up won't
see the logs or won't understand what to do about it.

That said, is this a security check, or a sanity check?  I can't really think of
how the create can succeed but this can fail, which implies to me that you're
checking for a race condition.  In that case, it should be expected to never
happen, so I think you should CHECK() or something, rather than masking it.

[jackhou1, 2014/04/24 02:34:54]

We don't want to create a socket in an unsafe directory. Changed this to a CHECK
with message.

+    NOTREACHED();
+    return false;
+  }
+
   // Setup the socket symlink and the two cookies.
   base::FilePath socket_target_path =
       socket_dir_.path().Append(chrome::kSingletonSocketFilename);
+  // Check that socket_target_path is short enough for a Unix domain socket.
+  DCHECK_GT(104u, socket_target_path.value().length());

[tapted, 2014/04/17 08:36:14]

This might only make sense on Mac, where the string length of
NSTemporaryDirectory() is somewhat predictable. Not sure if Linux makes many
guarantees about $TMP.

[mattm, 2014/04/17 21:44:55]

SetupSocket already has a CHECK() of the path length, does having a similar
dcheck here add anything?
Also shouldn't use a magic number.

[Scott Hess - ex-Googler, 2014/04/18 20:51:33]

I would think that would be why you'd want the check, because if it's too long
it will break the Unix domain socket.

I think it would be reasonable to just have a more practical check, that the
socket creation doesn't return ETOOLONG or whatever error it returns in that
case.

[jackhou1, 2014/04/24 02:34:54]

You're right, the check in SetupSocketAddr does the same thing.

+
   base::FilePath cookie(GenerateCookie());
   base::FilePath remote_cookie_path =
       socket_dir_.path().Append(chrome::kSingletonCookieFilename);
   UnlinkPath(socket_path_);
   UnlinkPath(cookie_path_);
   if (!SymlinkPath(socket_target_path, socket_path_) ||
       !SymlinkPath(cookie, cookie_path_) ||
       !SymlinkPath(cookie, remote_cookie_path)) {
     // We've already locked things, so we can't have lost the startup race,
     // but something doesn't like us.
@@ skipping 66 matching lines @@
 }
 
 void ProcessSingleton::KillProcess(int pid) {
   // TODO(james.su@gmail.com): Is SIGKILL ok?
   int rv = kill(static_cast<base::ProcessHandle>(pid), SIGKILL);
   // ESRCH = No Such Process (can happen if the other process is already in
   // progress of shutting down and finishes before we try to kill it).
   DCHECK(rv == 0 || errno == ESRCH) << "Error killing process: "
                                     << safe_strerror(errno);
 }