Split captive portal metrics out of SSLErrorClassification

chrome/browser/ssl/bad_clock_blocking_page.cc

 // Copyright 2015 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 "chrome/browser/ssl/bad_clock_blocking_page.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/build_time.h"
 #include "base/callback_helpers.h"
@@ skipping 165 matching lines @@
     const GURL& request_url,
     const base::Time& time_triggered,
     const base::Callback<void(bool)>& callback)
     : SecurityInterstitialPage(web_contents, request_url),
       callback_(callback),
       cert_error_(cert_error),
       ssl_info_(ssl_info),
       time_triggered_(time_triggered) {
   security_interstitials::MetricsHelper::ReportDetails reporting_info;
   reporting_info.metric_prefix = kMetricsName;
-  set_metrics_helper(new ChromeMetricsHelper(web_contents, request_url,
-                                             reporting_info, kMetricsName));
+  scoped_ptr<ChromeMetricsHelper> chrome_metrics_helper(new ChromeMetricsHelper(
+      web_contents, request_url, reporting_info, kMetricsName));
+  chrome_metrics_helper->StartRecordingCaptivePortalMetrics(false);

[estark, 2015/09/25 23:53:35]

For the sake of my understanding: is this measuring cases where the user's clock
is wrong and they see a clock interstitial, but they might also be in a captive
portal?

[felt, 2015/09/27 20:08:15]

Yes. I suppose we don't need to, although we always used to so I figure we ought
to continue for consistency. I'll cc meacer@ and ask him whether he has any
opinion.

+  set_metrics_helper(chrome_metrics_helper.Pass());
   metrics_helper()->RecordUserInteraction(
       security_interstitials::MetricsHelper::TOTAL_VISITS);
 
   // TODO(felt): Separate the clock statistics from the main ssl statistics.
-  scoped_ptr<SSLErrorClassification> classifier(
-      new SSLErrorClassification(web_contents, time_triggered_, request_url,
-                                 cert_error_, *ssl_info_.cert.get()));
-  classifier->RecordUMAStatistics(false);
+  SSLErrorClassification classifier(time_triggered_, request_url, cert_error_,
+                                    *ssl_info_.cert.get());
+  classifier.RecordUMAStatistics(false);
 }
 
 bool BadClockBlockingPage::ShouldCreateNewNavigation() const {
   return true;
 }
 
 InterstitialPageDelegate::TypeID BadClockBlockingPage::GetTypeForTesting()
     const {
   return BadClockBlockingPage::kTypeForTesting;
 }
 
 BadClockBlockingPage::~BadClockBlockingPage() {
+  metrics_helper()->RecordShutdownMetrics();
   if (!callback_.is_null()) {
     // Deny when the page is closed.
     NotifyDenyCertificate();
   }
 }
 
 void BadClockBlockingPage::PopulateInterstitialStrings(
     base::DictionaryValue* load_time_data) {
   CHECK(load_time_data);
   base::string16 url(GetFormattedHostName());
@@ skipping 136 matching lines @@
 
 void BadClockBlockingPage::NotifyDenyCertificate() {
   // It's possible that callback_ may not exist if the user clicks "Proceed"
   // followed by pressing the back button before the interstitial is hidden.
   // In that case the certificate will still be treated as allowed.
   if (callback_.is_null())
     return;
 
   base::ResetAndReturn(&callback_).Run(false);
 }