Introduce the ability to wait for the watcher process to initialize.

chrome/app/chrome_watcher_client_win.h

+// Copyright 2015 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_APP_CHROME_WATCHER_CLIENT_WIN_H_
+#define CHROME_APP_CHROME_WATCHER_CLIENT_WIN_H_
+
+#include <windows.h>
+
+#include "base/callback.h"
+#include "base/command_line.h"
+#include "base/macros.h"
+#include "base/process/process.h"
+#include "base/win/scoped_handle.h"
+
+// Launches a Chrome watcher process and permits the client to wait until the
+// process is fully initialized.
+class ChromeWatcherClient {
+ public:
+  // A CommandLineGenerator generates command lines that will launch a separate
+  // process and pass the supplied HANDLEs to WatcherMain in that process. The
+  // first HANDLE is an event that should be signaled when the watcher process
+  // is fully initialized. The second HANDLE is the process that the watcher

[Sigurður Ásgeirsson, 2015/01/29 16:59:56]

note that both handles must be inheritable?
Maybe also worth a note somewhere on the necessary lifetime of these handles, as
those are all rather unusual details?

[erikwright (departed), 2015/01/30 20:39:57]

Given that the command-line generator is not responsible for the inheritance I'm
not sure it's relevant. I added a brief comment here anyway.

I'm not sure what lifetime issues you're referring to. Again it's not relevant
to the generator (I think it's self-evident that the HANDLEs will still be valid
at the time the generated cmdline is actually executed, but even still, not the
generator's problem).

+  // process should watch.
+  typedef base::Callback<base::CommandLine(HANDLE, HANDLE)>
+      CommandLineGenerator;
+
+  // Constructs an instance that launches its watcher process using the command
+  // line generated by |command_line_generator|.
+  explicit ChromeWatcherClient(
+      const CommandLineGenerator& command_line_generator);
+
+  ~ChromeWatcherClient();
+
+  // Launches the watcher process such that the child process is able to inherit
+  // a handle to the current process. Returns true if the process is
+  // successfully launched.
+  bool LaunchWatcher();
+
+  // Blocks until the process, previously launched by LaunchWatcher, is either
+  // fully initialized or terminates. Returns true if the process successfully
+  // initializes.
+  bool WaitForInitialization();
+
+  // Terminates the previously launched watcher process with |exit_code| if it
+  // has not already exited. Blocks until the process has exited and returns its

[Sigurður Ásgeirsson, 2015/01/29 16:59:57]

This is hopefully for testing only?

[erikwright (departed), 2015/01/30 20:39:57]

Yes, although I can't think of a good reason to label it as such. In my opinion,
a ForTesting label is appropriate when the use of the method somehow violates
the contract or encapsulation of the class and thus must be prevented. While
there is no existing production use-case for this method, there would be no harm
in invoking it if one were to materialize.

For example, it could conceivably make sense to call it if the user disabled
crash reporting after the browser had been launched.

[Sigurður Ásgeirsson, 2015/02/02 16:59:20]

Not the way it's currently implemented.
TerminateProcess should never be used, any more than WriteProcessMemory (read:
used only when absolutely necessary :). It would be appropriate to expose a
"please shutdown in an orderly fashion" interface, though.

[erikwright (departed), 2015/02/03 20:05:34]

How about this? It's up to the client to ask the "watcher" to shut down, but we
provide a way to wait on it. This seems to respect encapsulation.

+  // exit code. Note that the return value might differ from |exit_code| if, for
+  // example, it exits naturally before the forced termination takes place. The
+  // return value is undefined if LaunchWatcher has not been called or failed,
+  // or if invoked multiple times.
+  int TerminateWatcher(int exit_code);
+
+  // Returns true if the process has previously been launched and
+  // TerminateWatcher has not yet been invoked.
+  bool launched() { return process_.IsValid(); }
+
+ private:
+  CommandLineGenerator command_line_generator_;
+  base::win::ScopedHandle on_initialized_event_;
+  base::Process process_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeWatcherClient);
+};
+
+#endif  // CHROME_APP_CHROME_WATCHER_CLIENT_WIN_H_