Revert "Split captive portal metrics out of SSLErrorClassification"

The Android clang trybots aren't happy with this change:
In file included from
../../chrome/browser/ssl/captive_portal_metrics_recorder.cc:5:
../../chrome/browser/ssl/captive_portal_metrics_recorder.h:42:8: error: private
field 'overridable_' is not used [-Werror,-Wunused-private-field]
  bool overridable_;
       ^
../../chrome/browser/ssl/captive_portal_metrics_recorder.h:43:8: error: private
field 'captive_portal_detection_enabled_' is not used
[-Werror,-Wunused-private-field]
  bool captive_portal_detection_enabled_;
       ^
../../chrome/browser/ssl/captive_portal_metrics_recorder.h:45:8: error: private
field 'captive_portal_probe_completed_' is not used
[-Werror,-Wunused-private-field]
  bool captive_portal_probe_completed_;
       ^
../../chrome/browser/ssl/captive_portal_metrics_recorder.h:47:8: error: private
field 'captive_portal_no_response_' is not used [-Werror,-Wunused-private-field]
  bool captive_portal_no_response_;
       ^
../../chrome/browser/ssl/captive_portal_metrics_recorder.h:48:8: error: private
field 'captive_portal_detected_' is not used [-Werror,-Wunused-private-field]
  bool captive_portal_detected_;
       ^
5 errors generated.

BUG=488673
TBR=felt@chromium.org

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

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

17 files changed, 219 insertions, 249 deletions

diff --git a/chrome/browser/interstitials/chrome_metrics_helper.cc b/chrome/browser/interstitials/chrome_metrics_helper.cc
index abda1aa..c886e44 100644
--- a/chrome/browser/interstitials/chrome_metrics_helper.cc
+++ b/chrome/browser/interstitials/chrome_metrics_helper.cc
@@ -7,7 +7,6 @@
 #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"
