[win] Change Chrome Watcher configuration API.

chrome/app/chrome_watcher_command_line_win_unittest.cc

+// 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 "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"
+#include "base/win/scoped_handle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class TestChromeWatcherCommandLineGenerator
+    : public ChromeWatcherCommandLineGenerator {
+ public:
+  explicit TestChromeWatcherCommandLineGenerator(
+      const base::FilePath& chrome_exe)
+      : ChromeWatcherCommandLineGenerator(chrome_exe) {
+  }
+
+  void ReleaseHandlesWithoutClosing() {
+    on_initialized_event_handle_.Take();
+    parent_process_handle_.Take();
+  }
+};
+
+TEST(ChromeWatcherCommandLineTest, UseOfBadHandlesFails) {
+  // Handles are always machine word aligned so there is no way these are valid.
+  HANDLE bad_handle_1 = reinterpret_cast<HANDLE>(0x01FC00B1);
+  HANDLE bad_handle_2 = reinterpret_cast<HANDLE>(0x01FC00B3);
+
+  // The above handles are duplicated by the generator.
+  TestChromeWatcherCommandLineGenerator generator(
+      base::FilePath(L"example.exe"));

[grt (UTC plus 2), 2015/12/22 16:31:31]

nit: FILE_PATH_LITERAL("example.exe") here and elsewhere

[chrisha, 2015/12/23 14:08:23]

Done.

+  EXPECT_FALSE(generator.SetOnInitializedEventHandle(bad_handle_1));
+  EXPECT_FALSE(generator.SetParentProcessHandle(bad_handle_2));
+
+  // Expect there to be two inherited handles created by the generator.

[grt (UTC plus 2), 2015/12/22 16:31:32]

two -> no?

[chrisha, 2015/12/23 14:08:23]

Done.

+  std::vector<HANDLE> handles;
+  generator.GetInheritedHandles(&handles);
+  EXPECT_TRUE(handles.empty());
+}
+
+TEST(ChromeWatcherCommandLineTest, BadCommandLineFailsInterpretation) {
+  // Create an invalid command-line that is missing several fields.
+  base::CommandLine cmd_line(base::FilePath(L"example.exe"));
+
+  // Parse the command line.
+  auto interpreted = ChromeWatcherCommandLine::InterpretCommandLine(cmd_line);
+  EXPECT_FALSE(interpreted.get());

[grt (UTC plus 2), 2015/12/22 16:31:31]

scoped_ptr can be used directly in boolean expressions, so this can be:
  EXPECT_FALSE(interpreted);
please update lines 86 and 130 accordingly

[chrisha, 2015/12/23 14:08:23]

Done.

+}
+
+TEST(ChromeWatcherCommandLineDeathTest, HandlesLeftUntakenCausesDeath) {
+  base::win::ScopedHandle process(::OpenProcess(

[grt (UTC plus 2), 2015/12/22 16:31:31]

can you use base::Process for this and other process handles?
  base::Process process(base::Process::OpenWithAccess(base::GetCurrentProcId(),
PROCESS_QUERY_INFORMATION | SYNCHRONIZE));

[chrisha, 2015/12/23 14:08:23]

Done.

+      PROCESS_QUERY_INFORMATION | SYNCHRONIZE,
+      FALSE,  // Not inheritable.
+      ::GetCurrentProcessId()));
+  ASSERT_TRUE(process.Get());
+
+  base::win::ScopedHandle event(::CreateEvent(nullptr, FALSE, FALSE, nullptr));
+  ASSERT_TRUE(event.Get());

[grt (UTC plus 2), 2015/12/22 16:31:32]

use IsValid() rather than Get() if these assertions are intended to check the
validity of a ScopedHandle

[chrisha, 2015/12/23 14:08:23]

Done.

+
+  // The above handles are duplicated by the generator.
+  TestChromeWatcherCommandLineGenerator generator(
+      base::FilePath(L"example.exe"));
+  generator.SetOnInitializedEventHandle(event.Get());
+  generator.SetParentProcessHandle(process.Get());
+
+  // Expect there to be two inherited handles created by the generator.
+  std::vector<HANDLE> handles;
+  generator.GetInheritedHandles(&handles);
+  EXPECT_EQ(2u, handles.size());

[grt (UTC plus 2), 2015/12/22 16:31:32]

nit: 2U here and line 118

[chrisha, 2015/12/23 14:08:23]

Done.

+
+  base::CommandLine cmd_line = generator.GenerateCommandLine();
+
+  // Release the handles from the generator. Ownership will be picked up by the
+  // interpreter, and this prevents the handle tracking from complaining about
+  // there being two owners of the same handle.
+  generator.ReleaseHandlesWithoutClosing();
+
+  // Parse the command line. This creates scoped handles around the same handles
+  // that are currently owned by the generator.
+  auto interpreted = ChromeWatcherCommandLine::InterpretCommandLine(cmd_line);
+  EXPECT_TRUE(interpreted.get());
+
+  // Leave the handles in the interpreter and expect it to explode upon
+  // destruction.
+  EXPECT_DEATH(interpreted.reset(), "Handles left untaken.");
+
+  // The above call to the destructor only runs in the context of the death test
+  // child process. To prevent the parent process from exploding in a similar
+  // fashion, release the handles so the destructor is happy.
+  interpreted->on_initialized_event_handle().Close();
+  interpreted->parent_process_handle().Close();
+}
+
+TEST(ChromeWatcherCommandLineTest, SuccessfulParse) {
+  base::win::ScopedHandle process(::OpenProcess(
+      PROCESS_QUERY_INFORMATION | SYNCHRONIZE,
+      FALSE,  // Not inheritable.
+      ::GetCurrentProcessId()));
+  ASSERT_TRUE(process.Get());
+
+  base::win::ScopedHandle event(::CreateEvent(nullptr, FALSE, FALSE, nullptr));
+  ASSERT_TRUE(event.Get());
+
+  // The above handles are duplicated by the generator.
+  TestChromeWatcherCommandLineGenerator generator(
+      base::FilePath(L"example.exe"));
+  generator.SetOnInitializedEventHandle(event.Get());
+  generator.SetParentProcessHandle(process.Get());
+
+  // Expect there to be two inherited handles created by the generator.
+  std::vector<HANDLE> handles;
+  generator.GetInheritedHandles(&handles);
+  EXPECT_EQ(2u, handles.size());
+
+  base::CommandLine cmd_line = generator.GenerateCommandLine();
+
+  // Release the handles from the generator. Ownership will be picked up by the
+  // interpreter, and this prevents the handle tracking from complaining about
+  // there being two owners of the same handle.
+  generator.ReleaseHandlesWithoutClosing();
+
+  // Parse the command line. This creates scoped handles around the same handles
+  // that are currently owned by the generator.
+  auto interpreted = ChromeWatcherCommandLine::InterpretCommandLine(cmd_line);
+  EXPECT_TRUE(interpreted.get());
+
+  EXPECT_EQ(::GetCurrentThreadId(), interpreted->main_thread_id());
+
+  // Explicitly take the handles from the interpreter so it doesn't explode.
+  base::win::ScopedHandle on_init, proc;
+  on_init.Set(interpreted->on_initialized_event_handle().Take());

[grt (UTC plus 2), 2015/12/22 16:31:31]

with moving, this can be:
  on_init = interpreted->TakeOnInitializedEventHandle();

[chrisha, 2015/12/23 14:08:23]

Done.

+  proc.Set(interpreted->parent_process_handle().Take());
+}
+
+TEST(ChromeWatcherCommandLineTest, BasicTest) {
+  // Ownership of these handles is passed to the ScopedHandles below via
+  // InterpretChromeWatcherCommandLine().
+  base::ProcessHandle current =
+      ::OpenProcess(PROCESS_QUERY_INFORMATION | SYNCHRONIZE,
+                    TRUE,  // Inheritable
+                    ::GetCurrentProcessId());
+  ASSERT_NE(nullptr, current);
+
+  HANDLE event = ::CreateEvent(nullptr, FALSE, FALSE, nullptr);
+  ASSERT_NE(nullptr, event);
+  DWORD current_thread_id = ::GetCurrentThreadId();
+  base::CommandLine cmd_line = GenerateChromeWatcherCommandLine(
+      base::FilePath(L"example.exe"), current, current_thread_id, event);
+
+  base::win::ScopedHandle current_result;
+  DWORD current_thread_id_result = 0;
+  base::win::ScopedHandle event_result;
+  ASSERT_TRUE(InterpretChromeWatcherCommandLine(
+      cmd_line, &current_result, &current_thread_id_result, &event_result));
+  ASSERT_EQ(current, current_result.Get());
+  ASSERT_EQ(current_thread_id, current_thread_id_result);
+  ASSERT_EQ(event, event_result.Get());
+}