Browser watcher, part deux.

components/browser_watcher/watcher_client_win.cc

Index: components/browser_watcher/watcher_client_win.cc
diff --git a/components/browser_watcher/watcher_client_win.cc b/components/browser_watcher/watcher_client_win.cc
new file mode 100644
index 0000000000000000000000000000000000000000..71f9f3c81afc0c87058cf85079ceb566f10d4b28
--- /dev/null
+++ b/components/browser_watcher/watcher_client_win.cc
@@ -0,0 +1,75 @@
+// Copyright (c) 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 "components/browser_watcher/watcher_client_win.h"
+
+#include <windows.h>
+
+#include "base/process/launch.h"
+#include "base/strings/stringprintf.h"
+#include "base/win/scoped_handle.h"

[erikwright (departed), 2014/11/18 20:00:05]

dup from header

[Sigurður Ásgeirsson, 2014/11/18 21:18:56]

Done.

+#include "base/win/windows_version.h"
+#include "components/browser_watcher/watcher_win.h"
+
+namespace browser_watcher {
+
+namespace {
+
+base::win::ScopedHandle OpenOwnProcessInheritable() {
+  return base::win::ScopedHandle(
+      ::OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION,
+                    TRUE,  // Ineritable handle.
+                    base::GetCurrentProcId()));
+}
+
+void AddHandleArgument(base::ProcessHandle handle,
+                       base::CommandLine* cmd_line) {
+  DCHECK(cmd_line);

[erikwright (departed), 2014/11/18 20:00:05]

base/logging.h

Though it's probably overkill for a single helper function in an anonymous
namespace in such a small file.

[Sigurður Ásgeirsson, 2014/11/18 21:18:56]

Done.

+
+  cmd_line->AppendSwitchASCII(ExitCodeWatcher::kParenthHandleSwitch,

[erikwright (departed), 2014/11/18 20:00:05]

Wow. My bad. In your other CL, I should have pointed out that the file name
should correspond to the class name.

Please rename watcher_win.{cc,h} to exit_code_watcher_win.{cc,h}

Can be a separate CL or this one as you see fit.

[Sigurður Ásgeirsson, 2014/11/18 21:18:56]

Done.

+                              base::StringPrintf("%d", handle));
+}
+
+}  // namespace
+
+WatcherClient::WatcherClient(const base::CommandLine& base_command_line) :
+    use_legacy_launch_(base::win::GetVersion() < base::win::VERSION_VISTA),
+    base_command_line_(base_command_line),
+    process_(base::kNullProcessHandle) {
+}
+
+base::win::ScopedHandle WatcherClient::LaunchWatcherProcess(
+    const base::CommandLine& cmd_line, base::ProcessHandle handle) {
+  base::HandlesToInheritVector to_inherit;
+  base::LaunchOptions options;
+  options.start_hidden = true;
+  if (use_legacy_launch_) {
+    // Launch the child process inheriting all handles on XP.
+    options.inherit_handles = true;
+  } else {
+    // Launch the child process inheriting only |handle| on
+    // Vista and better.
+    to_inherit.push_back(handle);
+    options.handles_to_inherit = &to_inherit;
+  }
+
+  base::ProcessHandle process = base::kNullProcessHandle;
+  base::LaunchProcess(cmd_line, options, &process);

[erikwright (departed), 2014/11/18 20:00:05]

Would some logging be appropriate if this fails?

[Sigurður Ásgeirsson, 2014/11/18 21:18:57]

Done, though in practice I don't think you'll get any logging from Chrome.exe?

+  return base::win::ScopedHandle(process);
+}
+
+void WatcherClient::LaunchWatcher() {
+  DCHECK(!process_.IsValid());

[erikwright (departed), 2014/11/18 20:00:05]

base/logging.h

[Sigurður Ásgeirsson, 2014/11/18 21:18:56]

Done.

+
+  // Build the command line for the watcher process.
+  base::win::ScopedHandle self(OpenOwnProcessInheritable());
+  DCHECK(self.IsValid());
+  base::CommandLine cmd_line(base_command_line_);
+  AddHandleArgument(self.Get(), &cmd_line);
+
+  // Launch it.
+  process_ = LaunchWatcherProcess(cmd_line, self.Get());

[erikwright (departed), 2014/11/18 20:00:05]

I wonder if the destructor of WatcherClient should check whether process_ is
still alive and report if it's not. Or maybe set a timer to check the launcher's
status after a few minutes of runtime. Or use an Event to signal successful
initialization of the watcher and report if it fails to signal in some delay. As
it stands, a failure during launch would be unreported. Or a crash after launch.

[Sigurður Ásgeirsson, 2014/11/18 21:18:56]

This is baked into chrome.exe, there's no infrastructure for what you request of
any kind.

+}
+
+}  // namespace browser_watcher