Split captive portal metrics out of SSLErrorClassification

chrome/browser/ssl/bad_clock_blocking_page.cc

Index: chrome/browser/ssl/bad_clock_blocking_page.cc
diff --git a/chrome/browser/ssl/bad_clock_blocking_page.cc b/chrome/browser/ssl/bad_clock_blocking_page.cc
index 2d89b105e6fc2f119465d7cec04f444c156c2b17..655f6deeea2e90db2bed7b911482fd1ab37a437b 100644
--- a/chrome/browser/ssl/bad_clock_blocking_page.cc
+++ b/chrome/browser/ssl/bad_clock_blocking_page.cc
@@ -183,16 +183,17 @@ BadClockBlockingPage::BadClockBlockingPage(
       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 {
@@ -205,6 +206,7 @@ InterstitialPageDelegate::TypeID BadClockBlockingPage::GetTypeForTesting()
 }
 
 BadClockBlockingPage::~BadClockBlockingPage() {
+  metrics_helper()->RecordShutdownMetrics();
   if (!callback_.is_null()) {
     // Deny when the page is closed.
     NotifyDenyCertificate();