Fix Windows client ExceptionHandlerTest tests

ExceptionHandlerTest.InvalidParameterMiniDumpTest and
ExceptionHandlerTest.PureVirtualCallMiniDumpTest both also exercise a
feature that if the MiniDumpWithFullMemory MINIDUMP_TYPE is used, both
UUID.dmp and UUID-full.dmp files are written.

This is currently broken, and requesting a minidump with
MiniDumpWithFullMemory MINIDUMP_TYPE fails, as the file handle for the full
dump is not set.

Call GenerateFullDumpFile() if MiniDumpWithFullMemory is requested, to
generate a filename for the full dump file and set the file handle.

Currently GenerateFullDumpFile() also generates another UUID for the full
dump filename, so also make the private method
MinidumpGenerator::GenerateDumpFilePath() idempotent (so the same UUID is
reused)

(Note that calling Generate(|Full)DumpFile() more than once is not
permitted, so there's no behaviour where this changed the UUID to preserve)

BUG=

Change-Id: I74304f38b398f53da1c24f368dedfba8463da9e5
Reviewed-on: https://chromium-review.googlesource.com/452978
Reviewed-by: Mike Frysinger <vapier@chromium.org>

4 files changed, 28 insertions, 6 deletions

diff --git a/src/client/windows/crash_generation/crash_generation_server.cc b/src/client/windows/crash_generation/crash_generation_server.cc
index bb0968fe..0af213ba 100644
--- a/src/client/windows/crash_generation/crash_generation_server.cc
+++ b/src/client/windows/crash_generation/crash_generation_server.cc
@@ -922,9 +922,21 @@ bool CrashGenerationServer::GenerateDump(const ClientInfo& client,
                                    client.assert_info(),
                                    client.dump_type(),
                                    true);
+
   if (!dump_generator.GenerateDumpFile(dump_path)) {
     return false;
   }
+
+  // If the client requests a full memory dump, we will write a normal mini
+  // dump and a full memory dump. Both dump files use the same uuid as file
+  // name prefix.
+  if (client.dump_type() & MiniDumpWithFullMemory) {
+    std::wstring full_dump_path;
+    if (!dump_generator.GenerateFullDumpFile(&full_dump_path)) {
+      return false;
+    }
+  }
+
   return dump_generator.WriteMinidump();
 }
 
diff --git a/src/client/windows/crash_generation/minidump_generator.cc b/src/client/windows/crash_generation/minidump_generator.cc
index 786c9b93..100e365a 100644
--- a/src/client/windows/crash_generation/minidump_generator.cc
+++ b/src/client/windows/crash_generation/minidump_generator.cc
@@ -271,12 +271,14 @@ MinidumpGenerator::MinidumpGenerator(
       dump_type_(dump_type),
       is_client_pointers_(is_client_pointers),
       dump_path_(dump_path),
+      uuid_generated_(false),
       dump_file_(INVALID_HANDLE_VALUE),
       full_dump_file_(INVALID_HANDLE_VALUE),
       dump_file_is_internal_(false),
       full_dump_file_is_internal_(false),
       additional_streams_(NULL),
       callback_info_(NULL) {
+  uuid_ = {0};
   InitializeCriticalSection(&module_load_sync_);
   InitializeCriticalSection(&get_proc_address_sync_);
 }
@@ -562,15 +564,17 @@ MinidumpGenerator::UuidCreateType MinidumpGenerator::GetCreateUuid() {
 }
 
 bool MinidumpGenerator::GenerateDumpFilePath(wstring* file_path) {
-  UUID id = {0};
+  if (!uuid_generated_) {
+    UuidCreateType create_uuid = GetCreateUuid();
+    if (!create_uuid) {
+      return false;
+    }
 
-  UuidCreateType create_uuid = GetCreateUuid();
-  if (!create_uuid) {
-    return false;
+    create_uuid(&uuid_);
+    uuid_generated_ = true;
   }
 
-  create_uuid(&id);
-  wstring id_str = GUIDString::GUIDToWString(&id);
+  wstring id_str = GUIDString::GUIDToWString(&uuid_);
 
   *file_path = dump_path_ + TEXT("\\") + id_str + TEXT(".dmp");
   return true;
diff --git a/src/client/windows/crash_generation/minidump_generator.h b/src/client/windows/crash_generation/minidump_generator.h
index a3c12305..a707c0bb 100644
--- a/src/client/windows/crash_generation/minidump_generator.h
+++ b/src/client/windows/crash_generation/minidump_generator.h
@@ -168,6 +168,10 @@ class MinidumpGenerator {
   // Folder path to store dump files.
   std::wstring dump_path_;
 
+  // UUID used to make dump file names.
+  UUID uuid_;
+  bool uuid_generated_;
+
   // The file where the dump will be written.
   HANDLE dump_file_;
 
diff --git a/src/client/windows/unittests/exception_handler_test.cc b/src/client/windows/unittests/exception_handler_test.cc
index 55275323..0e8ee9ff 100644
--- a/src/client/windows/unittests/exception_handler_test.cc
+++ b/src/client/windows/unittests/exception_handler_test.cc
@@ -247,6 +247,7 @@ TEST_F(ExceptionHandlerTest, InvalidParameterMiniDumpTest) {
   EXPECT_EXIT(DoCrashInvalidParameter(), ::testing::ExitedWithCode(0), "");
   ASSERT_TRUE(!dump_file.empty() && !full_dump_file.empty());
   ASSERT_TRUE(DoesPathExist(dump_file.c_str()));
+  ASSERT_TRUE(DoesPathExist(full_dump_file.c_str()));
 
   // Verify the dump for infos.
   DumpAnalysis mini(dump_file);
@@ -318,6 +319,7 @@ TEST_F(ExceptionHandlerTest, PureVirtualCallMiniDumpTest) {
   EXPECT_EXIT(DoCrashPureVirtualCall(), ::testing::ExitedWithCode(0), "");
   ASSERT_TRUE(!dump_file.empty() && !full_dump_file.empty());
   ASSERT_TRUE(DoesPathExist(dump_file.c_str()));
+  ASSERT_TRUE(DoesPathExist(full_dump_file.c_str()));
 
   // Verify the dump for infos.
   DumpAnalysis mini(dump_file);