Rework GetStabilityFileForProcess for use by Crashpad integration

components/browser_watcher/stability_paths.cc

Index: components/browser_watcher/stability_paths.cc
diff --git a/components/browser_watcher/stability_paths.cc b/components/browser_watcher/stability_paths.cc
index d8764166752db72ebbdf472b75f8e16f2be03889..6f210a1916a6c0205bc07eeac41cc3ae11da22e9 100644
--- a/components/browser_watcher/stability_paths.cc
+++ b/components/browser_watcher/stability_paths.cc
@@ -4,6 +4,10 @@
 
 #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"
@@ -15,22 +19,21 @@
 
 #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.

+
+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 creation_time = {};
   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
@@ -39,22 +42,32 @@ 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;
 }