Refactor the Windows MinidumpGenerator interface to get rid of the overloads when generating dumps.

All required params are now passed to the constructor and the various options are set through new methods.

BUG=N/A
TEST=Existing minidump generation tests
R=mark@chromium.org

Review URL: https://breakpad.appspot.com/1074002

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1274 4c0a9323-5329-0410-9bdc-e9ce6186880e

5 files changed, 262 insertions, 260 deletions

diff --git a/src/client/windows/crash_generation/crash_generation_server.cc b/src/client/windows/crash_generation/crash_generation_server.cc
index b4bb5c95..ca71c44f 100644
--- a/src/client/windows/crash_generation/crash_generation_server.cc
+++ b/src/client/windows/crash_generation/crash_generation_server.cc
@@ -121,17 +121,13 @@ CrashGenerationServer::CrashGenerationServer(
       upload_request_callback_(upload_request_callback),
       upload_context_(upload_context),
       generate_dumps_(generate_dumps),
-      dump_generator_(NULL),
+      dump_path_(*dump_path),
       server_state_(IPC_SERVER_STATE_UNINITIALIZED),
       shutting_down_(false),
       overlapped_(),
       client_info_(NULL),
       pre_fetch_custom_info_(true) {
   InitializeCriticalSection(&sync_);
-
-  if (dump_path) {
-    dump_generator_.reset(new MinidumpGenerator(*dump_path));
-  }
 }
 
 // This should never be called from the OnPipeConnected callback.
@@ -917,15 +913,19 @@ bool CrashGenerationServer::GenerateDump(const ClientInfo& client,
     return false;
   }
 
-  return dump_generator_->WriteMinidump(client.process_handle(),
-                                        client.pid(),
-                                        client_thread_id,
-                                        GetCurrentThreadId(),
-                                        client_ex_info,
-                                        client.assert_info(),
-                                        client.dump_type(),
-                                        true,
-                                        dump_path);
+  MinidumpGenerator dump_generator(dump_path_,
+                                   client.process_handle(),
+                                   client.pid(),
+                                   client_thread_id,
+                                   GetCurrentThreadId(),
+                                   client_ex_info,
+                                   client.assert_info(),
+                                   client.dump_type(),
+                                   true);
+  if (!dump_generator.GenerateDumpFile(dump_path)) {
+    return false;
+  }
+  return dump_generator.WriteMinidump();
 }
 
 }  // namespace google_breakpad
