Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(111)

Unified Diff: components/browser_watcher/watcher_metrics_provider_win.cc

Issue 792163002: Add ExitFunnel to prepare for instrumenting browser exits. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@lkgr
Patch Set: Address Erik's comments. Created 6 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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
index c6863f7723b6cbd2dca5baeafa35d6b983e77385..9ef6e6c53d2d81ca981f12e6a414dc2d1633566a 100644
--- a/components/browser_watcher/watcher_metrics_provider_win.cc
+++ b/components/browser_watcher/watcher_metrics_provider_win.cc
@@ -4,12 +4,14 @@
#include "components/browser_watcher/watcher_metrics_provider_win.h"
+#include <limits>
#include <vector>
#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/strings/utf_string_conversions.h"
#include "base/win/registry.h"
namespace browser_watcher {
@@ -60,10 +62,155 @@ bool IsDeadProcess(base::StringPiece16 key_name) {
return true;
}
+void RecordExitCodes(const base::string16& registry_path) {
+ base::win::RegKey regkey(HKEY_CURRENT_USER,
+ registry_path.c_str(),
+ KEY_QUERY_VALUE | KEY_SET_VALUE);
+ if (!regkey.Valid())
+ return;
+
+ size_t num = regkey.GetValueCount();
+ if (num == 0)
+ return;
+ std::vector<base::string16> to_delete;
+
+ // 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(
+ WatcherMetricsProviderWin::kBrowserExitCodeHistogramName,
+ base::HistogramBase::kUmaStabilityHistogramFlag);
+
+ for (size_t i = 0; i < num; ++i) {
+ base::string16 name;
+ if (regkey.GetValueNameAt(static_cast<int>(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)) {
+ 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());
+}
+
+void ReadSingleExitFunnel(
+ base::win::RegKey* parent_key, const base::char16* name,
+ std::vector<std::pair<base::string16, int64>>* events_out) {
+ DCHECK(parent_key);
+ DCHECK(name);
+ DCHECK(events_out);
+
+ base::win::RegKey regkey(parent_key->Handle(), name, KEY_READ | KEY_WRITE);
+ if (!regkey.Valid())
+ return;
+
+ // Exit early if no work to do.
+ size_t num = regkey.GetValueCount();
+ if (num == 0)
+ return;
+
+ // Enumerate the recorded events for this process for processing.
+ std::vector<std::pair<base::string16, int64>> events;
+ for (size_t i = 0; i < num; ++i) {
+ base::string16 event_name;
+ LONG res = regkey.GetValueNameAt(static_cast<int>(i), &event_name);
+ if (res == ERROR_SUCCESS) {
+ int64 event_time = 0;
+ res = regkey.ReadInt64(event_name.c_str(), &event_time);
+ if (res == ERROR_SUCCESS)
+ events.push_back(std::make_pair(event_name, event_time));
+ }
+ }
+
+ // Attempt to delete the values before reporting anything.
+ // Exit if this fails to make sure there is no double-reporting on e.g.
+ // permission problems or other corruption.
+ for (size_t i = 0; i < events.size(); ++i) {
+ const base::string16& event_name = events[i].first;
+ LONG res = regkey.DeleteValue(event_name.c_str());
+ if (res != ERROR_SUCCESS) {
+ LOG(ERROR) << "Failed to delete value " << event_name;
+ return;
+ }
+ }
+
+ events_out->swap(events);
+}
+
+void RecordSingleExitFunnel(base::win::RegKey* parent_key,
+ const base::char16* name) {
+ std::vector<std::pair<base::string16, int64>> events;
+ ReadSingleExitFunnel(parent_key, name, &events);
+
+ // Find the earliest event time.
+ int64 min_time = std::numeric_limits<int64>::max();
+ for (size_t i = 0; i < events.size(); ++i)
+ min_time = std::min(min_time, events[i].second);
+
+ // Record the exit funnel event times in a sparse stability histogram.
+ for (size_t i = 0; i < events.size(); ++i) {
+ std::string histogram_name(
+ WatcherMetricsProviderWin::kExitFunnelHistogramPrefix);
+ histogram_name.append(base::WideToUTF8(events[i].first));
+ base::TimeDelta event_time =
+ base::Time::FromInternalValue(events[i].second) -
+ base::Time::FromInternalValue(min_time);
+ base::HistogramBase* histogram =
+ base::SparseHistogram::FactoryGet(
+ histogram_name.c_str(),
+ base::HistogramBase::kUmaStabilityHistogramFlag);
+
+ // Record the time rounded up to the nearest millisecond.
+ histogram->Add(event_time.InMillisecondsRoundedUp());
+ }
+}
+
+void RecordExitFunnels(const base::string16& registry_path) {
+ base::win::RegistryKeyIterator it(HKEY_CURRENT_USER, registry_path.c_str());
+ if (!it.Valid())
+ return;
+
+ // Exit early if no work to do.
+ if (it.SubkeyCount() == 0)
+ return;
+
+ // Open the key we use for deletion preemptively to prevent reporting
+ // multiple times on permission problems.
+ base::win::RegKey key(HKEY_CURRENT_USER,
+ registry_path.c_str(),
+ KEY_QUERY_VALUE);
+ if (!key.Valid()) {
+ LOG(ERROR) << "Failed to open " << registry_path << " for writing.";
+ return;
+ }
+
+ std::vector<base::string16> to_delete;
+ for (; it.Valid(); ++it) {
+ RecordSingleExitFunnel(&key, it.Name());
+ to_delete.push_back(it.Name());
+ }
+
+ for (size_t i = 0; i < to_delete.size(); ++i) {
+ LONG res = key.DeleteEmptyKey(to_delete[i].c_str());
+ if (res != ERROR_SUCCESS)
+ LOG(ERROR) << "Failed to delete key " << to_delete[i];
+ }
+}
+
} // namespace
const char WatcherMetricsProviderWin::kBrowserExitCodeHistogramName[] =
"Stability.BrowserExitCodes";
+const char WatcherMetricsProviderWin::kExitFunnelHistogramPrefix[] =
+ "Stability.ExitFunnel.";
WatcherMetricsProviderWin::WatcherMetricsProviderWin(
const base::char16* registry_path) : registry_path_(registry_path) {
@@ -80,39 +227,8 @@ void WatcherMetricsProviderWin::ProvideStabilityMetrics(
// 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);
-
- size_t num = regkey.GetValueCount();
- if (num != 0) {
- std::vector<base::string16> to_delete;
-
- // 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,
- base::HistogramBase::kUmaStabilityHistogramFlag);
-
- for (size_t i = 0; i < num; ++i) {
- base::string16 name;
- if (regkey.GetValueNameAt(static_cast<int>(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)) {
- 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());
- }
+ RecordExitCodes(registry_path_);
+ RecordExitFunnels(registry_path_);
}
} // namespace browser_watcher

Powered by Google App Engine
This is Rietveld 408576698