Stability instrumentation: metrics for collection on crash

components/browser_watcher/stability_report_user_stream_data_source.cc

Index: components/browser_watcher/stability_report_user_stream_data_source.cc
diff --git a/components/browser_watcher/stability_report_user_stream_data_source.cc b/components/browser_watcher/stability_report_user_stream_data_source.cc
index 62fdf2235edd63531b4c6d53b8985b846af9632d..69bead0c1523e8f826019ec9d1dd435b6c8a990e 100644
--- a/components/browser_watcher/stability_report_user_stream_data_source.cc
+++ b/components/browser_watcher/stability_report_user_stream_data_source.cc
@@ -73,6 +73,20 @@ bool BufferExtensionStreamDataSource::ReadStreamData(Delegate* delegate) {
       data_.size() ? data_.data() : nullptr, data_.size());
 }
 
+// DO NOT CHANGE VALUES. This is logged persistently in a histogram.
+enum class CollectionEvent {

[bcwhite, 2017/05/31 15:41:19]

Do you track failures?  You could have a kAttempts that is counted every time
and the kSuccess, etc. to indicate the things that happened within that attempt.

[manzagop (departed), 2017/05/31 19:57:23]

I've added a bunch of states. Done.

+  kSuccess = 0,

[bcwhite, 2017/05/31 15:41:19]

Omit the numbers and just say "do not insert or remove values because..."

[manzagop (departed), 2017/05/31 19:57:23]

Done.

+  kPmaSetDeletedFailed = 1,
+  kCollectionEventMax = 2
+};
+
+void LogCollectionEvent(CollectionEvent event) {
+  UMA_HISTOGRAM_ENUMERATION("ActivityTracker.CollectCrash.Event", event,
+                            CollectionEvent::kCollectionEventMax);
+}
+
+// TODO(manzagop): collection should factor in whether this is a true crash or
+// dump without crashing.
 std::unique_ptr<BufferExtensionStreamDataSource> CollectReport(
     const base::FilePath& path) {
   StabilityReport report;
@@ -82,15 +96,9 @@ std::unique_ptr<BufferExtensionStreamDataSource> CollectReport(
   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 |
-                            base::File::FLAG_DELETE_ON_CLOSE);
-  UMA_HISTOGRAM_BOOLEAN("ActivityTracker.CollectCrash.OpenForDeleteSuccess",
-                        file.IsValid());
+  if (!MarkStabilityFileDeleted(path))
+    LogCollectionEvent(CollectionEvent::kPmaSetDeletedFailed);
+  LogCollectionEvent(CollectionEvent::kSuccess);
 
   std::unique_ptr<BufferExtensionStreamDataSource> source(
       new BufferExtensionStreamDataSource(kStabilityReportStreamType));