[win] Change Chrome Watcher configuration API.

chrome/app/chrome_watcher_command_line_win_unittest.cc

Index: chrome/app/chrome_watcher_command_line_win_unittest.cc
diff --git a/chrome/app/chrome_watcher_command_line_win_unittest.cc b/chrome/app/chrome_watcher_command_line_win_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9e4e6a0dd62a11d62b513f836e8b852df1dd5bd4
--- /dev/null
+++ b/chrome/app/chrome_watcher_command_line_win_unittest.cc
@@ -0,0 +1,168 @@
+// 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.h"
+#include "base/process/process_handle.h"
+#include "base/win/scoped_handle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+const wchar_t kExampleExe[] = FILE_PATH_LITERAL("example.exe");
+
+base::FilePath ExampleExe() {
+  base::FilePath example_exe(kExampleExe);
+  return example_exe;
+}
+
+}  // namespace
+
+class TestChromeWatcherCommandLineGenerator
+    : public ChromeWatcherCommandLineGenerator {
+ public:
+  TestChromeWatcherCommandLineGenerator()
+      : ChromeWatcherCommandLineGenerator(base::FilePath(kExampleExe)) {
+  }
+
+  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;

[Sigurður Ásgeirsson, 2016/01/08 16:55:19]

do you need to use the TEst class here?

[chrisha, 2016/01/08 22:29:00]

Not specifically in this test, but it just makes the code more consistent.

+  EXPECT_FALSE(generator.SetOnInitializedEventHandle(bad_handle_1));
+  EXPECT_FALSE(generator.SetParentProcessHandle(bad_handle_2));
+
+  // Expect there to be no inherited handles created by the generator.
+  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(ExampleExe());
+
+  // Parse the command line.
+  auto interpreted = ChromeWatcherCommandLine::InterpretCommandLine(cmd_line);
+  EXPECT_FALSE(interpreted);
+}
+
+TEST(ChromeWatcherCommandLineDeathTest, HandlesLeftUntakenCausesDeath) {
+  base::Process process(base::Process::OpenWithAccess(
+      base::GetCurrentProcId(), PROCESS_QUERY_INFORMATION | SYNCHRONIZE));
+  ASSERT_TRUE(process.Handle());
+
+  base::win::ScopedHandle event(::CreateEvent(nullptr, FALSE, FALSE, nullptr));
+  ASSERT_TRUE(event.IsValid());
+
+  // The above handles are duplicated by the generator.
+  TestChromeWatcherCommandLineGenerator generator;
+  generator.SetOnInitializedEventHandle(event.Get());
+  generator.SetParentProcessHandle(process.Handle());
+
+  // 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

[Sigurður Ásgeirsson, 2016/01/08 16:55:19]

Nit: explain why this is - e.g. in the normal case these should run in different
process instances. Maybe best to do this by commenting on teh Release* method.

[chrisha, 2016/01/08 22:29:00]

Done.

+  // 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);
+
+  // 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->TakeOnInitializedEventHandle().Close();
+  interpreted->TakeParentProcessHandle().Close();
+}
+
+TEST(ChromeWatcherCommandLineTest, SuccessfulParse) {
+  base::Process process(base::Process::OpenWithAccess(

[Sigurður Ásgeirsson, 2016/01/08 16:55:19]

This code repeats a fair bit - move to a fixture?

[chrisha, 2016/01/08 22:29:00]

Done.

+      base::GetCurrentProcId(), PROCESS_QUERY_INFORMATION | SYNCHRONIZE));
+  ASSERT_TRUE(process.Handle());
+
+  base::win::ScopedHandle event(::CreateEvent(nullptr, FALSE, FALSE, nullptr));
+  ASSERT_TRUE(event.IsValid());
+
+  // The above handles are duplicated by the generator.
+  TestChromeWatcherCommandLineGenerator generator;
+  generator.SetOnInitializedEventHandle(event.Get());
+  generator.SetParentProcessHandle(process.Handle());
+
+  // 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);
+
+  EXPECT_EQ(::GetCurrentThreadId(), interpreted->main_thread_id());
+
+  // Explicitly take the handles from the interpreter so it doesn't explode.
+  base::win::ScopedHandle on_init = interpreted->TakeOnInitializedEventHandle();
+  base::win::ScopedHandle proc = interpreted->TakeParentProcessHandle();
+  EXPECT_TRUE(on_init.IsValid());
+  EXPECT_TRUE(proc.IsValid());
+}
+
+TEST(ChromeWatcherCommandLineTest, BasicTest) {
+  // Ownership of these handles is passed to the ScopedHandles below via

[Sigurður Ásgeirsson, 2016/01/08 16:55:19]

This is a legacy test?

[chrisha, 2016/01/08 22:29:00]

Yup... commented that fact. It will be removed once we *switch* to using the new
classes. In the meantime, the code still uses the old mechanism.

+  // 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(
+      ExampleExe(), 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());
+}