Initial integration tests for the App Shim domain socket and IPC.

Integration testing for the app shim socket is difficult in
browser_tests because the nested temp dirs used by test swarming make a
deep directory hierarchy that exceeds the path limits for UNIX domain
sockets. This limit is only 104 characters because the kernel crams it
into a struct sockaddr_un. See `man 4 unix`.

This CL works around the path length limitation by creating a symlink in
a temporary folder under /tmp. Since app shims only exist on Mac, we
know /tmp will exist since Mac conforms to IEEE standard P1003.2 (POSIX,
part 2) via it's UNIX 03 certification.

Initial tests are added for non-argument "launch" IPC messages, and the
listening socket creation/destruction and error handling. This gives
explicit integration coverage to a range of fixes in the bugs below.

BUG=292566, 272577, 240554, 242941

Review URL: https://codereview.chromium.org/23463011

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

8 files changed, 366 insertions, 3 deletions

diff --git a/apps/DEPS b/apps/DEPS
index 62d6854..ee53482 100644
--- a/apps/DEPS
+++ b/apps/DEPS
@@ -60,6 +60,8 @@ specific_include_rules = {
     "+chrome/browser/extensions/extension_test_message_listener.h",
     "+chrome/browser/extensions/test_extension_environment.h",
     "+chrome/browser/extensions/test_extension_prefs.h",
+    "+chrome/browser/ui/browser.h",
+    "+chrome/test/base/in_process_browser_test.h",
     "+chrome/test/base/interactive_test_utils.h",
     "+chrome/test/base/testing_profile.h",
     # Temporary allowed testing include.
diff --git a/apps/app_shim/app_shim_host_manager_browsertest_mac.mm b/apps/app_shim/app_shim_host_manager_browsertest_mac.mm
new file mode 100644
index 0000000..bf325cc
--- /dev/null
+++ b/apps/app_shim/app_shim_host_manager_browsertest_mac.mm
@@ -0,0 +1,263 @@
+// 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 <unistd.h>
+
+#include "apps/app_shim/app_shim_messages.h"
+#include "apps/app_shim/test/app_shim_host_manager_test_api_mac.h"
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "base/path_service.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/mac/app_mode_common.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/test/test_utils.h"
+#include "ipc/ipc_channel_proxy.h"
+#include "ipc/ipc_listener.h"
+#include "ipc/ipc_message.h"
+
+namespace {
+
+const char kTestAppMode[] = "test_app";
+
+// A test version of the AppShimController IPC client in chrome_main_app_mode.
+class TestShimClient : public IPC::Listener {
+ public:
+  TestShimClient(const base::FilePath& socket_path);
+  virtual ~TestShimClient();
+
+  template <class T>
+  void Send(const T& message) {
+    channel_->Send(message);
+  }
+
+ private:
+  // IPC::Listener overrides:
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+  virtual void OnChannelError() OVERRIDE;
+
+  base::Thread io_thread_;
+  scoped_ptr<IPC::ChannelProxy> channel_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestShimClient);
+};
+
+TestShimClient::TestShimClient(const base::FilePath& socket_path)
+    : io_thread_("TestShimClientIO") {
+  base::Thread::Options io_thread_options;
+  io_thread_options.message_loop_type = base::MessageLoop::TYPE_IO;
+  io_thread_.StartWithOptions(io_thread_options);
+
+  IPC::ChannelHandle handle(socket_path.value());
+  channel_.reset(new IPC::ChannelProxy(handle, IPC::Channel::MODE_NAMED_CLIENT,
+      this, io_thread_.message_loop_proxy().get()));
+}
+
+TestShimClient::~TestShimClient() {}
+
+bool TestShimClient::OnMessageReceived(const IPC::Message& message) {
+  return true;
+}
+
+void TestShimClient::OnChannelError() {
+  // Client should not get any channel errors for the current set of tests.
+  PLOG(FATAL) << "ChannelError";
+}
+
+// Browser Test for AppShimHostManager to test IPC interactions across the
+// UNIX domain socket.
+class AppShimHostManagerBrowserTest : public InProcessBrowserTest,
+                                      public apps::AppShimHandler {
+ public:
+  AppShimHostManagerBrowserTest();
+  virtual ~AppShimHostManagerBrowserTest();
+
+ protected:
+  // Wait for OnShimLaunch, then send a quit, and wait for the response. Used to
+  // test launch behavior.
+  void RunAndExitGracefully();
+
+  // InProcessBrowserTest overrides:
+  virtual bool SetUpUserDataDirectory() OVERRIDE;
+
+  // AppShimHandler overrides:
+  virtual void OnShimLaunch(apps::AppShimHandler::Host* host,
+                            apps::AppShimLaunchType launch_type,
+                            const std::vector<base::FilePath>& files) OVERRIDE;
+  virtual void OnShimClose(apps::AppShimHandler::Host* host) OVERRIDE {}
+  virtual void OnShimFocus(apps::AppShimHandler::Host* host,
+                           apps::AppShimFocusType focus_type,
+                           const std::vector<base::FilePath>& files) OVERRIDE {}
+  virtual void OnShimSetHidden(apps::AppShimHandler::Host* host,
+                               bool hidden) OVERRIDE {}
+  virtual void OnShimQuit(apps::AppShimHandler::Host* host) OVERRIDE;
+
+  scoped_ptr<TestShimClient> test_client_;
+  base::FilePath short_socket_path_;
+  std::vector<base::FilePath> last_launch_files_;
+  apps::AppShimLaunchType last_launch_type_;
+
+private:
+  scoped_refptr<content::MessageLoopRunner> runner_;
+  base::ScopedTempDir short_temp_dir_;
+
+  int launch_count_;
+  int quit_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppShimHostManagerBrowserTest);
+};
+
+AppShimHostManagerBrowserTest::AppShimHostManagerBrowserTest()
+    : last_launch_type_(apps::APP_SHIM_LAUNCH_NUM_TYPES),
+      launch_count_(0),
+      quit_count_(0) {
+  apps::AppShimHandler::RegisterHandler(kTestAppMode, this);
+}
+
+AppShimHostManagerBrowserTest::~AppShimHostManagerBrowserTest() {
+  apps::AppShimHandler::RemoveHandler(kTestAppMode);
+}
+
+void AppShimHostManagerBrowserTest::RunAndExitGracefully() {
+  runner_ = new content::MessageLoopRunner();
+  EXPECT_EQ(0, launch_count_);
+  runner_->Run();  // Will stop in OnShimLaunch().
+  EXPECT_EQ(1, launch_count_);
+
+  runner_ = new content::MessageLoopRunner();
+  test_client_->Send(new AppShimHostMsg_QuitApp);
+  EXPECT_EQ(0, quit_count_);
+  runner_->Run();  // Will stop in OnShimQuit().
+  EXPECT_EQ(1, quit_count_);
+
+  test_client_.reset();
+}
+
+bool AppShimHostManagerBrowserTest::SetUpUserDataDirectory() {
+  // Create a symlink at /tmp/scoped_dir_XXXXXX/udd that points to the real user
+  // data dir, and use this as the domain socket path. This is required because
+  // there is a path length limit for named sockets that is exceeded in
+  // multi-process test spawning.
+  base::FilePath real_user_data_dir;
+  EXPECT_TRUE(PathService::Get(chrome::DIR_USER_DATA, &real_user_data_dir));
+  EXPECT_TRUE(
+      short_temp_dir_.CreateUniqueTempDirUnderPath(base::FilePath("/tmp")));
+  base::FilePath shortened_user_data_dir = short_temp_dir_.path().Append("udd");
+  EXPECT_EQ(0, ::symlink(real_user_data_dir.AsUTF8Unsafe().c_str(),
+                         shortened_user_data_dir.AsUTF8Unsafe().c_str()));
+
+  test::AppShimHostManagerTestApi::OverrideUserDataDir(shortened_user_data_dir);
+  short_socket_path_ =
+      shortened_user_data_dir.Append(app_mode::kAppShimSocketName);
+
+  return InProcessBrowserTest::SetUpUserDataDirectory();
+}
+
+void AppShimHostManagerBrowserTest::OnShimLaunch(
+    apps::AppShimHandler::Host* host,
+    apps::AppShimLaunchType launch_type,
+    const std::vector<base::FilePath>& files) {
+  host->OnAppLaunchComplete(apps::APP_SHIM_LAUNCH_SUCCESS);
+  ++launch_count_;
+  last_launch_type_ = launch_type;
+  last_launch_files_ = files;
+  runner_->Quit();
+}
+
+void AppShimHostManagerBrowserTest::OnShimQuit(
+    apps::AppShimHandler::Host* host) {
+  ++quit_count_;
+  runner_->Quit();
+}
+
+// Test regular launch, which would ask Chrome to launch the app.
+IN_PROC_BROWSER_TEST_F(AppShimHostManagerBrowserTest, LaunchNormal) {
+  test_client_.reset(new TestShimClient(short_socket_path_));
+  test_client_->Send(new AppShimHostMsg_LaunchApp(
+      browser()->profile()->GetPath(),
+      kTestAppMode,
+      apps::APP_SHIM_LAUNCH_NORMAL,
+      std::vector<base::FilePath>()));
+
+  RunAndExitGracefully();
+  EXPECT_EQ(apps::APP_SHIM_LAUNCH_NORMAL, last_launch_type_);
+  EXPECT_TRUE(last_launch_files_.empty());
+}
+
+// Test register-only launch, used when Chrome has already launched the app.
+IN_PROC_BROWSER_TEST_F(AppShimHostManagerBrowserTest, LaunchRegisterOnly) {
+  test_client_.reset(new TestShimClient(short_socket_path_));
+  test_client_->Send(new AppShimHostMsg_LaunchApp(
+      browser()->profile()->GetPath(),
+      kTestAppMode,
+      apps::APP_SHIM_LAUNCH_REGISTER_ONLY,
+      std::vector<base::FilePath>()));
+
+  RunAndExitGracefully();
+  EXPECT_EQ(apps::APP_SHIM_LAUNCH_REGISTER_ONLY, last_launch_type_);
+  EXPECT_TRUE(last_launch_files_.empty());
+}
+
+// Ensure the domain socket can be created in a fresh user data dir.
+IN_PROC_BROWSER_TEST_F(AppShimHostManagerBrowserTest,
+                       PRE_ReCreate) {
+  test::AppShimHostManagerTestApi test_api(
+      g_browser_process->platform_part()->app_shim_host_manager());
+  EXPECT_TRUE(test_api.factory());
+}
+
+// Ensure the domain socket 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());
+  EXPECT_TRUE(test_api.factory());
+}
+
+// Test for AppShimHostManager that fails to create the socket.
+class AppShimHostManagerBrowserTestFailsCreate :
+    public AppShimHostManagerBrowserTest {
+ public:
+  AppShimHostManagerBrowserTestFailsCreate() {}
+
+ private:
+  virtual bool SetUpUserDataDirectory() OVERRIDE;
+
+  base::ScopedTempDir barrier_dir_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppShimHostManagerBrowserTestFailsCreate);
+};
+
+bool AppShimHostManagerBrowserTestFailsCreate::SetUpUserDataDirectory() {
+  base::FilePath user_data_dir;
+  // Start in the "real" user data dir for this test. This is a meta-test for
+  // the symlinking steps used in the superclass. That is, by putting the
+  // clobber in the actual user data dir, the test will fail if the symlink
+  // does not actually point to the user data dir, since it won't be clobbered.
+  EXPECT_TRUE(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir));
+  base::FilePath socket_path =
+      user_data_dir.Append(app_mode::kAppShimSocketName);
+  // Create a "barrier" to forming the UNIX domain socket. This is just a
+  // pre-existing directory which can not be unlink()ed, in order to place a
+  // named socked there instead.
+  EXPECT_TRUE(barrier_dir_.Set(socket_path));
+  return AppShimHostManagerBrowserTest::SetUpUserDataDirectory();
+}
+
+// Test error handling. This is essentially testing for lifetime correctness
+// during startup for unexpected failures.
+IN_PROC_BROWSER_TEST_F(AppShimHostManagerBrowserTestFailsCreate,
+                       SocketFailure) {
+  test::AppShimHostManagerTestApi test_api(
+      g_browser_process->platform_part()->app_shim_host_manager());
+  EXPECT_FALSE(test_api.factory());
+}
+
+}  // namespace
diff --git a/apps/app_shim/app_shim_host_manager_mac.h b/apps/app_shim/app_shim_host_manager_mac.h
index 0b52bc4..73381ef 100644
--- a/apps/app_shim/app_shim_host_manager_mac.h
+++ b/apps/app_shim/app_shim_host_manager_mac.h
@@ -9,6 +9,14 @@
 #include "base/memory/ref_counted.h"
 #include "ipc/ipc_channel_factory.h"
 
