Make the histograms for setting the default browser consistent

chrome/browser/shell_integration.cc

Index: chrome/browser/shell_integration.cc
diff --git a/chrome/browser/shell_integration.cc b/chrome/browser/shell_integration.cc
index d8c5b8dcf38352aab3c7937c875c333fd463e3f6..49b1a6b53a1dde21567511fef3ca46ed822529c7 100644
--- a/chrome/browser/shell_integration.cc
+++ b/chrome/browser/shell_integration.cc
@@ -178,10 +178,10 @@ ShellIntegration::DefaultWebClientWorker::DefaultWebClientWorker(
 void ShellIntegration::DefaultWebClientWorker::StartCheckIsDefault() {
   if (observer_) {

[grt (UTC plus 2), 2015/11/03 21:12:21]

nit: omit braces

[Patrick Monette, 2015/11/10 19:22:47]

Done.

     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 +196,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 +236,17 @@ void ShellIntegration::DefaultWebClientWorker::OnCheckIsDefaultComplete(
     DefaultWebClientState state) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   UpdateUI(state);
+
+#if defined(OS_WIN)

[grt (UTC plus 2), 2015/11/03 21:12:21]

how about doing this for all platforms?

[Patrick Monette, 2015/11/10 19:22:47]

Done.

+  if (check_default_should_report_success_) {
+    check_default_should_report_success_ = false;
+
+    ReportAttemptResult(state == DefaultWebClientState::IS_DEFAULT
+                            ? AttemptResult::SUCCESS
+                            : AttemptResult::NO_ERRORS_NOT_DEFAULT);
+  }
+#endif  // defined(OS_WIN)
+
   // The worker has finished everything it needs to do, so free the observer
   // if we own it.
   if (observer_ && observer_->IsOwnedByWorker()) {
@@ -267,7 +277,12 @@ void ShellIntegration::DefaultWebClientWorker::OnSetAsDefaultAttemptComplete(
       observer_->OnSetAsDefaultConcluded(succeeded);
     }
 
-    ReportAttemptResult(result);
+    // We don't report a success yet because we have to make sure Chrome is the

[grt (UTC plus 2), 2015/11/03 21:12:21]

nit: avoid "we" in comments. how about "Report failures here. Successes are
reported in OnCheckIsDefaultComplete after checking that the change is
verified." or something like that.

[Patrick Monette, 2015/11/10 19:22:47]

Done.

+    // default browser first.
+    check_default_should_report_success_ = result == AttemptResult::SUCCESS;
+    if (!check_default_should_report_success_) {

[grt (UTC plus 2), 2015/11/03 21:12:21]

nit: omit braces

[Patrick Monette, 2015/11/10 19:22:47]

Done.

+      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
@@ -283,32 +298,35 @@ void ShellIntegration::DefaultWebClientWorker::ReportAttemptResult(
   if (!ShouldReportAttemptResults())
     return;
 
-  UMA_HISTOGRAM_ENUMERATION("DefaultBrowser.AsyncSetAsDefault.Result", result,
+  UMA_HISTOGRAM_ENUMERATION("SetDefaultBrowser.Result", result,

[grt (UTC plus 2), 2015/11/03 21:12:21]

how about recording this for all platforms? is there a downside?

[Patrick Monette, 2015/11/10 19:22:47]

Done.

                             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;
+  if (IsSetAsDefaultAsynchronous()) {
+    switch (result) {

[grt (UTC plus 2), 2015/11/03 21:12:21]

    const base::TimeTicks delta = base::TimeTicks::Now() - start_time_;

[Patrick Monette, 2015/11/10 19:22:47]

Done.

+      case SUCCESS:
+        UMA_HISTOGRAM_MEDIUM_TIMES("SetDefaultBrowser.AsyncDuration_Success",
+                                   base::TimeTicks::Now() - start_time_);

[grt (UTC plus 2), 2015/11/03 21:12:21]

delta

[Patrick Monette, 2015/11/10 19:22:47]

Done.

+        break;
+      case FAILURE:
+        UMA_HISTOGRAM_MEDIUM_TIMES("SetDefaultBrowser.AsyncDuration_Failure",
+                                   base::TimeTicks::Now() - start_time_);
+        break;
+      case ABANDONED:
+        UMA_HISTOGRAM_MEDIUM_TIMES("SetDefaultBrowser.AsyncDuration_Abandoned",
+                                   base::TimeTicks::Now() - start_time_);
+        break;
+      case RETRY:
+        UMA_HISTOGRAM_MEDIUM_TIMES("SetDefaultBrowser.AsyncDuration_Retry",
+                                   base::TimeTicks::Now() - start_time_);
+        break;
+      case NO_ERRORS_NOT_DEFAULT:
+        UMA_HISTOGRAM_MEDIUM_TIMES(
+            "SetDefaultBrowser.AsyncDuration_NoErrorsNotDefault",
+            base::TimeTicks::Now() - start_time_);
+        break;
+      default:
+        break;
+    }
   }
 }
 
@@ -318,12 +336,14 @@ bool ShellIntegration::DefaultWebClientWorker::InitializeSetAsDefault() {
 
 void ShellIntegration::DefaultWebClientWorker::FinalizeSetAsDefault() {}
 
-#if !defined(OS_WIN)
 // static
 bool ShellIntegration::DefaultWebClientWorker::ShouldReportAttemptResults() {
+#if defined(OS_WIN)
+  return true;
+#else  // !defined(OS_WIN)
   return false;
+#endif
 }
-#endif  // !defined(OS_WIN)
 
 void ShellIntegration::DefaultWebClientWorker::UpdateUI(
     DefaultWebClientState state) {