diff --git a/src/client/windows/crash_generation/crash_generation_server.h b/src/client/windows/crash_generation/crash_generation_server.h
index 07019858..0ea90e51 100644
--- a/src/client/windows/crash_generation/crash_generation_server.h
+++ b/src/client/windows/crash_generation/crash_generation_server.h
@@ -268,8 +268,8 @@ class CrashGenerationServer {
   // Wether to populate custom information up-front.
   bool pre_fetch_custom_info_;
 
-  // Instance of a mini dump generator.
-  scoped_ptr<MinidumpGenerator> dump_generator_;
+  // The dump path for the server.
+  const std::wstring dump_path_;
 
   // State of the server in performing the IPC with the client.
   // Note that since we restrict the pipe to one instance, we
diff --git a/src/client/windows/crash_generation/minidump_generator.cc b/src/client/windows/crash_generation/minidump_generator.cc
index 652c1c9a..22e8d365 100644
--- a/src/client/windows/crash_generation/minidump_generator.cc
+++ b/src/client/windows/crash_generation/minidump_generator.cc
@@ -243,10 +243,33 @@ ULONG CALLBACK HandleTraceData::RecordHandleOperations(
 
 namespace google_breakpad {
 
-MinidumpGenerator::MinidumpGenerator(const wstring& dump_path)
+MinidumpGenerator::MinidumpGenerator(
+    const std::wstring& dump_path,
+    const HANDLE process_handle,
+    const DWORD process_id,
+    const DWORD thread_id,
+    const DWORD requesting_thread_id,
+    EXCEPTION_POINTERS* exception_pointers,
+    MDRawAssertionInfo* assert_info,
+    const MINIDUMP_TYPE dump_type,
+    const bool is_client_pointers)
     : dbghelp_module_(NULL),
       rpcrt4_module_(NULL),
       dump_path_(dump_path),
+      process_handle_(process_handle),
+      process_id_(process_id),
+      thread_id_(thread_id),
+      requesting_thread_id_(requesting_thread_id),
+      exception_pointers_(exception_pointers),
+      assert_info_(assert_info),
+      dump_type_(dump_type),
+      is_client_pointers_(is_client_pointers),
+      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),
       write_dump_(NULL),
       create_uuid_(NULL) {
   InitializeCriticalSection(&module_load_sync_);
@@ -254,6 +277,14 @@ MinidumpGenerator::MinidumpGenerator(const wstring& dump_path)
 }
 
 MinidumpGenerator::~MinidumpGenerator() {
+  if (dump_file_is_internal_ && dump_file_ != INVALID_HANDLE_VALUE) {
+    CloseHandle(dump_file_);
+  }
+
+  if (full_dump_file_is_internal_ && full_dump_file_ != INVALID_HANDLE_VALUE) {
+    CloseHandle(full_dump_file_);
+  }
+
   if (dbghelp_module_) {
     FreeLibrary(dbghelp_module_);
   }
@@ -266,141 +297,10 @@ MinidumpGenerator::~MinidumpGenerator() {
   DeleteCriticalSection(&module_load_sync_);
 }
 
-bool MinidumpGenerator::WriteMinidump(HANDLE process_handle,
-                                      DWORD process_id,
-                                      DWORD thread_id,
-                                      DWORD requesting_thread_id,
-                                      EXCEPTION_POINTERS* exception_pointers,
-                                      MDRawAssertionInfo* assert_info,
-                                      MINIDUMP_TYPE dump_type,
-                                      bool is_client_pointers,
-                                      wstring* dump_path) {
-  // Just call the full WriteMinidump with NULL as the full_dump_path.
-  return this->WriteMinidump(process_handle, process_id, thread_id,
-                             requesting_thread_id, exception_pointers,
-                             assert_info, dump_type, is_client_pointers,
-                             dump_path, NULL);
-}
-
-bool MinidumpGenerator::WriteMinidump(HANDLE process_handle,
-                                      DWORD process_id,
-                                      DWORD thread_id,
-                                      DWORD requesting_thread_id,
-                                      EXCEPTION_POINTERS* exception_pointers,
-                                      MDRawAssertionInfo* assert_info,
-                                      MINIDUMP_TYPE dump_type,
-                                      bool is_client_pointers,
-                                      wstring* dump_path,
-                                      wstring* full_dump_path) {
-  wstring dump_file_path;
-  if (!GenerateDumpFilePath(&dump_file_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.
-  bool full_memory_dump = (dump_type & MiniDumpWithFullMemory) != 0;
-  wstring full_dump_file_path;
-  if (full_memory_dump) {
-    full_dump_file_path.assign(dump_file_path);
-    full_dump_file_path.resize(full_dump_file_path.size() - 4);  // strip .dmp
-    full_dump_file_path.append(TEXT("-full.dmp"));
-  }
-
-  HANDLE dump_file = CreateFile(dump_file_path.c_str(),
-                                GENERIC_WRITE,
-                                0,
-                                NULL,
-                                CREATE_NEW,
-                                FILE_ATTRIBUTE_NORMAL,
-                                NULL);
-
-  if (dump_file == INVALID_HANDLE_VALUE) {
-    return false;
-  }
-
-  HANDLE full_dump_file = INVALID_HANDLE_VALUE;
-  if (full_memory_dump) {
-    full_dump_file = CreateFile(full_dump_file_path.c_str(),
-                                GENERIC_WRITE,
-                                0,
-                                NULL,
-                                CREATE_NEW,
-                                FILE_ATTRIBUTE_NORMAL,
-                                NULL);
-
-    if (full_dump_file == INVALID_HANDLE_VALUE) {
-      CloseHandle(dump_file);
-      return false;
-    }
-  }
-
-  bool result = WriteMinidump(process_handle,
-                              process_id,
-                              thread_id,
-                              requesting_thread_id,
-                              exception_pointers,
-                              assert_info,
-                              dump_type,
-                              is_client_pointers,
-                              dump_file,
-                              full_dump_file);
-
-  // Store the path of the dump file in the out parameter if dump generation
-  // succeeded.
-  if (result && dump_path) {
-    *dump_path = dump_file_path;
-  }
-  if (result && full_memory_dump && full_dump_path) {
-    *full_dump_path = full_dump_file_path;
-  }
-
-  CloseHandle(dump_file);
-  if (full_dump_file != INVALID_HANDLE_VALUE)
-    CloseHandle(full_dump_file);
-
-  return result;
-}
-
-bool MinidumpGenerator::WriteMinidump(HANDLE process_handle,
-                                      DWORD process_id,
-                                      DWORD thread_id,
-                                      DWORD requesting_thread_id,
-                                      EXCEPTION_POINTERS* exception_pointers,
-                                      MDRawAssertionInfo* assert_info,
-                                      MINIDUMP_TYPE dump_type,
-                                      bool is_client_pointers,
-                                      HANDLE dump_file,
-                                      HANDLE full_dump_file) {
-  return WriteMinidump(process_handle,
-                       process_id,
-                       thread_id,
-                       requesting_thread_id,
-                       exception_pointers,
-                       assert_info,
-                       dump_type,
-                       is_client_pointers,
-                       dump_file,
-                       full_dump_file,
-                       NULL);
-}
-
-bool MinidumpGenerator::WriteMinidump(
-    HANDLE process_handle,
-    DWORD process_id,
-    DWORD thread_id,
-    DWORD requesting_thread_id,
-    EXCEPTION_POINTERS* exception_pointers,
-    MDRawAssertionInfo* assert_info,
-    MINIDUMP_TYPE dump_type,
-    bool is_client_pointers,
-    HANDLE dump_file,
-    HANDLE full_dump_file,
-    MINIDUMP_USER_STREAM_INFORMATION* additional_streams) {
-  bool full_memory_dump = (dump_type & MiniDumpWithFullMemory) != 0;
-  if (dump_file == INVALID_HANDLE_VALUE ||
-      (full_memory_dump && full_dump_file == INVALID_HANDLE_VALUE)) {
+bool MinidumpGenerator::WriteMinidump() {
+  bool full_memory_dump = (dump_type_ & MiniDumpWithFullMemory) != 0;
+  if (dump_file_ == INVALID_HANDLE_VALUE ||
+      (full_memory_dump && full_dump_file_ == INVALID_HANDLE_VALUE)) {
     return false;
   }
 
@@ -414,11 +314,11 @@ bool MinidumpGenerator::WriteMinidump(
 
   // Setup the exception information object only if it's a dump
   // due to an exception.
-  if (exception_pointers) {
+  if (exception_pointers_) {
     dump_exception_pointers = &dump_exception_info;
-    dump_exception_info.ThreadId = thread_id;
-    dump_exception_info.ExceptionPointers = exception_pointers;
-    dump_exception_info.ClientPointers = is_client_pointers;
+    dump_exception_info.ThreadId = thread_id_;
+    dump_exception_info.ExceptionPointers = exception_pointers_;
+    dump_exception_info.ClientPointers = is_client_pointers_;
   }
 
   // Add an MDRawBreakpadInfo stream to the minidump, to provide additional
@@ -428,17 +328,17 @@ bool MinidumpGenerator::WriteMinidump(
   // can function better with Breakpad-generated dumps when it is present.
   // The native debugger is not harmed by the presence of this information.
   MDRawBreakpadInfo breakpad_info = {0};
-  if (!is_client_pointers) {
+  if (!is_client_pointers_) {
     // Set the dump thread id and requesting thread id only in case of
     // in-process dump generation.
     breakpad_info.validity = MD_BREAKPAD_INFO_VALID_DUMP_THREAD_ID |
                              MD_BREAKPAD_INFO_VALID_REQUESTING_THREAD_ID;
-    breakpad_info.dump_thread_id = thread_id;
-    breakpad_info.requesting_thread_id = requesting_thread_id;
+    breakpad_info.dump_thread_id = thread_id_;
+    breakpad_info.requesting_thread_id = requesting_thread_id_;
   }
 
-  int additional_streams_count = additional_streams ?
-      additional_streams->UserStreamCount : 0;
+  int additional_streams_count = additional_streams_ ?
+      additional_streams_->UserStreamCount : 0;
   scoped_array<MINIDUMP_USER_STREAM> user_stream_array(
       new MINIDUMP_USER_STREAM[3 + additional_streams_count]);
   user_stream_array[0].Type = MD_BREAKPAD_INFO_STREAM;
@@ -449,29 +349,33 @@ bool MinidumpGenerator::WriteMinidump(
   user_streams.UserStreamCount = 1;
   user_streams.UserStreamArray = user_stream_array.get();
 
-  MDRawAssertionInfo* actual_assert_info = assert_info;
+  MDRawAssertionInfo* actual_assert_info = assert_info_;
   MDRawAssertionInfo client_assert_info = {0};
 
-  if (assert_info) {
+  if (assert_info_) {
     // If the assertion info object lives in the client process,
     // read the memory of the client process.
-    if (is_client_pointers) {
+    if (is_client_pointers_) {
       SIZE_T bytes_read = 0;
-      if (!ReadProcessMemory(process_handle,
-                             assert_info,
+      if (!ReadProcessMemory(process_handle_,
+                             assert_info_,
                              &client_assert_info,
                              sizeof(client_assert_info),
                              &bytes_read)) {
-        CloseHandle(dump_file);
-        if (full_dump_file != INVALID_HANDLE_VALUE)
-          CloseHandle(full_dump_file);
+        if (dump_file_is_internal_)
+          CloseHandle(dump_file_);
+        if (full_dump_file_is_internal_ &&
+            full_dump_file_ != INVALID_HANDLE_VALUE)
+          CloseHandle(full_dump_file_);
         return false;
       }
 
       if (bytes_read != sizeof(client_assert_info)) {
-        CloseHandle(dump_file);
-        if (full_dump_file != INVALID_HANDLE_VALUE)
-          CloseHandle(full_dump_file);
+        if (dump_file_is_internal_)
+          CloseHandle(dump_file_);
+        if (full_dump_file_is_internal_ &&
+            full_dump_file_ != INVALID_HANDLE_VALUE)
+          CloseHandle(full_dump_file_);
         return false;
       }
 
@@ -484,16 +388,16 @@ bool MinidumpGenerator::WriteMinidump(
     ++user_streams.UserStreamCount;
   }
 
-  if (additional_streams) {
+  if (additional_streams_) {
     for (size_t i = 0;
-         i < additional_streams->UserStreamCount;
+         i < additional_streams_->UserStreamCount;
          i++, user_streams.UserStreamCount++) {
       user_stream_array[user_streams.UserStreamCount].Type =
-          additional_streams->UserStreamArray[i].Type;
+          additional_streams_->UserStreamArray[i].Type;
       user_stream_array[user_streams.UserStreamCount].BufferSize =
-          additional_streams->UserStreamArray[i].BufferSize;
+          additional_streams_->UserStreamArray[i].BufferSize;
       user_stream_array[user_streams.UserStreamCount].Buffer =
-          additional_streams->UserStreamArray[i].Buffer;
+          additional_streams_->UserStreamArray[i].Buffer;
     }
   }
 
@@ -501,12 +405,14 @@ bool MinidumpGenerator::WriteMinidump(
   // the trace of operations for the offending handle value. Do nothing special
   // if the client already requested the handle trace to be stored in the dump.
   HandleTraceData handle_trace_data;
-  if (exception_pointers && (dump_type & MiniDumpWithHandleData) == 0) {
-    if (!handle_trace_data.CollectHandleData(process_handle,
-                                             exception_pointers)) {
-      CloseHandle(dump_file);
-      if (full_dump_file != INVALID_HANDLE_VALUE)
-        CloseHandle(full_dump_file);
+  if (exception_pointers_ && (dump_type_ & MiniDumpWithHandleData) == 0) {
+    if (!handle_trace_data.CollectHandleData(process_handle_,
+                                             exception_pointers_)) {
+      if (dump_file_is_internal_)
+        CloseHandle(dump_file_);
+      if (full_dump_file_is_internal_ &&
+          full_dump_file_ != INVALID_HANDLE_VALUE)
+        CloseHandle(full_dump_file_);
       return false;
     }
   }
@@ -514,12 +420,12 @@ bool MinidumpGenerator::WriteMinidump(
   bool result_full_memory = true;
   if (full_memory_dump) {
     result_full_memory = write_dump(
-        process_handle,
-        process_id,
-        full_dump_file,
-        static_cast<MINIDUMP_TYPE>((dump_type & (~MiniDumpNormal))
+        process_handle_,
+        process_id_,
+        full_dump_file_,
+        static_cast<MINIDUMP_TYPE>((dump_type_ & (~MiniDumpNormal))
                                     | MiniDumpWithHandleData),
-        exception_pointers ? &dump_exception_info : NULL,
+        exception_pointers_ ? &dump_exception_info : NULL,
         &user_streams,
         NULL) != FALSE;
   }
@@ -531,18 +437,81 @@ bool MinidumpGenerator::WriteMinidump(
   }
 
   bool result_minidump = write_dump(
-      process_handle,
-      process_id,
-      dump_file,
-      static_cast<MINIDUMP_TYPE>((dump_type & (~MiniDumpWithFullMemory))
+      process_handle_,
+      process_id_,
+      dump_file_,
+      static_cast<MINIDUMP_TYPE>((dump_type_ & (~MiniDumpWithFullMemory))
                                   | MiniDumpNormal),
-      exception_pointers ? &dump_exception_info : NULL,
+      exception_pointers_ ? &dump_exception_info : NULL,
       &user_streams,
-      NULL) != FALSE;
+      callback_info_) != FALSE;
 
   return result_minidump && result_full_memory;
 }
 
+bool MinidumpGenerator::GenerateDumpFile(wstring* dump_path) {
+  // The dump file was already set by handle or this function was previously
+  // called.
+  if (dump_file_ != INVALID_HANDLE_VALUE) {
+    return false;
+  }
+
+  wstring dump_file_path;
+  if (!GenerateDumpFilePath(&dump_file_path)) {
+    return false;
+  }
+
+  dump_file_ = CreateFile(dump_file_path.c_str(),
+                          GENERIC_WRITE,
+                          0,
+                          NULL,
+                          CREATE_NEW,
+                          FILE_ATTRIBUTE_NORMAL,
+                          NULL);
+  if (dump_file_ == INVALID_HANDLE_VALUE) {
+    return false;
+  }
+
+  dump_file_is_internal_ = true;
+  *dump_path = dump_file_path;
+  return true;
+}
+
+bool MinidumpGenerator::GenerateFullDumpFile(wstring* full_dump_path) {
+  // A full minidump was not requested.
+  if ((dump_type_ & MiniDumpWithFullMemory) == 0) {
+    return false;
+  }
+
+  // The dump file was already set by handle or this function was previously
+  // called.
+  if (full_dump_file_ != INVALID_HANDLE_VALUE) {
+    return false;
+  }
+
+  wstring full_dump_file_path;
+  if (!GenerateDumpFilePath(&full_dump_file_path)) {
+    return false;
+  }
+  full_dump_file_path.resize(full_dump_file_path.size() - 4);  // strip .dmp
+  full_dump_file_path.append(TEXT("-full.dmp"));
+
+  full_dump_file_ = CreateFile(full_dump_file_path.c_str(),
+                               GENERIC_WRITE,
+                               0,
+                               NULL,
+                               CREATE_NEW,
+                               FILE_ATTRIBUTE_NORMAL,
+                               NULL);
+  if (full_dump_file_ == INVALID_HANDLE_VALUE) {
+    return false;
+  }
+
+  full_dump_file_is_internal_ = true;
+  *full_dump_path = full_dump_file_path;
+  return true;
+}
+
 HMODULE MinidumpGenerator::GetDbghelpModule() {
   AutoCriticalSection lock(&module_load_sync_);
   if (!dbghelp_module_) {
diff --git a/src/client/windows/crash_generation/minidump_generator.h b/src/client/windows/crash_generation/minidump_generator.h
index c4c8fc3e..b95e04eb 100644
--- a/src/client/windows/crash_generation/minidump_generator.h
+++ b/src/client/windows/crash_generation/minidump_generator.h
@@ -44,66 +44,55 @@ namespace google_breakpad {
 // the clients to generate minidumps.
 class MinidumpGenerator {
  public:
-  // Creates an instance with the given dump path.
-  explicit MinidumpGenerator(const std::wstring& dump_path);
+  // Creates an instance with the given parameters.
+  // is_client_pointers specifies whether the exception_pointers and
+  // assert_info point into the process that is being dumped.
+  // Before calling WriteMinidump on the returned instance a dump file muct be
+  // specified by a call to either SetDumpFile() or GenerateDumpFile().
+  // If a full dump file will be requested via a subsequent call to either
+  // SetFullDumpFile or GenerateFullDumpFile() dump_type must include
+  // MiniDumpWithFullMemory.
+  MinidumpGenerator(const std::wstring& dump_path,
+                    const HANDLE process_handle,
+                    const DWORD process_id,
+                    const DWORD thread_id,
+                    const DWORD requesting_thread_id,
+                    EXCEPTION_POINTERS* exception_pointers,
+                    MDRawAssertionInfo* assert_info,
+                    const MINIDUMP_TYPE dump_type,
+                    const bool is_client_pointers);
 
   ~MinidumpGenerator();
 
+  void SetDumpFile(const HANDLE dump_file) { dump_file_ = dump_file; }
+  void SetFullDumpFile(const HANDLE full_dump_file) {
+    full_dump_file_ = full_dump_file;
+  }
+
+  // Generate the name for the dump file that will be written to once
+  // WriteMinidump() is called. Can only be called once and cannot be called
+  // if the dump file is set via SetDumpFile().
+  bool GenerateDumpFile(std::wstring* dump_path);
+
+  // Generate the name for the full dump file that will be written to once
+  // WriteMinidump() is called. Cannot be called unless the minidump type
+  // includes MiniDumpWithFullMemory. Can only be called once and cannot be
+  // called if the dump file is set via SetFullDumpFile().
+  bool GenerateFullDumpFile(std::wstring* full_dump_path);
+
+  void SetAdditionalStreams(
+      MINIDUMP_USER_STREAM_INFORMATION* additional_streams) {
+    additional_streams_ = additional_streams;
+  }
+
+  void SetCallback(MINIDUMP_CALLBACK_INFORMATION* callback_info) {
+    callback_info_ = callback_info;
+  }
+
   // Writes the minidump with the given parameters. Stores the
   // dump file path in the dump_path parameter if dump generation
   // succeeds.
-  bool WriteMinidump(HANDLE process_handle,
-                     DWORD process_id,
-                     DWORD thread_id,
-                     DWORD requesting_thread_id,
-                     EXCEPTION_POINTERS* exception_pointers,
-                     MDRawAssertionInfo* assert_info,
-                     MINIDUMP_TYPE dump_type,
-                     bool is_client_pointers,
-                     std::wstring* dump_path);
-
-  // Writes the minidump with the given parameters. Stores the dump file
-  // path in the dump_path (and full_dump_path) parameter if dump
-  // generation succeeds. full_dump_path and dump_path can be NULL.
-  bool WriteMinidump(HANDLE process_handle,
-                     DWORD process_id,
-                     DWORD thread_id,
-                     DWORD requesting_thread_id,
-                     EXCEPTION_POINTERS* exception_pointers,
-                     MDRawAssertionInfo* assert_info,
-                     MINIDUMP_TYPE dump_type,
-                     bool is_client_pointers,
-                     std::wstring* dump_path,
-                     std::wstring* full_dump_path);
-
-  // Writes the minidump with the given parameters. Writes the minidump and
-  // full dump to the file handles supplied. This allows the caller to handle
-  // the creation of the files for the dump. The file handles are not closed
-  // by this function.
-  bool WriteMinidump(HANDLE process_handle,
-                     DWORD process_id,
-                     DWORD thread_id,
-                     DWORD requesting_thread_id,
-                     EXCEPTION_POINTERS* exception_pointers,
-                     MDRawAssertionInfo* assert_info,
-                     MINIDUMP_TYPE dump_type,
-                     bool is_client_pointers,
-                     HANDLE dump_file,
-                     HANDLE full_dump_file);
-
-  // Writes the minidump with the given parameters. Allows the user to include
-  // additional streams in the dump that would not otherwise be included.
-  bool WriteMinidump(HANDLE process_handle,
-                     DWORD process_id,
-                     DWORD thread_id,
-                     DWORD requesting_thread_id,
-                     EXCEPTION_POINTERS* exception_pointers,
-                     MDRawAssertionInfo* assert_info,
-                     MINIDUMP_TYPE dump_type,
-                     bool is_client_pointers,
-                     HANDLE dump_file,
-                     HANDLE full_dump_file,
-                     MINIDUMP_USER_STREAM_INFORMATION* additional_streams);
+  bool WriteMinidump();
 
  private:
   // Function pointer type for MiniDumpWriteDump, which is looked up
@@ -149,9 +138,53 @@ class MinidumpGenerator {
   // Pointer to the UuidCreate function.
   UuidCreateType create_uuid_;
 
+  // Handle for the process to dump.
+  HANDLE process_handle_;
+
+  // Process ID for the process to dump.
+  DWORD process_id_;
+
+  // The crashing thread ID.
+  DWORD thread_id_;
+
+  // The thread ID which is requesting the dump.
+  DWORD requesting_thread_id_;
+
+  // Pointer to the exception information for the crash. This may point to an
+  // address in the crashing process so it should not be dereferenced.
+  EXCEPTION_POINTERS* exception_pointers_;
+
+  // Assertion info for the report.
+  MDRawAssertionInfo* assert_info_;
+
+  // Type of minidump to generate.
+  MINIDUMP_TYPE dump_type_;
+
+  // Specifies whether the exception_pointers_ reference memory in the crashing
+  // process.
+  bool is_client_pointers_;
+
   // Folder path to store dump files.
   std::wstring dump_path_;
 
+  // The file where the dump will be written.
+  HANDLE dump_file_;
+
+  // The file where the full dump will be written.
+  HANDLE full_dump_file_;
+
+  // Tracks whether the dump file handle is managed externally.
+  bool dump_file_is_internal_;
+
+  // Tracks whether the full dump file handle is managed externally.
+  bool full_dump_file_is_internal_;
+
+  // Additional streams to be written to the dump.
+  MINIDUMP_USER_STREAM_INFORMATION* additional_streams_;
+
+  // The user defined callback for the various stages of the dump process.
+  MINIDUMP_CALLBACK_INFORMATION* callback_info_;
+
   // Critical section to sychronize action of loading modules dynamically.
   CRITICAL_SECTION module_load_sync_;
 
diff --git a/src/client/windows/unittests/minidump_test.cc b/src/client/windows/unittests/minidump_test.cc
index e3097f28..8d2d726c 100644
--- a/src/client/windows/unittests/minidump_test.cc
+++ b/src/client/windows/unittests/minidump_test.cc
@@ -104,19 +104,19 @@ class MinidumpTest: public testing::Test {
       &ctx_record,
     };
 
-    MinidumpGenerator generator(dump_path_);
-
+    MinidumpGenerator generator(dump_path_,
+                                ::GetCurrentProcess(),
+                                ::GetCurrentProcessId(),
+                                ::GetCurrentThreadId(),
+                                ::GetCurrentThreadId(),
+                                &ex_ptrs,
+                                NULL,
+                                static_cast<MINIDUMP_TYPE>(flags),
+                                TRUE);
+    generator.GenerateDumpFile(&dump_file_);
+    generator.GenerateFullDumpFile(&full_dump_file_);
     // And write a dump
-    bool result = generator.WriteMinidump(::GetCurrentProcess(),
-                                          ::GetCurrentProcessId(),
-                                          ::GetCurrentThreadId(),
-                                          ::GetCurrentThreadId(),
-                                          &ex_ptrs,
-                                          NULL,
-                                          static_cast<MINIDUMP_TYPE>(flags),
-                                          TRUE,
-                                          &dump_file_,
-                                          &full_dump_file_);
+    bool result = generator.WriteMinidump();
     return result == TRUE;
   }