Stability instrumentation: metrics for collection on crash

components/browser_watcher/stability_paths.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_paths.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #endif  // defined(OS_WIN)
 
+#include <memory>
 #include <string>
+#include <utility>
 
+#include "base/debug/activity_tracker.h"
 #include "base/feature_list.h"
 #include "base/files/file.h"
+#include "base/files/memory_mapped_file.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/metrics/persistent_memory_allocator.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "components/browser_watcher/features.h"
 
 #if defined(OS_WIN)
 
 #include "third_party/crashpad/crashpad/util/win/time.h"
 
 namespace browser_watcher {
+
+using base::FilePersistentMemoryAllocator;
+using base::MemoryMappedFile;
+using base::PersistentMemoryAllocator;
+
 namespace {
 
 bool GetCreationTime(const base::Process& process, FILETIME* creation_time) {
   FILETIME ignore;
   return ::GetProcessTimes(process.Handle(), creation_time, &ignore, &ignore,
                            &ignore) != 0;
 }
 
+bool SetPmaFileDeleted(const base::FilePath& file_path) {
+  // Map the file read-write so it can guarantee consistency between
+  // the analyzer and any trackers that may still be active.
+  std::unique_ptr<MemoryMappedFile> mmfile(new MemoryMappedFile());

[Sigurður Ásgeirsson, 2017/05/31 17:19:05]

Does a MemoryMappedFile require that you allocate it from heap? If not, then why
not a local variable?

[bcwhite, 2017/05/31 18:19:26]

Owership of the object is passed via std::move to the
FilePersistentMemoryAllocator so has to be heap-based.

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

The allocator takes ownership of it at l.53. That said, the allocator doesn't
need to be on heap.

+  mmfile->Initialize(file_path, MemoryMappedFile::READ_WRITE);
+  if (!mmfile->IsValid())
+    return false;
+  if (!FilePersistentMemoryAllocator::IsFileAcceptable(*mmfile, true))
+    return false;
+  std::unique_ptr<FilePersistentMemoryAllocator> allocator(
+      new FilePersistentMemoryAllocator(std::move(mmfile), 0, 0,
+                                        base::StringPiece(), true));
+  allocator->SetMemoryState(PersistentMemoryAllocator::MEMORY_DELETED);
+  return true;
+}
+
 }  // namespace
 
 base::FilePath GetStabilityDir(const base::FilePath& user_data_dir) {
   return user_data_dir.AppendASCII("Stability");
 }
 
 base::FilePath GetStabilityFileForProcess(base::ProcessId pid,
                                           timeval creation_time,
                                           const base::FilePath& user_data_dir) {
   base::FilePath stability_dir = GetStabilityDir(user_data_dir);
@@ skipping 22 matching lines @@
       process.Pid(), crashpad::FiletimeToTimevalEpoch(creation_time),
       user_data_dir);
   return true;
 }
 
 base::FilePath::StringType GetStabilityFilePattern() {
   return base::FilePath::StringType(FILE_PATH_LITERAL("*-*")) +
          base::PersistentMemoryAllocator::kFileExtension;
 }
 
-void MarkStabilityFileForDeletion(const base::FilePath& user_data_dir) {
-  if (!base::FeatureList::IsEnabled(
-          browser_watcher::kStabilityDebuggingFeature)) {
-    return;
-  }
+void MarkOwnStabilityFileDeleted(const base::FilePath& user_data_dir) {
+  base::debug::GlobalActivityTracker* global_tracker =
+      base::debug::GlobalActivityTracker::Get();
+  if (!global_tracker)
+    return;  // No stability instrumentation.
 
-  base::FilePath stability_file;
-  if (!GetStabilityFileForProcess(base::Process::Current(), user_data_dir,
-                                  &stability_file)) {
-    // TODO(manzagop): add a metric for this.
-    return;
-  }
+  global_tracker->MarkDeleted();
 
   // 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.
-  base::File file(stability_file, base::File::FLAG_OPEN |
-                                      base::File::FLAG_READ |
-                                      base::File::FLAG_DELETE_ON_CLOSE);
+  base::FilePath stability_file;
+  if (!GetStabilityFileForProcess(base::Process::Current(), user_data_dir,
+                                  &stability_file)) {
+    return;  // TODO(manzagop): add a metric for this.

[Sigurður Ásgeirsson, 2017/05/31 17:19:05]

is now an opportune moment for addressing this TODO?

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

Done.

+  }
+  base::File deleter(stability_file, base::File::FLAG_OPEN |
+                                         base::File::FLAG_READ |
+                                         base::File::FLAG_DELETE_ON_CLOSE);
+}
+
+bool MarkStabilityFileDeleted(const base::FilePath& file_path) {
+  bool success = SetPmaFileDeleted(file_path);
+  base::File deleter(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ |
+                                    base::File::FLAG_DELETE_ON_CLOSE);
+  return success;
 }
 
 }  // namespace browser_watcher
 
 #endif  // defined(OS_WIN)