Split captive portal metrics out of SSLErrorClassification

SSLErrorClassification can be shared cross-platform if the Chrome-specific
captive portal logic is moved to its own helper. This also is a nice clean
separation of purposes, since the captive portal logic was fairly disjoint
from the other work being done in SSLErrorClassification. Now the captive
portal logic is handled as a metrics helper.

I also made the ownership of the metrics helper clearer by changing to use a
scoped_ptr.

BUG=488673
R=estark@chromium.org
TBR=mattm@chromium.org

Review URL: https://codereview.chromium.org/1365733005

Cr-Commit-Position: refs/heads/master@{#351195}

17 files changed, 249 insertions, 219 deletions

diff --git a/chrome/browser/interstitials/chrome_metrics_helper.cc b/chrome/browser/interstitials/chrome_metrics_helper.cc
index c886e44..abda1aa 100644
--- a/chrome/browser/interstitials/chrome_metrics_helper.cc
+++ b/chrome/browser/interstitials/chrome_metrics_helper.cc
@@ -7,6 +7,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ssl/captive_portal_metrics_recorder.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/rappor/rappor_service.h"
 #include "content/public/browser/web_contents.h"
@@ -35,6 +36,18 @@ ChromeMetricsHelper::ChromeMetricsHelper(
 
 ChromeMetricsHelper::~ChromeMetricsHelper() {}
 
+void ChromeMetricsHelper::StartRecordingCaptivePortalMetrics(bool overridable) {
+  captive_portal_recorder_.reset(
+      new CaptivePortalMetricsRecorder(web_contents_, overridable));
+}
+
+void ChromeMetricsHelper::RecordExtraShutdownMetrics() {
+  // The captive portal metrics should be recorded when the interstitial is
+  // closing (or destructing).
+  if (captive_portal_recorder_)
+    captive_portal_recorder_->RecordCaptivePortalUMAStatistics();
+}
+
 void ChromeMetricsHelper::RecordExtraUserDecisionMetrics(
     security_interstitials::MetricsHelper::Decision decision) {
 #if defined(ENABLE_EXTENSIONS)
diff --git a/chrome/browser/interstitials/chrome_metrics_helper.h b/chrome/browser/interstitials/chrome_metrics_helper.h
index e56e59a..bba24c2 100644
--- a/chrome/browser/interstitials/chrome_metrics_helper.h
+++ b/chrome/browser/interstitials/chrome_metrics_helper.h
@@ -18,9 +18,13 @@ namespace extensions {
 class ExperienceSamplingEvent;
 }
 
+class CaptivePortalMetricsRecorder;
+
 // This class adds desktop-Chrome-specific metrics (extension experience
 // sampling) to the security_interstitials::MetricsHelper. Together, they
 // record UMA, Rappor, and experience sampling metrics.
+
+// This class is meant to be used on the UI thread for captive portal metrics.
 class ChromeMetricsHelper : public security_interstitials::MetricsHelper {
  public:
   ChromeMetricsHelper(
@@ -30,12 +34,15 @@ class ChromeMetricsHelper : public security_interstitials::MetricsHelper {
       const std::string& sampling_event_name);
   ~ChromeMetricsHelper() override;
 
+  void StartRecordingCaptivePortalMetrics(bool overridable);
+
  protected:
   // security_interstitials::MetricsHelper methods:
   void RecordExtraUserDecisionMetrics(
       security_interstitials::MetricsHelper::Decision decision) override;
   void RecordExtraUserInteractionMetrics(
       security_interstitials::MetricsHelper::Interaction interaction) override;
+  void RecordExtraShutdownMetrics() override;
 
  private:
   content::WebContents* web_contents_;
@@ -44,6 +51,7 @@ class ChromeMetricsHelper : public security_interstitials::MetricsHelper {
 #if defined(ENABLE_EXTENSIONS)
   scoped_ptr<extensions::ExperienceSamplingEvent> sampling_event_;
 #endif
+  scoped_ptr<CaptivePortalMetricsRecorder> captive_portal_recorder_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeMetricsHelper);
 };
diff --git a/chrome/browser/interstitials/security_interstitial_page.cc b/chrome/browser/interstitials/security_interstitial_page.cc
index 876f55b..0cc658e 100644
--- a/chrome/browser/interstitials/security_interstitial_page.cc
+++ b/chrome/browser/interstitials/security_interstitial_page.cc
@@ -115,8 +115,8 @@ SecurityInterstitialPage::metrics_helper() const {
 }
 
 void SecurityInterstitialPage::set_metrics_helper(
-    security_interstitials::MetricsHelper* metrics_helper) {
-  metrics_helper_.reset(metrics_helper);
+    scoped_ptr<security_interstitials::MetricsHelper> metrics_helper) {
+  metrics_helper_ = metrics_helper.Pass();
 }
 
 base::string16 SecurityInterstitialPage::GetFormattedHostName() const {
diff --git a/chrome/browser/interstitials/security_interstitial_page.h b/chrome/browser/interstitials/security_interstitial_page.h
index 0bd632b..13c547d 100644
--- a/chrome/browser/interstitials/security_interstitial_page.h
+++ b/chrome/browser/interstitials/security_interstitial_page.h
@@ -99,7 +99,7 @@ class SecurityInterstitialPage : public content::InterstitialPageDelegate {
 
   security_interstitials::MetricsHelper* metrics_helper() const;
   void set_metrics_helper(
-      security_interstitials::MetricsHelper* metrics_helper);
+      scoped_ptr<security_interstitials::MetricsHelper> metrics_helper);
 
  private:
   scoped_ptr<security_interstitials::MetricsHelper> metrics_helper_;
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index c74fb7a..342e59d 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -172,8 +172,11 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
   reporting_info.extra_suffix = GetExtraMetricsSuffix();
   reporting_info.rappor_prefix = GetRapporPrefix();
   reporting_info.rappor_report_type = rappor::SAFEBROWSING_RAPPOR_TYPE;
-  set_metrics_helper(new ChromeMetricsHelper(
-      web_contents, request_url(), reporting_info, GetSamplingEventName()));
+  set_metrics_helper(
+      make_scoped_ptr(new ChromeMetricsHelper(web_contents, request_url(),
+                                              reporting_info,
+                                              GetSamplingEventName()))
+          .Pass());
   metrics_helper()->RecordUserDecision(
       security_interstitials::MetricsHelper::SHOW);
   metrics_helper()->RecordUserInteraction(
diff --git a/chrome/browser/ssl/bad_clock_blocking_page.cc b/chrome/browser/ssl/bad_clock_blocking_page.cc
index 2d89b10..655f6de 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);
+  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();
diff --git a/chrome/browser/ssl/captive_portal_metrics_recorder.cc b/chrome/browser/ssl/captive_portal_metrics_recorder.cc
new file mode 100644
index 0000000..f847f44
--- /dev/null
+++ b/chrome/browser/ssl/captive_portal_metrics_recorder.cc
@@ -0,0 +1,120 @@
+// 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/captive_portal_metrics_recorder.h"
+
+#include <vector>
+
+#include "base/metrics/histogram_macros.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/profiles/profile.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/web_contents.h"
+
+#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
+#include "chrome/browser/captive_portal/captive_portal_service.h"
+#include "chrome/browser/captive_portal/captive_portal_service_factory.h"
+#endif
+
+namespace {
+
+// Events for UMA. Do not reorder or change!
+enum SSLInterstitialCauseCaptivePortal {
+  CAPTIVE_PORTAL_ALL,
+  CAPTIVE_PORTAL_DETECTION_ENABLED,
+  CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE,
+  CAPTIVE_PORTAL_PROBE_COMPLETED,
+  CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE,
+  CAPTIVE_PORTAL_NO_RESPONSE,
+  CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE,
+  CAPTIVE_PORTAL_DETECTED,
+  CAPTIVE_PORTAL_DETECTED_OVERRIDABLE,
+  UNUSED_CAPTIVE_PORTAL_EVENT,
+};
+
+#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
+void RecordCaptivePortalEventStats(SSLInterstitialCauseCaptivePortal event) {
+  UMA_HISTOGRAM_ENUMERATION("interstitial.ssl.captive_portal", event,
+                            UNUSED_CAPTIVE_PORTAL_EVENT);
+}
+#endif
+
+}  // namespace
+
+CaptivePortalMetricsRecorder::CaptivePortalMetricsRecorder(
+    content::WebContents* web_contents,
+    bool overridable)
+    : overridable_(overridable),
+      captive_portal_detection_enabled_(false),
+      captive_portal_probe_completed_(false),
+      captive_portal_no_response_(false),
+      captive_portal_detected_(false) {
+#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  captive_portal_detection_enabled_ =
+      CaptivePortalServiceFactory::GetForProfile(profile)->enabled();
+  registrar_.Add(this, chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT,
+                 content::Source<Profile>(profile));
+#endif
+}
+
+CaptivePortalMetricsRecorder::~CaptivePortalMetricsRecorder() {}
+
+void CaptivePortalMetricsRecorder::RecordCaptivePortalUMAStatistics() const {
+#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
+  RecordCaptivePortalEventStats(CAPTIVE_PORTAL_ALL);
+  if (captive_portal_detection_enabled_)
+    RecordCaptivePortalEventStats(
+        overridable_ ? CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE
+                     : CAPTIVE_PORTAL_DETECTION_ENABLED);
+  if (captive_portal_probe_completed_)
+    RecordCaptivePortalEventStats(
+        overridable_ ? CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE
+                     : CAPTIVE_PORTAL_PROBE_COMPLETED);
+  // Log only one of portal detected and no response results.
+  if (captive_portal_detected_)
+    RecordCaptivePortalEventStats(overridable_
+                                      ? CAPTIVE_PORTAL_DETECTED_OVERRIDABLE
+                                      : CAPTIVE_PORTAL_DETECTED);
+  else if (captive_portal_no_response_)
+    RecordCaptivePortalEventStats(overridable_
+                                      ? CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE
+                                      : CAPTIVE_PORTAL_NO_RESPONSE);
+#endif
+}
+
+void CaptivePortalMetricsRecorder::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
+  // When detection is disabled, captive portal service always sends
+  // RESULT_INTERNET_CONNECTED. Ignore any probe results in that case.
+  if (!captive_portal_detection_enabled_)
+    return;
+  if (type == chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT) {
+    captive_portal_probe_completed_ = true;
+    CaptivePortalService::Results* results =
+        content::Details<CaptivePortalService::Results>(details).ptr();
+    // If a captive portal was detected at any point when the interstitial was
+    // displayed, assume that the interstitial was caused by a captive portal.
+    // Example scenario:
+    // 1- Interstitial displayed and captive portal detected, setting the flag.
+    // 2- Captive portal detection automatically opens portal login page.
+    // 3- User logs in on the portal login page.
+    // A notification will be received here for RESULT_INTERNET_CONNECTED. Make
+    // sure we don't clear the captive protal flag, since the interstitial was
+    // potentially caused by the captive portal.
+    captive_portal_detected_ =
+        captive_portal_detected_ ||
+        (results->result == captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL);
+    // Also keep track of non-HTTP portals and error cases.
+    captive_portal_no_response_ =
+        captive_portal_no_response_ ||
+        (results->result == captive_portal::RESULT_NO_RESPONSE);
+  }
+#endif
+}
diff --git a/chrome/browser/ssl/captive_portal_metrics_recorder.h b/chrome/browser/ssl/captive_portal_metrics_recorder.h
new file mode 100644
index 0000000..34a885d2
--- /dev/null
+++ b/chrome/browser/ssl/captive_portal_metrics_recorder.h
@@ -0,0 +1,53 @@
+// 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.
+
+#ifndef CHROME_BROWSER_SSL_CAPTIVE_PORTAL_METRICS_RECORDER_H_
+#define CHROME_BROWSER_SSL_CAPTIVE_PORTAL_METRICS_RECORDER_H_
+
+#include <string>
+#include <vector>
+
+#include "base/time/time.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "net/cert/x509_certificate.h"
+#include "url/gurl.h"
+
+namespace content {
+class WebContents;
+}
+
+// This class helps the SSL interstitial record captive portal-specific
+// metrics. It should only be used on the UI thread because its implementation
+// uses captive_portal::CaptivePortalService which can only be accessed on the
+// UI thread.
+class CaptivePortalMetricsRecorder : public content::NotificationObserver {
+ public:
+  CaptivePortalMetricsRecorder(content::WebContents* web_contents,
+                               bool overridable);
+  ~CaptivePortalMetricsRecorder() override;
+
+  // Should be called when the interstitial is closing.
+  void RecordCaptivePortalUMAStatistics() const;
+
+ private:
+  typedef std::vector<std::string> Tokens;
+
+  // content::NotificationObserver:
+  void Observe(int type,
+               const content::NotificationSource& source,
+               const content::NotificationDetails& details) override;
+
+  bool overridable_;
+  bool captive_portal_detection_enabled_;
+  // Did the probe complete before the interstitial was closed?
+  bool captive_portal_probe_completed_;
+  // Did the captive portal probe receive an error or get a non-HTTP response?
+  bool captive_portal_no_response_;
+  bool captive_portal_detected_;
+
+  content::NotificationRegistrar registrar_;
+};
+
+#endif  // CHROME_BROWSER_SSL_CAPTIVE_PORTAL_METRICS_RECORDER_H_
diff --git a/chrome/browser/ssl/ssl_blocking_page.cc b/chrome/browser/ssl/ssl_blocking_page.cc
index 2147785..721df33 100644
--- a/chrome/browser/ssl/ssl_blocking_page.cc
+++ b/chrome/browser/ssl/ssl_blocking_page.cc
@@ -143,8 +143,10 @@ SSLBlockingPage::SSLBlockingPage(content::WebContents* web_contents,
   reporting_info.metric_prefix = GetUmaHistogramPrefix();
   reporting_info.rappor_prefix = kSSLRapporPrefix;
   reporting_info.rappor_report_type = rappor::UMA_RAPPOR_TYPE;
-  set_metrics_helper(new ChromeMetricsHelper(
+  scoped_ptr<ChromeMetricsHelper> chrome_metrics_helper(new ChromeMetricsHelper(
       web_contents, request_url, reporting_info, GetSamplingEventName()));
+  chrome_metrics_helper->StartRecordingCaptivePortalMetrics(overridable_);
+  set_metrics_helper(chrome_metrics_helper.Pass());
   metrics_helper()->RecordUserDecision(
       security_interstitials::MetricsHelper::SHOW);
   metrics_helper()->RecordUserInteraction(
@@ -155,13 +157,9 @@ SSLBlockingPage::SSLBlockingPage(content::WebContents* web_contents,
       certificate_reporting::ErrorReport::INTERSTITIAL_SSL, overridable_,
       metrics_helper()));
 
-  ssl_error_classification_.reset(new SSLErrorClassification(
-      web_contents,
-      time_triggered_,
-      request_url,
-      cert_error_,
-      *ssl_info_.cert.get()));
-  ssl_error_classification_->RecordUMAStatistics(overridable_);
+  SSLErrorClassification error_classification(
+      time_triggered_, request_url, cert_error_, *ssl_info_.cert.get());
+  error_classification.RecordUMAStatistics(overridable_);
 
   // Creating an interstitial without showing (e.g. from chrome://interstitials)
   // it leaks memory, so don't create it here.
@@ -176,11 +174,7 @@ InterstitialPageDelegate::TypeID SSLBlockingPage::GetTypeForTesting() const {
 }
 
 SSLBlockingPage::~SSLBlockingPage() {
-#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
-  // Captive portal detection results can arrive anytime during the interstitial
-  // is being displayed, so record it when the interstitial is going away.
-  ssl_error_classification_->RecordCaptivePortalUMAStatistics(overridable_);
-#endif
+  metrics_helper()->RecordShutdownMetrics();
   if (!callback_.is_null()) {
     // The page is closed without the user having chosen what to do, default to
     // deny.
diff --git a/chrome/browser/ssl/ssl_blocking_page.h b/chrome/browser/ssl/ssl_blocking_page.h
index 0c45e31..5387c81 100644
--- a/chrome/browser/ssl/ssl_blocking_page.h
+++ b/chrome/browser/ssl/ssl_blocking_page.h
@@ -30,7 +30,6 @@ class PolicyTest_SSLErrorOverridingDisallowed_Test;
 }
 
 class CertReportHelper;
-class SSLErrorClassification;
 
 // This class is responsible for showing/hiding the interstitial page that is
 // shown when a certificate error happens.
@@ -121,7 +120,6 @@ class SSLBlockingPage : public SecurityInterstitialPage {
   // Did the user previously allow a bad certificate but the decision has now
   // expired?
   const bool expired_but_previously_allowed_;
-  scoped_ptr<SSLErrorClassification> ssl_error_classification_;
 
   // The time at which the interstitial was triggered. The interstitial
   // calculates all times relative to this.
diff --git a/chrome/browser/ssl/ssl_error_classification.cc b/chrome/browser/ssl/ssl_error_classification.cc
index ce43de5..a934274 100644
--- a/chrome/browser/ssl/ssl_error_classification.cc
+++ b/chrome/browser/ssl/ssl_error_classification.cc
@@ -7,27 +7,17 @@
 #include "chrome/browser/ssl/ssl_error_classification.h"
 
 #include "base/build_time.h"
-#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/profiles/profile.h"
 #include "components/ssl_errors/error_info.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/web_contents.h"
 #include "net/base/net_util.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/cert/x509_cert_types.h"
 #include "net/cert/x509_certificate.h"
 #include "url/gurl.h"
 
-#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
-#include "chrome/browser/captive_portal/captive_portal_service.h"
-#include "chrome/browser/captive_portal/captive_portal_service_factory.h"
-#endif
-
 #if defined(OS_WIN)
 #include "base/win/win_util.h"
 #include "base/win/windows_version.h"
@@ -51,27 +41,13 @@ enum SSLInterstitialCause {
   LIKELY_MULTI_TENANT_HOSTING,
   LOCALHOST,
   PRIVATE_URL,
-  AUTHORITY_ERROR_CAPTIVE_PORTAL,
+  AUTHORITY_ERROR_CAPTIVE_PORTAL,  // Deprecated in M47.
   SELF_SIGNED,
   EXPIRED_RECENTLY,
   LIKELY_SAME_DOMAIN,
   UNUSED_INTERSTITIAL_CAUSE_ENTRY,
 };
 
-// Events for UMA. Do not reorder or change!
-enum SSLInterstitialCauseCaptivePortal {
-  CAPTIVE_PORTAL_ALL,
-  CAPTIVE_PORTAL_DETECTION_ENABLED,
-  CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE,
-  CAPTIVE_PORTAL_PROBE_COMPLETED,
-  CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE,
-  CAPTIVE_PORTAL_NO_RESPONSE,
-  CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE,
-  CAPTIVE_PORTAL_DETECTED,
-  CAPTIVE_PORTAL_DETECTED_OVERRIDABLE,
-  UNUSED_CAPTIVE_PORTAL_EVENT,
-};
-
 void RecordSSLInterstitialCause(bool overridable, SSLInterstitialCause event) {
   if (overridable) {
     UMA_HISTOGRAM_ENUMERATION("interstitial.ssl.cause.overridable", event,
@@ -82,14 +58,6 @@ void RecordSSLInterstitialCause(bool overridable, SSLInterstitialCause event) {
   }
 }
 
-#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
-void RecordCaptivePortalEventStats(SSLInterstitialCauseCaptivePortal event) {
-  UMA_HISTOGRAM_ENUMERATION("interstitial.ssl.captive_portal",
-                            event,
-                            UNUSED_CAPTIVE_PORTAL_EVENT);
-}
-#endif
-
 int GetLevensteinDistance(const std::string& str1,
                           const std::string& str2) {
   if (str1 == str2)
@@ -121,62 +89,17 @@ base::Time g_testing_build_time;
 
 } // namespace
 
-SSLErrorClassification::SSLErrorClassification(
-    content::WebContents* web_contents,
-    const base::Time& current_time,
-    const GURL& url,
-    int cert_error,
-    const net::X509Certificate& cert)
-  : web_contents_(web_contents),
-    current_time_(current_time),
-    request_url_(url),
-    cert_error_(cert_error),
-    cert_(cert),
-    captive_portal_detection_enabled_(false),
-    captive_portal_probe_completed_(false),
-    captive_portal_no_response_(false),
-    captive_portal_detected_(false) {
-#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
-  Profile* profile = Profile::FromBrowserContext(
-      web_contents_->GetBrowserContext());
-  captive_portal_detection_enabled_ =
-      CaptivePortalServiceFactory::GetForProfile(profile)->enabled();
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT,
-                 content::Source<Profile>(profile));
-#endif
-}
+SSLErrorClassification::SSLErrorClassification(const base::Time& current_time,
+                                               const GURL& url,
+                                               int cert_error,
+                                               const net::X509Certificate& cert)
+    : current_time_(current_time),
+      request_url_(url),
+      cert_error_(cert_error),
+      cert_(cert) {}
 
 SSLErrorClassification::~SSLErrorClassification() { }
 
-void SSLErrorClassification::RecordCaptivePortalUMAStatistics(
-    bool overridable) const {
-#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
-  RecordCaptivePortalEventStats(CAPTIVE_PORTAL_ALL);
-  if (captive_portal_detection_enabled_)
-    RecordCaptivePortalEventStats(
-        overridable ?
-        CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE :
-        CAPTIVE_PORTAL_DETECTION_ENABLED);
-  if (captive_portal_probe_completed_)
-    RecordCaptivePortalEventStats(
-        overridable ?
-        CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE :
-        CAPTIVE_PORTAL_PROBE_COMPLETED);
-  // Log only one of portal detected and no response results.
-  if (captive_portal_detected_)
-    RecordCaptivePortalEventStats(
-        overridable ?
-        CAPTIVE_PORTAL_DETECTED_OVERRIDABLE :
-        CAPTIVE_PORTAL_DETECTED);
-  else if (captive_portal_no_response_)
-    RecordCaptivePortalEventStats(
-        overridable ?
-        CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE :
-        CAPTIVE_PORTAL_NO_RESPONSE);
-#endif
-}
-
 void SSLErrorClassification::RecordUMAStatistics(
     bool overridable) const {
   ssl_errors::ErrorInfo::ErrorType type =
@@ -225,8 +148,6 @@ void SSLErrorClassification::RecordUMAStatistics(
         RecordSSLInterstitialCause(overridable, LOCALHOST);
       if (IsHostnameNonUniqueOrDotless(hostname))
         RecordSSLInterstitialCause(overridable, PRIVATE_URL);
-      if (captive_portal_probe_completed_ && captive_portal_detected_)
-        RecordSSLInterstitialCause(overridable, AUTHORITY_ERROR_CAPTIVE_PORTAL);
       if (net::X509Certificate::IsSelfSigned(cert_.os_cert_handle()))
         RecordSSLInterstitialCause(overridable, SELF_SIGNED);
       break;
@@ -521,34 +442,3 @@ bool SSLErrorClassification::IsHostnameNonUniqueOrDotless(
   return net::IsHostnameNonUnique(hostname) ||
          hostname.find('.') == std::string::npos;
 }
-
-void SSLErrorClassification::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
-  // When detection is disabled, captive portal service always sends
-  // RESULT_INTERNET_CONNECTED. Ignore any probe results in that case.
-  if (!captive_portal_detection_enabled_)
-    return;
-  if (type == chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT) {
-    captive_portal_probe_completed_ = true;
-    CaptivePortalService::Results* results =
-        content::Details<CaptivePortalService::Results>(details).ptr();
-    // If a captive portal was detected at any point when the interstitial was
-    // displayed, assume that the interstitial was caused by a captive portal.
-    // Example scenario:
-    // 1- Interstitial displayed and captive portal detected, setting the flag.
-    // 2- Captive portal detection automatically opens portal login page.
-    // 3- User logs in on the portal login page.
-    // A notification will be received here for RESULT_INTERNET_CONNECTED. Make
-    // sure we don't clear the captive protal flag, since the interstitial was
-    // potentially caused by the captive portal.
-    captive_portal_detected_ = captive_portal_detected_ ||
-        (results->result == captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL);
-    // Also keep track of non-HTTP portals and error cases.
-    captive_portal_no_response_ = captive_portal_no_response_ ||
-        (results->result == captive_portal::RESULT_NO_RESPONSE);
-  }
-#endif
-}
diff --git a/chrome/browser/ssl/ssl_error_classification.h b/chrome/browser/ssl/ssl_error_classification.h
index bdbb04e..e273365 100644
--- a/chrome/browser/ssl/ssl_error_classification.h
+++ b/chrome/browser/ssl/ssl_error_classification.h
@@ -10,29 +10,16 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/time/time.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 #include "net/cert/x509_certificate.h"
 #include "url/gurl.h"
 
-namespace content {
-class WebContents;
-}
-
-// This class classifies characteristics of SSL errors, including information
-// about captive portal detection.
-//
-// This class should only be used on the UI thread because its
-// implementation uses captive_portal::CaptivePortalService which can only be
-// accessed on the UI thread.
-class SSLErrorClassification : public content::NotificationObserver {
+class SSLErrorClassification {
  public:
-  SSLErrorClassification(content::WebContents* web_contents,
-                         const base::Time& current_time,
+  SSLErrorClassification(const base::Time& current_time,
                          const GURL& url,
                          int cert_error,
                          const net::X509Certificate& cert);
-  ~SSLErrorClassification() override;
+  ~SSLErrorClassification();
 
   // Returns true if the system time is in the past.
   static bool IsUserClockInThePast(const base::Time& time_now);
@@ -63,7 +50,6 @@ class SSLErrorClassification : public content::NotificationObserver {
                                    std::string* www_match_host_name);
 
   void RecordUMAStatistics(bool overridable) const;
-  void RecordCaptivePortalUMAStatistics(bool overridable) const;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest, TestDateInvalidScore);
@@ -132,24 +118,10 @@ class SSLErrorClassification : public content::NotificationObserver {
 
   static Tokens Tokenize(const std::string& name);
 
-  // content::NotificationObserver:
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
-
-  content::WebContents* web_contents_;
   base::Time current_time_;
   const GURL request_url_;
   int cert_error_;
   const net::X509Certificate& cert_;
-  bool captive_portal_detection_enabled_;
-  // Did the probe complete before the interstitial was closed?
-  bool captive_portal_probe_completed_;
-  // Did the captive portal probe receive an error or get a non-HTTP response?
-  bool captive_portal_no_response_;
-  bool captive_portal_detected_;
-
-  content::NotificationRegistrar registrar_;
 };
 
 #endif  // CHROME_BROWSER_SSL_SSL_ERROR_CLASSIFICATION_H_
diff --git a/chrome/browser/ssl/ssl_error_classification_unittest.cc b/chrome/browser/ssl/ssl_error_classification_unittest.cc
index 318dbc9..b84676e 100644
--- a/chrome/browser/ssl/ssl_error_classification_unittest.cc
+++ b/chrome/browser/ssl/ssl_error_classification_unittest.cc
@@ -7,8 +7,6 @@
 #include "base/files/file_path.h"
 #include "base/strings/string_split.h"
 #include "base/time/time.h"
-#include "chrome/test/base/chrome_render_view_host_test_harness.h"
-#include "content/public/browser/web_contents.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_data_directory.h"
 #include "net/cert/x509_cert_types.h"
@@ -19,14 +17,8 @@
 #include "url/gurl.h"
 
 using base::Time;
-using content::WebContents;
 
-class SSLErrorClassificationTest : public ChromeRenderViewHostTestHarness {
- public:
-  SSLErrorClassificationTest() {
-    SetThreadBundleOptions(content::TestBrowserThreadBundle::REAL_IO_THREAD);
-  }
-};
+class SSLErrorClassificationTest : public testing::Test {};
 
 TEST_F(SSLErrorClassificationTest, TestNameMismatch) {
   scoped_refptr<net::X509Certificate> google_cert(
@@ -41,16 +33,11 @@ TEST_F(SSLErrorClassificationTest, TestNameMismatch) {
   std::vector<std::vector<std::string>> dns_name_tokens_google;
   dns_name_tokens_google.push_back(dns_names_google);
   int cert_error = net::ERR_CERT_COMMON_NAME_INVALID;
-  WebContents* contents = web_contents();
   {
     GURL origin("https://google.com");
     std::vector<std::string> host_name_tokens = base::SplitString(
         origin.host(), ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
-    SSLErrorClassification ssl_error(contents,
-                                     time,
-                                     origin,
-                                     cert_error,
-                                     *google_cert);
+    SSLErrorClassification ssl_error(time, origin, cert_error, *google_cert);
     EXPECT_TRUE(ssl_error.IsWWWSubDomainMatch());
     EXPECT_FALSE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                              dns_name_tokens_google));
@@ -65,11 +52,7 @@ TEST_F(SSLErrorClassificationTest, TestNameMismatch) {
     GURL origin("https://foo.blah.google.com");
     std::vector<std::string> host_name_tokens = base::SplitString(
         origin.host(), ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
-    SSLErrorClassification ssl_error(contents,
-                                     time,
-                                     origin,
-                                     cert_error,
-                                     *google_cert);
+    SSLErrorClassification ssl_error(time, origin, cert_error, *google_cert);
     EXPECT_FALSE(ssl_error.IsWWWSubDomainMatch());
     EXPECT_FALSE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                              dns_name_tokens_google));
@@ -82,11 +65,7 @@ TEST_F(SSLErrorClassificationTest, TestNameMismatch) {
     GURL origin("https://foo.www.google.com");
     std::vector<std::string> host_name_tokens = base::SplitString(
         origin.host(), ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
-    SSLErrorClassification ssl_error(contents,
-                                     time,
-                                     origin,
-                                     cert_error,
-                                     *google_cert);
+    SSLErrorClassification ssl_error(time, origin, cert_error, *google_cert);
     EXPECT_FALSE(ssl_error.IsWWWSubDomainMatch());
     EXPECT_TRUE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                             dns_name_tokens_google));
@@ -99,11 +78,7 @@ TEST_F(SSLErrorClassificationTest, TestNameMismatch) {
      GURL origin("https://www.google.com.foo");
      std::vector<std::string> host_name_tokens = base::SplitString(
          origin.host(), ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
-     SSLErrorClassification ssl_error(contents,
-                                      time,
-                                      origin,
-                                      cert_error,
-                                      *google_cert);
+     SSLErrorClassification ssl_error(time, origin, cert_error, *google_cert);
      EXPECT_FALSE(ssl_error.IsWWWSubDomainMatch());
      EXPECT_FALSE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                               dns_name_tokens_google));
@@ -116,11 +91,7 @@ TEST_F(SSLErrorClassificationTest, TestNameMismatch) {
     GURL origin("https://www.foogoogle.com.");
     std::vector<std::string> host_name_tokens = base::SplitString(
         origin.host(), ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
-    SSLErrorClassification ssl_error(contents,
-                                     time,
-                                     origin,
-                                     cert_error,
-                                     *google_cert);
+    SSLErrorClassification ssl_error(time, origin, cert_error, *google_cert);
     EXPECT_FALSE(ssl_error.IsWWWSubDomainMatch());
     EXPECT_FALSE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                              dns_name_tokens_google));
@@ -142,11 +113,7 @@ TEST_F(SSLErrorClassificationTest, TestNameMismatch) {
     GURL origin("https://a.b.webkit.org");
     std::vector<std::string> host_name_tokens = base::SplitString(
         origin.host(), ".", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
-    SSLErrorClassification ssl_error(contents,
-                                     time,
-                                     origin,
-                                     cert_error,
-                                     *webkit_cert);
+    SSLErrorClassification ssl_error(time, origin, cert_error, *webkit_cert);
     EXPECT_FALSE(ssl_error.IsWWWSubDomainMatch());
     EXPECT_FALSE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                              dns_name_tokens_webkit));
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index bebab2b..4ca7612 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2729,6 +2729,8 @@
     'chrome_browser_ssl_sources': [
       'browser/ssl/bad_clock_blocking_page.cc',
       'browser/ssl/bad_clock_blocking_page.h',
+      'browser/ssl/captive_portal_metrics_recorder.cc',
+      'browser/ssl/captive_portal_metrics_recorder.h',
       'browser/ssl/cert_report_helper.cc',
       'browser/ssl/cert_report_helper.h',
       'browser/ssl/chrome_ssl_host_state_delegate.cc',
diff --git a/components/security_interstitials/core/metrics_helper.cc b/components/security_interstitials/core/metrics_helper.cc
index da7002b..7466562 100644
--- a/components/security_interstitials/core/metrics_helper.cc
+++ b/components/security_interstitials/core/metrics_helper.cc
@@ -134,6 +134,10 @@ void MetricsHelper::RecordUserInteraction(Interaction interaction) {
   RecordExtraUserInteractionMetrics(interaction);
 }
 
+void MetricsHelper::RecordShutdownMetrics() {
+  RecordExtraShutdownMetrics();
+}
+
 int MetricsHelper::NumVisits() {
   return num_visits_;
 }
diff --git a/components/security_interstitials/core/metrics_helper.h b/components/security_interstitials/core/metrics_helper.h
index 064a180..e610201 100644
--- a/components/security_interstitials/core/metrics_helper.h
+++ b/components/security_interstitials/core/metrics_helper.h
@@ -91,14 +91,17 @@ class MetricsHelper {
   // histogram and potentially in a RAPPOR metric.
   void RecordUserDecision(Decision decision);
   void RecordUserInteraction(Interaction interaction);
+  void RecordShutdownMetrics();
+
   // Number of times user visited this origin before. -1 means not-yet-set.
   int NumVisits();
 
  protected:
   // Subclasses should implement any embedder-specific recording logic in these
-  // methods. They'll be invoked from RecordUserDecision/Interaction.
+  // methods. They'll be invoked from the matching Record methods.
   virtual void RecordExtraUserDecisionMetrics(Decision decision) = 0;
   virtual void RecordExtraUserInteractionMetrics(Interaction interaction) = 0;
+  virtual void RecordExtraShutdownMetrics() = 0;
 
  private:
   // Used to query the HistoryService to see if the URL is in history.  It will
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 67c873e..9cee900 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -71478,7 +71478,8 @@ To add a new entry, add it with any value and run test to compute valid value.
   </int>
   <int value="10"
       label="AUTHORTIY_ERROR_CAPTIVE_PORTAL: Captive portal was detected">
-    This cause is recorded only for CERT_AUTHORITY_INVALID errors.
+    This cause is recorded only for CERT_AUTHORITY_INVALID errors. (Deprecated
+    in M47.)
   </int>
   <int value="11" label="SELF_SIGNED: The cert is self-signed">
     This cause is recorded only for CERT_AUTHORITY_INVALID errors.