Added histograms on process singleton create when remote process exists and we cannot notify it

chrome/browser/process_singleton_win.cc

Index: chrome/browser/process_singleton_win.cc
diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc
index 2bb2b68581e222c0df07c23a401dc216534ce16a..3b4a3e747083584230f5455f5d29521889863d10 100644
--- a/chrome/browser/process_singleton_win.cc
+++ b/chrome/browser/process_singleton_win.cc
@@ -11,6 +11,7 @@
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/debug/activity_tracker.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
@@ -178,6 +179,50 @@ bool DisplayShouldKillMessageBox() {
          chrome::MESSAGE_BOX_RESULT_NO;
 }
 
+// Function was copied from Process::Terminate.
+void TerminateProcessWithHistograms(const base::Process& process,
+                                    int exit_code) {
+  DCHECK(process.IsValid());
+  base::TimeTicks start_time = base::TimeTicks::Now();
+  bool result = (::TerminateProcess(process.Handle(), exit_code) != FALSE);
+  DWORD terminate_error = 0;
+  if (result) {
+    DWORD wait_error = 0;
+    // The process may not end immediately due to pending I/O
+    DWORD wait_result = ::WaitForSingleObject(process.Handle(), 60 * 1000);
+    if (wait_result != WAIT_OBJECT_0) {
+      if (wait_result == WAIT_FAILED)
+        wait_error = ::GetLastError();
+      UMA_HISTOGRAM_ENUMERATION(
+          "Chrome.ProcessSingleton.RemoteProcessInteractionResult",
+          ProcessSingleton::TERMINATE_WAIT_TIMEOUT,
+          ProcessSingleton::MAX_HUNG_REMOTE_PROCESS_ACTION);
+      DPLOG(ERROR) << "Error waiting for process exit";
+    } else {
+      UMA_HISTOGRAM_ENUMERATION(
+          "Chrome.ProcessSingleton.RemoteProcessInteractionResult",
+          ProcessSingleton::TERMINATE_SUCCEEDED,
+          ProcessSingleton::MAX_HUNG_REMOTE_PROCESS_ACTION);
+    }
+    UMA_HISTOGRAM_TIMES("Chrome.ProcessSingleton.TerminateProcessTime",
+                        base::TimeTicks::Now() - start_time);
+    UMA_HISTOGRAM_SPARSE_SLOWLY(
+        "Chrome.ProcessSingleton.TerminationWaitErrorCode.Windows", wait_error);
+    base::debug::GlobalActivityTracker::RecordProcessExitIfEnabled(
+        process.Pid(), exit_code);
+  } else {
+    terminate_error = ::GetLastError();
+    UMA_HISTOGRAM_ENUMERATION(
+        "Chrome.ProcessSingleton.RemoteProcessInteractionResult",
+        ProcessSingleton::TERMINATE_FAILED,
+        ProcessSingleton::MAX_HUNG_REMOTE_PROCESS_ACTION);
+    DPLOG(ERROR) << "Unable to terminate process";
+  }
+  UMA_HISTOGRAM_SPARSE_SLOWLY(
+      "Chrome.ProcessSingleton.ProcessTerminateErrorCode.Windows",

[gab, 2017/05/10 15:16:50]

These histograms will be examined filtered per platform anyways, so I don't
think we need different names.

Just like say "Chrome.ProcessSingleton.RemoteProcessInteractionResult" doesn't
mean the same thing on each platform.

[Alexey Seren, 2017/05/11 13:42:05]

Unfortunately we cannot do it because each platform specific histogram has it
own enum (see histograms.xml):


<histogram name="Chrome.ProcessSingleton.TerminateProcessErrorCode.Posix"
    enum="OSAgnosticErrno">
  <owner>aseren@yandex-team.ru</owner>
  <summary>
    The error code of remote process termination on Posix in case when remote
    process hung.
  </summary>
</histogram>

<histogram name="Chrome.ProcessSingleton.TerminateProcessErrorCode.Windows"
    enum="WinGetLastError">
  <owner>aseren@yandex-team.ru</owner>
  <summary>
    The error code of remote process termination on Windows in case when remote
    process hung.
  </summary>
</histogram>

[gab, 2017/05/11 15:10:03]

s/OSAgnosticErrno/PosixErrno/ (maybe there's already such a suffix in
histograms.xml? I didn't look...)

+      terminate_error);
+}
+
 }  // namespace
 
 // Microsoft's Softricity virtualization breaks the sandbox processes.
@@ -242,6 +287,9 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
       return PROCESS_NOTIFIED;
     case chrome::NOTIFY_FAILED:
       remote_window_ = NULL;
+      UMA_HISTOGRAM_ENUMERATION(
+          "Chrome.ProcessSingleton.RemoteProcessInteractionResult",
+          RUNNING_PROCESS_NOTIFY_FAILED, MAX_HUNG_REMOTE_PROCESS_ACTION);
       return PROCESS_NONE;
     case chrome::NOTIFY_WINDOW_HUNG:
       // Fall through and potentially terminate the hung browser.
@@ -252,6 +300,9 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
   DWORD process_id = 0;
   DWORD thread_id = ::GetWindowThreadProcessId(remote_window_, &process_id);
   if (!thread_id || !process_id) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "Chrome.ProcessSingleton.RemoteProcessInteractionResult",
+        REMOTE_PROCESS_NOT_FOUND, MAX_HUNG_REMOTE_PROCESS_ACTION);
     remote_window_ = NULL;
     return PROCESS_NONE;
   }
@@ -272,9 +323,14 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
     // The user denied. Quit silently.
     return PROCESS_NOTIFIED;
   }
+  UMA_HISTOGRAM_ENUMERATION(
+      "Chrome.ProcessSingleton.RemoteHungProcessTerminateReason",
+      visible_window ? USER_ACCEPTED_TERMINATION : NO_VISIBLE_WINDOW_FOUND,
+      MAX_REMOTE_HUNG_PROCESS_TERMINATE_REASON);
 
   // Time to take action. Kill the browser process.
-  process.Terminate(content::RESULT_CODE_HUNG, true);
+  TerminateProcessWithHistograms(process, content::RESULT_CODE_HUNG);
+
   remote_window_ = NULL;
   return PROCESS_NONE;
 }