Stability instrumentation Crashpad integration

components/browser_watcher/stability_report_user_stream_data_source.cc

+// Copyright 2017 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.
+
+#include "components/browser_watcher/stability_report_user_stream_data_source.h"
+
+#include <string>
+#include <utility>
+
+#include "base/files/file.h"
+#include "base/files/file_util.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string16.h"
+#include "base/time/time.h"
+#include "components/browser_watcher/minidump_user_streams.h"
+#include "components/browser_watcher/stability_paths.h"
+#include "components/browser_watcher/stability_report_extractor.h"
+#include "third_party/crashpad/crashpad/minidump/minidump_user_extension_stream_data_source.h"
+#include "third_party/crashpad/crashpad/snapshot/process_snapshot.h"
+
+namespace browser_watcher {
+
+namespace {
+
+bool GetStabilityFileName(const base::string16& user_data_dir,
+                          crashpad::ProcessSnapshot* process_snapshot,
+                          base::FilePath* stability_file) {
+  DCHECK(process_snapshot);
+  DCHECK(stability_file);
+
+  timeval creation_time{};
+  process_snapshot->ProcessStartTime(&creation_time);
+
+  *stability_file =
+      GetStabilityFileForProcess(process_snapshot->ProcessID(), creation_time,
+                                 base::FilePath(user_data_dir));
+  return true;

[Sigurður Ásgeirsson, 2017/05/11 15:33:40]

return void as this can't fail (at present).

[manzagop (departed), 2017/05/11 19:14:53]

Done.

+}
+
+class BufferExtensionStreamDataSource final
+    : public crashpad::MinidumpUserExtensionStreamDataSource {
+ public:
+  BufferExtensionStreamDataSource(uint32_t stream_type,
+                                  const StabilityReport& report);

[Sigurður Ásgeirsson, 2017/05/11 15:33:40]

you could also use a "bool Init()" method to initialize the data_ member if you
want to detect serialization failure.

[manzagop (departed), 2017/05/11 19:14:53]

Done.

+
+  size_t StreamDataSize() override;
+  bool ReadStreamData(Delegate* delegate) override;
+
+ private:
+  std::string data_;
+
+  DISALLOW_COPY_AND_ASSIGN(BufferExtensionStreamDataSource);
+};
+
+BufferExtensionStreamDataSource::BufferExtensionStreamDataSource(
+    uint32_t stream_type,
+    const StabilityReport& report)
+    : crashpad::MinidumpUserExtensionStreamDataSource(stream_type) {
+  if (!report.SerializeToString(&data_))
+    data_.clear();
+}
+
+size_t BufferExtensionStreamDataSource::StreamDataSize() {
+  return data_.size();
+}
+
+bool BufferExtensionStreamDataSource::ReadStreamData(Delegate* delegate) {
+  return delegate->ExtensionStreamDataSourceRead(
+      data_.size() ? data_.data() : nullptr, data_.size());
+}
+
+std::unique_ptr<BufferExtensionStreamDataSource> CollectReport(
+    const base::FilePath& path) {
+  StabilityReport report;
+  CollectionStatus status = Extract(path, &report);

[Sigurður Ásgeirsson, 2017/05/11 15:33:40]

You can use histograms here to make sure there's a record of failures. The
Crashpad handler metrics hookup will provide.

[manzagop (departed), 2017/05/11 19:14:53]

Done.

+  if (status != SUCCESS)
+    return nullptr;
+
+  // Open (with delete) and then immediately close the file by going out of
+  // scope. This should cause the stability debugging file to be deleted prior
+  // to the next execution.
+  // TODO(manzagop): set the persistent allocator file's state to deleted in
+  //     case the file can't be deleted.
+  base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ |

[Sigurður Ásgeirsson, 2017/05/11 15:33:40]

also leave a trace of failure here?

[manzagop (departed), 2017/05/11 19:14:53]

Done.

+                            base::File::FLAG_DELETE_ON_CLOSE);
+
+  return base::WrapUnique(
+      new BufferExtensionStreamDataSource(kStabilityReportStreamType, report));
+}
+
+}  // namespace
+
+StabilityReportUserStreamDataSource::StabilityReportUserStreamDataSource(
+    const base::string16& user_data_dir)
+    : user_data_dir_(user_data_dir) {}
+
+std::unique_ptr<crashpad::MinidumpUserExtensionStreamDataSource>
+StabilityReportUserStreamDataSource::ProduceStreamData(
+    crashpad::ProcessSnapshot* process_snapshot) {
+  DCHECK(process_snapshot);
+
+  if (user_data_dir_.empty())
+    return nullptr;
+
+  base::FilePath stability_file;
+  GetStabilityFileName(user_data_dir_, process_snapshot, &stability_file);
+  if (!PathExists(stability_file)) {
+    // Either this is not an instrumented process, or the stability file cannot

[Sigurður Ásgeirsson, 2017/05/11 15:33:40]

is this going to be true for e.g. renderers and !browser processes? If so maybe
a quick comment is in order?

[manzagop (departed), 2017/05/11 19:14:53]

Done.

+    // be found.
+    return nullptr;
+  }
+
+  return CollectReport(stability_file);
+}
+
+}  // namespace browser_watcher