Index: components/browser_watcher/watcher_metrics_provider_win.cc |
diff --git a/components/browser_watcher/watcher_metrics_provider_win.cc b/components/browser_watcher/watcher_metrics_provider_win.cc |
new file mode 100644 |
index 0000000000000000000000000000000000000000..f4c582ec6754408c596373bcb52ea3dd6c8252ea |
--- /dev/null |
+++ b/components/browser_watcher/watcher_metrics_provider_win.cc |
@@ -0,0 +1,108 @@ |
+// Copyright (c) 2014 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/watcher_metrics_provider_win.h" |
+ |
+#include "base/metrics/sparse_histogram.h" |
+#include "base/process/process_handle.h" |
+#include "base/strings/string_number_conversions.h" |
+#include "base/strings/string_piece.h" |
+#include "base/win/registry.h" |
+ |
+namespace browser_watcher { |
+ |
+namespace { |
+ |
+// This function does soft matching on the PID recorded in the key only. |
+// Due to PID reuse, the possibility exists that the process that's now live |
+// with the given PID is not the same process the data was recorded for. |
+// This doesn't matter for the purpose, as eventually the data will be |
+// scavenged and reported. |
+bool IsDeadProcess(base::StringPiece16 key_name) { |
+ // Truncate the input string to the first occurrence of '-', if one exists. |
+ size_t num_end = key_name.find(L'-'); |
+ if (num_end != base::StringPiece16::npos) |
+ key_name = key_name.substr(0, num_end); |
+ |
+ // Convert to the numeric PID. |
+ int pid = 0; |
erikwright (departed)
2014/11/18 21:13:36
this CL and the other one I noticed you are using
Sigurður Ásgeirsson
2014/11/18 21:52:36
Yeah, in practice the PID space is pretty small th
|
+ if (!base::StringToInt(key_name, &pid) || pid == 0) |
+ return true; |
+ |
+ // This is a very inexpensive check for the common case of our own PID. |
+ if (static_cast<base::ProcessId>(pid) == base::GetCurrentProcId()) |
+ return false; |
+ |
+ // The process is not our own - see whether a process with this PID exists. |
+ // This is more expensive than the above check, but should also be very rare, |
+ // as this only happens more than once for a given PID if a user is running |
+ // multiple Chrome instances concurrently. |
+ base::ProcessHandle process = base::kNullProcessHandle; |
+ if (base::OpenProcessHandle(static_cast<base::ProcessId>(pid), &process)) { |
+ base::CloseProcessHandle(process); |
+ |
+ // The fact that it was possible to open the process says it's live. |
+ return false; |
+ } |
+ |
+ return true; |
+} |
+ |
+} // namespace |
+ |
+// TODO(siggi): is this an acceptable name? |
erikwright (departed)
2014/11/18 21:13:36
Seems reasonable to me.
Sigurður Ásgeirsson
2014/11/18 21:52:37
Acknowledged.
|
+const char WatcherMetricsProviderWin::kBrowserExitCodeHistogramName[] = |
+ "Stability.BrowserExitCodes"; |
+ |
+WatcherMetricsProviderWin::WatcherMetricsProviderWin( |
+ const base::char16* registry_path) : registry_path_(registry_path) { |
+} |
+ |
+WatcherMetricsProviderWin::~WatcherMetricsProviderWin() { |
+} |
+ |
+void WatcherMetricsProviderWin::ProvideStabilityMetrics( |
+ metrics::SystemProfileProto* /* system_profile_proto */) { |
+ // Note that if there are multiple instances of Chrome running in the same |
+ // user account, there's a small race that will double-reporti the exit codes |
erikwright (departed)
2014/11/18 21:13:36
reporti -> report
Sigurður Ásgeirsson
2014/11/18 21:52:37
Done.
|
+ // from both/multiple instances. This ought to be vanishingly rare and will |
+ // only manifest as low-level "random" noise. To work around this it would be |
+ // necessary to implement some form of global locking, which is not worth it |
+ // here. |
+ base::win::RegKey regkey(HKEY_CURRENT_USER, |
+ registry_path_.c_str(), |
+ KEY_QUERY_VALUE | KEY_SET_VALUE); |
+ |
+ std::vector<base::string16> to_delete; |
erikwright (departed)
2014/11/18 21:13:36
move inside the if(num!=0) block
erikwright (departed)
2014/11/18 21:13:36
<vector>
Sigurður Ásgeirsson
2014/11/18 21:52:37
Done.
Sigurður Ásgeirsson
2014/11/18 21:52:37
Done.
|
+ size_t num = regkey.GetValueCount(); |
+ if (num != 0) { |
+ // Record the exit codes in a sparse stability histogram, as the range of |
+ // values used to report failures is large. |
+ base::HistogramBase* exit_code_histogram = |
+ base::SparseHistogram::FactoryGet(kBrowserExitCodeHistogramName, |
erikwright (departed)
2014/11/18 21:13:36
Is this not available using the macros?
Sigurður Ásgeirsson
2014/11/18 21:52:37
This particular variant is not present in the macr
|
+ base::HistogramBase::kUmaStabilityHistogramFlag); |
+ |
+ for (size_t i = 0; i < num; ++i) { |
+ base::string16 name; |
+ if (regkey.GetValueNameAt(i, &name) == ERROR_SUCCESS) { |
+ DWORD exit_code = 0; |
+ if (regkey.ReadValueDW(name.c_str(), &exit_code) == ERROR_SUCCESS) { |
+ // Do not report exit codes for processes that are still live, |
+ // notably for our own process. |
+ if (exit_code != STILL_ACTIVE || IsDeadProcess(name)) { |
+ // Tag the value for deletion. |
erikwright (departed)
2014/11/18 21:13:36
Obvious.
Sigurður Ásgeirsson
2014/11/18 21:52:37
Done.
|
+ to_delete.push_back(name); |
+ exit_code_histogram->Add(exit_code); |
+ } |
+ } |
+ } |
+ } |
+ |
+ // Delete the values reported above. |
+ for (size_t i = 0; i < to_delete.size(); ++i) |
+ regkey.DeleteValue(to_delete[i].c_str()); |
+ } |
+} |
+ |
+} // namespace browser_watcher |