Fix extension unpacking on linux/mac

Add a unittest that pretends to be the UtilityProcessHost and launches a chrome utility process for unpacking a theme.

Review URL: http://codereview.chromium.org/147001


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

6 files changed, 177 insertions, 9 deletions

diff --git a/chrome/browser/utility_process_host.cc b/chrome/browser/utility_process_host.cc
index e515eeb..0d9aaa6 100644
--- a/chrome/browser/utility_process_host.cc
+++ b/chrome/browser/utility_process_host.cc
@@ -14,6 +14,9 @@
 
 #if defined(OS_WIN)
 #include "chrome/browser/sandbox_policy.h"
+#elif defined(OS_POSIX)
+#include "base/global_descriptors_posix.h"
+#include "chrome/common/chrome_descriptors.h"
 #endif
 
 UtilityProcessHost::UtilityProcessHost(ResourceDispatcherHost* rdh,
@@ -45,15 +48,23 @@ bool UtilityProcessHost::StartWebResourceUnpacker(const std::string& data) {
   return true;
 }
 
+std::wstring UtilityProcessHost::GetUtilityProcessCmd() {
+  std::wstring exe_path = CommandLine::ForCurrentProcess()->GetSwitchValue(
+      switches::kBrowserSubprocessPath);
+  if (exe_path.empty()) {
+    PathService::Get(base::FILE_EXE, &exe_path);
+  }
+  return exe_path;
+}
+
 bool UtilityProcessHost::StartProcess(const FilePath& exposed_dir) {
   if (!CreateChannel())
     return false;
 
-  std::wstring exe_path = CommandLine::ForCurrentProcess()->GetSwitchValue(
-      switches::kBrowserSubprocessPath);
+  std::wstring exe_path = GetUtilityProcessCmd();
   if (exe_path.empty()) {
-    if (!PathService::Get(base::FILE_EXE, &exe_path))
-      return false;
+    NOTREACHED() << "Unable to get utility process binary name.";
+    return false;
   }
 
   CommandLine cmd_line(exe_path);
@@ -64,13 +75,37 @@ bool UtilityProcessHost::StartProcess(const FilePath& exposed_dir) {
 
   base::ProcessHandle process;
 #if defined(OS_WIN)
-  if (exposed_dir.empty())
+  if (!UseSandbox()) {
+    // Don't use the sandbox during unit tests.
+    base::LaunchApp(cmd_line, false, false, &process);
+  } else if (exposed_dir.empty()) {
     process = sandbox::StartProcess(&cmd_line);
-  else
+  } else {
     process = sandbox::StartProcessWithAccess(&cmd_line, exposed_dir);
+  }
 #else
-  // TODO(port): sandbox
-  base::LaunchApp(cmd_line, false, false, &process);
+  // TODO(port): Sandbox this on Linux/Mac.  Also, zygote this to work with
+  // Linux updating.
+  const CommandLine& browser_command_line = *CommandLine::ForCurrentProcess();
+  bool has_cmd_prefix = browser_command_line.HasSwitch(
+      switches::kUtilityCmdPrefix);
+  if (has_cmd_prefix) {
+    // launch the utility child process with some prefix (usually "xterm -e gdb
+    // --args").
+    cmd_line.PrependWrapper(browser_command_line.GetSwitchValue(
+        switches::kUtilityCmdPrefix));
+  }
+
+  // This code is duplicated with browser_render_process_host.cc and
+  // plugin_process_host.cc, but there's not a good place to de-duplicate it.
+  // Maybe we can merge this into sandbox::StartProcess which will set up
+  // everything before calling LaunchApp?
+  base::file_handle_mapping_vector fds_to_map;
+  const int ipcfd = channel().GetClientFileDescriptor();
+  if (ipcfd > -1)
+    fds_to_map.push_back(std::pair<int, int>(
+        ipcfd, kPrimaryIPCChannel + base::GlobalDescriptors::kBaseDescriptor));
+  base::LaunchApp(cmd_line.argv(), fds_to_map, false, &process);
 #endif
   if (!process)
     return false;
diff --git a/chrome/browser/utility_process_host.h b/chrome/browser/utility_process_host.h
index 86b7935..2028f03 100644
--- a/chrome/browser/utility_process_host.h
+++ b/chrome/browser/utility_process_host.h
@@ -79,6 +79,13 @@ class UtilityProcessHost : public ChildProcessHost {
   // web resource server format(s).
   bool StartWebResourceUnpacker(const std::string& data);
 
+ protected:
+  // Allow these methods to be overridden for tests.
+  virtual std::wstring GetUtilityProcessCmd();
+  virtual bool UseSandbox() {
+    return true;
+  }
+
  private:
   // Starts a process.  Returns true iff it succeeded.
   bool StartProcess(const FilePath& exposed_dir);
diff --git a/chrome/browser/utility_process_host_unittest.cc b/chrome/browser/utility_process_host_unittest.cc
new file mode 100644
index 0000000..b48d95a
--- /dev/null
+++ b/chrome/browser/utility_process_host_unittest.cc
@@ -0,0 +1,118 @@
+// Copyright (c) 2009 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/utility_process_host.h"
+
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/message_loop.h"
+#include "base/path_service.h"
+#include "base/string_util.h"
+#include "build/build_config.h"
+#include "chrome/browser/renderer_host/resource_dispatcher_host.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_paths.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class UtilityProcessHostTest : public testing::Test {
+ public:
+  UtilityProcessHostTest() {
+  }
+
+ protected:
+  MessageLoopForIO message_loop_;
+};
+
+class TestUtilityProcessHostClient : public UtilityProcessHost::Client {
+ public:
+  explicit TestUtilityProcessHostClient(MessageLoop* message_loop)
+      : message_loop_(message_loop), success_(false) {
+  }
+
+  // UtilityProcessHost::Client methods.
+  virtual void OnProcessCrashed() {
+    NOTREACHED();
+  }
+
+  virtual void OnUnpackExtensionSucceeded(const DictionaryValue& manifest) {
+    success_ = true;
+    message_loop_->PostTask(FROM_HERE, new MessageLoop::QuitTask);
+  }
+
+  virtual void OnUnpackExtensionFailed(const std::string& error_message) {
+    NOTREACHED();
+  }
+
+  virtual void OnUnpackWebResourceSucceeded(
+      const ListValue& json_data) {
+    NOTREACHED();
+  }
+
+  virtual void OnUnpackWebResourceFailed(
+      const std::string& error_message) {
+    NOTREACHED();
+  }
+
+  bool success() {
+    return success_;
+  }
+
+ private:
+  MessageLoop* message_loop_;
+  bool success_;
+};
+
+class TestUtilityProcessHost : public UtilityProcessHost {
+ public:
+  TestUtilityProcessHost(TestUtilityProcessHostClient* client,
+                         MessageLoop* loop_io)
+      : UtilityProcessHost(new ResourceDispatcherHost(NULL), client, loop_io) {
+  }
+
+ protected:
+  virtual std::wstring GetUtilityProcessCmd() {
+    FilePath exe_path;
+    PathService::Get(base::DIR_EXE, &exe_path);
+    exe_path = exe_path.AppendASCII(WideToASCII(
+        chrome::kBrowserProcessExecutablePath));
+    return exe_path.ToWStringHack();
+  }
+
+  virtual bool UseSandbox() {
+    return false;
+  }
+
+ private:
+};
+
+TEST_F(UtilityProcessHostTest, ExtensionUnpacker) {
+  // Copy the test extension into a temp dir and install from the temp dir.
+  FilePath extension_file;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extension_file));
+  extension_file = extension_file.AppendASCII("extensions")
+                                 .AppendASCII("theme.crx");
+  FilePath temp_extension_dir;
+  ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &temp_extension_dir));
+  temp_extension_dir = temp_extension_dir.AppendASCII("extension_test");
+  ASSERT_TRUE(file_util::CreateDirectory(temp_extension_dir));
+  ASSERT_TRUE(file_util::CopyFile(extension_file,
+                                  temp_extension_dir.AppendASCII("theme.crx")));
+
+  scoped_refptr<TestUtilityProcessHostClient> client(
+      new TestUtilityProcessHostClient(&message_loop_));
+  TestUtilityProcessHost* process_host = new TestUtilityProcessHost(
+      client.get(), &message_loop_);
+  process_host->StartExtensionUnpacker(
+      temp_extension_dir.AppendASCII("theme.crx"));
+  message_loop_.Run();
+  EXPECT_TRUE(client->success());
+
+  // Clean up the temp dir.
+  file_util::Delete(temp_extension_dir, true);
+}
+
+}  // namespace
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 8a06add..1d95e7b 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -3488,6 +3488,7 @@
         'browser/tabs/tab_strip_model_unittest.cc',
         'browser/task_manager_unittest.cc',
         'browser/theme_resources_util_unittest.cc',
