OLD | NEW |
---|---|
(Empty) | |
1 // Copyright (c) 2014 The Chromium Authors. All rights reserved. | |
2 // Use of this source code is governed by a BSD-style license that can be | |
3 // found in the LICENSE file. | |
4 | |
5 #include "components/browser_watcher/watcher_metrics_provider_win.h" | |
6 | |
7 #include "base/metrics/sparse_histogram.h" | |
8 #include "base/process/process_handle.h" | |
9 #include "base/strings/string_number_conversions.h" | |
10 #include "base/strings/string_piece.h" | |
11 #include "base/win/registry.h" | |
12 | |
13 namespace browser_watcher { | |
14 | |
15 namespace { | |
16 | |
17 // This function does soft matching on the PID recorded in the key only. | |
18 // Due to PID reuse, the possibility exists that the process that's now live | |
19 // with the given PID is not the same process the data was recorded for. | |
20 // This doesn't matter for the purpose, as eventually the data will be | |
21 // scavenged and reported. | |
22 bool IsDeadProcess(base::StringPiece16 key_name) { | |
23 // Truncate the input string to the first occurrence of '-', if one exists. | |
24 size_t num_end = key_name.find(L'-'); | |
25 if (num_end != base::StringPiece16::npos) | |
26 key_name = key_name.substr(0, num_end); | |
27 | |
28 // Convert to the numeric PID. | |
29 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
| |
30 if (!base::StringToInt(key_name, &pid) || pid == 0) | |
31 return true; | |
32 | |
33 // This is a very inexpensive check for the common case of our own PID. | |
34 if (static_cast<base::ProcessId>(pid) == base::GetCurrentProcId()) | |
35 return false; | |
36 | |
37 // The process is not our own - see whether a process with this PID exists. | |
38 // This is more expensive than the above check, but should also be very rare, | |
39 // as this only happens more than once for a given PID if a user is running | |
40 // multiple Chrome instances concurrently. | |
41 base::ProcessHandle process = base::kNullProcessHandle; | |
42 if (base::OpenProcessHandle(static_cast<base::ProcessId>(pid), &process)) { | |
43 base::CloseProcessHandle(process); | |
44 | |
45 // The fact that it was possible to open the process says it's live. | |
46 return false; | |
47 } | |
48 | |
49 return true; | |
50 } | |
51 | |
52 } // namespace | |
53 | |
54 // 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.
| |
55 const char WatcherMetricsProviderWin::kBrowserExitCodeHistogramName[] = | |
56 "Stability.BrowserExitCodes"; | |
57 | |
58 WatcherMetricsProviderWin::WatcherMetricsProviderWin( | |
59 const base::char16* registry_path) : registry_path_(registry_path) { | |
60 } | |
61 | |
62 WatcherMetricsProviderWin::~WatcherMetricsProviderWin() { | |
63 } | |
64 | |
65 void WatcherMetricsProviderWin::ProvideStabilityMetrics( | |
66 metrics::SystemProfileProto* /* system_profile_proto */) { | |
67 // Note that if there are multiple instances of Chrome running in the same | |
68 // 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.
| |
69 // from both/multiple instances. This ought to be vanishingly rare and will | |
70 // only manifest as low-level "random" noise. To work around this it would be | |
71 // necessary to implement some form of global locking, which is not worth it | |
72 // here. | |
73 base::win::RegKey regkey(HKEY_CURRENT_USER, | |
74 registry_path_.c_str(), | |
75 KEY_QUERY_VALUE | KEY_SET_VALUE); | |
76 | |
77 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.
| |
78 size_t num = regkey.GetValueCount(); | |
79 if (num != 0) { | |
80 // Record the exit codes in a sparse stability histogram, as the range of | |
81 // values used to report failures is large. | |
82 base::HistogramBase* exit_code_histogram = | |
83 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
| |
84 base::HistogramBase::kUmaStabilityHistogramFlag); | |
85 | |
86 for (size_t i = 0; i < num; ++i) { | |
87 base::string16 name; | |
88 if (regkey.GetValueNameAt(i, &name) == ERROR_SUCCESS) { | |
89 DWORD exit_code = 0; | |
90 if (regkey.ReadValueDW(name.c_str(), &exit_code) == ERROR_SUCCESS) { | |
91 // Do not report exit codes for processes that are still live, | |
92 // notably for our own process. | |
93 if (exit_code != STILL_ACTIVE || IsDeadProcess(name)) { | |
94 // Tag the value for deletion. | |
erikwright (departed)
2014/11/18 21:13:36
Obvious.
Sigurður Ásgeirsson
2014/11/18 21:52:37
Done.
| |
95 to_delete.push_back(name); | |
96 exit_code_histogram->Add(exit_code); | |
97 } | |
98 } | |
99 } | |
100 } | |
101 | |
102 // Delete the values reported above. | |
103 for (size_t i = 0; i < to_delete.size(); ++i) | |
104 regkey.DeleteValue(to_delete[i].c_str()); | |
105 } | |
106 } | |
107 | |
108 } // namespace browser_watcher | |
OLD | NEW |