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

apps/app_shim/app_shim_host_manager_mac.mm

 // Copyright 2013 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 "apps/app_shim/app_shim_host_manager_mac.h"
 
 #include "apps/app_shim/app_shim_handler_mac.h"
 #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"
 #include "chrome/browser/browser_process.h"
 #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;
 
 namespace {
 
 void CreateAppShimHost(const IPC::ChannelHandle& handle) {
   // AppShimHost takes ownership of itself.
   (new AppShimHost)->ServeChannel(handle);
 }
 
 }  // namespace
 
-const base::FilePath* AppShimHostManager::g_override_user_data_dir_ = NULL;
-
 AppShimHostManager::AppShimHostManager() {}
 
 void AppShimHostManager::Init() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   apps::AppShimHandler::SetDefaultHandler(&extension_app_shim_handler_);
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&AppShimHostManager::InitOnFileThread, this));
 }
 
 AppShimHostManager::~AppShimHostManager() {
   apps::AppShimHandler::SetDefaultHandler(NULL);
 }
 
 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));
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&AppShimHostManager::ListenOnIOThread, this));
 }
 
 void AppShimHostManager::ListenOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ skipping 11 matching lines @@
       BrowserThread::UI, FROM_HERE,
       base::Bind(&CreateAppShimHost, handle));
 }
 
 void AppShimHostManager::OnListenError() {
   // TODO(tapted): Set a timeout and attempt to reconstruct the channel. Until
   // cases where the error could occur are better known, just reset the factory
   // to allow failure to be communicated via the test API.
   factory_.reset();
 }