Postmortem report collection: validate internal state

base/debug/activity_analyzer.cc

Index: base/debug/activity_analyzer.cc
diff --git a/base/debug/activity_analyzer.cc b/base/debug/activity_analyzer.cc
index 3c672341dd0794754a20bd960712ebe381837a0c..561f1cc32315142f88b3a29941e827638aeac85f 100644
--- a/base/debug/activity_analyzer.cc
+++ b/base/debug/activity_analyzer.cc
@@ -5,6 +5,7 @@
 #include "base/debug/activity_analyzer.h"
 
 #include <algorithm>
+#include <utility>
 
 #include "base/files/file.h"
 #include "base/files/file_path.h"
@@ -12,6 +13,7 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 
@@ -21,6 +23,24 @@ namespace debug {
 namespace {
 // An empty snapshot that can be returned when there otherwise is none.
 LazyInstance<ActivityUserData::Snapshot>::Leaky g_empty_user_data_snapshot;
+
+#if !defined(OS_NACL)
+// DO NOT CHANGE VALUES. This is logged persistently in a histogram.
+enum AnalyzerCreationError {
+  kInvalidMemoryMappedFile = 0,

[bcwhite, 2017/04/19 18:54:07]

It's standard practice not to specify the numerical values.  Noting that it's
only okay to append (before "max") is sufficient.

[manzagop (departed), 2017/04/19 19:26:57]

Done.

+  kPmaBadFile = 1,
+  kPmaUninitialized = 2,
+  kPmaDeleted = 3,
+  kPmaCorrupt = 4,
+  kAnalyzerCreationErrorMax = 5
+};
+
+void LogAnalyzerCreationError(AnalyzerCreationError error) {
+  UMA_HISTOGRAM_ENUMERATION("ActivityTracker.Collect.AnalyzerCreationError",
+                            error, kAnalyzerCreationErrorMax);
+}
+#endif  // !defined(OS_NACL)
+
 }  // namespace
 
 ThreadActivityAnalyzer::Snapshot::Snapshot() {}
@@ -63,7 +83,9 @@ void ThreadActivityAnalyzer::AddGlobalInformation(
 
 GlobalActivityAnalyzer::GlobalActivityAnalyzer(
     std::unique_ptr<PersistentMemoryAllocator> allocator)
-    : allocator_(std::move(allocator)), allocator_iterator_(allocator_.get()) {}
+    : allocator_(std::move(allocator)), allocator_iterator_(allocator_.get()) {
+  DCHECK(allocator_);
+}
 
 GlobalActivityAnalyzer::~GlobalActivityAnalyzer() {}
 
@@ -75,15 +97,35 @@ std::unique_ptr<GlobalActivityAnalyzer> GlobalActivityAnalyzer::CreateWithFile(
   // the analyzer and any trackers that my still be active.
   std::unique_ptr<MemoryMappedFile> mmfile(new MemoryMappedFile());
   mmfile->Initialize(file_path, MemoryMappedFile::READ_WRITE);
-  if (!mmfile->IsValid())
+  if (!mmfile->IsValid()) {
+    LogAnalyzerCreationError(kInvalidMemoryMappedFile);
     return nullptr;
+  }
 
-  if (!FilePersistentMemoryAllocator::IsFileAcceptable(*mmfile, true))
+  if (!FilePersistentMemoryAllocator::IsFileAcceptable(*mmfile, true)) {
+    LogAnalyzerCreationError(kPmaBadFile);
     return nullptr;
+  }
 
-  return WrapUnique(
-      new GlobalActivityAnalyzer(MakeUnique<FilePersistentMemoryAllocator>(
-          std::move(mmfile), 0, 0, base::StringPiece(), true)));
+  std::unique_ptr<FilePersistentMemoryAllocator> allocator(
+      new FilePersistentMemoryAllocator(std::move(mmfile), 0, 0,
+                                        base::StringPiece(), true));
+  if (allocator->GetMemoryState() ==
+      PersistentMemoryAllocator::MEMORY_UNINITIALIZED) {
+    LogAnalyzerCreationError(kPmaUninitialized);
+    return nullptr;
+  }
+  if (allocator->GetMemoryState() ==
+      PersistentMemoryAllocator::MEMORY_DELETED) {
+    LogAnalyzerCreationError(kPmaDeleted);
+    return nullptr;
+  }
+  if (allocator->IsCorrupt()) {
+    LogAnalyzerCreationError(kPmaCorrupt);
+    return nullptr;
+  }
+
+  return WrapUnique(new GlobalActivityAnalyzer(std::move(allocator)));
 }
 #endif  // !defined(OS_NACL)
 
@@ -215,6 +257,11 @@ GlobalActivityAnalyzer::GetProgramLocationFromAddress(uint64_t address) {
   return { 0, 0 };
 }
 
+bool GlobalActivityAnalyzer::IsDataComplete() const {
+  DCHECK(allocator_);
+  return !allocator_->IsFull();
+}
+
 GlobalActivityAnalyzer::UserDataSnapshot::UserDataSnapshot() {}
 GlobalActivityAnalyzer::UserDataSnapshot::UserDataSnapshot(
     const UserDataSnapshot& rhs) = default;