Implement native messaging host launcher for Windows.

BUG=142915

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

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

12 files changed, 148 insertions, 59 deletions

diff --git a/chrome/browser/extensions/api/messaging/native_message_process_host.cc b/chrome/browser/extensions/api/messaging/native_message_process_host.cc
index af1be73..41048ec 100644
--- a/chrome/browser/extensions/api/messaging/native_message_process_host.cc
+++ b/chrome/browser/extensions/api/messaging/native_message_process_host.cc
@@ -45,7 +45,6 @@ NativeMessageProcessHost::NativeMessageProcessHost(
       destination_port_(destination_port),
       launcher_(launcher.Pass()),
       closed_(false),
-      native_process_handle_(base::kNullProcessHandle),
       read_pending_(false),
       read_eof_(false),
       write_pending_(false) {
@@ -104,18 +103,16 @@ void NativeMessageProcessHost::LaunchHostProcess() {
 }
 
 void NativeMessageProcessHost::OnHostProcessLaunched(
-    base::ProcessHandle native_process_handle,
+    bool result,
     base::PlatformFile read_file,
     base::PlatformFile write_file) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
-  if (native_process_handle == base::kNullProcessHandle) {
+  if (!result) {
     OnError();
     return;
   }
 
-  native_process_handle_ = native_process_handle;
-
   read_file_ = read_file;
   read_stream_.reset(new net::FileStream(
       read_file, base::PLATFORM_FILE_READ | base::PLATFORM_FILE_ASYNC, NULL));
@@ -311,17 +308,6 @@ void NativeMessageProcessHost::Close() {
   content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
       base::Bind(&Client::CloseChannel, weak_client_ui_,
           destination_port_, true));
-
-  if (native_process_handle_ != base::kNullProcessHandle) {
-    // Give the process some time to shutdown, then try and kill it.
-    content::BrowserThread::PostDelayedTask(
-      content::BrowserThread::IO, FROM_HERE,
-      base::Bind(base::IgnoreResult(&base::KillProcess),
-                 native_process_handle_, 0,
-                 false /* don't wait for exit */),
-      base::TimeDelta::FromMilliseconds(kExitTimeoutMS));
-    native_process_handle_ = base::kNullProcessHandle;
-  }
 }
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/messaging/native_message_process_host.h b/chrome/browser/extensions/api/messaging/native_message_process_host.h
index 129cb07..834eb43 100644
--- a/chrome/browser/extensions/api/messaging/native_message_process_host.h
+++ b/chrome/browser/extensions/api/messaging/native_message_process_host.h
@@ -88,10 +88,9 @@ class NativeMessageProcessHost
   void LaunchHostProcess();
 
   // Callback for NativeProcessLauncher::Launch().
-  void OnHostProcessLaunched(
-      base::ProcessHandle native_process_handle,
-      base::PlatformFile read_file,
-      base::PlatformFile write_file);
+  void OnHostProcessLaunched(bool result,
+                             base::PlatformFile read_file,
+                             base::PlatformFile write_file);
 
   // Helper methods to read incoming messages.
   void WaitRead();
@@ -130,9 +129,6 @@ class NativeMessageProcessHost
   // due to an error.
   bool closed_;
 
-  // Handle of the native host process.
-  base::ProcessHandle native_process_handle_;
-
   // Input stream handle and reader.
   base::PlatformFile read_file_;
   scoped_ptr<net::FileStream> read_stream_;
diff --git a/chrome/browser/extensions/api/messaging/native_messaging_apitest_posix.cc b/chrome/browser/extensions/api/messaging/native_messaging_apitest.cc
index 3649b83..3649b83 100644
--- a/chrome/browser/extensions/api/messaging/native_messaging_apitest_posix.cc
+++ b/chrome/browser/extensions/api/messaging/native_messaging_apitest.cc
diff --git a/chrome/browser/extensions/api/messaging/native_process_launcher.cc b/chrome/browser/extensions/api/messaging/native_process_launcher.cc
index ce223ae..36beaf2 100644
--- a/chrome/browser/extensions/api/messaging/native_process_launcher.cc
+++ b/chrome/browser/extensions/api/messaging/native_process_launcher.cc
@@ -9,6 +9,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
+#include "base/sequenced_task_runner.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/common/chrome_paths.h"
 
@@ -34,6 +35,7 @@ class NativeProcessLauncherImpl : public NativeProcessLauncher {
     void Launch(const std::string& native_host_name,
                 LaunchedCallback callback);
     void Detach();
+
    private:
     friend class base::RefCountedThreadSafe<Core>;
     virtual ~Core();
@@ -41,7 +43,7 @@ class NativeProcessLauncherImpl : public NativeProcessLauncher {
     void DoLaunchOnThreadPool(const std::string& native_host_name,
                               LaunchedCallback callback);
     void CallCallbackOnIOThread(LaunchedCallback callback,
-                                base::ProcessHandle native_process_handle,
+                                bool result,
                                 base::PlatformFile read_file,
                                 base::PlatformFile write_file);
 
@@ -95,46 +97,38 @@ void NativeProcessLauncherImpl::Core::DoLaunchOnThreadPool(
     content::BrowserThread::PostTask(
         content::BrowserThread::IO, FROM_HERE,
         base::Bind(&NativeProcessLauncherImpl::Core::CallCallbackOnIOThread,
-                   this, callback, base::kNullProcessHandle,
+                   this, callback, false,
                    base::kInvalidPlatformFileValue,
                    base::kInvalidPlatformFileValue));
     return;
   }
 
-  base::ProcessHandle native_process_handle;
   base::PlatformFile read_file;
   base::PlatformFile write_file;
-  if (!NativeProcessLauncher::LaunchNativeProcess(
-          native_host_program, &native_process_handle,
-          &read_file, &write_file)) {
-    native_process_handle = base::kNullProcessHandle;
-  }
+  bool result = NativeProcessLauncher::LaunchNativeProcess(
+      native_host_program, &read_file, &write_file);
 
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&NativeProcessLauncherImpl::Core::CallCallbackOnIOThread,
-                 this, callback, native_process_handle, read_file, write_file));
+                 this, callback, result, read_file, write_file));
 }
 
 void NativeProcessLauncherImpl::Core::CallCallbackOnIOThread(
     LaunchedCallback callback,
-    base::ProcessHandle native_process_handle,
+    bool result,
     base::PlatformFile read_file,
     base::PlatformFile write_file) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
   if (detached_) {
-    // Kill the process if it's started already.
-    if (native_process_handle != base::kNullProcessHandle) {
-      content::BrowserThread::PostBlockingPoolTask(
-          FROM_HERE,
-          base::Bind(base::IgnoreResult(&base::KillProcess),
-                     native_process_handle, 0,
-                     false /* don't wait for exit */));
-    }
+    if (read_file != base::kInvalidPlatformFileValue)
+      base::ClosePlatformFile(read_file);
+    if (write_file != base::kInvalidPlatformFileValue)
+      base::ClosePlatformFile(write_file);
     return;
   }
 
