Refactor captive portal code from SSLBlockingPage to SSLErrorClassification

A function is also added to check the type of the network the user is
connected to.

CL originally written by radhikabhar@ as 400323002, but not landed before
her internship ended.

BUG=395295
TBR=meacer@chromium.org, dcheng@chromium.org

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

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

chrome/browser/ssl/ssl_blocking_page.cc

@@ -47,11 +47,6 @@
 #include "ui/base/webui/jstemplate_builder.h"
 #include "ui/base/webui/web_ui_util.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(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/api/experience_sampling_private/experience_sampling.h"
 #endif
@@ -110,14 +105,15 @@ enum SSLBlockingPageEvent {
   SHOW_NEW_SITE,
   PROCEED_NEW_SITE,
   PROCEED_MANUAL_NONOVERRIDABLE,
-  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,
+  // Captive Portal errors moved to ssl_error_classification.
+  DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED,
+  DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE,
+  DEPRECATED_CAPTIVE_PORTAL_PROBE_COMPLETED,
+  DEPRECATED_CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE,
+  DEPRECATED_CAPTIVE_PORTAL_NO_RESPONSE,
+  DEPRECATED_CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE,
+  DEPRECATED_CAPTIVE_PORTAL_DETECTED,
+  DEPRECATED_CAPTIVE_PORTAL_DETECTED_OVERRIDABLE,
   UNUSED_BLOCKING_PAGE_EVENT,
 };
 
