Always use Mojo ChannelMac in app shims

ChannelMac is now the default Mojo channel implementation, and the
feature condition between it and ChannelPosix can be removed. This
removes the now-dead code for manipulating the UNIX domain socket app
shims used to connect with over ChannelPosix.

Bug: 973882
Change-Id: I67ffa068c814c459002e63c587225394d072271f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1677286
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#673723}

chrome/app_shim/app_shim_controller.h

@@ -71,13 +71,17 @@ class AppShimController : public chrome::mojom::AppShim {
   // Terminates the app shim process.
   void Close();
 
+  // Returns the connection to the AppShimHostManager in the browser. Returns
+  // an invalid endpoint if it is not available yet.
+  mojo::PlatformChannelEndpoint GetBrowserEndpoint();
+
   // Sets up a connection to the AppShimHostManager at the given Mach
   // endpoint name.
   static mojo::PlatformChannelEndpoint ConnectToBrowser(
       const mojo::NamedPlatformChannel::ServerName& server_name);
 
   // Connects to Chrome and sends a LaunchApp message.
-  void InitBootstrapPipe();
+  void InitBootstrapPipe(mojo::PlatformChannelEndpoint endpoint);
 
   // Find a running instance of Chrome and set |chrome_running_app_| to it. If
   // none exists, launch Chrome, and set |chrome_running_app_|.

chrome/app_shim/app_shim_controller.mm

@@ -30,7 +30,6 @@
 #include "components/remote_cocoa/common/application.mojom.h"
 #include "content/public/browser/remote_cocoa.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
-#include "mojo/public/cpp/platform/features.h"
 #include "mojo/public/cpp/platform/named_platform_channel.h"
 #include "mojo/public/cpp/platform/platform_channel.h"
 #include "ui/accelerated_widget_mac/window_resize_helper_mac.h"
@@ -129,21 +128,9 @@ void AppShimController::PollForChromeReady(
   if ([chrome_running_app_ isTerminated])
     LOG(FATAL) << "Running chrome instance terminated before connecting.";
 
-  // Check to see if the app shim socket symlink path exists. If it does, then
-  // we know that the AppShimHostManager is listening in the browser process,
-  // and we can connect.
-  CHECK(!params_.user_data_dir.empty());
-  base::FilePath symlink_path =
-      params_.user_data_dir.Append(app_mode::kAppShimSocketSymlinkName);
-
-  // If the MojoChannelMac signal file is present, the browser is running
-  // with Mach IPC instead of socket-based IPC, and the shim can connect
-  // via that.
-  base::FilePath mojo_channel_mac_signal_file =
-      params_.user_data_dir.Append(app_mode::kMojoChannelMacSignalFile);
-  if (base::PathExists(symlink_path) ||
-      base::PathExists(mojo_channel_mac_signal_file)) {
-    InitBootstrapPipe();
+  mojo::PlatformChannelEndpoint endpoint = GetBrowserEndpoint();
+  if (endpoint.is_valid()) {
+    InitBootstrapPipe(std::move(endpoint));
     return;
   }
 
@@ -158,6 +145,19 @@ void AppShimController::PollForChromeReady(
       kPollPeriodMsec);
 }
 
+mojo::PlatformChannelEndpoint AppShimController::GetBrowserEndpoint() {
+  NSString* browser_bundle_id =
+      base::mac::ObjCCast<NSString>([[NSBundle mainBundle]
+          objectForInfoDictionaryKey:app_mode::kBrowserBundleIDKey]);
+  CHECK(browser_bundle_id);
+
+  std::string name_fragment = base::StringPrintf(
+      "%s.%s.%s", base::SysNSStringToUTF8(browser_bundle_id).c_str(),
+      app_mode::kAppShimBootstrapNameFragment,
+      base::MD5String(params_.user_data_dir.value()).c_str());
+  return ConnectToBrowser(name_fragment);
+}
+
 // static
 mojo::PlatformChannelEndpoint AppShimController::ConnectToBrowser(
     const mojo::NamedPlatformChannel::ServerName& server_name) {
@@ -170,6 +170,11 @@ mojo::PlatformChannelEndpoint AppShimController::ConnectToBrowser(
   // remote (receive) end.
   mojo::PlatformChannelEndpoint server_endpoint =
       mojo::NamedPlatformChannel::ConnectToServer(server_name);
+  // The browser may still be in the process of launching, so the endpoint
+  // may not yet be available.
+  if (!server_endpoint.is_valid())
+    return mojo::PlatformChannelEndpoint();
+
   mojo::PlatformChannel channel;
   mach_msg_base_t message{};
   message.header.msgh_id = app_mode::kBootstrapMsgId;
@@ -188,50 +193,14 @@ mojo::PlatformChannelEndpoint AppShimController::ConnectToBrowser(
   return channel.TakeRemoteEndpoint();
 }
 
-void AppShimController::InitBootstrapPipe() {
+void AppShimController::InitBootstrapPipe(
+    mojo::PlatformChannelEndpoint endpoint) {
   SetUpMenu();
 
   // Chrome will relaunch shims when relaunching apps.
   [NSApp disableRelaunchOnLogin];
   CHECK(!params_.user_data_dir.empty());
 
-  mojo::PlatformChannelEndpoint endpoint;
-
-  // Check for the signal file. If this file is present, then Chrome is running
-  // with features::kMojoChannelMac and the app shim needs to connect over
-  // Mach IPC. The FeatureList also needs to be initialized with that on, so
-  // Mojo internals use the right transport mechanism.
-  base::FilePath mojo_channel_mac_signal_file =
-      params_.user_data_dir.Append(app_mode::kMojoChannelMacSignalFile);
-  if (base::PathExists(mojo_channel_mac_signal_file)) {
-    base::FeatureList::InitializeInstance(mojo::features::kMojoChannelMac.name,
-                                          std::string());
-
-    NSString* browser_bundle_id =
-        base::mac::ObjCCast<NSString>([[NSBundle mainBundle]
-            objectForInfoDictionaryKey:app_mode::kBrowserBundleIDKey]);
-    CHECK(browser_bundle_id);
-
-    std::string name_fragment = base::StringPrintf(
-        "%s.%s.%s", base::SysNSStringToUTF8(browser_bundle_id).c_str(),
-        app_mode::kAppShimBootstrapNameFragment,
-        base::MD5String(params_.user_data_dir.value()).c_str());
-
-    endpoint = ConnectToBrowser(name_fragment);
-  } else {
-    base::FilePath symlink_path =
-        params_.user_data_dir.Append(app_mode::kAppShimSocketSymlinkName);
-    base::FilePath socket_path;
-    if (base::ReadSymbolicLink(symlink_path, &socket_path)) {
-      app_mode::VerifySocketPermissions(socket_path);
-    } else {
-      // The path in the user data dir is not a symlink, try connecting
-      // directly.
-      socket_path = symlink_path;
-    }
-    endpoint = mojo::NamedPlatformChannel::ConnectToServer(socket_path.value());
-  }
-
   CreateChannelAndSendLaunchApp(std::move(endpoint));
 }
 

chrome/browser/apps/app_shim/BUILD.gn

@@ -20,8 +20,6 @@ source_set("app_shim") {
     "extension_app_shim_handler_mac.h",
     "mach_bootstrap_acceptor.cc",
     "mach_bootstrap_acceptor.h",
-    "unix_domain_socket_acceptor.cc",
-    "unix_domain_socket_acceptor.h",
   ]
 
   deps = [

chrome/browser/apps/app_shim/app_shim_host_manager_browsertest_mac.mm

@@ -29,7 +29,6 @@
 #include "components/version_info/version_info.h"
 #include "content/public/test/test_utils.h"
 #include "mojo/public/cpp/bindings/binding.h"
-#include "mojo/public/cpp/platform/features.h"
 #include "mojo/public/cpp/platform/named_platform_channel.h"
 #include "mojo/public/cpp/platform/platform_channel.h"
 #include "mojo/public/cpp/system/isolated_connection.h"
@@ -92,32 +91,19 @@ TestShimClient::TestShimClient()
   base::FilePath user_data_dir;
   CHECK(base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir));
 
-  mojo::PlatformChannelEndpoint endpoint;
-
-  if (base::FeatureList::IsEnabled(mojo::features::kMojoChannelMac)) {
-    std::string name_fragment =
-        base::StringPrintf("%s.%s.%s", base::mac::BaseBundleID(),
-                           app_mode::kAppShimBootstrapNameFragment,
-                           base::MD5String(user_data_dir.value()).c_str());
-    endpoint = ConnectToBrowser(name_fragment);
-  } else {
-    base::FilePath symlink_path =
-        user_data_dir.Append(app_mode::kAppShimSocketSymlinkName);
-
-    base::FilePath socket_path;
-    base::ScopedAllowBlockingForTesting allow_blocking;
-    CHECK(base::ReadSymbolicLink(symlink_path, &socket_path));
-    app_mode::VerifySocketPermissions(socket_path);
-    endpoint = mojo::NamedPlatformChannel::ConnectToServer(socket_path.value());
-  }
+  std::string name_fragment =
+      base::StringPrintf("%s.%s.%s", base::mac::BaseBundleID(),
+                         app_mode::kAppShimBootstrapNameFragment,
+                         base::MD5String(user_data_dir.value()).c_str());
+  mojo::PlatformChannelEndpoint endpoint = ConnectToBrowser(name_fragment);
+
   mojo::ScopedMessagePipeHandle message_pipe =
       mojo_connection_.Connect(std::move(endpoint));
   host_bootstrap_ = chrome::mojom::AppShimHostBootstrapPtr(
       chrome::mojom::AppShimHostBootstrapPtrInfo(std::move(message_pipe), 0));
 }
 
-// Browser Test for AppShimHostManager to test IPC interactions across the
-// UNIX domain socket.
+// Browser Test for AppShimHostManager to test IPC interactions.
 class AppShimHostManagerBrowserTest : public InProcessBrowserTest,
                                       public apps::AppShimHandler,
                                       public chrome::mojom::AppShimHost {
@@ -233,58 +219,43 @@ IN_PROC_BROWSER_TEST_F(AppShimHostManagerBrowserTest, LaunchRegisterOnly) {
   EXPECT_TRUE(last_launch_files_.empty());
 }
 
-// Ensure the domain socket can be created in a fresh user data dir.
+// Ensure bootstrap name registers.
 IN_PROC_BROWSER_TEST_F(AppShimHostManagerBrowserTest,
                        PRE_ReCreate) {
   test::AppShimHostManagerTestApi test_api(
       g_browser_process->platform_part()->app_shim_host_manager());
-  if (base::FeatureList::IsEnabled(mojo::features::kMojoChannelMac)) {
-    EXPECT_TRUE(test_api.mach_acceptor());
-  } else {
-    EXPECT_TRUE(test_api.acceptor());
-  }
+  EXPECT_TRUE(test_api.mach_acceptor());
 }
 
-// Ensure the domain socket can be re-created after a prior browser process has
+// Ensure the bootstrap name can be re-created after a prior browser process has
 // quit.
 IN_PROC_BROWSER_TEST_F(AppShimHostManagerBrowserTest,
                        ReCreate) {
   test::AppShimHostManagerTestApi test_api(
       g_browser_process->platform_part()->app_shim_host_manager());
-  if (base::FeatureList::IsEnabled(mojo::features::kMojoChannelMac)) {
-    EXPECT_TRUE(test_api.mach_acceptor());
-  } else {
-    EXPECT_TRUE(test_api.acceptor());
-  }
+  EXPECT_TRUE(test_api.mach_acceptor());
 }
 
 // Tests for the files created by AppShimHostManager.
-class AppShimHostManagerBrowserTestSocketFiles
+class AppShimHostManagerBrowserTestSymlink
     : public AppShimHostManagerBrowserTest {
  public:
-  AppShimHostManagerBrowserTestSocketFiles() {}
+  AppShimHostManagerBrowserTestSymlink() {}
 
  protected:
-  base::FilePath directory_in_tmp_;
-  base::FilePath symlink_path_;
   base::FilePath version_path_;
 
  private:
   bool SetUpUserDataDirectory() override;
   void TearDownInProcessBrowserTestFixture() override;
 
-  DISALLOW_COPY_AND_ASSIGN(AppShimHostManagerBrowserTestSocketFiles);
+  DISALLOW_COPY_AND_ASSIGN(AppShimHostManagerBrowserTestSymlink);
 };
 
-bool AppShimHostManagerBrowserTestSocketFiles::SetUpUserDataDirectory() {
+bool AppShimHostManagerBrowserTestSymlink::SetUpUserDataDirectory() {
   // Create an existing symlink. It should be replaced by AppShimHostManager.
   base::FilePath user_data_dir;
   EXPECT_TRUE(base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir));
-  symlink_path_ = user_data_dir.Append(app_mode::kAppShimSocketSymlinkName);
-  base::FilePath temp_dir;
-  base::PathService::Get(base::DIR_TEMP, &temp_dir);
-  EXPECT_TRUE(base::CreateSymbolicLink(temp_dir.Append("chrome-XXXXXX"),
-                                       symlink_path_));
 
   // Create an invalid RunningChromeVersion file.
   version_path_ =
@@ -294,34 +265,14 @@ bool AppShimHostManagerBrowserTestSocketFiles::SetUpUserDataDirectory() {
   return AppShimHostManagerBrowserTest::SetUpUserDataDirectory();
 }
 
-void AppShimHostManagerBrowserTestSocketFiles::
+void AppShimHostManagerBrowserTestSymlink::
     TearDownInProcessBrowserTestFixture() {
   // Check that created files have been deleted.
-  EXPECT_FALSE(base::PathExists(directory_in_tmp_));
-  EXPECT_FALSE(base::PathExists(symlink_path_));
   EXPECT_FALSE(base::PathExists(version_path_));
 }
 
-IN_PROC_BROWSER_TEST_F(AppShimHostManagerBrowserTestSocketFiles,
-                       ReplacesSymlinkAndCleansUpFiles) {
-  if (base::FeatureList::IsEnabled(mojo::features::kMojoChannelMac))
-    return;
-
-  // Get the directory created by AppShimHostManager.
-  test::AppShimHostManagerTestApi test_api(
-      g_browser_process->platform_part()->app_shim_host_manager());
-  directory_in_tmp_ = test_api.directory_in_tmp();
-
-  // Check that socket files have been created.
-  base::ScopedAllowBlockingForTesting allow_blocking;
-  EXPECT_TRUE(base::PathExists(directory_in_tmp_));
-  EXPECT_TRUE(base::PathExists(symlink_path_));
-
-  // Check that the symlink has been replaced.
-  base::FilePath socket_path;
-  ASSERT_TRUE(base::ReadSymbolicLink(symlink_path_, &socket_path));
-  EXPECT_EQ(app_mode::kAppShimSocketShortName, socket_path.BaseName().value());
-
+IN_PROC_BROWSER_TEST_F(AppShimHostManagerBrowserTestSymlink,
+                       RunningChromeVersionCorrectlyWritten) {
   // Check that the RunningChromeVersion file is correctly written.
   base::FilePath version;
   EXPECT_TRUE(base::ReadSymbolicLink(version_path_, &version));

chrome/browser/apps/app_shim/app_shim_host_manager_mac.h

@@ -10,7 +10,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/apps/app_shim/mach_bootstrap_acceptor.h"
-#include "chrome/browser/apps/app_shim/unix_domain_socket_acceptor.h"
 #include "content/public/browser/browser_thread.h"
 #include "mojo/public/cpp/platform/platform_channel_endpoint.h"
 
@@ -26,10 +25,10 @@ namespace test {
 class AppShimHostManagerTestApi;
 }
 
-// The AppShimHostManager receives connections from app shims on a UNIX
-// socket (|acceptor_|) and creates a helper object to manage the connection.
-class AppShimHostManager : public apps::UnixDomainSocketAcceptor::Delegate,
-                           public apps::MachBootstrapAcceptor::Delegate,
+// The AppShimHostManager receives connections from app shims on a Mach
+// bootstrap namespace entry (mach_acceptor_) and creates a helper object
+// to manage the connection.
+class AppShimHostManager : public apps::MachBootstrapAcceptor::Delegate,
                            public base::RefCountedThreadSafe<
                                AppShimHostManager,
                                content::BrowserThread::DeleteOnUIThread> {
@@ -53,10 +52,6 @@ class AppShimHostManager : public apps::UnixDomainSocketAcceptor::Delegate,
   friend class test::AppShimHostManagerTestApi;
   virtual ~AppShimHostManager();
 
-  // UnixDomainSocketAcceptor::Delegate implementation.
-  void OnClientConnected(mojo::PlatformChannelEndpoint endpoint) override;
-  void OnListenError() override;
-
   // MachBootstrapAcceptor::Delegate:
   void OnClientConnected(mojo::PlatformChannelEndpoint endpoint,
                          base::ProcessId peer_pid) override;
@@ -64,12 +59,8 @@ class AppShimHostManager : public apps::UnixDomainSocketAcceptor::Delegate,
   // The |acceptor_| must be created on a thread which allows blocking I/O.
   void InitOnBackgroundThread();
 
-  // Called on the IO thread to begin listening for connections from app shims.
-  void ListenOnIOThread();
-
   base::FilePath directory_in_tmp_;
 
-  std::unique_ptr<apps::UnixDomainSocketAcceptor> acceptor_;
   std::unique_ptr<apps::MachBootstrapAcceptor> mach_acceptor_;
 
   std::unique_ptr<apps::ExtensionAppShimHandler> extension_app_shim_handler_;

chrome/browser/apps/app_shim/app_shim_host_manager_mac.mm

@@ -22,38 +22,6 @@
 #include "chrome/common/mac/app_mode_common.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_task_traits.h"
-#include "mojo/public/cpp/platform/features.h"
-
-namespace {
-
-base::FilePath GetDirectoryInTmpTemplate(const base::FilePath& user_data_dir) {
-  base::FilePath temp_dir;
-  CHECK(base::PathService::Get(base::DIR_TEMP, &temp_dir));
-  // Check that it's shorter than the IPC socket length (104) minus the
-  // intermediate folder ("/chrome-XXXXXX/") and kAppShimSocketShortName.
-  DCHECK_GT(83u, temp_dir.value().length());
-  return temp_dir.Append("chrome-XXXXXX");
-}
-
-void DeleteSocketFiles(const base::FilePath& directory_in_tmp,
-                       const base::FilePath& symlink_path,
-                       const base::FilePath& version_path,
-                       const base::FilePath& mojo_channel_mac_signal_file) {
-  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
-                                                base::BlockingType::MAY_BLOCK);
-
-  // Delete in reverse order of creation.
-  if (!version_path.empty())
-    base::DeleteFile(version_path, false);
-  if (!symlink_path.empty())
-    base::DeleteFile(symlink_path, false);
-  if (!directory_in_tmp.empty())
-    base::DeleteFile(directory_in_tmp, true);
-  if (!mojo_channel_mac_signal_file.empty())
-    base::DeleteFile(mojo_channel_mac_signal_file, false);
-}
-
-}  // namespace
 
 AppShimHostManager::AppShimHostManager() {}
 
@@ -72,8 +40,6 @@ void AppShimHostManager::Init() {
 }
 
 AppShimHostManager::~AppShimHostManager() {
-  base::CreateSingleThreadTaskRunnerWithTraits({content::BrowserThread::IO})
-      ->DeleteSoon(FROM_HERE, std::move(acceptor_));
   base::CreateSingleThreadTaskRunnerWithTraits({content::BrowserThread::IO})
       ->DeleteSoon(FROM_HERE, std::move(mach_acceptor_));
 
@@ -84,23 +50,19 @@ AppShimHostManager::~AppShimHostManager() {
     return;
 
   apps::AppShimHandler::SetDefaultHandler(NULL);
+
   base::FilePath user_data_dir;
-  base::FilePath symlink_path;
-  base::FilePath version_path;
-  base::FilePath mojo_channel_mac_signal_file;
   if (base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
-    symlink_path = user_data_dir.Append(app_mode::kAppShimSocketSymlinkName);
-    version_path =
+    base::FilePath version_path =
         user_data_dir.Append(app_mode::kRunningChromeVersionSymlinkName);
-    mojo_channel_mac_signal_file =
-        user_data_dir.Append(app_mode::kMojoChannelMacSignalFile);
+
+    base::PostTaskWithTraits(
+        FROM_HERE,
+        {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
+         base::TaskShutdownBehavior::BLOCK_SHUTDOWN},
+        base::BindOnce(base::IgnoreResult(&base::DeleteFile), version_path,
+                       false));
   }
-  base::PostTaskWithTraits(
-      FROM_HERE,
-      {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
-       base::TaskShutdownBehavior::BLOCK_SHUTDOWN},
-      base::BindOnce(&DeleteSocketFiles, directory_in_tmp_, symlink_path,
-                     version_path, mojo_channel_mac_signal_file));
 }
 
 void AppShimHostManager::InitOnBackgroundThread() {
@@ -110,62 +72,18 @@ void AppShimHostManager::InitOnBackgroundThread() {
   if (!base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir))
     return;
 
+  std::string name_fragment =
+      std::string(app_mode::kAppShimBootstrapNameFragment) + "." +
+      base::MD5String(user_data_dir.value());
+  mach_acceptor_ =
+      std::make_unique<apps::MachBootstrapAcceptor>(name_fragment, this);
+  mach_acceptor_->Start();
+
+  // TODO(rsesek): Delete this after a little while, to ensure everything
+  // is cleaned up.
   base::FilePath mojo_channel_mac_signal_file =
       user_data_dir.Append(app_mode::kMojoChannelMacSignalFile);
-  if (base::FeatureList::IsEnabled(mojo::features::kMojoChannelMac)) {
-    std::string name_fragment =
-        std::string(app_mode::kAppShimBootstrapNameFragment) + "." +
-        base::MD5String(user_data_dir.value());
-    mach_acceptor_ =
-        std::make_unique<apps::MachBootstrapAcceptor>(name_fragment, this);
-    mach_acceptor_->Start();
-
-    // App shims do not have Finch initialized and so cannot test for the
-    // kMojoChannelMac feature. Instead, put a file in the profile directory.
-    // The presence of this file will indicate that app shims should connect
-    // over a Mach channel instead.
-    char data = 1;
-    if (base::WriteFile(mojo_channel_mac_signal_file, &data, 1) != 1) {
-      LOG(ERROR) << "Failed to write MojoChannelMac signal file.";
-    }
-    return;
-  } else {
-    base::DeleteFile(mojo_channel_mac_signal_file, false);
-  }
-
-  // The socket path must be shorter than 104 chars (IPC::kMaxSocketNameLength).
-  // To accommodate this, we use a short path in /tmp/ that is generated from a
-  // hash of the user data dir.
-  std::string directory_string =
-      GetDirectoryInTmpTemplate(user_data_dir).value();
-
-  // mkdtemp() replaces trailing X's randomly and creates the directory.
-  if (!mkdtemp(&directory_string[0])) {
-    PLOG(ERROR) << directory_string;
-    return;
-  }
-
-  directory_in_tmp_ = base::FilePath(directory_string);
-  // Check that the directory was created with the correct permissions.
-  int dir_mode = 0;
-  if (!base::GetPosixFilePermissions(directory_in_tmp_, &dir_mode) ||
-      dir_mode != base::FILE_PERMISSION_USER_MASK) {
-    NOTREACHED();
-    return;
-  }
-
-  // UnixDomainSocketAcceptor creates the socket immediately.
-  base::FilePath socket_path =
-      directory_in_tmp_.Append(app_mode::kAppShimSocketShortName);
-  acceptor_.reset(new apps::UnixDomainSocketAcceptor(socket_path, this));
-
-  // Create a symlink to the socket in the user data dir. This lets the shim
-  // process started from Finder find the actual socket path by following the
-  // symlink with ::readlink().
-  base::FilePath symlink_path =
-      user_data_dir.Append(app_mode::kAppShimSocketSymlinkName);
-  base::DeleteFile(symlink_path, false);
-  base::CreateSymbolicLink(socket_path, symlink_path);
+  base::DeleteFile(mojo_channel_mac_signal_file, false);
 
   // Create a symlink containing the current version string. This allows the
   // shim to load the same framework version as the currently running Chrome
@@ -175,28 +93,6 @@ void AppShimHostManager::InitOnBackgroundThread() {
   base::DeleteFile(version_path, false);
   base::CreateSymbolicLink(base::FilePath(version_info::GetVersionNumber()),
                            version_path);
-
-  base::CreateSingleThreadTaskRunnerWithTraits({content::BrowserThread::IO})
-      ->PostTask(FROM_HERE,
-                 base::BindOnce(&AppShimHostManager::ListenOnIOThread, this));
-}
-
-void AppShimHostManager::ListenOnIOThread() {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  if (!acceptor_->Listen()) {
-    base::CreateSingleThreadTaskRunnerWithTraits({content::BrowserThread::UI})
-        ->PostTask(FROM_HERE,
-                   base::BindOnce(&AppShimHostManager::OnListenError, this));
-  }
-}
-
-void AppShimHostManager::OnClientConnected(
-    mojo::PlatformChannelEndpoint endpoint) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  base::CreateSingleThreadTaskRunnerWithTraits({content::BrowserThread::UI})
-      ->PostTask(FROM_HERE,
-                 base::BindOnce(&AppShimHostBootstrap::CreateForChannel,
-                                std::move(endpoint)));
 }
 
 void AppShimHostManager::OnClientConnected(
@@ -208,10 +104,3 @@ void AppShimHostManager::OnClientConnected(
           base::BindOnce(&AppShimHostBootstrap::CreateForChannelAndPeerID,
                          std::move(endpoint), peer_pid));
 }
-
-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 acceptor
-  // to allow failure to be communicated via the test API.
-  acceptor_.reset();
-}

chrome/browser/apps/app_shim/test/app_shim_host_manager_test_api_mac.cc

@@ -19,10 +19,6 @@ AppShimHostManagerTestApi::AppShimHostManagerTestApi(
   DCHECK(host_manager_);
 }
 
-apps::UnixDomainSocketAcceptor* AppShimHostManagerTestApi::acceptor() {
-  return host_manager_->acceptor_.get();
-}
-
 apps::MachBootstrapAcceptor* AppShimHostManagerTestApi::mach_acceptor() {
   return host_manager_->mach_acceptor_.get();
 }

chrome/browser/apps/app_shim/test/app_shim_host_manager_test_api_mac.h

@@ -18,7 +18,6 @@ class FilePath;
 namespace apps {
 class ExtensionAppShimHandler;
 class MachBootstrapAcceptor;
-class UnixDomainSocketAcceptor;
 }
 
 namespace test {
@@ -27,8 +26,6 @@ class AppShimHostManagerTestApi {
  public:
   explicit AppShimHostManagerTestApi(AppShimHostManager* host_manager);
 
-  apps::UnixDomainSocketAcceptor* acceptor();
-
   apps::MachBootstrapAcceptor* mach_acceptor();
 
   const base::FilePath& directory_in_tmp();

chrome/browser/apps/app_shim/unix_domain_socket_acceptor.cc

-// Copyright 2014 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/apps/app_shim/unix_domain_socket_acceptor.h"
-
-#include <utility>
-
-#include "base/logging.h"
-#include "base/message_loop/message_loop_current.h"
-#include "mojo/public/cpp/platform/socket_utils_posix.h"
-
-namespace apps {
-
-UnixDomainSocketAcceptor::UnixDomainSocketAcceptor(const base::FilePath& path,
-                                                   Delegate* delegate)
-    : server_listen_connection_watcher_(FROM_HERE),
-      named_pipe_(path.value()),
-      delegate_(delegate) {
-  mojo::NamedPlatformChannel::Options options;
-  options.server_name = named_pipe_;
-  mojo::NamedPlatformChannel channel(options);
-  listen_handle_ = channel.TakeServerEndpoint();
-  DCHECK(delegate_);
-}
-
-UnixDomainSocketAcceptor::~UnixDomainSocketAcceptor() {
-  Close();
-}
-
-bool UnixDomainSocketAcceptor::Listen() {
-  if (!listen_handle_.is_valid())
-    return false;
-
-  // Watch the fd for connections, and turn any connections into
-  // active sockets.
-  base::MessageLoopCurrentForIO::Get()->WatchFileDescriptor(
-      listen_handle_.platform_handle().GetFD().get(), true,
-      base::MessagePumpForIO::WATCH_READ, &server_listen_connection_watcher_,
-      this);
-  return true;
-}
-
-// Called by libevent when we can read from the fd without blocking.
-void UnixDomainSocketAcceptor::OnFileCanReadWithoutBlocking(int fd) {
-  DCHECK_EQ(fd, listen_handle_.platform_handle().GetFD().get());
-  base::ScopedFD connection_fd;
-  if (!mojo::AcceptSocketConnection(fd, &connection_fd)) {
-    Close();
-    delegate_->OnListenError();
-    return;
-  }
-
-  if (!connection_fd.is_valid()) {
-    // The accept() failed, but not in such a way that the factory needs to be
-    // shut down.
-    return;
-  }
-
-  delegate_->OnClientConnected(mojo::PlatformChannelEndpoint(
-      mojo::PlatformHandle(std::move(connection_fd))));
-}
-
-void UnixDomainSocketAcceptor::OnFileCanWriteWithoutBlocking(int fd) {
-  NOTREACHED() << "Listen fd should never be writable.";
-}
-
-void UnixDomainSocketAcceptor::Close() {
-  if (!listen_handle_.is_valid())
-    return;
-  listen_handle_.reset();
-  if (unlink(named_pipe_.c_str()) < 0)
-    PLOG(ERROR) << "unlink";
-
-  // Unregister libevent for the listening socket and close it.
-  server_listen_connection_watcher_.StopWatchingFileDescriptor();
-}
-
-}  // namespace apps

chrome/browser/apps/app_shim/unix_domain_socket_acceptor.h

-// Copyright 2014 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.
-
-#ifndef CHROME_BROWSER_APPS_APP_SHIM_UNIX_DOMAIN_SOCKET_ACCEPTOR_H_
-#define CHROME_BROWSER_APPS_APP_SHIM_UNIX_DOMAIN_SOCKET_ACCEPTOR_H_
-
-#include "base/files/file_path.h"
-#include "base/macros.h"
-#include "base/message_loop/message_pump_for_io.h"
-#include "mojo/public/cpp/platform/named_platform_channel.h"
-#include "mojo/public/cpp/platform/platform_channel_endpoint.h"
-#include "mojo/public/cpp/platform/platform_channel_server_endpoint.h"
-
-namespace apps {
-
-// A UnixDomainSocketAcceptor listens on a UNIX domain socket. When a
-// client connects to the socket, it accept()s the connection and
-// passes the new FD to the delegate. The delegate is then responsible
-// for creating a new IPC::Channel for the FD.
-class UnixDomainSocketAcceptor : public base::MessagePumpForIO::FdWatcher {
- public:
-  class Delegate {
-   public:
-    // Called when a client connects to the factory. It is the delegate's
-    // responsibility to create an IPC::Channel for the handle, or else close
-    // the file descriptor contained therein.
-    virtual void OnClientConnected(mojo::PlatformChannelEndpoint endpoint) = 0;
-
-    // Called when an error occurs and the channel is closed.
-    virtual void OnListenError() = 0;
-  };
-
-  UnixDomainSocketAcceptor(const base::FilePath& path, Delegate* delegate);
-
-  ~UnixDomainSocketAcceptor() override;
-
-  // Call this to start listening on the socket.
-  bool Listen();
-
-  // Close and unlink the socket, and stop accepting connections.
-  void Close();
-
- private:
-  void OnFileCanReadWithoutBlocking(int fd) override;
-  void OnFileCanWriteWithoutBlocking(int fd) override;
-
-  base::MessagePumpForIO::FdWatchController server_listen_connection_watcher_;
-  mojo::NamedPlatformChannel::ServerName named_pipe_;
-  Delegate* delegate_;
-  mojo::PlatformChannelServerEndpoint listen_handle_;
-
-  DISALLOW_COPY_AND_ASSIGN(UnixDomainSocketAcceptor);
-};
-
-}  // namespace apps
-
-#endif  // CHROME_BROWSER_APPS_APP_SHIM_UNIX_DOMAIN_SOCKET_ACCEPTOR_H_

chrome/common/mac/app_mode_common.h

@@ -34,13 +34,6 @@ class NSString;
 
 namespace app_mode {
 
-// The IPC socket used to communicate between app shims and Chrome will be
-// created under a temporary directory with this name.
-extern const char kAppShimSocketShortName[];
-// A symlink to allow the app shim to find the socket will be created under the
-// user data dir with this name.
-extern const char kAppShimSocketSymlinkName[];
-
 // Mach message ID used by the shim to connect to Chrome.
 constexpr mach_msg_id_t kBootstrapMsgId = 'apps';
 
@@ -50,6 +43,7 @@ constexpr mach_msg_id_t kBootstrapMsgId = 'apps';
 // of the full profile directory path.
 extern const char kAppShimBootstrapNameFragment[];
 
+// TODO(rsesek): Delete this after ensuring the file has been cleaned up.
 // The name of a file placed in the profile directory to indicate that app
 // shims should connect over Mach IPC rather than a UNIX domain socket. If
 // a file named this does not exist, app shims will fall back to the UNIX
@@ -198,10 +192,6 @@ struct ChromeAppModeInfo {
   const char* profile_dir;
 };
 
-// Check that the socket and its parent directory have the correct permissions
-// and are owned by the user.
-void VerifySocketPermissions(const base::FilePath& socket_path);
-
 }  // namespace app_mode
 
 #endif  // CHROME_COMMON_MAC_APP_MODE_COMMON_H_

chrome/common/mac/app_mode_common.mm

@@ -7,13 +7,8 @@
 #import <Foundation/Foundation.h>
 #include <type_traits>
 
-#include "base/files/file_util.h"
-
 namespace app_mode {
 
-const char kAppShimSocketShortName[] = "Socket";
-const char kAppShimSocketSymlinkName[] = "App Shim Socket";
-
 const char kAppShimBootstrapNameFragment[] = "apps";
 const char kMojoChannelMacSignalFile[] = "apps_use_mojo_channel_mac";
 
@@ -78,12 +73,4 @@ static_assert(
     "ChromeAppModeInfo layout has changed; bump the APP_SHIM_VERSION_NUMBER "
     "in chrome/common/mac/app_mode_common.h. (And fix this static_assert.)");
 
-void VerifySocketPermissions(const base::FilePath& socket_path) {
-  CHECK(base::PathIsWritable(socket_path));
-  base::FilePath socket_dir = socket_path.DirName();
-  int socket_dir_mode = 0;
-  CHECK(base::GetPosixFilePermissions(socket_dir, &socket_dir_mode));
-  CHECK_EQ(0700, socket_dir_mode);
-}
-
 }  // namespace app_mode