handler: Add user extensibility stream call-out.

Bug: crashpad:167
Test: Add crashpad_handler_test.
Change-Id: I79b0b71dc4f61e6dce6bc10083e2f924dc83c940
Reviewed-on: https://chromium-review.googlesource.com/463746
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

build/run_tests.py

@@ -38,6 +38,11 @@ def main(args):
       'crashpad_test_test',
       'crashpad_util_test',
   ]
+
+  if sys.platform == 'win32':
+    tests.append('crashpad_handler_test')
+    tests = sorted(tests)
+
   for test in tests:
     print '-' * 80
     print test

crashpad.gyp

@@ -22,6 +22,7 @@
         'client/client_test.gyp:*',
         'compat/compat.gyp:*',
         'handler/handler.gyp:*',
+        'handler/handler_test.gyp:*',
         'minidump/minidump.gyp:*',
         'minidump/minidump_test.gyp:*',
         'snapshot/snapshot.gyp:*',

handler/crashpad_handler_test.cc

@@ -0,0 +1,132 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <windows.h>
+#include <dbghelp.h>
+#include <string.h>
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "base/files/file_path.h"
+#include "client/crash_report_database.h"
+#include "client/crashpad_client.h"
+#include "gtest/gtest.h"
+#include "minidump/test/minidump_file_writer_test_util.h"
+#include "test/test_paths.h"
+#include "test/win/win_multiprocess_with_temp_dir.h"
+#include "util/file/file_reader.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+void StartAndCrashWithExtendedHandler(const base::FilePath& temp_dir) {
+  base::FilePath handler_path = TestPaths::Executable().DirName().Append(
+      FILE_PATH_LITERAL("crashpad_handler_test_extended_handler.exe"));
+
+  CrashpadClient client;
+  ASSERT_TRUE(client.StartHandler(handler_path,
+                                  temp_dir,
+                                  base::FilePath(),
+                                  "",
+                                  std::map<std::string, std::string>(),
+                                  std::vector<std::string>(),
+                                  false,
+                                  false));
+
+  __debugbreak();
+}
+
+class CrashWithExtendedHandler final : public WinMultiprocessWithTempDir {
+ public:
+  CrashWithExtendedHandler() : WinMultiprocessWithTempDir() {}
+  ~CrashWithExtendedHandler() {}
+
+ private:
+  void ValidateGeneratedDump();
+
+  void WinMultiprocessParent() override {
+    SetExpectedChildExitCode(EXCEPTION_BREAKPOINT);
+  }
+
+  void WinMultiprocessChild() override {
+    StartAndCrashWithExtendedHandler(GetTempDirPath());
+  }
+
+  void WinMultiprocessParentAfterChild(HANDLE child) override {
+    // At this point the child has exited, which means the crash report should
+    // have been written.
+    ValidateGeneratedDump();
+
+    // Delegate the cleanup to the superclass.
+    WinMultiprocessWithTempDir::WinMultiprocessParentAfterChild(child);
+  }
+};
+
+void CrashWithExtendedHandler::ValidateGeneratedDump() {
+  // Open the database and find the sole dump that should have been created.
+  std::unique_ptr<CrashReportDatabase> database(
+      CrashReportDatabase::Initialize(GetTempDirPath()));
+  ASSERT_TRUE(database);
+
+  std::vector<CrashReportDatabase::Report> reports;
+  ASSERT_EQ(database->GetCompletedReports(&reports),
+            CrashReportDatabase::kNoError);
+  ASSERT_EQ(reports.size(), 1u);
+
+  // Open the dump and validate that it has the extension stream with the
+  // expected contents.
+  FileReader reader;
+  ASSERT_TRUE(reader.Open(reports[0].file_path));
+
+  // Read the header.
+  MINIDUMP_HEADER header = {};
+  ASSERT_TRUE(reader.ReadExactly(&header, sizeof(header)));
+
+  // Read the directory.
+  std::vector<MINIDUMP_DIRECTORY> directory(header.NumberOfStreams);
+  ASSERT_TRUE(reader.SeekSet(header.StreamDirectoryRva));
+  ASSERT_TRUE(reader.ReadExactly(directory.data(),
+                                 directory.size() * sizeof(directory[0])));
+
+  // Search for the extension stream.
+  size_t found_extension_streams = 0;
+  for (const auto& entry : directory) {
+    if (entry.StreamType == 0xCAFEBABE) {
+      ++found_extension_streams;
+
+      ASSERT_TRUE(reader.SeekSet(entry.Location.Rva));
+
+      std::vector<char> data;
+      data.resize(entry.Location.DataSize);
+
+      ASSERT_TRUE(reader.ReadExactly(data.data(), data.size()));
+
+      static const char kExpectedData[] = "Injected extension stream!";
+      EXPECT_EQ(memcmp(kExpectedData, data.data(), sizeof(kExpectedData)), 0);
+    }
+  }
+
+  EXPECT_EQ(found_extension_streams, 1u);
+}
+
+TEST(CrashpadHandler, ExtensibilityCalloutsWork) {
+  WinMultiprocessWithTempDir::Run<CrashWithExtendedHandler>();
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad

handler/crashpad_handler_test_extended_handler.cc

@@ -0,0 +1,70 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "build/build_config.h"
+#include "handler/handler_main.h"
+#include "minidump/test/minidump_user_extension_stream_util.h"
+#include "tools/tool_support.h"
+
+#if defined(OS_WIN)
+#include <windows.h>
+#endif
+
+namespace {
+
+class TestUserStreamDataSource : public crashpad::UserStreamDataSource {
+ public:
+  TestUserStreamDataSource() {}
+
+  std::unique_ptr<crashpad::MinidumpUserExtensionStreamDataSource>
+  ProduceStreamData(crashpad::ProcessSnapshot* process_snapshot) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestUserStreamDataSource);
+};
+
+std::unique_ptr<crashpad::MinidumpUserExtensionStreamDataSource>
+TestUserStreamDataSource::ProduceStreamData(
+    crashpad::ProcessSnapshot* process_snapshot) {
+  static const char kTestData[] = "Injected extension stream!";
+
+  return base::WrapUnique(new crashpad::test::BufferExtensionStreamDataSource(
+      0xCAFEBABE, kTestData, sizeof(kTestData)));
+}
+
+int ExtendedHandlerMain(int argc, char* argv[]) {
+  crashpad::UserStreamDataSources user_stream_data_sources;
+  user_stream_data_sources.push_back(
+      base::WrapUnique(new TestUserStreamDataSource()));
+
+  return crashpad::HandlerMain(argc, argv, &user_stream_data_sources);
+}
+
+}  // namespace
+
+#if defined(OS_MACOSX)
+
+int main(int argc, char* argv[]) {
+  return ExtendedHandlerMain(argc, argv);
+}
+
+#elif defined(OS_WIN)
+
+int wmain(int argc, wchar_t* argv[]) {
+  return crashpad::ToolSupport::Wmain(argc, argv, &ExtendedHandlerMain);
+}
+
+#endif  // OS_MACOSX