@@ -167,38 +163,11 @@ void RecordSSLBlockingPageDetailedStats(bool proceed,
                                         bool overridable,
                                         bool internal,
                                         int num_visits,
-                                        bool captive_portal_detection_enabled,
-                                        bool captive_portal_probe_completed,
-                                        bool captive_portal_no_response,
-                                        bool captive_portal_detected,
                                         bool expired_but_previously_allowed) {
   UMA_HISTOGRAM_ENUMERATION("interstitial.ssl_error_type",
       SSLErrorInfo::NetErrorToErrorType(cert_error), SSLErrorInfo::END_OF_ENUM);
   RecordSSLExpirationPageEventState(
       expired_but_previously_allowed, proceed, overridable);
-#if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
-  if (captive_portal_detection_enabled)
-    RecordSSLBlockingPageEventStats(
-        overridable ?
-        CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE :
-        CAPTIVE_PORTAL_DETECTION_ENABLED);
-  if (captive_portal_probe_completed)
-    RecordSSLBlockingPageEventStats(
-        overridable ?
-        CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE :
-        CAPTIVE_PORTAL_PROBE_COMPLETED);
-  // Log only one of portal detected and no response results.
-  if (captive_portal_detected)
-    RecordSSLBlockingPageEventStats(
-        overridable ?
-        CAPTIVE_PORTAL_DETECTED_OVERRIDABLE :
-        CAPTIVE_PORTAL_DETECTED);
-  else if (captive_portal_no_response)
-    RecordSSLBlockingPageEventStats(
-        overridable ?
-        CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE :
-        CAPTIVE_PORTAL_NO_RESPONSE);
-#endif
   if (!overridable) {
     if (proceed) {
       RecordSSLBlockingPageEventStats(PROCEED_MANUAL_NONOVERRIDABLE);
@@ -351,10 +320,6 @@ SSLBlockingPage::SSLBlockingPage(content::WebContents* web_contents,
       interstitial_page_(NULL),
       internal_(false),
       num_visits_(-1),
-      captive_portal_detection_enabled_(false),
-      captive_portal_probe_completed_(false),
-      captive_portal_no_response_(false),
-      captive_portal_detected_(false),
       expired_but_previously_allowed_(
           (options_mask & EXPIRED_BUT_PREVIOUSLY_ALLOWED) != 0) {
   Profile* profile = Profile::FromBrowserContext(
@@ -378,20 +343,16 @@ SSLBlockingPage::SSLBlockingPage(content::WebContents* web_contents,
     }
   }
 
-  SSLErrorClassification ssl_error_classification(
+  ssl_error_classification_.reset(new SSLErrorClassification(
+      web_contents_,
       base::Time::NowFromSystemTime(),
       request_url_,
-      *ssl_info_.cert.get());
-  ssl_error_classification.RecordUMAStatistics(overridable_, cert_error_);
+      cert_error_,
+      *ssl_info_.cert.get()));
+  ssl_error_classification_->RecordUMAStatistics(overridable_);
 
 #if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
-  CaptivePortalService* captive_portal_service =
-      CaptivePortalServiceFactory::GetForProfile(profile);
-  captive_portal_detection_enabled_ = captive_portal_service ->enabled();
-  captive_portal_service ->DetectCaptivePortal();
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT,
-                 content::Source<Profile>(profile));
+  ssl_error_classification_->RecordCaptivePortalUMAStatistics(overridable_);
 #endif
 
 #if defined(ENABLE_EXTENSIONS)
@@ -414,16 +375,27 @@ SSLBlockingPage::SSLBlockingPage(content::WebContents* web_contents,
 }
 
 SSLBlockingPage::~SSLBlockingPage() {
+  // InvalidCommonNameSeverityScore() and InvalidDateSeverityScore() are in the
+  // destructor because they depend on knowing whether captive portal detection
+  // happened before the user made a decision.
+  SSLErrorInfo::ErrorType type =
+      SSLErrorInfo::NetErrorToErrorType(cert_error_);
+  switch (type) {
+    case SSLErrorInfo::CERT_DATE_INVALID:
+      ssl_error_classification_->InvalidDateSeverityScore();
+      break;
+    case SSLErrorInfo::CERT_COMMON_NAME_INVALID:
+      ssl_error_classification_->InvalidCommonNameSeverityScore();
+      break;
+    default:
+      break;
+  }
   if (!callback_.is_null()) {
     RecordSSLBlockingPageDetailedStats(false,
                                        cert_error_,
                                        overridable_,
                                        internal_,
                                        num_visits_,
-                                       captive_portal_detection_enabled_,
-                                       captive_portal_probe_completed_,
-                                       captive_portal_no_response_,
-                                       captive_portal_detected_,
                                        expired_but_previously_allowed_);
     // The page is closed without the user having chosen what to do, default to
     // deny.
@@ -621,16 +593,13 @@ void SSLBlockingPage::OnProceed() {
                                      overridable_,
                                      internal_,
                                      num_visits_,
-                                     captive_portal_detection_enabled_,
-                                     captive_portal_probe_completed_,
-                                     captive_portal_no_response_,
-                                     captive_portal_detected_,
                                      expired_but_previously_allowed_);
 #if defined(ENABLE_EXTENSIONS)
   // ExperienceSampling: Notify that user decided to proceed.
   if (sampling_event_.get())
     sampling_event_->CreateUserDecisionEvent(ExperienceSamplingEvent::kProceed);
 #endif
+
   // Accepting the certificate resumes the loading of the page.
   NotifyAllowCertificate();
 }
@@ -641,10 +610,6 @@ void SSLBlockingPage::OnDontProceed() {
                                      overridable_,
                                      internal_,
                                      num_visits_,
-                                     captive_portal_detection_enabled_,
-                                     captive_portal_probe_completed_,
-                                     captive_portal_no_response_,
-                                     captive_portal_detected_,
                                      expired_but_previously_allowed_);
 #if defined(ENABLE_EXTENSIONS)
   // ExperienceSampling: Notify that user decided to not proceed.
@@ -695,35 +660,3 @@ void SSLBlockingPage::OnGotHistoryCount(bool success,
                                         base::Time first_visit) {
   num_visits_ = num_visits;
 }
-
-void SSLBlockingPage::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 portal 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
-}

chrome/browser/ssl/ssl_blocking_page.h

@@ -32,15 +32,12 @@ class ExperienceSamplingEvent;
 }
 #endif
 
+class SSLErrorClassification;
+
 // This class is responsible for showing/hiding the interstitial page that is
 // shown when a certificate error happens.
 // It deletes itself when the interstitial page is closed.
-//
-// 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 SSLBlockingPage : public content::InterstitialPageDelegate,
-                        public content::NotificationObserver {
+class SSLBlockingPage : public content::InterstitialPageDelegate {
  public:
   // These represent the commands sent from the interstitial JavaScript. They
   // are defined in chrome/browser/resources/ssl/ssl_errors_common.js.
@@ -100,12 +97,6 @@ class SSLBlockingPage : public content::InterstitialPageDelegate,
   // Used to query the HistoryService to see if the URL is in history. For UMA.
   void OnGotHistoryCount(bool success, int num_visits, base::Time first_visit);
 
-  // content::NotificationObserver:
-  virtual void Observe(
-      int type,
-      const content::NotificationSource& source,
-      const content::NotificationDetails& details) OVERRIDE;
-
   base::Callback<void(bool)> callback_;
 
   content::WebContents* web_contents_;
@@ -124,20 +115,10 @@ class SSLBlockingPage : public content::InterstitialPageDelegate,
   int num_visits_;
   // Used for getting num_visits_.
   base::CancelableTaskTracker request_tracker_;
-  // Is captive portal detection enabled?
-  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_;
-  // Was a captive portal detected?
-  bool captive_portal_detected_;
   // Did the user previously allow a bad certificate but the decision has now
   // expired?
   const bool expired_but_previously_allowed_;
-
-  // For the FieldTrial: this contains the name of the condition.
-  std::string trial_condition_;
+  scoped_ptr<SSLErrorClassification> ssl_error_classification_;
 
 #if defined(ENABLE_EXTENSIONS)
   // For Chrome Experience Sampling Platform: this maintains event state.

chrome/browser/ssl/ssl_error_classification.h

@@ -9,17 +9,29 @@
 #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"
 
-// This class calculates the severity scores for the different type of SSL
-// errors.
-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();
+  virtual ~SSLErrorClassification();
 
   // Returns true if the system time is in the past.
   static bool IsUserClockInThePast(const base::Time& time_now);
@@ -31,18 +43,19 @@ class SSLErrorClassification {
   static bool IsWindowsVersionSP3OrLower();
 
   // A function which calculates the severity score when the ssl error is
-  // CERT_DATE_INVALID, returns a score between 0.0 and 1.0, higher values
-  // being more severe, indicating how severe the certificate's invalid
-  // date error is.
-  float InvalidDateSeverityScore(int cert_error) const;
+  // |CERT_DATE_INVALID|. The calculated score is between 0.0 and 1.0, higher
+  // being more severe, indicating how severe the certificate's
+  // date invalid error is.
+  void InvalidDateSeverityScore();
 
   // A function which calculates the severity score when the ssl error is
-  // when the SSL error is |CERT_COMMON_NAME_INVALID|, returns a score between
-  // between 0.0 and 1.0, higher values being more severe, indicating how
-  // severe the certificate's common name invalid error is.
-  float InvalidCommonNameSeverityScore(int cert_error) const;
+  // |CERT_COMMON_NAME_INVALID|. The calculated score is between 0.0 and 1.0,
+  // higher being more severe, indicating how severe the certificate's common
+  // name invalid error is.
+  void InvalidCommonNameSeverityScore();
 
-  void RecordUMAStatistics(bool overridable, int cert_error);
+  void RecordUMAStatistics(bool overridable) const;
+  void RecordCaptivePortalUMAStatistics(bool overridable) const;
   base::TimeDelta TimePassedSinceExpiry() const;
 
  private:
@@ -90,8 +103,6 @@ class SSLErrorClassification {
   // fields.
   bool IsCertLikelyFromMultiTenantHosting() const;
 
-  float CalculateScoreTimePassedSinceExpiry() const;
-
   static std::vector<Tokens> GetTokenizedDNSNames(
       const std::vector<std::string>& dns_names);
 
@@ -109,13 +120,32 @@ class SSLErrorClassification {
 
   static Tokens Tokenize(const std::string& name);
 
+  float CalculateScoreTimePassedSinceExpiry() const;
+  float CalculateScoreEnvironments() const;
+
+  // content::NotificationObserver:
+  virtual void Observe(
+      int type,
+      const content::NotificationSource& source,
+      const content::NotificationDetails& details) OVERRIDE;
+
+  content::WebContents* web_contents_;
   // This stores the current time.
   base::Time current_time_;
-
   const GURL& request_url_;
-
+  int cert_error_;
   // This stores the certificate.
   const net::X509Certificate& cert_;
+  // Is captive portal detection enabled?
+  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_;
+  // Was a captive portal detected?
+  bool captive_portal_detected_;
+
+  content::NotificationRegistrar registrar_;
 };
 
 #endif  // CHROME_BROWSER_SSL_SSL_ERROR_CLASSIFICATION_H_

chrome/browser/ssl/ssl_error_classification_unittest.cc

@@ -7,6 +7,9 @@
 #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"
 #include "net/cert/x509_certificate.h"
@@ -16,34 +19,56 @@
 #include "url/gurl.h"
 
 using base::Time;
+using content::WebContents;
 
-TEST(SSLErrorClassificationTest, TestDateInvalidScore) {
+class SSLErrorClassificationTest : public ChromeRenderViewHostTestHarness {
+ public:
+  SSLErrorClassificationTest() {
+    SetThreadBundleOptions(content::TestBrowserThreadBundle::REAL_IO_THREAD);
+  }
+};
+
+TEST_F(SSLErrorClassificationTest, TestDateInvalidScore) {
   base::FilePath certs_dir = net::GetTestCertsDirectory();
   scoped_refptr<net::X509Certificate> expired_cert =
       net::ImportCertFromFile(certs_dir, "expired_cert.pem");
   base::Time time;
   GURL origin("https://example.com");
+  int cert_error = net::ERR_CERT_DATE_INVALID;
+  WebContents* contents = web_contents();
 
   {
     EXPECT_TRUE(base::Time::FromString("Wed, 03 Jan 2007 12:00:00 GMT", &time));
-    SSLErrorClassification ssl_error(time, origin, *expired_cert);
+    SSLErrorClassification ssl_error(contents,
+                                     time,
+                                     origin,
+                                     cert_error,
+                                     *expired_cert);
     EXPECT_FLOAT_EQ(0.2f, ssl_error.CalculateScoreTimePassedSinceExpiry());
   }
 
   {
     EXPECT_TRUE(base::Time::FromString("Sat, 06 Jan 2007 12:00:00 GMT", &time));
-    SSLErrorClassification ssl_error(time, origin, *expired_cert);
+    SSLErrorClassification ssl_error(contents,
+                                     time,
+                                     origin,
+                                     cert_error,
+                                     *expired_cert);
     EXPECT_FLOAT_EQ(0.3f, ssl_error.CalculateScoreTimePassedSinceExpiry());
   }
 
   {
     EXPECT_TRUE(base::Time::FromString("Mon, 08 Jan 2007 12:00:00 GMT", &time));
-    SSLErrorClassification ssl_error(time, origin, *expired_cert);
+    SSLErrorClassification ssl_error(contents,
+                                     time,
+                                     origin,
+                                     cert_error,
+                                     *expired_cert);
     EXPECT_FLOAT_EQ(0.4f, ssl_error.CalculateScoreTimePassedSinceExpiry());
   }
 }
 
-TEST(SSLErrorClassificationTest, TestNameMismatch) {
+TEST_F(SSLErrorClassificationTest, TestNameMismatch) {
   scoped_refptr<net::X509Certificate> google_cert(
       net::X509Certificate::CreateFromBytes(
           reinterpret_cast<const char*>(google_der), sizeof(google_der)));
@@ -55,12 +80,18 @@ TEST(SSLErrorClassificationTest, TestNameMismatch) {
   dns_names_google.push_back("com");
   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::string host_name = origin.host();
     std::vector<std::string> host_name_tokens;
     base::SplitStringDontTrim(host_name, '.', &host_name_tokens);
-    SSLErrorClassification ssl_error(time, origin, *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));
@@ -75,7 +106,11 @@ TEST(SSLErrorClassificationTest, TestNameMismatch) {
     std::string host_name = origin.host();
     std::vector<std::string> host_name_tokens;
     base::SplitStringDontTrim(host_name, '.', &host_name_tokens);
-    SSLErrorClassification ssl_error(time, origin, *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));
@@ -88,7 +123,11 @@ TEST(SSLErrorClassificationTest, TestNameMismatch) {
     std::string host_name = origin.host();
     std::vector<std::string> host_name_tokens;
     base::SplitStringDontTrim(host_name, '.', &host_name_tokens);
-    SSLErrorClassification ssl_error(time, origin, *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));
@@ -101,7 +140,11 @@ TEST(SSLErrorClassificationTest, TestNameMismatch) {
      std::string host_name = origin.host();
      std::vector<std::string> host_name_tokens;
      base::SplitStringDontTrim(host_name, '.', &host_name_tokens);
-     SSLErrorClassification ssl_error(time, origin, *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));
@@ -114,7 +157,11 @@ TEST(SSLErrorClassificationTest, TestNameMismatch) {
     std::string host_name = origin.host();
     std::vector<std::string> host_name_tokens;
     base::SplitStringDontTrim(host_name, '.', &host_name_tokens);
-    SSLErrorClassification ssl_error(time, origin, *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));
@@ -136,7 +183,11 @@ TEST(SSLErrorClassificationTest, TestNameMismatch) {
     std::string host_name = origin.host();
     std::vector<std::string> host_name_tokens;
     base::SplitStringDontTrim(host_name, '.', &host_name_tokens);
-    SSLErrorClassification ssl_error(time, origin, *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));
@@ -147,7 +198,7 @@ TEST(SSLErrorClassificationTest, TestNameMismatch) {
   }
 }
 
-TEST(SSLErrorClassificationTest, TestHostNameHasKnownTLD) {
+TEST_F(SSLErrorClassificationTest, TestHostNameHasKnownTLD) {
   std::string url1 = "www.google.com";
   std::string url2 = "b.appspot.com";
   std::string url3 = "a.private";

tools/metrics/histograms/histograms.xml

@@ -10436,6 +10436,17 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="interstitial.ssl.captive_portal" enum="SSLCaptivePortal">
+  <owner>meacer@chromium.org</owner>
+  <summary>
+    Record possible states of captive portals. This histogram is emitted
+    (possibly multiple times to different buckets) whenever a ssl interstitial
+    page is displayed and captive portal detection is enabled. The captive
+    portal technique forces a client on a network to see a special web page
+    (usually for authentication purposes) before using the internet normally.
+  </summary>
+</histogram>
+
 <histogram name="interstitial.ssl.cause.nonoverridable"
     enum="SSLNonAttackCauses">
   <owner>felt@chromium.org</owner>
@@ -49871,6 +49882,44 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="5" label="DEPRECATION_RAZE_FAILED">Raze failed.</int>
 </enum>
 
+<enum name="SSLCaptivePortal" type="int">
+  <int value="0"
+      label="Chrome captive portal detection enabled
+             (CAPTIVE_PORTAL_DETECTION_ENABLED)"/>
+  <int value="1"
+      label="Chrome captive portal detection enabled on an overridable SSL
+             error page (CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE)">
+    This is a subset of CAPTIVE_PORTAL_DETECTION_ENABLED (bucket 0), the only
+    difference is that it is for overridable errors.
+  </int>
+  <int value="2"
+      label="Received a captive portal probe result.
+             (CAPTIVE_PORTAL_PROBE_COMPLETED)">
+    Was the captive portal probe completed before the interstitial was closed?
+    Captive Portal won't be detected unless ::Observe is triggered which might
+    be a few seconds later.
+  </int>
+  <int value="3"
+      label="Received a captive portal result on an overridable SSL error page
+             (CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE)">
+    This is a subset of CAPTIVE_PORTAL_PROBE_COMPLETED (bucket 2), the only
+    difference is that it is for overridable errors.
+  </int>
+  <int value="4"
+      label="Received no response or Non-HTTP login page
+             (CAPTIVE_PORTAL_NO_RESPONSE)"/>
+  <int value="5"
+      label="Received no response or Non-HTTP login page on an overridable
+             SSL error page (CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE)"/>
+  <int value="6" label="Detected captive portal (CAPTIVE_PORTAL_DETECTED)"/>
+  <int value="7"
+      label="Detected captive portal on an overridable SSL error page
+             (CAPTIVE_PORTAL_DETECTED_OVERRIDABLE)">
+    This is a subset of CAPTIVE_PORTAL_DETECTED (bucket 6), the only difference
+    is that it is for overridable errors.
+  </int>
+</enum>
+
 <enum name="SSLCipherSuite" type="int">
   <summary>SSL/TLS cipher suites from the IANA registry</summary>
   <int value="0" label="TLS_NULL_WITH_NULL_NULL"/>
@@ -50274,26 +50323,28 @@ To add a new entry, add it with any value and run test to compute valid value.
       label="User manually typed proceed (PROCEED_MANUAL_NONOVERRIDABLE)"/>
   <int value="17"
       label="Chrome captive portal detection enabled
-             (CAPTIVE_PORTAL_DETECTION_ENABLED)"/>
+             (DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED)"/>
   <int value="18"
       label="Chrome captive portal detection enabled on an overridable SSL
-             error page (CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE)"/>
+             error page
+             (DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE)"/>
   <int value="19"
       label="Received a captive portal result
-             (CAPTIVE_PORTAL_PROBE_COMPLETED)"/>
+             (DEPRECATED_CAPTIVE_PORTAL_PROBE_COMPLETED)"/>
   <int value="20"
       label="Received a captive portal result on an overridable SSL error
-             page (CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE)"/>
+             page (DEPRECATED_CAPTIVE_PORTAL_PROBE_COMPLETED_OVERRIDABLE)"/>
   <int value="21"
       label="Received no response or Non-HTTP login page
-             (CAPTIVE_PORTAL_NO_RESPONSE)"/>
+             (DEPRECATED_CAPTIVE_PORTAL_NO_RESPONSE)"/>
   <int value="22"
       label="Received no response or Non-HTTP login page on an overridable
-             SSL error page (CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE)"/>
+             SSL error page
+             (DEPRECATED_CAPTIVE_PORTAL_NO_RESPONSE_OVERRIDABLE)"/>
   <int value="23" label="Detected captive portal (CAPTIVE_PORTAL_DETECTED)"/>
   <int value="24"
       label="Detected captive portal on an overridable SSL error page
-             (CAPTIVE_PORTAL_DETECTED_OVERRIDABLE)"/>
+             (DEPRECATED_CAPTIVE_PORTAL_DETECTED_OVERRIDABLE)"/>
 </enum>
 
 <enum name="StartupURLsMigration" type="int">