+namespace base {
+class FilePath;
+}
+
+namespace test {
+class AppShimHostManagerTestApi;
+}
+
 // The AppShimHostManager receives connections from app shims on a UNIX
 // socket (|factory_|) and creates a helper object to manage the connection.
 class AppShimHostManager
@@ -28,6 +36,7 @@ class AppShimHostManager
 
  private:
   friend class base::RefCountedThreadSafe<AppShimHostManager>;
+  friend class test::AppShimHostManagerTestApi;
   virtual ~AppShimHostManager();
 
   // IPC::ChannelFactory::Delegate implementation.
@@ -42,6 +51,9 @@ class AppShimHostManager
   // Called on the IO thread to begin listening for connections from app shims.
   void ListenOnIOThread();
 
+  // If set, used instead of chrome::DIR_USER_DATA for placing the socket.
+  static const base::FilePath* g_override_user_data_dir_;
+
   scoped_ptr<IPC::ChannelFactory> factory_;
 
   apps::ExtensionAppShimHandler extension_app_shim_handler_;
diff --git a/apps/app_shim/app_shim_host_manager_mac.mm b/apps/app_shim/app_shim_host_manager_mac.mm
index e4f18a6..e28970e 100644
--- a/apps/app_shim/app_shim_host_manager_mac.mm
+++ b/apps/app_shim/app_shim_host_manager_mac.mm
@@ -26,6 +26,8 @@ void CreateAppShimHost(const IPC::ChannelHandle& handle) {
 
 }  // namespace
 
