Collect unclean shutdown debug information

components/browser_watcher/postmortem_minidump_writer_win.cc

+// Copyright 2016 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.
+//
+// Note: aside from using windows headers to obtain the definitions of minidump
+//     structures, nothing here is windows specific. This seems like the best
+//     approach given this code is for tempory experimentation on Windows.

[scottmg, 2016/08/03 22:47:06]

temporary

[manzagop (departed), 2016/08/10 15:59:52]

Done.

+//     Longer term, CrashPad will take over the minidump writing in this case as
+//     well.
+
+#include "components/browser_watcher/postmortem_minidump_writer.h"
+
+#include <windows.h>  // NOLINT
+#include <dbghelp.h>
+
+#include <string>
+
+#include "base/files/file_util.h"
+#include "base/numerics/safe_math.h"
+
+namespace browser_watcher {
+
+// The stream type assigned to the minidump stream that holds the serialized
+// stability report.
+// Note: the value was obtained by adding 1 to the stream type used for holding
+// the SyzyAsan proto.
+// TODO(manzagop): centralize the stream type definitions to avoid issues.
+const uint32_t kStabilityReportStreamType = 0x4B6B0002;

[scottmg, 2016/08/03 22:47:06]

(Kk02... "K"as"k"o lives on, I guess?

[manzagop (departed), 2016/08/10 15:59:52]

Yeah, I really had no clue what value to use!

Is there:
- a better value to use
- a mechanism to reserve that value wrt chromium?

Eg
https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/minidump/m...

+
+PostmortemMinidumpWriter::PostmortemMinidumpWriter() : cursor_(0U) {}
+
+bool PostmortemMinidumpWriter::WriteDump(const base::FilePath& mindump_path,
+                                         const StabilityReport& report) {
+  // Open the minidump file.
+  minidump_.reset(base::OpenFile(mindump_path, "wb"));
+  if (minidump_.get() == nullptr)
+    return false;

[scottmg, 2016/08/03 22:47:06]

Is it useful to log on any of these failures? Or if not, maybe generally
reporting back high level success/failure via UMA?

[manzagop (departed), 2016/08/10 15:59:52]

Totally! It's a major TODO to sprinkle some UMA reporting (it's in
postmortem.h). I'll bundle the addition of logging in that TODO if that's ok
with you.

I wasn't sure where the data harvesting would happen and thought there might be
complexity to adding UMA depending on where it lives. At this point, I'm
thinking I'll put this in the browser watcher process.

+
+  // Allocate space for the header.
+  Position pos = 0U;
+  if (!Allocate(sizeof(MINIDUMP_HEADER), &pos))
+    return false;
+  DCHECK_EQ(kHeaderPos, pos);
+
+  // Write the proto to the file.
+  std::string serialized_report;
+  report.SerializeToString(&serialized_report);
+  Position report_pos = 0U;
+  if (!AppendBytes(serialized_report, &report_pos))
+    return false;
+
+  // Write the directory entry for the stability report's stream.

[scottmg, 2016/08/03 22:47:06]

As Mark described, you'll have to add keys for the Breakpad server-side to
accept this.

[manzagop (departed), 2016/08/10 15:59:52]

Done. I added product and version. Hopefully correctly!

+  MINIDUMP_DIRECTORY report_directory = {0};
+  report_directory.StreamType = kStabilityReportStreamType;
+  report_directory.Location.Rva = report_pos;
+  report_directory.Location.DataSize = serialized_report.length();
+  Position directory_pos = 0U;
+  if (!Append(report_directory, &directory_pos))
+    return false;
+
+  // Write the header.
+  MINIDUMP_HEADER header;
+  header.Signature = MINIDUMP_SIGNATURE;
+  header.NumberOfStreams = 1;
+  header.StreamDirectoryRva = directory_pos;
+  return Write(kHeaderPos, header);
+}
+
+bool PostmortemMinidumpWriter::Allocate(size_t size_bytes, Position* pos) {
+  DCHECK(pos);
+  *pos = cursor_;
+  return IncrementCursor(size_bytes);
+}
+
+bool PostmortemMinidumpWriter::WriteBytes(Position pos,
+                                          size_t size_bytes,
+                                          const void* data) {
+  DCHECK(data);
+  DCHECK(minidump_.get());
+
+  // Validate the write does not extend past the cursor.
+  base::CheckedNumeric<Position> pos_end = pos;
+  pos_end += size_bytes;
+  if (!pos_end.IsValid() || pos_end.ValueOrDie() > cursor_)
+    return false;
+
+  // Seek and write.
+  if (fseek(minidump_.get(), pos, SEEK_SET) != 0)
+    return false;
+  return fwrite(data, sizeof(char), size_bytes, minidump_.get()) == size_bytes;
+}
+
+bool PostmortemMinidumpWriter::AppendBytes(base::StringPiece data,
+                                           Position* pos) {
+  DCHECK(pos);
+  if (!Allocate(data.length(), pos))
+    return false;
+  return WriteBytes(*pos, data.length(), data.data());
+}
+
+bool PostmortemMinidumpWriter::IncrementCursor(size_t size_bytes) {
+  base::CheckedNumeric<Position> cur = cursor_;
+  cur += size_bytes;
+  if (!cur.IsValid())
+    return false;
+
+  cursor_ += size_bytes;
+  return true;
+}
+
+}  // namespace browser_watcher