Rework GetStabilityFileForProcess for use by Crashpad integration

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 <string>
 
 #include "base/feature_list.h"
 #include "base/files/file.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"

[Mark Mentovai, 2017/05/04 16:47:10]

You’ll probably need to update a DEPS file to be able to do this too.

[manzagop (departed), 2017/05/04 19:52:41]

components/browser_watcher/DEPS already has "+third_party/crashpad/crashpad"
because it registers postmortem dumps to a crashpad database.

+
 namespace browser_watcher {
 namespace {
 
-bool GetCreationTime(const base::Process& process, base::Time* time) {
-  DCHECK(time);
+const uint64_t kMicrosecondsPerSecond = static_cast<uint64_t>(1E6);

[Mark Mentovai, 2017/05/04 16:47:10]

This is only used in one spot, you can move it to that spot.

Also, it can be constexpr, not just const.

[manzagop (departed), 2017/05/04 19:52:41]

Done.

 
-  FILETIME creation_time = {};
+bool GetCreationTime(const base::Process& process, FILETIME* creation_time) {
+  DCHECK(creation_time);

[Mark Mentovai, 2017/05/04 16:47:10]

It’s OK to not do this and let GetProcessTimes() crash.

[manzagop (departed), 2017/05/04 19:52:41]

Done.

+
   FILETIME ignore1 = {};
   FILETIME ignore2 = {};

[Mark Mentovai, 2017/05/04 16:47:10]

Is it OK to use the same “ignore” each time? We do that in a Crashpad test, and
it seems fine there.

[manzagop (departed), 2017/05/04 19:52:41]

Looks like exit_code_watcher_win.cc does this too.

Done.

   FILETIME ignore3 = {};

[Mark Mentovai, 2017/05/04 16:47:10]

And since you’re just gonna ignore it, there’s no sense in initializing it with
= {}.

[manzagop (departed), 2017/05/04 19:52:41]

Done.

-  if (!::GetProcessTimes(process.Handle(), &creation_time, &ignore1, &ignore2,
-                         &ignore3)) {
-    return false;
-  }
-  *time = base::Time::FromFileTime(creation_time);
-  return true;
+  return ::GetProcessTimes(process.Handle(), creation_time, &ignore1, &ignore2,
+                           &ignore3) != 0;
 }
 
 }  // 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);
+
+  int64_t creation_time_us =
+      creation_time.tv_sec * kMicrosecondsPerSecond + creation_time.tv_usec;
+  std::string file_name = base::StringPrintf("%u-%llu", pid, creation_time_us);
+  return stability_dir.AppendASCII(file_name).AddExtension(
+      base::PersistentMemoryAllocator::kFileExtension);
+}
+
 bool GetStabilityFileForProcess(const base::Process& process,
                                 const base::FilePath& user_data_dir,
                                 base::FilePath* file_path) {
   DCHECK(file_path);
-  base::FilePath stability_dir = GetStabilityDir(user_data_dir);
 
-  // Build the name using the pid and creation time. On windows, this is unique
-  // even after the process exits.
-  base::Time creation_time;
+  FILETIME creation_time;
   if (!GetCreationTime(process, &creation_time))
     return false;
 
-  std::string file_name =
-      base::StringPrintf("%u-%llu", process.Pid(), creation_time.ToJavaTime());
-  *file_path = stability_dir.AppendASCII(file_name).AddExtension(
-      base::PersistentMemoryAllocator::kFileExtension);
+  // We rely on Crashpad's conversion to ensure the resulting filename is the
+  // same as on crash, when the creation time is obtained via Crashpad.
+  *file_path = GetStabilityFileForProcess(
+      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(
@@ skipping 12 matching lines @@
   // 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);
 }
 
 }  // namespace browser_watcher
 
 #endif  // defined(OS_WIN)