+const base::FilePath* AppShimHostManager::g_override_user_data_dir_ = NULL;
+
 AppShimHostManager::AppShimHostManager() {}
 
 void AppShimHostManager::Init() {
@@ -43,11 +45,14 @@ AppShimHostManager::~AppShimHostManager() {
 void AppShimHostManager::InitOnFileThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   base::FilePath user_data_dir;
-  if (!PathService::Get(chrome::DIR_USER_DATA, &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.";
     return;
   }
+
   base::FilePath socket_path =
       user_data_dir.Append(app_mode::kAppShimSocketName);
   factory_.reset(new IPC::ChannelFactory(socket_path, this));
@@ -58,7 +63,11 @@ void AppShimHostManager::InitOnFileThread() {
 
 void AppShimHostManager::ListenOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  factory_->Listen();
+  if (!factory_->Listen()) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&AppShimHostManager::OnListenError, this));
+  }
 }
 
 void AppShimHostManager::OnClientConnected(
@@ -70,5 +79,8 @@ void AppShimHostManager::OnClientConnected(
 }
 
 void AppShimHostManager::OnListenError() {
-  // TODO(jeremya): set a timeout and attempt to reconstruct the channel.
+  // 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();
 }
diff --git a/apps/app_shim/test/app_shim_host_manager_test_api_mac.cc b/apps/app_shim/test/app_shim_host_manager_test_api_mac.cc
new file mode 100644
index 0000000..2bd83fd
--- /dev/null
+++ b/apps/app_shim/test/app_shim_host_manager_test_api_mac.cc
@@ -0,0 +1,30 @@
+// 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/test/app_shim_host_manager_test_api_mac.h"
+
+#include "apps/app_shim/app_shim_host_manager_mac.h"
+#include "base/files/file_path.h"
+
+namespace test {
+
+// static
+void AppShimHostManagerTestApi::OverrideUserDataDir(
+    const base::FilePath& user_data_dir) {
+  CR_DEFINE_STATIC_LOCAL(base::FilePath, override_user_data_dir, ());
+  override_user_data_dir = user_data_dir;
+  AppShimHostManager::g_override_user_data_dir_ = &override_user_data_dir;
+}
+
+AppShimHostManagerTestApi::AppShimHostManagerTestApi(
+    AppShimHostManager* host_manager)
+    : host_manager_(host_manager) {
+  DCHECK(host_manager_);
+}
+
+IPC::ChannelFactory* AppShimHostManagerTestApi::factory() {
+  return host_manager_->factory_.get();
+}
+
+}  // namespace test
diff --git a/apps/app_shim/test/app_shim_host_manager_test_api_mac.h b/apps/app_shim/test/app_shim_host_manager_test_api_mac.h
new file mode 100644
index 0000000..dde8782
--- /dev/null
+++ b/apps/app_shim/test/app_shim_host_manager_test_api_mac.h
@@ -0,0 +1,38 @@
+// 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.
+
+#ifndef APPS_APP_SHIM_TEST_APP_SHIM_HOST_MANAGER_TEST_API_MAC_H
+#define APPS_APP_SHIM_TEST_APP_SHIM_HOST_MANAGER_TEST_API_MAC_H
+
+#include "base/basictypes.h"
+
+class AppShimHostManager;
+
+namespace base {
+class FilePath;
+}
+
+namespace IPC {
+class ChannelFactory;
+}
+
+namespace test {
+
+class AppShimHostManagerTestApi {
+ public:
+  static void OverrideUserDataDir(const base::FilePath& user_data_dir);
+
+  explicit AppShimHostManagerTestApi(AppShimHostManager* host_manager);
+
+  IPC::ChannelFactory* factory();
+
+ private:
+  AppShimHostManager* host_manager_;  // Not owned.
+
+  DISALLOW_COPY_AND_ASSIGN(AppShimHostManagerTestApi);
+};
+
+}  // namespace test
+
+#endif  // APPS_APP_SHIM_TEST_APP_SHIM_HOST_MANAGER_TEST_API_MAC_H
diff --git a/chrome/browser/browser_process_platform_part_mac.mm b/chrome/browser/browser_process_platform_part_mac.mm
index 415c715..5a78d7b 100644
--- a/chrome/browser/browser_process_platform_part_mac.mm
+++ b/chrome/browser/browser_process_platform_part_mac.mm
@@ -25,6 +25,9 @@ void BrowserProcessPlatformPart::AttemptExit() {
 }
 
 void BrowserProcessPlatformPart::PreMainMessageLoopRun() {
+  // AppShimHostManager can not simply be reset, otherwise destroying the old
+  // domain socket will cause the just-created socket to be unlinked.
+  DCHECK(!app_shim_host_manager_);
   app_shim_host_manager_ = new AppShimHostManager;
   // Init needs to be called after assigning to a scoped_refptr (i.e. after
   // incrementing the refcount).
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index e69dd1e..fc25bf2 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -908,6 +908,9 @@
       ],
       'sources': [
         '../apps/app_restore_service_browsertest.cc',
+        '../apps/app_shim/app_shim_host_manager_browsertest_mac.mm',
+        '../apps/app_shim/test/app_shim_host_manager_test_api_mac.cc',
+        '../apps/app_shim/test/app_shim_host_manager_test_api_mac.h',
         '../apps/load_and_launch_browsertest.cc',
         # TODO(blundell): Bring up a components_browsertests target and move
         # this test to be in that target. crbug.com/283846