win: Make reading CrashpadInfo work across bitness

snapshot/win/pe_image_annotations_reader_test.cc

Index: snapshot/win/pe_image_annotations_reader_test.cc
diff --git a/snapshot/win/pe_image_annotations_reader_test.cc b/snapshot/win/pe_image_annotations_reader_test.cc
index 2c88417d0a49dd8e53c36f0fff3451aa0569748e..5d5d34dc991898d23486223b348015a6517df17b 100644
--- a/snapshot/win/pe_image_annotations_reader_test.cc
+++ b/snapshot/win/pe_image_annotations_reader_test.cc
@@ -22,12 +22,15 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
 #include "client/crashpad_info.h"
 #include "client/simple_string_dictionary.h"
 #include "gtest/gtest.h"
 #include "snapshot/win/pe_image_reader.h"
 #include "snapshot/win/process_reader_win.h"
+#include "test/paths.h"
+#include "test/win/child_launcher.h"
 #include "test/win/win_multiprocess.h"
 #include "util/file/file_io.h"
 #include "util/win/process_info.h"
@@ -44,97 +47,105 @@ enum TestType {
   kCrashDebugBreak,
 };
 
-template <TestType Type>
-class TestPEImageAnnotationsReader final : public WinMultiprocess {
- public:
-  TestPEImageAnnotationsReader() {}
-  ~TestPEImageAnnotationsReader() {}
-
- private:
-  // WinMultiprocess:
-
-  void WinMultiprocessParent() override {
-    ProcessReaderWin process_reader;
-    ASSERT_TRUE(process_reader.Initialize(ChildProcess(),
-                                          ProcessSuspensionState::kRunning));
-
-    // Wait for the child process to indicate that it's done setting up its
-    // annotations via the CrashpadInfo interface.
-    char c;
-    CheckedReadFile(ReadPipeHandle(), &c, sizeof(c));
-
-    // Verify the "simple map" annotations set via the CrashpadInfo interface.
-    const std::vector<ProcessInfo::Module>& modules = process_reader.Modules();
-    std::map<std::string, std::string> all_annotations_simple_map;
-    for (const ProcessInfo::Module& module : modules) {
-      PEImageReader pe_image_reader;
-      pe_image_reader.Initialize(&process_reader,
-                                 module.dll_base,
-                                 module.size,
-                                 base::UTF16ToUTF8(module.name));
-      PEImageAnnotationsReader module_annotations_reader(
-          &process_reader, &pe_image_reader, module.name);
-      std::map<std::string, std::string> module_annotations_simple_map =
-          module_annotations_reader.SimpleMap();
-      all_annotations_simple_map.insert(module_annotations_simple_map.begin(),
-                                        module_annotations_simple_map.end());
-    }
-
-    EXPECT_GE(all_annotations_simple_map.size(), 5u);
-    EXPECT_EQ("crash", all_annotations_simple_map["#TEST# pad"]);
-    EXPECT_EQ("value", all_annotations_simple_map["#TEST# key"]);
-    EXPECT_EQ("y", all_annotations_simple_map["#TEST# x"]);
-    EXPECT_EQ("shorter", all_annotations_simple_map["#TEST# longer"]);
-    EXPECT_EQ("", all_annotations_simple_map["#TEST# empty_value"]);
-
-    if (Type == kCrashDebugBreak)
-      SetExpectedChildExitCode(STATUS_BREAKPOINT);
-
-    // Tell the child process to continue.
-    CheckedWriteFile(WritePipeHandle(), &c, sizeof(c));
+void TestAnnotationsOnCrash(TestType type,
+                            const base::string16& directory_modification) {
+  // Spawn a child process, passing it the pipe name to connect to.
+  base::FilePath test_executable = Paths::Executable();
+  std::wstring child_test_executable =
+      test_executable.DirName()
+          .Append(directory_modification)
+          .Append(test_executable.BaseName().RemoveFinalExtension().value() +
+                  L"_simple_annotations.exe")
+          .value();
+  ChildLauncher child(child_test_executable, L"");
+  child.Start();
+
+  // Wait for the child process to indicate that it's done setting up its
+  // annotations via the CrashpadInfo interface.
+  char c;
+  CheckedReadFile(child.stdout_read_handle(), &c, sizeof(c));
+
+  ProcessReaderWin process_reader;
+  ASSERT_TRUE(process_reader.Initialize(child.process_handle(),
+	  ProcessSuspensionState::kRunning));

[Mark Mentovai, 2015/09/22 13:17:29]

Spotted a tab here.

[scottmg, 2015/09/22 17:25:59]

Gah, new OS install and haven't reconfigured everything yet (apparently). Done.

+
+  // Verify the "simple map" annotations set via the CrashpadInfo interface.
+  const std::vector<ProcessInfo::Module>& modules = process_reader.Modules();
+  std::map<std::string, std::string> all_annotations_simple_map;
+  for (const ProcessInfo::Module& module : modules) {
+    PEImageReader pe_image_reader;
+    pe_image_reader.Initialize(&process_reader,
+                               module.dll_base,
+                               module.size,
+                               base::UTF16ToUTF8(module.name));
+    PEImageAnnotationsReader module_annotations_reader(
+        &process_reader, &pe_image_reader, module.name);
+    std::map<std::string, std::string> module_annotations_simple_map =
+        module_annotations_reader.SimpleMap();
+    all_annotations_simple_map.insert(module_annotations_simple_map.begin(),
+                                      module_annotations_simple_map.end());
   }
 
-  void WinMultiprocessChild() override {
-    CrashpadInfo* crashpad_info = CrashpadInfo::GetCrashpadInfo();
-
-    // This is "leaked" to crashpad_info.
-    SimpleStringDictionary* simple_annotations = new SimpleStringDictionary();
-    simple_annotations->SetKeyValue("#TEST# pad", "break");
-    simple_annotations->SetKeyValue("#TEST# key", "value");
-    simple_annotations->SetKeyValue("#TEST# pad", "crash");
-    simple_annotations->SetKeyValue("#TEST# x", "y");
-    simple_annotations->SetKeyValue("#TEST# longer", "shorter");
-    simple_annotations->SetKeyValue("#TEST# empty_value", "");
-
-    crashpad_info->set_simple_annotations(simple_annotations);
-
-    // Tell the parent that the environment has been set up.
-    char c = '\0';
-    CheckedWriteFile(WritePipeHandle(), &c, sizeof(c));
-
-    // Wait for the parent to indicate that it's safe to continue/crash.
-    CheckedReadFile(ReadPipeHandle(), &c, sizeof(c));
-
-    switch (Type) {
-      case kDontCrash:
-        break;
-
-      case kCrashDebugBreak:
-        __debugbreak();
-        break;
-    }
+  EXPECT_GE(all_annotations_simple_map.size(), 5u);
+  EXPECT_EQ("crash", all_annotations_simple_map["#TEST# pad"]);
+  EXPECT_EQ("value", all_annotations_simple_map["#TEST# key"]);
+  EXPECT_EQ("y", all_annotations_simple_map["#TEST# x"]);
+  EXPECT_EQ("shorter", all_annotations_simple_map["#TEST# longer"]);
+  EXPECT_EQ("", all_annotations_simple_map["#TEST# empty_value"]);
+
+  // Tell the child process to continue.
+  switch (type) {
+    case kDontCrash:
+      c = ' ';
+      break;
+    case kCrashDebugBreak:
+      c = 'd';
+      break;
+    default:
+      ADD_FAILURE();

[Mark Mentovai, 2015/09/22 13:17:29]

Should this be FAIL() instead to cause an immediate return (like ASSERT_*()?)

[scottmg, 2015/09/22 17:25:59]

Done.

   }
-
-  DISALLOW_COPY_AND_ASSIGN(TestPEImageAnnotationsReader);
-};
+  CheckedWriteFile(child.stdin_write_handle(), &c, sizeof(c));
+
+  DWORD exit_code = child.WaitForExit();
+  switch (type) {

[Mark Mentovai, 2015/09/22 13:17:29]

Optional: instead of having two switches on the same thing, you could have just
the one above, also responsible for setting DWORD expected_exit_code.

[scottmg, 2015/09/22 17:25:59]

Done.

+    case kDontCrash:
+      EXPECT_EQ(0, exit_code);
+      break;
+    case kCrashDebugBreak:
+      EXPECT_EQ(STATUS_BREAKPOINT, exit_code);
+      break;
+    default:
+      ADD_FAILURE();
+  }
+}
 
 TEST(PEImageAnnotationsReader, DontCrash) {
-  WinMultiprocess::Run<TestPEImageAnnotationsReader<kDontCrash>>();
+  TestAnnotationsOnCrash(kDontCrash, FILE_PATH_LITERAL("."));
 }
 
 TEST(PEImageAnnotationsReader, CrashDebugBreak) {
-  WinMultiprocess::Run<TestPEImageAnnotationsReader<kCrashDebugBreak>>();
+  TestAnnotationsOnCrash(kCrashDebugBreak, FILE_PATH_LITERAL("."));
+}
+
+#if defined(ARCH_CPU_64_BITS)
+TEST(PEImageAnnotationsReader, DontCrashWOW64) {
+#ifndef NDEBUG
+  TestAnnotationsOnCrash(kDontCrash, FILE_PATH_LITERAL("..\\..\\out\\Debug"));
+#else
+  TestAnnotationsOnCrash(kDontCrash, FILE_PATH_LITERAL("..\\..\\out\\Release"));
+#endif
+}
+
+TEST(PEImageAnnotationsReader, CrashDebugBreakWOW64) {
+#ifndef NDEBUG
+  TestAnnotationsOnCrash(kCrashDebugBreak,
+                         FILE_PATH_LITERAL("..\\..\\out\\Debug"));
+#else
+  TestAnnotationsOnCrash(kCrashDebugBreak,
+                         FILE_PATH_LITERAL("..\\..\\out\\Release"));
+#endif
 }
+#endif  // ARCH_CPU_64_BITS
 
 }  // namespace
 }  // namespace test