-  callback.Run(native_process_handle, read_file, write_file);
+  callback.Run(result, read_file, write_file);
 }
 
 NativeProcessLauncherImpl::NativeProcessLauncherImpl()
diff --git a/chrome/browser/extensions/api/messaging/native_process_launcher.h b/chrome/browser/extensions/api/messaging/native_process_launcher.h
index cbb9bd3..f8188d4 100644
--- a/chrome/browser/extensions/api/messaging/native_process_launcher.h
+++ b/chrome/browser/extensions/api/messaging/native_process_launcher.h
@@ -16,10 +16,10 @@ namespace extensions {
 
 class NativeProcessLauncher {
  public:
-  // Callback that's called after the process has been launched.
-  // |native_process_handle| is set to base::kNullProcessHandle in case of a
-  // failure.
-  typedef base::Callback<void (base::ProcessHandle native_process_handle,
+  // Callback that's called after the process has been launched. |result| is
+  // set to false in case of a failure. Handler must take ownership of the IO
+  // handles.
+  typedef base::Callback<void (bool result,
                                base::PlatformFile read_file,
                                base::PlatformFile write_file)> LaunchedCallback;
 
@@ -31,14 +31,13 @@ class NativeProcessLauncher {
   // Launches native host with the specified name asynchronously. |callback| is
   // called after the process has been started. If the launcher is destroyed
   // before the callback is called then the call is canceled and the process is
-  // killed if it has been started already.
+  // stopped if it has been started already (by closing IO pipes).
   virtual void Launch(const std::string& native_host_name,
                       LaunchedCallback callback) const = 0;
 
  protected:
   static bool LaunchNativeProcess(
       const base::FilePath& path,
-      base::ProcessHandle* native_process_handle,
       base::PlatformFile* read_file,
       base::PlatformFile* write_file);
 
diff --git a/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc b/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc
index 6297271..8b1ab22 100644
--- a/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc
+++ b/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc
@@ -15,7 +15,6 @@ namespace extensions {
 // static
 bool NativeProcessLauncher::LaunchNativeProcess(
     const base::FilePath& path,
-    base::ProcessHandle* native_process_handle,
     base::PlatformFile* read_file,
     base::PlatformFile* write_file) {
   base::FileHandleMappingVector fd_map;
@@ -41,7 +40,8 @@ bool NativeProcessLauncher::LaunchNativeProcess(
   CommandLine line(path);
   base::LaunchOptions options;
   options.fds_to_remap = &fd_map;
-  if (!base::LaunchProcess(line, options, native_process_handle)) {
+  int process_id;
+  if (!base::LaunchProcess(line, options, &process_id)) {
     LOG(ERROR) << "Error launching process";
     return false;
   }
diff --git a/chrome/browser/extensions/api/messaging/native_process_launcher_win.cc b/chrome/browser/extensions/api/messaging/native_process_launcher_win.cc
index 5a0d9c9..2b12eb1 100644
--- a/chrome/browser/extensions/api/messaging/native_process_launcher_win.cc
+++ b/chrome/browser/extensions/api/messaging/native_process_launcher_win.cc
@@ -6,21 +6,120 @@
 
 #include <windows.h>
 
+#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/process_util.h"
 #include "base/stringprintf.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/utf_string_conversions.h"
+#include "base/win/scoped_handle.h"
+#include "crypto/random.h"
 
 namespace extensions {
 
 // static
 bool NativeProcessLauncher::LaunchNativeProcess(
     const base::FilePath& path,
-    base::ProcessHandle* native_process_handle,
     base::PlatformFile* read_file,
     base::PlatformFile* write_file) {
-  NOTREACHED();
-  return false;
+  // Timeout for the IO pipes.
+  const DWORD kTimeoutMs = 5000;
+
+  // Windows will use default buffer size when 0 is passed to
+  // CreateNamedPipeW().
+  const DWORD kBufferSize = 0;
+
+  if (!path.IsAbsolute()) {
+    LOG(ERROR) << "Native Messaging host path must be absolute.";
+    return false;
+  }
+
+  uint64 pipe_name_token;
+  crypto::RandBytes(&pipe_name_token, sizeof(pipe_name_token));
+  string16 out_pipe_name = base::StringPrintf(
+      L"\\\\.\\pipe\\chrome.nativeMessaging.out.%llx", pipe_name_token);
+  string16 in_pipe_name = base::StringPrintf(
+      L"\\\\.\\pipe\\chrome.nativeMessaging.in.%llx", pipe_name_token);
+
+  // Create the pipes to read and write from.
+  base::win::ScopedHandle stdout_pipe(
+      CreateNamedPipeW(out_pipe_name.c_str(),
+                       PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED |
+                           FILE_FLAG_FIRST_PIPE_INSTANCE,
+                       PIPE_TYPE_BYTE, 1, kBufferSize, kBufferSize,
+                       kTimeoutMs, NULL));
+  if (!stdout_pipe.IsValid()) {
+    LOG(ERROR) << "Failed to create pipe " << out_pipe_name;
+    return false;
+  }
+
+  base::win::ScopedHandle stdin_pipe(
+      CreateNamedPipeW(in_pipe_name.c_str(),
+                       PIPE_ACCESS_OUTBOUND | FILE_FLAG_OVERLAPPED |
+                           FILE_FLAG_FIRST_PIPE_INSTANCE,
+                       PIPE_TYPE_BYTE, 1, kBufferSize, kBufferSize,
+                       kTimeoutMs, NULL));
+  if (!stdin_pipe.IsValid()) {
+    LOG(ERROR) << "Failed to create pipe " << in_pipe_name;
+    return false;
+  }
+
+  DWORD comspec_length = ::GetEnvironmentVariable(L"COMSPEC", NULL, 0);
+  if (comspec_length == 0) {
+    LOG(ERROR) << "COMSPEC is not set";
+    return false;
+  }
+  scoped_ptr<wchar_t[]> comspec(new wchar_t[comspec_length]);
+  ::GetEnvironmentVariable(L"COMSPEC", comspec.get(), comspec_length);
+
+  // 'start' command has a moronic syntax: if first argument is quoted then it
+  // interprets it as a command title. Host path must always be in quotes, so
+  // we always need to specify the title as the first argument.
+  string16 command = base::StringPrintf(
+      L"%ls /c start \"Chrome Native Messaging Host\" /b "
+      L"\"%ls\" < %ls > %ls",
+      comspec.get(), path.value().c_str(),
+      in_pipe_name.c_str(), out_pipe_name.c_str());
+
+  base::LaunchOptions options;
+  options.start_hidden = true;
+  base::ProcessHandle cmd_handle;
+  if (!base::LaunchProcess(command.c_str(), options, &cmd_handle)) {
+    LOG(ERROR) << "Error launching process " << path.MaybeAsASCII();
+    return false;
+  }
+
+  bool stdout_connected = ConnectNamedPipe(stdout_pipe.Get(), NULL) ?
+      TRUE : GetLastError() == ERROR_PIPE_CONNECTED;
+  bool stdin_connected = ConnectNamedPipe(stdin_pipe.Get(), NULL) ?
+      TRUE : GetLastError() == ERROR_PIPE_CONNECTED;
+  if (!stdout_connected || !stdin_connected) {
+    base::KillProcess(cmd_handle, 0, false);
+    base::CloseProcessHandle(cmd_handle);
+    LOG(ERROR) << "Failed to connect IO pipes when starting "
+               << path.MaybeAsASCII();
+    return false;
+  }
+
+  // Check that cmd.exe has completed with 0 exit code to make sure it was
+  // able to connect IO pipes.
+  int error_code;
+  if (!base::WaitForExitCodeWithTimeout(
+          cmd_handle, &error_code,
+          base::TimeDelta::FromMilliseconds(kTimeoutMs)) ||
+      error_code != 0) {
+    LOG(ERROR) << "cmd.exe did not exit cleanly";
+    base::KillProcess(cmd_handle, 0, false);
+    base::CloseProcessHandle(cmd_handle);
+    return false;
+  }
+
+  base::CloseProcessHandle(cmd_handle);
+
+  *read_file = stdout_pipe.Take();
+  *write_file = stdin_pipe.Take();
+
+  return true;
 }
 
 }  // namespace extensions
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index a3838fd..6dbf313 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1153,7 +1153,7 @@
         'browser/extensions/api/media_galleries_private/media_galleries_eject_apitest.cc',
         'browser/extensions/api/media_galleries_private/media_galleries_private_apitest.cc',
         'browser/extensions/api/media_galleries_private/media_galleries_watch_apitest.cc',
-        'browser/extensions/api/messaging/native_messaging_apitest_posix.cc',
+        'browser/extensions/api/messaging/native_messaging_apitest.cc',
         'browser/extensions/api/metrics_private/metrics_apitest.cc',
         'browser/extensions/api/module/module_apitest.cc',
         'browser/extensions/api/notification/notification_apitest.cc',
diff --git a/chrome/common/extensions/docs/examples/extensions/native_messaging/popup.js b/chrome/common/extensions/docs/examples/extensions/native_messaging/popup.js
index a37f5ea..07c6da4 100644
--- a/chrome/common/extensions/docs/examples/extensions/native_messaging/popup.js
+++ b/chrome/common/extensions/docs/examples/extensions/native_messaging/popup.js
@@ -14,7 +14,8 @@ function gotNativeMessage(message) {
 
 function sendNativeMessage() {
   if (!port) {
-    port = chrome.runtime.connectNative('echo.py');
+    app = navigator.platform.match(/win/i) ? 'echo.bat' : 'echo.py';
+    port = chrome.extension.connectNative(app);
     port.onMessage.addListener(gotNativeMessage);
     document.getElementById('input-text').style.display = 'block';
     document.getElementById('send-native-message').innerHTML = 'Send Message';
diff --git a/chrome/test/data/extensions/api_test/native_messaging/test.js b/chrome/test/data/extensions/api_test/native_messaging/test.js
index 8df94bd..1ea0a3b 100644
--- a/chrome/test/data/extensions/api_test/native_messaging/test.js
+++ b/chrome/test/data/extensions/api_test/native_messaging/test.js
@@ -2,23 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+var appName = navigator.platform.match(/win/i) ? 'echo.bat' : 'echo.py';
+
 chrome.test.getConfig(function(config) {
     chrome.test.runTests([
 
       function sendMessageWithCallback() {
         var message = {"text": "Hi there!", "number": 3};
         chrome.runtime.sendNativeMessage(
-            'echo.py', message,
+            appName, message,
             chrome.test.callbackPass(function(nativeResponse) {
           var expectedResponse = {"id": 1, "echo": message};
           chrome.test.assertEq(expectedResponse, nativeResponse);
         }));
       },
 
-      // The goal of this test, is just not to crash.
+      // The goal of this test is just not to crash.
       function sendMessageWithoutCallback() {
         var message = {"text": "Hi there!", "number": 3};
-        chrome.runtime.sendNativeMessage('echo.py', message);
+        chrome.extension.sendNativeMessage(appName, message);
         chrome.test.succeed(); // Mission Complete
       },
 
@@ -31,7 +33,7 @@ chrome.test.getConfig(function(config) {
                                  {"id": 3, "echo": messagesToSend[2]}];
         var currentMessage = 0;
 
-        port = chrome.runtime.connectNative('echo.py');
+        port = chrome.extension.connectNative(appName);
         port.postMessage(messagesToSend[currentMessage]);
 
         port.onMessage.addListener(function(message) {
diff --git a/chrome/test/data/native_messaging/native_hosts/echo.bat b/chrome/test/data/native_messaging/native_hosts/echo.bat
new file mode 100755
index 0000000..1080f2b
--- /dev/null
+++ b/chrome/test/data/native_messaging/native_hosts/echo.bat
@@ -0,0 +1,6 @@
+@echo off
+:: Copyright (c) 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.
+
+python "%~dp0/echo.py"
diff --git a/chrome/test/data/native_messaging/native_hosts/empty_app.bat b/chrome/test/data/native_messaging/native_hosts/empty_app.bat
new file mode 100755
index 0000000..c16178f
--- /dev/null
+++ b/chrome/test/data/native_messaging/native_hosts/empty_app.bat
@@ -0,0 +1,6 @@
+@echo off
+:: Copyright (c) 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.
+
+python "%~dp0/empty_app.py"