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

Unified Diff: chrome/browser/shell_integration.cc

Issue 1426663005: Make the histograms for setting the default browser consistent (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fixed name collision Created 5 years, 1 month 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
« no previous file with comments | « chrome/browser/shell_integration.h ('k') | chrome/browser/shell_integration_win.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/shell_integration.cc
diff --git a/chrome/browser/shell_integration.cc b/chrome/browser/shell_integration.cc
index d8c5b8dcf38352aab3c7937c875c333fd463e3f6..c1f83461247867c57c29bcd66914515d18c8240f 100644
--- a/chrome/browser/shell_integration.cc
+++ b/chrome/browser/shell_integration.cc
@@ -10,6 +10,7 @@
#include "base/metrics/histogram_macros.h"
#include "base/prefs/pref_service.h"
#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_restrictions.h"
#include "base/timer/timer.h"
@@ -176,12 +177,12 @@ ShellIntegration::DefaultWebClientWorker::DefaultWebClientWorker(
: observer_(observer) {}
void ShellIntegration::DefaultWebClientWorker::StartCheckIsDefault() {
- if (observer_) {
+ if (observer_)
observer_->SetDefaultWebClientUIState(STATE_PROCESSING);
- BrowserThread::PostTask(
- BrowserThread::FILE, FROM_HERE,
- base::Bind(&DefaultWebClientWorker::CheckIsDefault, this));
- }
+
+ BrowserThread::PostTask(
+ BrowserThread::FILE, FROM_HERE,
+ base::Bind(&DefaultWebClientWorker::CheckIsDefault, this));
}
void ShellIntegration::DefaultWebClientWorker::StartSetAsDefault() {
@@ -196,15 +197,14 @@ void ShellIntegration::DefaultWebClientWorker::StartSetAsDefault() {
}
set_as_default_in_progress_ = true;
- bool interactive_permitted = false;
+ bool interactive_permitted = true;
if (observer_) {
observer_->SetDefaultWebClientUIState(STATE_PROCESSING);
interactive_permitted = observer_->IsInteractiveSetDefaultPermitted();
-
- // The initialization is only useful when there is an observer.
- set_as_default_initialized_ = InitializeSetAsDefault();
}
+ set_as_default_initialized_ = InitializeSetAsDefault();
+
// Remember the start time.
start_time_ = base::TimeTicks::Now();
@@ -237,6 +237,15 @@ void ShellIntegration::DefaultWebClientWorker::OnCheckIsDefaultComplete(
DefaultWebClientState state) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
UpdateUI(state);
+
+ if (check_default_should_report_success_) {
+ check_default_should_report_success_ = false;
+
+ ReportAttemptResult(state == DefaultWebClientState::IS_DEFAULT
+ ? AttemptResult::SUCCESS
+ : AttemptResult::NO_ERRORS_NOT_DEFAULT);
+ }
+
// The worker has finished everything it needs to do, so free the observer
// if we own it.
if (observer_ && observer_->IsOwnedByWorker()) {
@@ -250,7 +259,7 @@ void ShellIntegration::DefaultWebClientWorker::OnSetAsDefaultAttemptComplete(
DCHECK_CURRENTLY_ON(BrowserThread::UI);
// Hold on to a reference because if this was called via the default browser
// callback in StartupBrowserCreator, clearing the callback in
- // FinalizeSetAsDefault would otherwise remove the last reference and delete
+ // FinalizeSetAsDefault() would otherwise remove the last reference and delete
// us in the middle of this function.
scoped_refptr<DefaultWebClientWorker> scoped_ref(this);
@@ -267,7 +276,11 @@ void ShellIntegration::DefaultWebClientWorker::OnSetAsDefaultAttemptComplete(
observer_->OnSetAsDefaultConcluded(succeeded);
}
- ReportAttemptResult(result);
+ // Report failures here. Successes are reported in
+ // OnCheckIsDefaultComplete() after checking that the change is verified.
+ check_default_should_report_success_ = result == AttemptResult::SUCCESS;
+ if (!check_default_should_report_success_)
+ ReportAttemptResult(result);
// Start the default browser check which will notify the observer as to
// whether Chrome is really the default browser. This is needed because
@@ -280,35 +293,24 @@ void ShellIntegration::DefaultWebClientWorker::OnSetAsDefaultAttemptComplete(
void ShellIntegration::DefaultWebClientWorker::ReportAttemptResult(
AttemptResult result) {
- if (!ShouldReportAttemptResults())
- return;
-
- UMA_HISTOGRAM_ENUMERATION("DefaultBrowser.AsyncSetAsDefault.Result", result,
- AttemptResult::NUM_ATTEMPT_RESULT_TYPES);
-
- switch (result) {
- case SUCCESS:
- UMA_HISTOGRAM_MEDIUM_TIMES(
- "DefaultBrowser.AsyncSetAsDefault.Duration_Success",
- base::TimeTicks::Now() - start_time_);
- break;
- case FAILURE:
- UMA_HISTOGRAM_MEDIUM_TIMES(
- "DefaultBrowser.AsyncSetAsDefault.Duration_Failure",
- base::TimeTicks::Now() - start_time_);
- break;
- case ABANDONED:
- UMA_HISTOGRAM_MEDIUM_TIMES(
- "DefaultBrowser.AsyncSetAsDefault.Duration_Abandoned",
- base::TimeTicks::Now() - start_time_);
- break;
- case RETRY:
- UMA_HISTOGRAM_MEDIUM_TIMES(
- "DefaultBrowser.AsyncSetAsDefault.Duration_Retry",
- base::TimeTicks::Now() - start_time_);
- break;
- default:
- break;
+ const char* histogram_prefix = GetHistogramPrefix();
+
+ // Report result.
+ base::LinearHistogram::FactoryGet(
+ base::StringPrintf("%s.SetDefaultResult", histogram_prefix), 1,
+ AttemptResult::NUM_ATTEMPT_RESULT_TYPES,
+ AttemptResult::NUM_ATTEMPT_RESULT_TYPES + 1,
+ base::HistogramBase::kUmaTargetedHistogramFlag)
+ ->Add(result);
+
+ // Report asynchronous duration.
+ if (IsSetAsDefaultAsynchronous() && ShouldReportDurationForResult(result)) {
+ base::Histogram::FactoryTimeGet(
+ base::StringPrintf("%s.SetDefaultAsyncDuration_%s", histogram_prefix,
+ AttemptResultToString(result)),
+ base::TimeDelta::FromMilliseconds(10), base::TimeDelta::FromMinutes(3),
+ 50, base::HistogramBase::kUmaTargetedHistogramFlag)
+ ->AddTime(base::TimeTicks::Now() - start_time_);
}
}
@@ -318,13 +320,6 @@ bool ShellIntegration::DefaultWebClientWorker::InitializeSetAsDefault() {
void ShellIntegration::DefaultWebClientWorker::FinalizeSetAsDefault() {}
-#if !defined(OS_WIN)
-// static
-bool ShellIntegration::DefaultWebClientWorker::ShouldReportAttemptResults() {
- return false;
-}
-#endif // !defined(OS_WIN)
-
void ShellIntegration::DefaultWebClientWorker::UpdateUI(
DefaultWebClientState state) {
if (observer_) {
@@ -344,6 +339,40 @@ void ShellIntegration::DefaultWebClientWorker::UpdateUI(
}
}
+// static
+bool ShellIntegration::DefaultWebClientWorker::ShouldReportDurationForResult(
+ AttemptResult result) {
+ return result == SUCCESS || result == FAILURE || result == ABANDONED ||
+ result == RETRY;
+}
+
+// static
+const char* ShellIntegration::DefaultWebClientWorker::AttemptResultToString(
+ AttemptResult result) {
+ switch (result) {
+ case SUCCESS:
+ return "Success";
+ case ALREADY_DEFAULT:
+ return "AlreadyDefault";
+ case FAILURE:
+ return "Failure";
+ case ABANDONED:
+ return "Abandoned";
+ case LAUNCH_FAILURE:
+ return "LaunchFailure";
+ case OTHER_WORKER:
+ return "OtherWorker";
+ case RETRY:
+ return "Retry";
+ case NO_ERRORS_NOT_DEFAULT:
+ return "NoErrorsNotDefault";
+ case NUM_ATTEMPT_RESULT_TYPES:
+ break;
+ }
+ NOTREACHED();
+ return "";
+}
+
///////////////////////////////////////////////////////////////////////////////
// ShellIntegration::DefaultBrowserWorker
//
@@ -408,6 +437,10 @@ void ShellIntegration::DefaultBrowserWorker::SetAsDefault(
result));
}
+const char* ShellIntegration::DefaultBrowserWorker::GetHistogramPrefix() {
+ return "DefaultBrowser";
+}
+
///////////////////////////////////////////////////////////////////////////////
// ShellIntegration::DefaultProtocolClientWorker
//
@@ -457,3 +490,8 @@ void ShellIntegration::DefaultProtocolClientWorker::SetAsDefault(
base::Bind(&DefaultProtocolClientWorker::OnSetAsDefaultAttemptComplete,
this, result));
}
+
+const char*
+ShellIntegration::DefaultProtocolClientWorker::GetHistogramPrefix() {
+ return "DefaultProtocolClient";
+}
« no previous file with comments | « chrome/browser/shell_integration.h ('k') | chrome/browser/shell_integration_win.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698