Refactor the captive portal code to move from the ssl_blocking_page class to the ssl_error_classific

chrome/browser/ssl/ssl_blocking_page.cc

Index: chrome/browser/ssl/ssl_blocking_page.cc
diff --git a/chrome/browser/ssl/ssl_blocking_page.cc b/chrome/browser/ssl/ssl_blocking_page.cc
index ca97c2b60a0a90009235bd3716fc0b3b657f2126..7efc9812a4e8c013e29099285d3bfbb83fcc896d 100644
--- a/chrome/browser/ssl/ssl_blocking_page.cc
+++ b/chrome/browser/ssl/ssl_blocking_page.cc
@@ -116,14 +116,14 @@ 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,
+  DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED,  // Captive Portal errors moved
+  DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE,  // to
+  DEPRECATED_CAPTIVE_PORTAL_PROBE_COMPLETED,  // ssl_error_classification.
+  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,

[meacer, 2014/07/21 18:45:14]

histograms.xml have entries for the deprecated enums. I think you'll need to
mark them as deprecated as well.

[radhikabhar, 2014/07/23 21:41:29]

Done.

   UNUSED_BLOCKING_PAGE_EVENT,
 };
 
@@ -138,36 +138,9 @@ void RecordSSLBlockingPageDetailedStats(
     int cert_error,
     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) {
+    int num_visits) {
   UMA_HISTOGRAM_ENUMERATION("interstitial.ssl_error_type",
       SSLErrorInfo::NetErrorToErrorType(cert_error), SSLErrorInfo::END_OF_ENUM);
-#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);
@@ -262,10 +235,15 @@ SSLBlockingPage::SSLBlockingPage(
           &request_tracker_);
     }
   }
+
+  SSLErrorClassification ssl_error_classification(
+      web_contents_,
+      base::Time::NowFromSystemTime(),
+      *ssl_info_.cert.get());
   if (SSLErrorInfo::NetErrorToErrorType(cert_error_) ==
       SSLErrorInfo::CERT_DATE_INVALID) {
-    SSLErrorClassification::RecordUMAStatistics(overridable_ &&
-                                                !strict_enforcement_);
+    ssl_error_classification.RecordUMAStatistics(
+        overridable_ && !strict_enforcement_);
   }
 
 #if defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
@@ -276,6 +254,8 @@ SSLBlockingPage::SSLBlockingPage(
   registrar_.Add(this,
                  chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT,
                  content::Source<Profile>(profile));
+  ssl_error_classification.RecordCaptivePortalUMAStatistics(
+      overridable_ && !strict_enforcement_);
 #endif
 
   interstitial_page_ = InterstitialPage::Create(
@@ -289,11 +269,7 @@ SSLBlockingPage::~SSLBlockingPage() {
                                        cert_error_,
                                        overridable_ && !strict_enforcement_,
                                        internal_,
-                                       num_visits_,
-                                       captive_portal_detection_enabled_,
-                                       captive_portal_probe_completed_,
-                                       captive_portal_no_response_,
-                                       captive_portal_detected_);
+                                       num_visits_);
     // The page is closed without the user having chosen what to do, default to
     // deny.
     NotifyDenyCertificate();
@@ -634,11 +610,7 @@ void SSLBlockingPage::OnProceed() {
                                      cert_error_,
                                      overridable_ && !strict_enforcement_,
                                      internal_,
-                                     num_visits_,
-                                     captive_portal_detection_enabled_,
-                                     captive_portal_probe_completed_,
-                                     captive_portal_no_response_,
-                                     captive_portal_detected_);
+                                     num_visits_);
   // Accepting the certificate resumes the loading of the page.
   NotifyAllowCertificate();
 }
@@ -648,11 +620,7 @@ void SSLBlockingPage::OnDontProceed() {
                                      cert_error_,
                                      overridable_ && !strict_enforcement_,
                                      internal_,
-                                     num_visits_,
-                                     captive_portal_detection_enabled_,
-                                     captive_portal_probe_completed_,
-                                     captive_portal_no_response_,
-                                     captive_portal_detected_);
+                                     num_visits_);
   NotifyDenyCertificate();
 }