handler/handler.gyp

@@ -45,6 +45,8 @@
         'mac/exception_handler_server.h',
         'prune_crash_reports_thread.cc',
         'prune_crash_reports_thread.h',
+        'user_stream_data_source.cc',
+        'user_stream_data_source.h',
         'win/crash_report_exception_handler.cc',
         'win/crash_report_exception_handler.h',
       ],

handler/handler_main.cc

@@ -388,7 +388,9 @@ void MonitorSelf(const Options& options) {
 
 }  // namespace
 
-int HandlerMain(int argc, char* argv[]) {
+int HandlerMain(int argc,
+                char* argv[],
+                const UserStreamDataSources* user_stream_sources) {
   InstallCrashHandler();
   CallMetricsRecordNormalExit metrics_record_normal_exit;
 
@@ -727,8 +729,10 @@ int HandlerMain(int argc, char* argv[]) {
                                       PruneCondition::GetDefault());
   prune_thread.Start();
 
-  CrashReportExceptionHandler exception_handler(
+  CrashReportExceptionHandler exception_handler(database.get(),
-      database.get(), &upload_thread, &options.annotations);
+                                                &upload_thread,
+                                                &options.annotations,
+                                                user_stream_sources);
 
 #if defined(OS_WIN)
   if (options.initial_client_data.IsValid()) {

handler/handler_main.h

@@ -15,13 +15,22 @@
 #ifndef CRASHPAD_HANDLER_HANDLER_MAIN_H_
 #define CRASHPAD_HANDLER_HANDLER_MAIN_H_
 
+#include "handler/user_stream_data_source.h"
+
 namespace crashpad {
 
 //! \brief The `main()` of the `crashpad_handler` binary.
 //!
 //! This is exposed so that `crashpad_handler` can be embedded into another
 //! binary, but called and used as if it were a standalone executable.
-int HandlerMain(int argc, char* argv[]);
+//!
+//! \param[in] user_stream_sources An optional vector containing the
+//!     extensibility data sources to call on crash. Each time a minidump is
+//!     created, the sources are called in turn. Any streams returned are added
+//!     to the minidump.
+int HandlerMain(int argc,
+                char* argv[],
+                const UserStreamDataSources* user_stream_sources);
 
 }  // namespace crashpad
 

handler/handler_test.gyp

@@ -0,0 +1,64 @@
+# Copyright 2017 The Crashpad Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+{
+  'includes': [
+    '../build/crashpad.gypi',
+  ],
+  'targets': [
+    {
+      'target_name': 'crashpad_handler_test_extended_handler',
+      'type': 'executable',
+      'dependencies': [
+        '../compat/compat.gyp:crashpad_compat',
+        '../minidump/minidump_test.gyp:crashpad_minidump_test_lib',
+        '../third_party/mini_chromium/mini_chromium.gyp:base',
+        '../tools/tools.gyp:crashpad_tool_support',
+        'handler.gyp:crashpad_handler_lib',
+      ],
+      'include_dirs': [
+        '..',
+      ],
+      'sources': [
+        'crashpad_handler_test_extended_handler.cc',
+      ],
+    },
+  ],
+  'conditions': [
+    ['OS=="win"', {
+      'targets': [{
+        # The handler is only tested on Windows for now.
+        'target_name': 'crashpad_handler_test',
+        'type': 'executable',
+        'dependencies': [
+          'crashpad_handler_test_extended_handler',
+          'handler.gyp:crashpad_handler_lib',
+          '../client/client.gyp:crashpad_client',
+          '../compat/compat.gyp:crashpad_compat',
+          '../test/test.gyp:crashpad_gtest_main',
+          '../test/test.gyp:crashpad_test',
+          '../third_party/gtest/gtest.gyp:gtest',
+          '../third_party/mini_chromium/mini_chromium.gyp:base',
+          '../util/util.gyp:crashpad_util',
+        ],
+        'include_dirs': [
+          '..',
+        ],
+        'sources': [
+          'crashpad_handler_test.cc',
+        ],
+      }],
+    }],
+  ],
+}

handler/mac/crash_report_exception_handler.cc

@@ -22,6 +22,7 @@
 #include "base/strings/stringprintf.h"
 #include "client/settings.h"
 #include "minidump/minidump_file_writer.h"
+#include "minidump/minidump_user_extension_stream_data_source.h"
 #include "snapshot/crashpad_info_client_options.h"
 #include "snapshot/mac/process_snapshot_mac.h"
 #include "util/file/file_writer.h"
@@ -41,11 +42,12 @@ namespace crashpad {
 CrashReportExceptionHandler::CrashReportExceptionHandler(
     CrashReportDatabase* database,
     CrashReportUploadThread* upload_thread,
-    const std::map<std::string, std::string>* process_annotations)
+    const std::map<std::string, std::string>* process_annotations,
+    const UserStreamDataSources* user_stream_data_sources)
     : database_(database),
       upload_thread_(upload_thread),
-      process_annotations_(process_annotations) {
+      process_annotations_(process_annotations),
-}
+      user_stream_data_sources_(user_stream_data_sources) {}
 
 CrashReportExceptionHandler::~CrashReportExceptionHandler() {
 }
@@ -169,6 +171,9 @@ kern_return_t CrashReportExceptionHandler::CatchMachException(
 
     MinidumpFileWriter minidump;
     minidump.InitializeFromSnapshot(&process_snapshot);
+    AddUserExtensionStreams(
+        user_stream_data_sources_, &process_snapshot, &minidump);
+
     if (!minidump.WriteEverything(&file_writer)) {
       Metrics::ExceptionCaptureResult(
           Metrics::CaptureResult::kMinidumpWriteFailed);

handler/mac/crash_report_exception_handler.h

@@ -23,6 +23,7 @@
 #include "base/macros.h"
 #include "client/crash_report_database.h"
 #include "handler/crash_report_upload_thread.h"
+#include "handler/user_stream_data_source.h"
 #include "util/mach/exc_server_variants.h"
 
 namespace crashpad {
@@ -49,7 +50,8 @@ class CrashReportExceptionHandler : public UniversalMachExcServer::Interface {
   CrashReportExceptionHandler(
       CrashReportDatabase* database,
       CrashReportUploadThread* upload_thread,
-      const std::map<std::string, std::string>* process_annotations);
+      const std::map<std::string, std::string>* process_annotations,
+      const UserStreamDataSources* user_stream_data_sources);
 
   ~CrashReportExceptionHandler();
 
@@ -77,6 +79,7 @@ class CrashReportExceptionHandler : public UniversalMachExcServer::Interface {
   CrashReportDatabase* database_;  // weak
   CrashReportUploadThread* upload_thread_;  // weak
   const std::map<std::string, std::string>* process_annotations_;  // weak
+  const UserStreamDataSources* user_stream_data_sources_;  // weak
 
   DISALLOW_COPY_AND_ASSIGN(CrashReportExceptionHandler);
 };

handler/main.cc

@@ -23,10 +23,18 @@
 
 #if defined(OS_MACOSX)
 int main(int argc, char* argv[]) {
-  return crashpad::HandlerMain(argc, argv);
+  return crashpad::HandlerMain(argc, argv, nullptr);
 }
 #elif defined(OS_WIN)
+namespace {
+
+int HandlerMainAdaptor(int argc, char* argv[]) {
+  return crashpad::HandlerMain(argc, argv, nullptr);
+}
+
+}  // namespace
+
 int APIENTRY wWinMain(HINSTANCE, HINSTANCE, wchar_t*, int) {
-  return crashpad::ToolSupport::Wmain(__argc, __wargv, crashpad::HandlerMain);
+  return crashpad::ToolSupport::Wmain(__argc, __wargv, HandlerMainAdaptor);
 }
 #endif  // OS_MACOSX

handler/user_stream_data_source.cc

@@ -0,0 +1,43 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "handler/user_stream_data_source.h"
+
+#include "base/logging.h"
+#include "minidump/minidump_file_writer.h"
+#include "minidump/minidump_user_extension_stream_data_source.h"
+#include "snapshot/process_snapshot.h"
+
+namespace crashpad {
+
+void AddUserExtensionStreams(
+    const UserStreamDataSources* user_stream_data_sources,
+    ProcessSnapshot* process_snapshot,
+    MinidumpFileWriter* minidump_file_writer) {
+  if (!user_stream_data_sources)
+    return;
+  for (const auto& source : *user_stream_data_sources) {
+    std::unique_ptr<MinidumpUserExtensionStreamDataSource> data_source(
+        source->ProduceStreamData(process_snapshot));
+    if (data_source &&
+        !minidump_file_writer->AddUserExtensionStream(std::move(data_source))) {
+      // This should only happen if multiple user stream sources yield the
+      // same stream type. It's the user's responsibility to make sure
+      // sources don't collide on the same stream type.
+      LOG(ERROR) << "AddUserExtensionStream failed";
+    }
+  }
+}
+
+}  // namespace crashpad

handler/user_stream_data_source.h

@@ -0,0 +1,68 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_HANDLER_USER_STREAM_DATA_SOURCE_H_
+#define CRASHPAD_HANDLER_USER_STREAM_DATA_SOURCE_H_
+
+#include <memory>
+#include <vector>
+
+namespace crashpad {
+
+class MinidumpFileWriter;
+class MinidumpUserExtensionStreamDataSource;
+class ProcessSnapshot;
+
+//! \brief Extensibility interface for embedders who wish to add custom streams
+//!     to minidumps.
+class UserStreamDataSource {
+ public:
+  virtual ~UserStreamDataSource() {}
+
+  //! \brief Produce the contents for an extension stream for a crashed program.
+  //!
+  //! Called after \a process_snapshot has been initialized for the crashed
+  //! process to (optionally) produce the contents of a user extension stream
+  //! that will be attached to the minidump.
+  //!
+  //! \param[in] process_snapshot An initialized snapshot for the crashed
+  //!     process.
+  //!
+  //! \return A new data source for the stream to add to the minidump or
+  //!      `nullptr` on failure or to opt out of adding a stream.
+  virtual std::unique_ptr<MinidumpUserExtensionStreamDataSource>
+  ProduceStreamData(ProcessSnapshot* process_snapshot) = 0;
+};
+
+using UserStreamDataSources =
+    std::vector<std::unique_ptr<UserStreamDataSource>>;
+
+//! \brief Adds user extension streams to a minidump.
+//!
+//! Dispatches to each source in \a user_stream_data_sources and adds returned
+//! extension streams to \a minidump_file_writer.
+//!
+//! \param[in] user_stream_data_sources A pointer to the data sources, or
+//!     `nullptr`.
+//! \param[in] process_snapshot An initialized snapshot to the crashing process.
+//! \param[in] minidump_file_writer Any extension streams will be added to this
+//!     minidump.
+void AddUserExtensionStreams(
+    const UserStreamDataSources* user_stream_data_sources,
+    ProcessSnapshot* process_snapshot,
+    MinidumpFileWriter* minidump_file_writer);
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_HANDLER_USER_STREAM_DATA_SOURCE_H_

handler/win/crash_report_exception_handler.cc

@@ -20,6 +20,7 @@
 #include "client/settings.h"
 #include "handler/crash_report_upload_thread.h"
 #include "minidump/minidump_file_writer.h"
+#include "minidump/minidump_user_extension_stream_data_source.h"
 #include "snapshot/win/process_snapshot_win.h"
 #include "util/file/file_writer.h"
 #include "util/misc/metrics.h"
@@ -32,11 +33,12 @@ namespace crashpad {
 CrashReportExceptionHandler::CrashReportExceptionHandler(
     CrashReportDatabase* database,
     CrashReportUploadThread* upload_thread,
-    const std::map<std::string, std::string>* process_annotations)
+    const std::map<std::string, std::string>* process_annotations,
+    const UserStreamDataSources* user_stream_data_sources)
     : database_(database),
       upload_thread_(upload_thread),
-      process_annotations_(process_annotations) {
+      process_annotations_(process_annotations),
-}
+      user_stream_data_sources_(user_stream_data_sources) {}
 
 CrashReportExceptionHandler::~CrashReportExceptionHandler() {
 }
@@ -107,6 +109,9 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException(
 
     MinidumpFileWriter minidump;
     minidump.InitializeFromSnapshot(&process_snapshot);
+    AddUserExtensionStreams(
+        user_stream_data_sources_, &process_snapshot, &minidump);
+
     if (!minidump.WriteEverything(&file_writer)) {
       LOG(ERROR) << "WriteEverything failed";
       Metrics::ExceptionCaptureResult(

handler/win/crash_report_exception_handler.h

@@ -21,6 +21,7 @@
 #include <string>
 
 #include "base/macros.h"
+#include "handler/user_stream_data_source.h"
 #include "util/win/exception_handler_server.h"
 
 namespace crashpad {
@@ -50,7 +51,8 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate {
   CrashReportExceptionHandler(
       CrashReportDatabase* database,
       CrashReportUploadThread* upload_thread,
-      const std::map<std::string, std::string>* process_annotations);
+      const std::map<std::string, std::string>* process_annotations,
+      const UserStreamDataSources* user_stream_data_sources);
 
   ~CrashReportExceptionHandler() override;
 
@@ -68,6 +70,7 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate {
   CrashReportDatabase* database_;  // weak
   CrashReportUploadThread* upload_thread_;  // weak
   const std::map<std::string, std::string>* process_annotations_;  // weak
+  const UserStreamDataSources* user_stream_data_sources_;  // weak
 
   DISALLOW_COPY_AND_ASSIGN(CrashReportExceptionHandler);
 };

minidump/minidump_file_writer.cc

@@ -206,9 +206,8 @@ bool MinidumpFileWriter::AddUserExtensionStream(
   DCHECK_EQ(state(), kStateMutable);
 
   auto user_stream = base::WrapUnique(new MinidumpUserStreamWriter());
-  user_stream->InitializeFromBuffer(user_extension_stream_data->stream_type(),
+  user_stream->InitializeFromUserExtensionStream(
-                                    user_extension_stream_data->buffer(),
+      std::move(user_extension_stream_data));
-                                    user_extension_stream_data->buffer_size());
 
   return AddStream(std::move(user_stream));
 }

minidump/minidump_file_writer_test.cc

@@ -28,6 +28,7 @@
 #include "minidump/minidump_user_extension_stream_data_source.h"
 #include "minidump/minidump_writable.h"
 #include "minidump/test/minidump_file_writer_test_util.h"
+#include "minidump/test/minidump_user_extension_stream_util.h"
 #include "minidump/test/minidump_writable_test_util.h"
 #include "snapshot/test/test_cpu_context.h"
 #include "snapshot/test/test_exception_snapshot.h"
@@ -137,14 +138,14 @@ TEST(MinidumpFileWriter, AddUserExtensionStream) {
   const size_t kStreamSize = arraysize(kStreamData);
   const MinidumpStreamType kStreamType = static_cast<MinidumpStreamType>(0x4d);
 
-  auto stream = base::WrapUnique(new MinidumpUserExtensionStreamDataSource(
+  auto data_source = base::WrapUnique(new test::BufferExtensionStreamDataSource(
       kStreamType, kStreamData, kStreamSize));
-  ASSERT_TRUE(minidump_file.AddUserExtensionStream(std::move(stream)));
+  ASSERT_TRUE(minidump_file.AddUserExtensionStream(std::move(data_source)));
 
   // Adding the same stream type a second time should fail.
-  stream = base::WrapUnique(new MinidumpUserExtensionStreamDataSource(
+  data_source = base::WrapUnique(new test::BufferExtensionStreamDataSource(
       kStreamType, kStreamData, kStreamSize));
-  ASSERT_FALSE(minidump_file.AddUserExtensionStream(std::move(stream)));
+  ASSERT_FALSE(minidump_file.AddUserExtensionStream(std::move(data_source)));
 
   StringFile string_file;
   ASSERT_TRUE(minidump_file.WriteEverything(&string_file));
@@ -172,6 +173,37 @@ TEST(MinidumpFileWriter, AddUserExtensionStream) {
   EXPECT_EQ(memcmp(stream_data, kStreamData, kStreamSize), 0);
 }
 
+TEST(MinidumpFileWriter, AddEmptyUserExtensionStream) {
+  MinidumpFileWriter minidump_file;
+  const time_t kTimestamp = 0x155d2fb8;
+  minidump_file.SetTimestamp(kTimestamp);
+
+  const MinidumpStreamType kStreamType = static_cast<MinidumpStreamType>(0x4d);
+
+  auto data_source = base::WrapUnique(
+      new test::BufferExtensionStreamDataSource(kStreamType, nullptr, 0));
+  ASSERT_TRUE(minidump_file.AddUserExtensionStream(std::move(data_source)));
+
+  StringFile string_file;
+  ASSERT_TRUE(minidump_file.WriteEverything(&string_file));
+
+  const size_t kDirectoryOffset = sizeof(MINIDUMP_HEADER);
+  const size_t kStreamOffset = kDirectoryOffset + sizeof(MINIDUMP_DIRECTORY);
+  const size_t kFileSize = kStreamOffset;
+
+  ASSERT_EQ(string_file.string().size(), kFileSize);
+
+  const MINIDUMP_DIRECTORY* directory;
+  const MINIDUMP_HEADER* header =
+      MinidumpHeaderAtStart(string_file.string(), &directory);
+  ASSERT_NO_FATAL_FAILURE(VerifyMinidumpHeader(header, 1, kTimestamp));
+  ASSERT_TRUE(directory);
+
+  EXPECT_EQ(directory[0].StreamType, kStreamType);
+  EXPECT_EQ(directory[0].Location.DataSize, 0u);
+  EXPECT_EQ(directory[0].Location.Rva, kStreamOffset);
+}
+
 TEST(MinidumpFileWriter, ThreeStreams) {
   MinidumpFileWriter minidump_file;
   const time_t kTimestamp = 0x155d2fb8;

minidump/minidump_test.gyp

@@ -17,10 +17,39 @@
     '../build/crashpad.gypi',
   ],
   'targets': [
+    {
+      'target_name': 'crashpad_minidump_test_lib',
+      'type': 'static_library',
+      'dependencies': [
+        'minidump.gyp:crashpad_minidump',
+        '../third_party/gtest/gtest.gyp:gtest',
+        '../third_party/mini_chromium/mini_chromium.gyp:base',
+      ],
+      'include_dirs': [
+        '..',
+      ],
+      'sources': [
+        'test/minidump_context_test_util.cc',
+        'test/minidump_context_test_util.h',
+        'test/minidump_file_writer_test_util.cc',
+        'test/minidump_file_writer_test_util.h',
+        'test/minidump_memory_writer_test_util.cc',
+        'test/minidump_memory_writer_test_util.h',
+        'test/minidump_rva_list_test_util.cc',
+        'test/minidump_rva_list_test_util.h',
+        'test/minidump_string_writer_test_util.cc',
+        'test/minidump_string_writer_test_util.h',
+        'test/minidump_user_extension_stream_util.cc',
+        'test/minidump_user_extension_stream_util.h',
+        'test/minidump_writable_test_util.cc',
+        'test/minidump_writable_test_util.h',
+      ],
+    },
     {
       'target_name': 'crashpad_minidump_test',
       'type': 'executable',
       'dependencies': [
+        'crashpad_minidump_test_lib',
         'minidump.gyp:crashpad_minidump',
         '../snapshot/snapshot_test.gyp:crashpad_snapshot_test_lib',
         '../test/test.gyp:crashpad_gtest_main',
@@ -36,8 +65,8 @@
         'minidump_context_writer_test.cc',
         'minidump_crashpad_info_writer_test.cc',
         'minidump_exception_writer_test.cc',
-        'minidump_handle_writer_test.cc',
         'minidump_file_writer_test.cc',
+        'minidump_handle_writer_test.cc',
         'minidump_memory_info_writer_test.cc',
         'minidump_memory_writer_test.cc',
         'minidump_misc_info_writer_test.cc',
@@ -52,18 +81,6 @@
         'minidump_unloaded_module_writer_test.cc',
         'minidump_user_stream_writer_test.cc',
         'minidump_writable_test.cc',
-        'test/minidump_context_test_util.cc',
-        'test/minidump_context_test_util.h',
-        'test/minidump_file_writer_test_util.cc',
-        'test/minidump_file_writer_test_util.h',
-        'test/minidump_memory_writer_test_util.cc',
-        'test/minidump_memory_writer_test_util.h',
-        'test/minidump_rva_list_test_util.cc',
-        'test/minidump_rva_list_test_util.h',
-        'test/minidump_string_writer_test_util.cc',
-        'test/minidump_string_writer_test_util.h',
-        'test/minidump_writable_test_util.cc',
-        'test/minidump_writable_test_util.h',
       ],
     },
   ],

minidump/minidump_user_extension_stream_data_source.cc

@@ -17,12 +17,8 @@
 namespace crashpad {
 
 MinidumpUserExtensionStreamDataSource::MinidumpUserExtensionStreamDataSource(
-    uint32_t stream_type,
+    uint32_t stream_type)
-    const void* buffer,
+    : stream_type_(static_cast<MinidumpStreamType>(stream_type)) {}
-    size_t buffer_size)
-    : stream_type_(static_cast<MinidumpStreamType>(stream_type)),
-      buffer_(buffer),
-      buffer_size_(buffer_size) {}
 
 MinidumpUserExtensionStreamDataSource::
     ~MinidumpUserExtensionStreamDataSource() {}

minidump/minidump_user_extension_stream_data_source.h

@@ -27,25 +27,54 @@ namespace crashpad {
 //! \brief Describes a user extension data stream in a minidump.
 class MinidumpUserExtensionStreamDataSource {
  public:
+  //! \brief An interface implemented by readers of
+  //!     MinidumpUserExtensionStreamDataSource.
+  class Delegate {
+   public:
+    //! \brief Called by  MinidumpUserExtensionStreamDataSource::Read() to
+    //!     provide data requested by a call to that method.
+    //!
+    //! \param[in] data A pointer to the data that was read. The callee does not
+    //!     take ownership of this data. This data is only valid for the
+    //!     duration of the call to this method. This parameter may be `nullptr`
+    //!     if \a size is `0`.
+    //! \param[in] size The size of the data that was read.
+    //!
+    //! \return `true` on success, `false` on failure.
+    //!     MinidumpUserExtensionStreamDataSource::ReadStreamData() will use
+    //!     this as its own return value.
+    virtual bool ExtensionStreamDataSourceRead(const void* data,
+                                               size_t size) = 0;
+
+   protected:
+    ~Delegate() {}
+  };
+
   //! \brief Constructs a MinidumpUserExtensionStreamDataSource.
   //!
   //! \param[in] stream_type The type of the user extension stream.
-  //! \param[in] buffer Points to the data for this stream. \a buffer is not
+  explicit MinidumpUserExtensionStreamDataSource(uint32_t stream_type);
-  //!     owned, and must outlive the use of this object.
+  virtual ~MinidumpUserExtensionStreamDataSource();
-  //! \param[in] buffer_size The length of data in \a buffer.
-  MinidumpUserExtensionStreamDataSource(uint32_t stream_type,
-                                        const void* buffer,
-                                        size_t buffer_size);
-  ~MinidumpUserExtensionStreamDataSource();
 
   MinidumpStreamType stream_type() const { return stream_type_; }
-  const void* buffer() const { return buffer_; }
+
-  size_t buffer_size() const { return buffer_size_; }
+  //! \brief The size of this data stream.
+  virtual size_t StreamDataSize() = 0;
+
+  //! \brief Calls Delegate::UserStreamDataSourceRead(), providing it with
+  //!     the stream data.
+  //!
+  //! Implementations do not necessarily compute the stream data prior to
+  //! this method being called. The stream data may be computed or loaded
+  //! lazily and may be discarded after being passed to the delegate.
+  //!
+  //! \return `false` on failure, otherwise, the return value of
+  //!     Delegate::ExtensionStreamDataSourceRead(), which should be `true` on
+  //!     success and `false` on failure.
+  virtual bool ReadStreamData(Delegate* delegate) = 0;
 
  private:
   MinidumpStreamType stream_type_;
-  const void* buffer_;  // weak
-  size_t buffer_size_;
 
   DISALLOW_COPY_AND_ASSIGN(MinidumpUserExtensionStreamDataSource);
 };

minidump/minidump_user_stream_writer.cc

@@ -56,22 +56,32 @@ class MinidumpUserStreamWriter::SnapshotContentsWriter final
   DISALLOW_COPY_AND_ASSIGN(SnapshotContentsWriter);
 };
 
-class MinidumpUserStreamWriter::BufferContentsWriter final
+class MinidumpUserStreamWriter::ExtensionStreamContentsWriter final
-    : public MinidumpUserStreamWriter::ContentsWriter {
+    : public MinidumpUserStreamWriter::ContentsWriter,
+      public MinidumpUserExtensionStreamDataSource::Delegate {
  public:
-  BufferContentsWriter(const void* buffer, size_t buffer_size)
+  explicit ExtensionStreamContentsWriter(
-      : buffer_(buffer), buffer_size_(buffer_size) {}
+      std::unique_ptr<MinidumpUserExtensionStreamDataSource> data_source)
+      : data_source_(std::move(data_source)), writer_(nullptr) {}
 
   bool WriteContents(FileWriterInterface* writer) override {
-    return writer->Write(buffer_, buffer_size_);
+    DCHECK(!writer_);
+
+    writer_ = writer;
+    return data_source_->ReadStreamData(this);
+  }
+
+  size_t GetSize() const override { return data_source_->StreamDataSize(); }
+
+  bool ExtensionStreamDataSourceRead(const void* data, size_t size) override {
+    return writer_->Write(data, size);
   }
-  size_t GetSize() const override { return buffer_size_; }
 
  private:
-  const void* buffer_;
+  std::unique_ptr<MinidumpUserExtensionStreamDataSource> data_source_;
-  size_t buffer_size_;
+  FileWriterInterface* writer_;
 
-  DISALLOW_COPY_AND_ASSIGN(BufferContentsWriter);
+  DISALLOW_COPY_AND_ASSIGN(ExtensionStreamContentsWriter);
 };
 
 MinidumpUserStreamWriter::MinidumpUserStreamWriter() : stream_type_() {}
@@ -89,16 +99,14 @@ void MinidumpUserStreamWriter::InitializeFromSnapshot(
       base::WrapUnique(new SnapshotContentsWriter(stream->memory()));
 }
 
-void MinidumpUserStreamWriter::InitializeFromBuffer(
+void MinidumpUserStreamWriter::InitializeFromUserExtensionStream(
-    MinidumpStreamType stream_type,
+    std::unique_ptr<MinidumpUserExtensionStreamDataSource> data_source) {
-    const void* buffer,
-    size_t buffer_size) {
   DCHECK_EQ(state(), kStateMutable);
   DCHECK(!contents_writer_.get());
 
-  stream_type_ = stream_type;
+  stream_type_ = data_source->stream_type();
-  contents_writer_ =
+  contents_writer_ = base::WrapUnique(
-      base::WrapUnique(new BufferContentsWriter(buffer, buffer_size));
+      new ExtensionStreamContentsWriter(std::move(data_source)));
 }
 
 bool MinidumpUserStreamWriter::Freeze() {

minidump/minidump_user_stream_writer.h

@@ -25,6 +25,7 @@
 #include "minidump/minidump_extensions.h"
 #include "minidump/minidump_stream_writer.h"
 #include "minidump/minidump_writable.h"
+#include "minidump/minidump_user_extension_stream_data_source.h"
 #include "snapshot/module_snapshot.h"
 
 namespace crashpad {
@@ -42,17 +43,13 @@ class MinidumpUserStreamWriter final : public internal::MinidumpStreamWriter {
   //! \note Valid in #kStateMutable.
   void InitializeFromSnapshot(const UserMinidumpStream* stream);
 
-  //! \brief Initializes a MINIDUMP_USER_STREAM based on \a stream_type,
+  //! \brief Initializes a MINIDUMP_USER_STREAM based on \a data_source.
-  //!     \a buffer and \a buffer_size.
   //!
-  //! \param[in] stream_type The type of the stream.
+  //! \param[in] data_source The content and type of the stream.
-  //! \param[in] buffer The data for the stream.
-  //! \param[in] buffer_size The length of \a buffer, and the resulting stream.
   //!
   //! \note Valid in #kStateMutable.
-  void InitializeFromBuffer(MinidumpStreamType stream_type,
+  void InitializeFromUserExtensionStream(
-                            const void* buffer,
+      std::unique_ptr<MinidumpUserExtensionStreamDataSource> data_source);
-                            size_t buffer_size);
 
  protected:
   // MinidumpWritable:
@@ -67,7 +64,7 @@ class MinidumpUserStreamWriter final : public internal::MinidumpStreamWriter {
  private:
   class ContentsWriter;
   class SnapshotContentsWriter;
-  class BufferContentsWriter;
+  class ExtensionStreamContentsWriter;
 
   std::unique_ptr<ContentsWriter> contents_writer_;
 

minidump/minidump_user_stream_writer_test.cc

@@ -21,6 +21,7 @@
 #include "gtest/gtest.h"
 #include "minidump/minidump_file_writer.h"
 #include "minidump/test/minidump_file_writer_test_util.h"
+#include "minidump/test/minidump_user_extension_stream_util.h"
 #include "minidump/test/minidump_writable_test_util.h"
 #include "snapshot/test/test_memory_snapshot.h"
 #include "util/file/string_file.h"
@@ -74,10 +75,12 @@ TEST(MinidumpUserStreamWriter, InitializeFromSnapshotNoData) {
       string_file.string(), &user_stream_location, kTestStreamId, 0u));
 }
 
-TEST(MinidumpUserStreamWriter, InitializeFromBufferNoData) {
+TEST(MinidumpUserStreamWriter, InitializeFromUserExtensionStreamNoData) {
   MinidumpFileWriter minidump_file_writer;
+  auto data_source = base::WrapUnique(
+      new test::BufferExtensionStreamDataSource(kTestStreamId, nullptr, 0));
   auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter());
-  user_stream_writer->InitializeFromBuffer(kTestStreamId, nullptr, 0);
+  user_stream_writer->InitializeFromUserExtensionStream(std::move(data_source));
   minidump_file_writer.AddStream(std::move(user_stream_writer));
 
   StringFile string_file;
@@ -121,12 +124,13 @@ TEST(MinidumpUserStreamWriter, InitializeFromSnapshotOneStream) {
 
 TEST(MinidumpUserStreamWriter, InitializeFromBufferOneStream) {
   MinidumpFileWriter minidump_file_writer;
-  auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter());
 
   const size_t kStreamSize = 128;
   std::vector<uint8_t> data(kStreamSize, 'c');
-  user_stream_writer->InitializeFromBuffer(
+  auto data_source = base::WrapUnique(new test::BufferExtensionStreamDataSource(
-      kTestStreamId, &data[0], data.size());
+      kTestStreamId, &data[0], data.size()));
+  auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter());
+  user_stream_writer->InitializeFromUserExtensionStream(std::move(data_source));
   minidump_file_writer.AddStream(std::move(user_stream_writer));
 
   StringFile string_file;

minidump/test/minidump_user_extension_stream_util.cc

@@ -0,0 +1,43 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "minidump/test/minidump_user_extension_stream_util.h"
+
+#include <string.h>
+
+namespace crashpad {
+namespace test {
+
+BufferExtensionStreamDataSource::BufferExtensionStreamDataSource(
+    uint32_t stream_type,
+    const void* data,
+    size_t data_size)
+    : MinidumpUserExtensionStreamDataSource(stream_type) {
+  data_.resize(data_size);
+
+  if (data_size)
+    memcpy(data_.data(), data, data_size);
+}
+
+size_t BufferExtensionStreamDataSource::StreamDataSize() {
+  return data_.size();
+}
+
+bool BufferExtensionStreamDataSource::ReadStreamData(Delegate* delegate) {
+  return delegate->ExtensionStreamDataSourceRead(
+      data_.size() ? data_.data() : nullptr, data_.size());
+}
+
+}  // namespace test
+}  // namespace crashpad

minidump/test/minidump_user_extension_stream_util.h

@@ -0,0 +1,53 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_MINIDUMP_TEST_MINIDUMP_USER_EXTENSION_STREAM_UTIL_H_
+#define CRASHPAD_MINIDUMP_TEST_MINIDUMP_USER_EXTENSION_STREAM_UTIL_H_
+
+#include "minidump/minidump_user_extension_stream_data_source.h"
+
+#include <stdint.h>
+#include <sys/types.h>
+
+#include <vector>
+
+namespace crashpad {
+namespace test {
+
+//! \brief A user extension data source that wraps a buffer.
+class BufferExtensionStreamDataSource final
+    : public MinidumpUserExtensionStreamDataSource {
+ public:
+  //! \brief Creates a data source with \a stream_type.
+  //!
+  //! param[in] stream_type The type of the stream.
+  //! param[in] data The data of the stream.
+  //! param[in] data_size The length of \a data.
+  BufferExtensionStreamDataSource(uint32_t stream_type,
+                                  const void* data,
+                                  size_t data_size);
+
+  size_t StreamDataSize() override;
+  bool ReadStreamData(Delegate* delegate) override;
+
+ private:
+  std::vector<uint8_t> data_;
+
+  DISALLOW_COPY_AND_ASSIGN(BufferExtensionStreamDataSource);
+};
+
+}  // namespace test
+}  // namespace crashpad
+
+#endif  // CRASHPAD_MINIDUMP_TEST_MINIDUMP_USER_EXTENSION_STREAM_UTIL_H_

test/win/win_multiprocess_with_temp_dir.h

@@ -77,4 +77,4 @@ class WinMultiprocessWithTempDir : public WinMultiprocess {
 }  // namespace test
 }  // namespace crashpad
 
-#endif  // CRASHPAD_TEST_WIN_WIN_MULTIPROCESS_H_
+#endif  // CRASHPAD_TEST_WIN_WIN_MULTIPROCESS_WITH_TEMPDIR_H_