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

BUG=

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

Cr-Commit-Position: refs/heads/master@{#314563}

chrome/BUILD.gn

@@ -45,6 +45,10 @@ if (!is_android && (!is_win || link_chrome_on_windows)) {
         "app/chrome_crash_reporter_client.h",
         "app/chrome_exe.rc",
         "app/chrome_exe_main_win.cc",
+        "app/chrome_watcher_client_win.cc",
+        "app/chrome_watcher_client_win.h",
+        "app/chrome_watcher_command_line_win.cc",
+        "app/chrome_watcher_command_line_win.h",
         "app/client_util.cc",
         "app/client_util.h",
         "app/chrome_watcher_command_line_win.cc",

chrome/app/chrome_watcher_client_unittest_win.cc

+// 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.
+
+#include "chrome/app/chrome_watcher_client_win.h"
+
+#include <windows.h>
+#include <string>
+#include "base/base_switches.h"
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/logging.h"
+#include "base/process/process_handle.h"
+#include "base/strings/string16.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/test/multiprocess_test.h"
+#include "base/threading/simple_thread.h"
+#include "base/time/time.h"
+#include "base/win/scoped_handle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/multiprocess_func_list.h"
+
+namespace {
+
+const char kParentHandle[] = "parent-handle";
+const char kEventHandle[] = "event-handle";
+const char kNamedEventSuffix[] = "named-event-suffix";
+
+const base::char16 kExitEventBaseName[] = L"ChromeWatcherClientTestExitEvent_";
+const base::char16 kInitializeEventBaseName[] =
+    L"ChromeWatcherClientTestInitializeEvent_";
+
+base::win::ScopedHandle InterpretHandleSwitch(base::CommandLine& cmd_line,
+                                              const char* switch_name) {
+  std::string str_handle =
+      cmd_line.GetSwitchValueASCII(switch_name);
+  if (str_handle.empty()) {
+    LOG(ERROR) << "Switch " << switch_name << " unexpectedly absent.";
+    return base::win::ScopedHandle();
+  }
+
+  unsigned int_handle = 0;
+  if (!base::StringToUint(str_handle, &int_handle)) {
+    LOG(ERROR) << "Switch " << switch_name << " has invalid value "
+               << str_handle;
+    return base::win::ScopedHandle();
+  }
+
+  return base::win::ScopedHandle(
+      reinterpret_cast<base::ProcessHandle>(int_handle));
+}
+
+// Simulates a Chrome watcher process. Exits when the global exit event is
+// signaled. Signals the "on initialized" event (passed on the command-line)
+// when the global initialization event is signaled.
+MULTIPROCESS_TEST_MAIN(ChromeWatcherClientTestProcess) {
+  base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess();
+
+  base::string16 named_event_suffix =
+      base::ASCIIToUTF16(cmd_line->GetSwitchValueASCII(kNamedEventSuffix));
+  if (named_event_suffix.empty()) {
+    LOG(ERROR) << "Switch " << kNamedEventSuffix << " unexpectedly absent.";
+    return 1;
+  }
+
+  base::win::ScopedHandle exit_event(::CreateEvent(
+      NULL, FALSE, FALSE, (kExitEventBaseName + named_event_suffix).c_str()));
+  if (!exit_event.IsValid()) {
+    LOG(ERROR) << "Failed to create event named "
+               << kExitEventBaseName + named_event_suffix;
+    return 1;
+  }
+
+  base::win::ScopedHandle initialize_event(
+      ::CreateEvent(NULL, FALSE, FALSE,
+                    (kInitializeEventBaseName + named_event_suffix).c_str()));
+  if (!initialize_event.IsValid()) {
+    LOG(ERROR) << "Failed to create event named "
+               << kInitializeEventBaseName + named_event_suffix;
+    return 1;
+  }
+
+  base::win::ScopedHandle parent_process(
+      InterpretHandleSwitch(*cmd_line, kParentHandle));
+  if (!parent_process.IsValid())
+    return 1;
+
+  base::win::ScopedHandle on_initialized_event(
+      InterpretHandleSwitch(*cmd_line, kEventHandle));
+  if (!on_initialized_event.IsValid())
+    return 1;
+
+  while (true) {
+    // We loop as a convenient way to continue waiting for the exit_event after
+    // the initialize_event is signaled. We expect to get initialize_event zero
+    // or one times before exit_event, never more.
+    HANDLE handles[] = {exit_event.Get(), initialize_event.Get()};
+    DWORD result =
+        ::WaitForMultipleObjects(arraysize(handles), handles, FALSE, INFINITE);
+    switch (result) {
+      case WAIT_OBJECT_0:
+        // exit_event
+        return 0;
+      case WAIT_OBJECT_0 + 1:
+        // initialize_event
+        ::SetEvent(on_initialized_event.Get());
+        break;
+      case WAIT_FAILED:
+        PLOG(ERROR) << "Unexpected failure in WaitForMultipleObjects.";
+        return 1;
+      default:
+        NOTREACHED() << "Unexpected result from WaitForMultipleObjects: "
+                     << result;
+        return 1;
+    }
+  }
+}
+
+// Implements a thread to launch the ChromeWatcherClient and block on
+// EnsureInitialized. Provides various helpers to interact with the
+// ChromeWatcherClient.
+class ChromeWatcherClientThread : public base::SimpleThread {
+ public:
+  ChromeWatcherClientThread()
+      : client_(base::Bind(&ChromeWatcherClientThread::GenerateCommandLine,
+                           base::Unretained(this))),
+        complete_(false, false),
+        result_(false),
+        SimpleThread("ChromeWatcherClientTest thread") {}
+
+  // Waits up to |timeout| for the call to EnsureInitialized to complete. If it
+  // does, sets |result| to the return value of EnsureInitialized and returns
+  // true. Otherwise returns false.
+  bool WaitForResultWithTimeout(base::TimeDelta timeout, bool* result) {
+    if (!complete_.TimedWait(timeout))
+      return false;
+    *result = result_;
+    return true;
+  }
+
+  // Waits indefinitely for the call to WaitForInitialization to complete.
+  // Returns the return value of WaitForInitialization.
+  bool WaitForResult() {
+    complete_.Wait();
+    return result_;
+  }
+
+  ChromeWatcherClient& client() { return client_; }
+
+  base::string16 NamedEventSuffix() {
+    return base::UintToString16(base::GetCurrentProcId());
+  }
+
+  // base::SimpleThread implementation.
+  void Run() override {
+    result_ = client_.LaunchWatcher();
+    if (result_)
+      result_ = client_.EnsureInitialized();
+    complete_.Signal();
+  }
+
+ private:
+  // Returns a command line to launch back into ChromeWatcherClientTestProcess.
+  base::CommandLine GenerateCommandLine(HANDLE parent_handle,
+                                        HANDLE on_initialized_event) {
+    base::CommandLine ret = base::GetMultiProcessTestChildBaseCommandLine();
+    ret.AppendSwitchASCII(switches::kTestChildProcess,
+                          "ChromeWatcherClientTestProcess");
+    ret.AppendSwitchASCII(kEventHandle,
+                          base::UintToString(reinterpret_cast<unsigned int>(
+                              on_initialized_event)));
+    ret.AppendSwitchASCII(
+        kParentHandle,
+        base::UintToString(reinterpret_cast<unsigned int>(parent_handle)));
+    ret.AppendSwitchASCII(kNamedEventSuffix,
+                          base::UTF16ToASCII(NamedEventSuffix()));
+    return ret;
+  }
+
+  // The instance under test.
+  ChromeWatcherClient client_;
+  // Signaled when WaitForInitialization returns.
+  base::WaitableEvent complete_;
+  // The return value of WaitForInitialization.
+  bool result_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeWatcherClientThread);
+};
+
+}  // namespace
+
+class ChromeWatcherClientTest : public testing::Test {
+ protected:
+  // Sends a signal to the simulated watcher process to exit. Returns true if
+  // successful.
+  bool SignalExit() { return ::SetEvent(exit_event_.Get()) != FALSE; }
+
+  // Sends a signal to the simulated watcher process to signal its
+  // "initialization". Returns true if successful.
+  bool SignalInitialize() {
+    return ::SetEvent(initialize_event_.Get()) != FALSE;
+  }
+
+  // The helper thread, which also provides access to the ChromeWatcherClient.
+  ChromeWatcherClientThread& thread() { return thread_; }
+
+  // testing::Test implementation.
+  void SetUp() override {
+    exit_event_.Set(::CreateEvent(
+        NULL, FALSE, FALSE,
+        (kExitEventBaseName + thread_.NamedEventSuffix()).c_str()));
+    ASSERT_TRUE(exit_event_.IsValid());
+    initialize_event_.Set(::CreateEvent(
+        NULL, FALSE, FALSE,
+        (kInitializeEventBaseName + thread_.NamedEventSuffix()).c_str()));
+    ASSERT_TRUE(initialize_event_.IsValid());
+  }
+
+  void TearDown() override {
+    // Even if we never launched, the following is harmless.
+    SignalExit();
+    int exit_code = 0;
+    thread_.client().WaitForExit(&exit_code);
+    thread_.Join();
+  }
+
+ private:
+  // Used to launch and block on the Chrome watcher process in a background
+  // thread.
+  ChromeWatcherClientThread thread_;
+  // Used to signal the Chrome watcher process to exit.
+  base::win::ScopedHandle exit_event_;
+  // Used to signal the Chrome watcher process to signal its own
+  // initialization..
+  base::win::ScopedHandle initialize_event_;
+};
+
+TEST_F(ChromeWatcherClientTest, SuccessTest) {
+  thread().Start();
+  bool result = false;
+  // Give a broken implementation a chance to exit unexpectedly.
+  ASSERT_FALSE(thread().WaitForResultWithTimeout(
+      base::TimeDelta::FromMilliseconds(100), &result));
+  ASSERT_TRUE(SignalInitialize());
+  ASSERT_TRUE(thread().WaitForResult());
+  // The watcher should still be running. Give a broken implementation a chance
+  // to exit unexpectedly, then signal it to exit.
+  int exit_code = 0;
+  ASSERT_FALSE(thread().client().WaitForExitWithTimeout(
+      base::TimeDelta::FromMilliseconds(100), &exit_code));
+  SignalExit();
+  ASSERT_TRUE(thread().client().WaitForExit(&exit_code));
+  ASSERT_EQ(0, exit_code);
+}
+
+TEST_F(ChromeWatcherClientTest, FailureTest) {
+  thread().Start();
+  bool result = false;
+  // Give a broken implementation a chance to exit unexpectedly.
+  ASSERT_FALSE(thread().WaitForResultWithTimeout(
+      base::TimeDelta::FromMilliseconds(100), &result));
+  ASSERT_TRUE(SignalExit());
+  ASSERT_FALSE(thread().WaitForResult());
+  int exit_code = 0;
+  ASSERT_TRUE(
+      thread().client().WaitForExitWithTimeout(base::TimeDelta(), &exit_code));
+  ASSERT_EQ(0, exit_code);
+}

chrome/app/chrome_watcher_client_win.cc

+// 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.
+
+#include "chrome/app/chrome_watcher_client_win.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "components/browser_watcher/watcher_client_win.h"
+
+namespace {
+
+// Because we can only bind parameters from the left, |parent_process| must be
+// the last parameter of the method that we bind int a
+// BrowserWatcherClient::CommandLineGenerator. The ChromeWatcherClient API is
+// more intuitive if the ChromeWatcherClient::CommandLineGenerator takes
+// |parent_process| as its second (of three) parameters, so we use this
+// intermediate function to swap the order.
+base::CommandLine InvokeCommandLineGenerator(
+    const ChromeWatcherClient::CommandLineGenerator& command_line_generator,
+    HANDLE on_initialized_event,
+    HANDLE parent_process) {
+  return command_line_generator.Run(parent_process, on_initialized_event);
+}
+
+}  // namespace
+
+ChromeWatcherClient::ChromeWatcherClient(
+    const CommandLineGenerator& command_line_generator)
+    : command_line_generator_(command_line_generator) {
+}
+
+ChromeWatcherClient::~ChromeWatcherClient() {
+}
+
+bool ChromeWatcherClient::LaunchWatcher() {
+  // Create an inheritable event that the child process will signal when it has
+  // completed initialization.
+  SECURITY_ATTRIBUTES on_initialized_event_attributes = {
+      sizeof(SECURITY_ATTRIBUTES),  // nLength
+      nullptr,                      // lpSecurityDescriptor
+      TRUE                          // bInheritHandle
+  };
+  on_initialized_event_.Set(::CreateEvent(&on_initialized_event_attributes,
+                                          TRUE,  // manual reset
+                                          FALSE, nullptr));
+  if (!on_initialized_event_.IsValid()) {
+    DPLOG(ERROR) << "Failed to create an event.";
+    return false;
+  }
+
+  // Configure the basic WatcherClient, binding in the initialization event
+  // HANDLE.
+  browser_watcher::WatcherClient watcher_client(
+      base::Bind(&InvokeCommandLineGenerator, command_line_generator_,
+                 on_initialized_event_.Get()));
+  // Indicate that the event HANDLE should be inherited.
+  watcher_client.AddInheritedHandle(on_initialized_event_.Get());
+  // Launch the watcher.
+  watcher_client.LaunchWatcher();
+  // Grab a handle to the watcher so that we may later wait on its
+  // initialization.
+  process_ = watcher_client.process().Duplicate();
+  if (!process_.IsValid())
+    on_initialized_event_.Close();
+  return process_.IsValid();
+}
+
+bool ChromeWatcherClient::EnsureInitialized() {
+  if (!process_.IsValid())
+    return false;
+
+  DCHECK(on_initialized_event_.IsValid());
+
+  HANDLE handles[] = {on_initialized_event_.Get(), process_.Handle()};
+  DWORD result = ::WaitForMultipleObjects(arraysize(handles), handles,
+                                          FALSE, INFINITE);
+
+  switch (result) {
+    case WAIT_OBJECT_0:
+      return true;
+    case WAIT_OBJECT_0 + 1:
+      LOG(ERROR) << "Chrome watcher process failed to launch.";
+      return false;
+    case WAIT_FAILED:
+      DPLOG(ERROR) << "Failure while waiting on Chrome watcher process launch.";
+      return false;
+    default:
+      NOTREACHED() << "Unexpected result while waiting on Chrome watcher "
+                      "process launch: " << result;
+      return false;
+  }
+}
+
+bool ChromeWatcherClient::WaitForExit(int* exit_code) {
+  return process_.IsValid() && process_.WaitForExit(exit_code);
+}
+
+bool ChromeWatcherClient::WaitForExitWithTimeout(base::TimeDelta timeout,
+                                                 int* exit_code) {
+  return process_.IsValid() &&
+         process_.WaitForExitWithTimeout(timeout, exit_code);
+}

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/time/time.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 HANDLE values to WatcherMain in that process.
+  // The first HANDLE is the process that the watcher process should watch. The
+  // second HANDLE is an event that should be signaled when the watcher process
+  // is fully initialized. The process will be launched such that the HANDLEs
+  // are inherited by the new process.
+  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 has terminated. Returns true if the process
+  // successfully initializes. May be called multiple times.
+  bool EnsureInitialized();
+
+  // Waits for the process to exit. Returns true on success. It is up to the
+  // client to somehow signal the process to exit.
+  bool WaitForExit(int* exit_code);
+
+  // Same as WaitForExit() but only waits for up to |timeout|.
+  bool WaitForExitWithTimeout(base::TimeDelta timeout, int* exit_code);
+
+ 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_

chrome/app/chrome_watcher_command_line_unittest_win.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/app/chrome_watcher_command_line_win.h"
 
+#include <windows.h>
+
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/process/process_handle.h"
@@ -11,10 +13,19 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 TEST(ChromeWatcherCommandLineTest, BasicTest) {
+  // Ownership of these handles is passed to the ScopedHandles below via
+  // InterpretChromeWatcherCommandLine().
   base::ProcessHandle current = nullptr;
   ASSERT_TRUE(base::OpenProcessHandle(base::GetCurrentProcId(), &current));
-  base::CommandLine cmd_line =
-      GenerateChromeWatcherCommandLine(base::FilePath(L"example.exe"), current);
-  base::win::ScopedHandle result = InterpretChromeWatcherCommandLine(cmd_line);
-  ASSERT_EQ(current, result.Get());
+  HANDLE event = ::CreateEvent(nullptr, FALSE, FALSE, nullptr);
+
+  base::CommandLine cmd_line = GenerateChromeWatcherCommandLine(
+      base::FilePath(L"example.exe"), current, event);
+
+  base::win::ScopedHandle current_result;
+  base::win::ScopedHandle event_result;
+  ASSERT_TRUE(InterpretChromeWatcherCommandLine(cmd_line, &current_result,
+                                                &event_result));
+  ASSERT_EQ(current, current_result.Get());
+  ASSERT_EQ(event, event_result.Get());
 }

chrome/app/chrome_watcher_command_line_win.cc

@@ -14,41 +14,91 @@
 #include "chrome/common/chrome_switches.h"
 
 namespace {
+
+const char kOnIninitializedEventHandleSwitch[] = "on-initialized-event-handle";
 const char kParentHandleSwitch[] = "parent-handle";
+
+void AppendHandleSwitch(const std::string& switch_name,
+                        HANDLE handle,
+                        base::CommandLine* command_line) {
+  command_line->AppendSwitchASCII(
+      switch_name, base::UintToString(reinterpret_cast<unsigned int>(handle)));
+}
+
+HANDLE ReadHandleFromSwitch(const base::CommandLine& command_line,
+                            const std::string& switch_name) {
+  std::string switch_string = command_line.GetSwitchValueASCII(switch_name);
+  unsigned int switch_uint = 0;
+  if (switch_string.empty() ||
+      !base::StringToUint(switch_string, &switch_uint)) {
+    DLOG(ERROR) << "Missing or invalid " << switch_name << " argument.";
+    return nullptr;
+  }
+  return reinterpret_cast<HANDLE>(switch_uint);
+}
+
 }  // namespace
 
 base::CommandLine GenerateChromeWatcherCommandLine(
     const base::FilePath& chrome_exe,
-    HANDLE parent_process) {
+    HANDLE parent_process,
+    HANDLE on_initialized_event) {
   base::CommandLine command_line(chrome_exe);
   command_line.AppendSwitchASCII(switches::kProcessType, "watcher");
-  command_line.AppendSwitchASCII(
-      kParentHandleSwitch,
-      base::UintToString(reinterpret_cast<unsigned int>(parent_process)));
+  AppendHandleSwitch(kOnIninitializedEventHandleSwitch, on_initialized_event,
+                     &command_line);
+  AppendHandleSwitch(kParentHandleSwitch, parent_process, &command_line);
   return command_line;
 }
 
-base::win::ScopedHandle InterpretChromeWatcherCommandLine(
-    const base::CommandLine& command_line) {
-  std::string parent_handle_str =
-      command_line.GetSwitchValueASCII(kParentHandleSwitch);
-  unsigned parent_handle_uint = 0;
-  if (parent_handle_str.empty() ||
-      !base::StringToUint(parent_handle_str, &parent_handle_uint)) {
-    LOG(ERROR) << "Missing or invalid " << kParentHandleSwitch << " argument.";
-    return base::win::ScopedHandle();
+bool InterpretChromeWatcherCommandLine(
+    const base::CommandLine& command_line,
+    base::win::ScopedHandle* parent_process,
+    base::win::ScopedHandle* on_initialized_event) {
+  DCHECK(on_initialized_event);
+  DCHECK(parent_process);
+
+  // For consistency, always close any existing HANDLEs here.
+  on_initialized_event->Close();
+  parent_process->Close();
+
+  HANDLE parent_handle =
+      ReadHandleFromSwitch(command_line, kParentHandleSwitch);
+  HANDLE on_initialized_event_handle =
+      ReadHandleFromSwitch(command_line, kOnIninitializedEventHandleSwitch);
+
+  if (parent_handle) {
+    // Initial test of the handle, a zero PID indicates invalid handle, or not
+    // a process handle. In this case, parsing fails and we avoid closing the
+    // handle.
+    DWORD process_pid = ::GetProcessId(parent_handle);
+    if (process_pid == 0) {
+      DLOG(ERROR) << "Invalid " << kParentHandleSwitch
+                  << " argument. Can't get parent PID.";
+    } else {
+      parent_process->Set(parent_handle);
+    }
+  }
+
+  if (on_initialized_event_handle) {
+    DWORD result = ::WaitForSingleObject(on_initialized_event_handle, 0);
+    if (result == WAIT_FAILED) {
+      DPLOG(ERROR)
+          << "Unexpected error while testing the initialization event.";
+    } else if (result != WAIT_TIMEOUT) {
+      DLOG(ERROR) << "Unexpected result while testing the initialization event "
+                     "with WaitForSingleObject: " << result;
+    } else {
+      on_initialized_event->Set(on_initialized_event_handle);
+    }
   }
 
-  HANDLE parent_process = reinterpret_cast<HANDLE>(parent_handle_uint);
-  // Initial test of the handle, a zero PID indicates invalid handle, or not
-  // a process handle. In this case, parsing fails and we avoid closing the
-  // handle.
-  DWORD process_pid = ::GetProcessId(parent_process);
-  if (process_pid == 0) {
-    LOG(ERROR) << "Invalid " << kParentHandleSwitch
-               << " argument. Can't get parent PID.";
-    return base::win::ScopedHandle();
+  if (!on_initialized_event->IsValid() || !parent_process->IsValid()) {
+    // If one was valid and not the other, free the valid one.
+    on_initialized_event->Close();
+    parent_process->Close();
+    return false;
   }
 
-  return base::win::ScopedHandle(parent_process);
+  return true;
 }

chrome/app/chrome_watcher_command_line_win.h

@@ -15,15 +15,21 @@ class FilePath;
 }  // namespace base
 
 // Generates a CommandLine that will launch |chrome_exe| in Chrome Watcher mode
-// to observe |parent_process|.
+// to observe |parent_process|. The watcher process will signal
+// |on_initialized_event| when its initialization is complete.
 base::CommandLine GenerateChromeWatcherCommandLine(
     const base::FilePath& chrome_exe,
-    HANDLE parent_process);
+    HANDLE parent_process,
+    HANDLE on_initialized_event);
 
 // Interprets the Command Line used to launch a Chrome Watcher process and
-// extracts the parent process HANDLE. Verifies that the handle is usable in
-// this process before returning it, and returns NULL in case of a failure.
-base::win::ScopedHandle InterpretChromeWatcherCommandLine(
-    const base::CommandLine& command_line);
+// extracts the parent process and initialization event HANDLEs. Verifies that
+// the handles are usable in this process before returning them. Returns true if
+// both handles are successfully parsed and false otherwise. If only one of the
+// handles can be parsed, it will be closed.
+bool InterpretChromeWatcherCommandLine(
+    const base::CommandLine& command_line,
+    base::win::ScopedHandle* parent_process,
+    base::win::ScopedHandle* on_initialized_event);
 
 #endif  // CHROME_APP_CHROME_WATCHER_COMMAND_LINE_WIN_H_

chrome/app/client_util.cc

@@ -21,6 +21,7 @@
 #include "base/win/scoped_handle.h"
 #include "base/win/windows_version.h"
 #include "chrome/app/chrome_crash_reporter_client.h"
+#include "chrome/app/chrome_watcher_client_win.h"
 #include "chrome/app/chrome_watcher_command_line_win.h"
 #include "chrome/app/client_util.h"
 #include "chrome/app/image_pre_reader_win.h"
@@ -33,7 +34,6 @@
 #include "chrome/installer/util/google_update_settings.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/util_constants.h"
-#include "components/browser_watcher/watcher_client_win.h"
 #include "components/crash/app/breakpad_win.h"
 #include "components/crash/app/crash_reporter_client.h"
 #include "components/metrics/client_info.h"
@@ -179,10 +179,12 @@ int MainDllLoader::Launch(HINSTANCE instance) {
   }
 
   if (process_type_ == "watcher") {
-    base::win::ScopedHandle parent_process =
-        InterpretChromeWatcherCommandLine(cmd_line);
-    if (!parent_process.IsValid())
+    base::win::ScopedHandle parent_process;
+    base::win::ScopedHandle on_initialized_event;
+    if (!InterpretChromeWatcherCommandLine(cmd_line, &parent_process,
+                                           &on_initialized_event)) {
       return chrome::RESULT_CODE_UNSUPPORTED_PARAM;
+    }
 
     // Intentionally leaked.
     HMODULE watcher_dll = Load(&version, &file);
@@ -193,7 +195,7 @@ int MainDllLoader::Launch(HINSTANCE instance) {
         reinterpret_cast<ChromeWatcherMainFunction>(
             ::GetProcAddress(watcher_dll, kChromeWatcherDLLEntrypoint));
     return watcher_main(chrome::kBrowserExitCodesRegistryPath,
-                        parent_process.Take());
+                        parent_process.Take(), on_initialized_event.Take());
   }
 
   // Initialize the sandbox services.
@@ -260,8 +262,7 @@ void ChromeDllLoader::OnBeforeLaunch(const std::string& process_type,
     if (g_chrome_crash_client.Get().GetCollectStatsConsent()) {
       base::char16 exe_path[MAX_PATH];
       ::GetModuleFileNameW(nullptr, exe_path, arraysize(exe_path));
-
-      browser_watcher::WatcherClient watcher_client(base::Bind(
+      ChromeWatcherClient watcher_client(base::Bind(
           &GenerateChromeWatcherCommandLine, base::FilePath(exe_path)));
       watcher_client.LaunchWatcher();
     }

chrome/chrome_exe.gypi

@@ -62,6 +62,8 @@
         'app/chrome_exe_main_mac.cc',
         'app/chrome_exe_main_win.cc',
         'app/chrome_exe_resource.h',
+        'app/chrome_watcher_client_win.cc',
+        'app/chrome_watcher_client_win.h',
         'app/chrome_watcher_command_line_win.cc',
         'app/chrome_watcher_command_line_win.h',
         'app/client_util.cc',

chrome/chrome_tests_unit.gypi

@@ -2605,6 +2605,8 @@
         '..',
       ],
       'sources': [
+        'app/chrome_watcher_client_unittest_win.cc',
+        'app/chrome_watcher_client_win.cc',
         'app/chrome_watcher_command_line_unittest_win.cc',
         'app/chrome_watcher_command_line_win.cc',
         'app/delay_load_hook_win.cc',

chrome/chrome_watcher/chrome_watcher_main.cc

@@ -34,23 +34,27 @@ const GUID kChromeWatcherTraceProviderName = {
         { 0x80, 0xc1, 0x52, 0x7f, 0xea, 0x23, 0xe3, 0xa7 } };
 
 // Takes care of monitoring a browser. This class watches for a browser's exit
-// code, as well as listening for WM_ENDSESSION messages. Events are recorded
-// in an exit funnel, for reporting the next time Chrome runs.
+// code, as well as listening for WM_ENDSESSION messages. Events are recorded in
+// an exit funnel, for reporting the next time Chrome runs.
 class BrowserMonitor {
  public:
   BrowserMonitor(base::RunLoop* run_loop, const base::char16* registry_path);
   ~BrowserMonitor();
 
-  // Starts the monitor, returns true on success.
+  // Initiates the asynchronous monitoring process, returns true on success.
+  // |on_initialized_event| will be signaled immediately before blocking on the
+  // exit of |process|.
   bool StartWatching(const base::char16* registry_path,
-                     base::Process process);
+                     base::Process process,
+                     base::win::ScopedHandle on_initialized_event);
 
  private:
   // Called from EndSessionWatcherWindow on a end session messages.
   void OnEndSessionMessage(UINT message, LPARAM lparam);
 
-  // Blocking function that runs on |background_thread_|.
-  void Watch();
+  // Blocking function that runs on |background_thread_|. Signals
+  // |on_initialized_event| before waiting for the browser process to exit.
+  void Watch(base::win::ScopedHandle on_initialized_event);
 
   // Posted to main thread from Watch when browser exits.
   void BrowserExited();
@@ -89,8 +93,10 @@ BrowserMonitor::BrowserMonitor(base::RunLoop* run_loop,
 BrowserMonitor::~BrowserMonitor() {
 }
 
-bool BrowserMonitor::StartWatching(const base::char16* registry_path,
-                                   base::Process process) {
+bool BrowserMonitor::StartWatching(
+    const base::char16* registry_path,
+    base::Process process,
+    base::win::ScopedHandle on_initialized_event) {
   if (!exit_code_watcher_.Initialize(process.Pass()))
     return false;
 
@@ -104,8 +110,9 @@ bool BrowserMonitor::StartWatching(const base::char16* registry_path,
     return false;
   }
 
-  if (!background_thread_.task_runner()->PostTask(FROM_HERE,
-        base::Bind(&BrowserMonitor::Watch, base::Unretained(this)))) {
+  if (!background_thread_.task_runner()->PostTask(
+          FROM_HERE, base::Bind(&BrowserMonitor::Watch, base::Unretained(this),
+                                base::Passed(on_initialized_event.Pass())))) {
     background_thread_.Stop();
     return false;
   }
@@ -137,10 +144,15 @@ void BrowserMonitor::OnEndSessionMessage(UINT message, LPARAM lparam) {
     run_loop_->Quit();
 }
 
-void BrowserMonitor::Watch() {
+void BrowserMonitor::Watch(base::win::ScopedHandle on_initialized_event) {
   // This needs to run on an IO thread.
   DCHECK_NE(main_thread_, base::MessageLoopProxy::current());
 
+  // Signal our client now that the Kasko reporter is initialized and we have
+  // cleared all of the obstacles that might lead to an early exit.
+  ::SetEvent(on_initialized_event.Get());
+  on_initialized_event.Close();
+
   exit_code_watcher_.WaitForExit();
   exit_funnel_.RecordEvent(L"BrowserExit");
 
@@ -177,8 +189,10 @@ void BrowserMonitor::BrowserExited() {
 // The main entry point to the watcher, declared as extern "C" to avoid name
 // mangling.
 extern "C" int WatcherMain(const base::char16* registry_path,
-                           HANDLE process_handle) {
+                           HANDLE process_handle,
+                           HANDLE on_initialized_event_handle) {
   base::Process process(process_handle);
+  base::win::ScopedHandle on_initialized_event(on_initialized_event_handle);
 
   // The exit manager is in charge of calling the dtors of singletons.
   base::AtExitManager exit_manager;
@@ -197,8 +211,10 @@ extern "C" int WatcherMain(const base::char16* registry_path,
 
   base::RunLoop run_loop;
   BrowserMonitor monitor(&run_loop, registry_path);
-  if (!monitor.StartWatching(registry_path, process.Pass()))
+  if (!monitor.StartWatching(registry_path, process.Pass(),
+                             on_initialized_event.Pass())) {
     return 1;
+  }
 
   run_loop.Run();
 

chrome/chrome_watcher/chrome_watcher_main_api.h

@@ -16,8 +16,11 @@ extern const char kChromeWatcherDLLEntrypoint[];
 
 // The type of the watcher DLL's main entry point.
 // Watches |parent_process| and records its exit code under |registry_path| in
-// HKCU. Takes ownership of |parent_process|.
-typedef int (*ChromeWatcherMainFunction)(const base::char16* registry_path,
-                                         HANDLE parent_process);
+// HKCU. |on_initialized_event| will be signaled once the process is fully
+// initialized. Takes ownership of |parent_process| and |on_initialized_event|.
+typedef int (*ChromeWatcherMainFunction)(
+    const base::char16* registry_path,
+    HANDLE parent_process,
+    HANDLE on_initialized_event);
 
 #endif  // CHROME_CHROME_WATCHER_CHROME_WATCHER_MAIN_API_H_

components/browser_watcher/watcher_client_win.cc

@@ -48,6 +48,8 @@ void WatcherClient::LaunchWatcher() {
     // Launch the child process inheriting only |self| on
     // Vista and better.
     to_inherit.push_back(self.Handle());
+    to_inherit.insert(to_inherit.end(), inherited_handles_.begin(),
+                      inherited_handles_.end());
     options.handles_to_inherit = &to_inherit;
   }
 
@@ -56,4 +58,8 @@ void WatcherClient::LaunchWatcher() {
     LOG(ERROR) << "Failed to launch browser watcher.";
 }
 
+void WatcherClient::AddInheritedHandle(HANDLE handle) {
+  inherited_handles_.push_back(handle);
+}
+
 }  // namespace browser_watcher

components/browser_watcher/watcher_client_win.h

@@ -32,12 +32,19 @@ class WatcherClient {
   // a non-threadsafe legacy launch mode that's compatible with Windows XP.
   void LaunchWatcher();
 
+  // Ensures that |handle| may be inherited by the watcher process. |handle|
+  // must still be inheritable, and it's the client's responsibility to
+  // communicate the value of |handle| to the launched process.
+  void AddInheritedHandle(HANDLE handle);
+
+  // Returns the launched process.
+  const base::Process& process() const { return process_; }
+
   // Accessors, exposed only for testing.
   bool use_legacy_launch() const { return use_legacy_launch_; }
   void set_use_legacy_launch(bool use_legacy_launch) {
       use_legacy_launch_ = use_legacy_launch;
   }
-  base::ProcessHandle process() const { return process_.Handle(); }
 
  private:
   // If true, the watcher process will be launched with XP legacy handle
@@ -52,6 +59,8 @@ class WatcherClient {
   // LaunchWatcher() call.
   base::Process process_;
 
+  std::vector<HANDLE> inherited_handles_;
+
   DISALLOW_COPY_AND_ASSIGN(WatcherClient);
 };
 

components/browser_watcher/watcher_client_win_unittest.cc

@@ -137,25 +137,11 @@ class WatcherClientTest : public base::MultiProcessTest {
                       base::Unretained(this), handle_policy);
   }
 
-  void AssertSuccessfulExitCode(base::ProcessHandle handle) {
-    ASSERT_NE(base::kNullProcessHandle, handle);
-
-    // Duplicate the process handle to work around the fact that
-    // WaitForExitCode closes it(!!!).
-    base::ProcessHandle dupe = NULL;
-    ASSERT_TRUE(::DuplicateHandle(base::GetCurrentProcessHandle(),
-                                  handle,
-                                  base::GetCurrentProcessHandle(),
-                                  &dupe,
-                                  SYNCHRONIZE | PROCESS_QUERY_INFORMATION,
-                                  FALSE,
-                                  0));
-    ASSERT_NE(base::kNullProcessHandle, dupe);
+  void AssertSuccessfulExitCode(base::Process process) {
+    ASSERT_TRUE(process.IsValid());
     int exit_code = 0;
-    if (!base::WaitForExitCode(dupe, &exit_code)) {
-      base::CloseProcessHandle(dupe);
-      FAIL() << "WaitForExitCode failed.";
-    }
+    if (!process.WaitForExit(&exit_code))
+      FAIL() << "Process::WaitForExit failed.";
     ASSERT_EQ(0, exit_code);
   }
 
@@ -177,7 +163,8 @@ TEST_F(WatcherClientTest, LaunchWatcherSucceeds) {
 
   client.LaunchWatcher();
 
-  ASSERT_NO_FATAL_FAILURE(AssertSuccessfulExitCode(client.process()));
+  ASSERT_NO_FATAL_FAILURE(
+      AssertSuccessfulExitCode(client.process().Duplicate()));
 }
 
 TEST_F(WatcherClientTest, LaunchWatcherLegacyModeSucceeds) {
@@ -190,7 +177,8 @@ TEST_F(WatcherClientTest, LaunchWatcherLegacyModeSucceeds) {
 
   client.LaunchWatcher();
 
-  ASSERT_NO_FATAL_FAILURE(AssertSuccessfulExitCode(client.process()));
+  ASSERT_NO_FATAL_FAILURE(
+      AssertSuccessfulExitCode(client.process().Duplicate()));
 }
 
 TEST_F(WatcherClientTest, LegacyModeDetectedOnXP) {
@@ -205,7 +193,8 @@ TEST_F(WatcherClientTest, LegacyModeDetectedOnXP) {
 
   client.LaunchWatcher();
 
-  ASSERT_NO_FATAL_FAILURE(AssertSuccessfulExitCode(client.process()));
+  ASSERT_NO_FATAL_FAILURE(
+      AssertSuccessfulExitCode(client.process().Duplicate()));
 }
 
 }  // namespace browser_watcher