@@ -36,18 +35,6 @@ 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 bba24c2..e56e59a 100644
--- a/chrome/browser/interstitials/chrome_metrics_helper.h
+++ b/chrome/browser/interstitials/chrome_metrics_helper.h
@@ -18,13 +18,9 @@ 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(
@@ -34,15 +30,12 @@ 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_;
@@ -51,7 +44,6 @@ 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 0cc658e..876f55b 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(
-    scoped_ptr<security_interstitials::MetricsHelper> metrics_helper) {
-  metrics_helper_ = metrics_helper.Pass();
+    security_interstitials::MetricsHelper* metrics_helper) {
+  metrics_helper_.reset(metrics_helper);
 }
 
 base::string16 SecurityInterstitialPage::GetFormattedHostName() const {
diff --git a/chrome/browser/interstitials/security_interstitial_page.h b/chrome/browser/interstitials/security_interstitial_page.h
index 13c547d..0bd632b 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(
-      scoped_ptr<security_interstitials::MetricsHelper> metrics_helper);
+      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 342e59d..c74fb7a 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -172,11 +172,8 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
   reporting_info.extra_suffix = GetExtraMetricsSuffix();
   reporting_info.rappor_prefix = GetRapporPrefix();
   reporting_info.rappor_report_type = rappor::SAFEBROWSING_RAPPOR_TYPE;
-  set_metrics_helper(
-      make_scoped_ptr(new ChromeMetricsHelper(web_contents, request_url(),
-                                              reporting_info,
-                                              GetSamplingEventName()))
-          .Pass());
+  set_metrics_helper(new ChromeMetricsHelper(
+      web_contents, request_url(), reporting_info, GetSamplingEventName()));
   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 655f6de..2d89b10 100644
--- a/chrome/browser/ssl/bad_clock_blocking_page.cc
+++ b/chrome/browser/ssl/bad_clock_blocking_page.cc
@@ -183,17 +183,16 @@ BadClockBlockingPage::BadClockBlockingPage(
       time_triggered_(time_triggered) {
   security_interstitials::MetricsHelper::ReportDetails reporting_info;
   reporting_info.metric_prefix = 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());
+  set_metrics_helper(new ChromeMetricsHelper(web_contents, request_url,
+                                             reporting_info, kMetricsName));
   metrics_helper()->RecordUserInteraction(
       security_interstitials::MetricsHelper::TOTAL_VISITS);
 
   // TODO(felt): Separate the clock statistics from the main ssl statistics.
-  SSLErrorClassification classifier(time_triggered_, request_url, cert_error_,
-                                    *ssl_info_.cert.get());
-  classifier.RecordUMAStatistics(false);
+  scoped_ptr<SSLErrorClassification> classifier(
+      new SSLErrorClassification(web_contents, time_triggered_, request_url,
+                                 cert_error_, *ssl_info_.cert.get()));
+  classifier->RecordUMAStatistics(false);
 }
 
 bool BadClockBlockingPage::ShouldCreateNewNavigation() const {
@@ -206,7 +205,6 @@ 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
deleted file mode 100644
index f847f44..0000000
--- a/chrome/browser/ssl/captive_portal_metrics_recorder.cc
+++ /dev/null
@@ -1,120 +0,0 @@
-// 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
deleted file mode 100644
index 34a885d2..0000000
--- a/chrome/browser/ssl/captive_portal_metrics_recorder.h
+++ /dev/null
@@ -1,53 +0,0 @@
-// 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 721df33..2147785 100644
--- a/chrome/browser/ssl/ssl_blocking_page.cc
+++ b/chrome/browser/ssl/ssl_blocking_page.cc
@@ -143,10 +143,8 @@ 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;
-  scoped_ptr<ChromeMetricsHelper> chrome_metrics_helper(new ChromeMetricsHelper(
+  set_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(
@@ -157,9 +155,13 @@ SSLBlockingPage::SSLBlockingPage(content::WebContents* web_contents,
       certificate_reporting::ErrorReport::INTERSTITIAL_SSL, overridable_,
       metrics_helper()));
 
-  SSLErrorClassification error_classification(
-      time_triggered_, request_url, cert_error_, *ssl_info_.cert.get());
-  error_classification.RecordUMAStatistics(overridable_);
+  ssl_error_classification_.reset(new SSLErrorClassification(
+      web_contents,
+      time_triggered_,
+      request_url,
+      cert_error_,
+      *ssl_info_.cert.get()));
+  ssl_error_classification_->RecordUMAStatistics(overridable_);
 
   // Creating an interstitial without showing (e.g. from chrome://interstitials)
   // it leaks memory, so don't create it here.
@@ -174,7 +176,11 @@ InterstitialPageDelegate::TypeID SSLBlockingPage::GetTypeForTesting() const {
 }
 
 SSLBlockingPage::~SSLBlockingPage() {
-  metrics_helper()->RecordShutdownMetrics();
+#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
   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 5387c81..0c45e31 100644
--- a/chrome/browser/ssl/ssl_blocking_page.h
+++ b/chrome/browser/ssl/ssl_blocking_page.h
@@ -30,6 +30,7 @@ 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.
@@ -120,6 +121,7 @@ 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 a934274..ce43de5 100644
--- a/chrome/browser/ssl/ssl_error_classification.cc
+++ b/chrome/browser/ssl/ssl_error_classification.cc
@@ -7,17 +7,27 @@
 #include "chrome/browser/ssl/ssl_error_classification.h"
 
 #include "base/build_time.h"
-#include "base/metrics/histogram_macros.h"
+#include "base/metrics/histogram.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"
@@ -41,13 +51,27 @@ enum SSLInterstitialCause {
   LIKELY_MULTI_TENANT_HOSTING,
   LOCALHOST,
   PRIVATE_URL,
-  AUTHORITY_ERROR_CAPTIVE_PORTAL,  // Deprecated in M47.
+  AUTHORITY_ERROR_CAPTIVE_PORTAL,
   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,
@@ -58,6 +82,14 @@ 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)
@@ -89,17 +121,62 @@ base::Time g_testing_build_time;
 
 } // namespace
 
-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(
+    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() { }
 
+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 =
@@ -148,6 +225,8 @@ 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;
@@ -442,3 +521,34 @@ 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 e273365..bdbb04e 100644
--- a/chrome/browser/ssl/ssl_error_classification.h
+++ b/chrome/browser/ssl/ssl_error_classification.h
@@ -10,16 +10,29 @@
 
 #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"
 
-class SSLErrorClassification {
+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 {
  public:
-  SSLErrorClassification(const base::Time& current_time,
+  SSLErrorClassification(content::WebContents* web_contents,
+                         const base::Time& current_time,
                          const GURL& url,
                          int cert_error,
                          const net::X509Certificate& cert);
-  ~SSLErrorClassification();
+  ~SSLErrorClassification() override;
 
   // Returns true if the system time is in the past.
   static bool IsUserClockInThePast(const base::Time& time_now);
@@ -50,6 +63,7 @@ class SSLErrorClassification {
                                    std::string* www_match_host_name);
 
   void RecordUMAStatistics(bool overridable) const;
+  void RecordCaptivePortalUMAStatistics(bool overridable) const;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest, TestDateInvalidScore);
@@ -118,10 +132,24 @@ class SSLErrorClassification {
 
   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 b84676e..318dbc9 100644
--- a/chrome/browser/ssl/ssl_error_classification_unittest.cc
+++ b/chrome/browser/ssl/ssl_error_classification_unittest.cc
@@ -7,6 +7,8 @@
 #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"
@@ -17,8 +19,14 @@
 #include "url/gurl.h"
 
 using base::Time;
+using content::WebContents;
 
-class SSLErrorClassificationTest : public testing::Test {};
+class SSLErrorClassificationTest : public ChromeRenderViewHostTestHarness {
+ public:
+  SSLErrorClassificationTest() {
+    SetThreadBundleOptions(content::TestBrowserThreadBundle::REAL_IO_THREAD);
+  }
+};
 
 TEST_F(SSLErrorClassificationTest, TestNameMismatch) {
   scoped_refptr<net::X509Certificate> google_cert(
@@ -33,11 +41,16 @@ 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(time, origin, cert_error, *google_cert);
+    SSLErrorClassification ssl_error(contents,
+                                     time,
+                                     origin,
+                                     cert_error,
+                                     *google_cert);
     EXPECT_TRUE(ssl_error.IsWWWSubDomainMatch());
     EXPECT_FALSE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                              dns_name_tokens_google));
@@ -52,7 +65,11 @@ 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(time, origin, cert_error, *google_cert);
+    SSLErrorClassification ssl_error(contents,
+                                     time,
+                                     origin,
+                                     cert_error,
+                                     *google_cert);
     EXPECT_FALSE(ssl_error.IsWWWSubDomainMatch());
     EXPECT_FALSE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                              dns_name_tokens_google));
@@ -65,7 +82,11 @@ 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(time, origin, cert_error, *google_cert);
+    SSLErrorClassification ssl_error(contents,
+                                     time,
+                                     origin,
+                                     cert_error,
+                                     *google_cert);
     EXPECT_FALSE(ssl_error.IsWWWSubDomainMatch());
     EXPECT_TRUE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                             dns_name_tokens_google));
@@ -78,7 +99,11 @@ 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(time, origin, cert_error, *google_cert);
+     SSLErrorClassification ssl_error(contents,
+                                      time,
+                                      origin,
+                                      cert_error,
+                                      *google_cert);
      EXPECT_FALSE(ssl_error.IsWWWSubDomainMatch());
      EXPECT_FALSE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                               dns_name_tokens_google));
@@ -91,7 +116,11 @@ 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(time, origin, cert_error, *google_cert);
+    SSLErrorClassification ssl_error(contents,
+                                     time,
+                                     origin,
+                                     cert_error,
+                                     *google_cert);
     EXPECT_FALSE(ssl_error.IsWWWSubDomainMatch());
     EXPECT_FALSE(ssl_error.NameUnderAnyNames(host_name_tokens,
                                              dns_name_tokens_google));
@@ -113,7 +142,11 @@ 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(time, origin, cert_error, *webkit_cert);
+    SSLErrorClassification ssl_error(contents,
+                                     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 4ca7612..bebab2b 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2729,8 +2729,6 @@
     '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 7466562..da7002b 100644
--- a/components/security_interstitials/core/metrics_helper.cc
+++ b/components/security_interstitials/core/metrics_helper.cc
@@ -134,10 +134,6 @@ 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 e610201..064a180 100644
--- a/components/security_interstitials/core/metrics_helper.h
+++ b/components/security_interstitials/core/metrics_helper.h
@@ -91,17 +91,14 @@ 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 the matching Record methods.
+  // methods. They'll be invoked from RecordUserDecision/Interaction.
   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 9cee900..67c873e 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -71478,8 +71478,7 @@ 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. (Deprecated
-    in M47.)
+    This cause is recorded only for CERT_AUTHORITY_INVALID errors.
   </int>
   <int value="11" label="SELF_SIGNED: The cert is self-signed">
     This cause is recorded only for CERT_AUTHORITY_INVALID errors.