Put app shim IPC socket in a temporary directory. (Mac)

apps/app_shim/app_shim_host_manager_mac.mm

Index: apps/app_shim/app_shim_host_manager_mac.mm
diff --git a/apps/app_shim/app_shim_host_manager_mac.mm b/apps/app_shim/app_shim_host_manager_mac.mm
index eeaa114adea0e03b5921483ca98379a4ffeee43b..dc1579bd89c976aa0dd7758841eeb8e7c3a8566e 100644
--- a/apps/app_shim/app_shim_host_manager_mac.mm
+++ b/apps/app_shim/app_shim_host_manager_mac.mm
@@ -8,6 +8,7 @@
 #include "apps/app_shim/app_shim_host_mac.h"
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/path_service.h"
@@ -15,7 +16,6 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/mac/app_mode_common.h"
-#include "ipc/unix_domain_socket_util.h"
 
 using content::BrowserThread;
 
@@ -28,8 +28,6 @@ void CreateAppShimHost(const IPC::ChannelHandle& handle) {
 
 }  // namespace
 
-const base::FilePath* AppShimHostManager::g_override_user_data_dir_ = NULL;
-
 AppShimHostManager::AppShimHostManager() {}
 
 void AppShimHostManager::Init() {
@@ -47,23 +45,19 @@ AppShimHostManager::~AppShimHostManager() {
 void AppShimHostManager::InitOnFileThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   base::FilePath user_data_dir;
-  if (g_override_user_data_dir_) {
-    user_data_dir = *g_override_user_data_dir_;
-  } else if (!PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
-    LOG(ERROR) << "Couldn't get user data directory while creating App Shim "
-               << "Host manager.";
+  if (!PathService::Get(chrome::DIR_USER_DATA, &user_data_dir))
     return;
-  }
 
-  base::FilePath socket_path =
-      user_data_dir.Append(app_mode::kAppShimSocketName);
-  // This mirrors a check in unix_domain_socket_util.cc which will guarantee
-  // failure and spam log files on bots because they have deeply nested paths to
-  // |user_data_dir| when swarming. See http://crbug.com/240554. Shim tests that
-  // run on the bots must override the path using AppShimHostManagerTestApi.
-  if (socket_path.value().length() >= IPC::kMaxSocketNameLength &&
-      CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) {
-    return;
+  // The socket cannot be created if its path is longer than 128 chars
+  // (IPC::kMaxSocketNameLength). To circumvent this, we use a short path in

[tapted, 2013/11/22 03:29:43]

nit: circumvent sounds dodgy, perhaps "accommodate" :)

[jackhou1, 2013/11/22 04:25:00]

Done.

+  // /tmp/ that is generated from a hash of the user data dir.
+  base::FilePath socket_path = app_mode::GetShortSocketPath(user_data_dir);
+  if (base::DirectoryExists(socket_path.DirName())) {
+    if (!base::PathIsWritable(socket_path.DirName()))
+      return;
+  } else {
+    if (!file_util::CreateDirectory(socket_path.DirName()))

[tapted, 2013/11/22 03:29:43]

hm - I think we should try to remove this folder when Chrome exits. There may
even be things set up to complain in buildbots if there is stuff lying around
after a clean exit.

Also you might be able to reduce the branching or make things more symmetric by
going straight to `CreateDirectory` in the first condition rather than
`DirectoryExists`.

[jackhou1, 2013/11/22 04:25:00]

I'm just deleting the directory on shutdown if we were the one to create it. Is
it ok to delete it on the UI thread if it's during shutdown? If not, is there a
way to have it delete on the file thread?

[tapted, 2013/11/22 05:04:31]

I'd probably delete it unconditionally - otherwise any Chrome crash would put it
there and it would never go away again.

For deleting on the UI thread... I don't actually know - does a post task not
get flushed? void IPC::ChannelFactory::Close() kinda gets around it just by
doing ::unlink so doesn't get checked by thread_restrictions.

[jackhou1, 2013/11/22 05:51:03]

Done.

I thought PostTask didn't work, but I was doing it wrong. Using PostTask now,
also destroying the factory beforehand.

+      return;
   }
 
   factory_.reset(new IPC::ChannelFactory(socket_path, this));