+        'browser/utility_process_host_unittest.cc',
         'browser/views/bookmark_editor_view_unittest.cc',
         'browser/views/keyword_editor_view_unittest.cc',
         'browser/visitedlink_unittest.cc',
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index b7c03b1..fbb4497 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -424,9 +424,14 @@ const wchar_t kEnableRendererAccessibility[] = L"enable-renderer-accessibility";
 const wchar_t kTestName[]                      = L"test-name";
 
 // On POSIX only: the contents of this flag are prepended to the renderer
-// command line. (Useful values might be "valgrind" or "gdb --args")
+// command line. Useful values might be "valgrind" or "xterm -e gdb --args".
 const wchar_t kRendererCmdPrefix[]             = L"renderer-cmd-prefix";
 
+// On POSIX only: the contents of this flag are prepended to the utility
+// process command line. Useful values might be "valgrind" or "xterm -e gdb
+// --args".
+const wchar_t kUtilityCmdPrefix[]             = L"utility-cmd-prefix";
+
 // Temparary option for new ftp implemetation.
 const wchar_t kNewFtp[]                        = L"new-ftp";
 
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index fa10386..98c9601 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -157,6 +157,8 @@ extern const wchar_t kTestName[];
 
 extern const wchar_t kRendererCmdPrefix[];
 
+extern const wchar_t kUtilityCmdPrefix[];
+
 extern const wchar_t kNewFtp[];
 
 extern const wchar_t kIPCUseFIFO[];