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

chrome/browser/ssl/ssl_error_classification.h

 // Copyright 2014 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_SSL_ERROR_CLASSIFICATION_H_
 #define CHROME_BROWSER_SSL_SSL_ERROR_CLASSIFICATION_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"
 
+namespace content {
+class WebContents;
+}
+
 // This class calculates the severity scores for the different type of SSL
 // errors.
-class SSLErrorClassification {
+class SSLErrorClassification : public content::NotificationObserver {
  public:
-  SSLErrorClassification(base::Time current_time,
-                         const net::X509Certificate& cert);
-  ~SSLErrorClassification();
+  SSLErrorClassification(content::WebContents& web_contents,
+                         base::Time current_time,
+                         const::net::X509Certificate& cert);

[meacer, 2014/07/19 00:06:45]

You don't need :: before net here, do you?

const net::X509Certificate should be OK.

[radhikabhar, 2014/07/21 16:05:11]

Done.

+  virtual ~SSLErrorClassification();
 
-  // This method checks whether the user clock is in the past or not.
+  // This method checks whether the system time is in the past.
   static bool IsUserClockInThePast(base::Time time_now);
 
   // This method checks whether the system time is too far in the future or
   // the user is using a version of Chrome which is more than 1 year old.
   static bool IsUserClockInTheFuture(base::Time time_now);
 
+  // This method checks whether captive protals have been detected or not.
+  void CaptivePortalDetect(bool record_histograms,

[meacer, 2014/07/19 00:06:45]

Maybe I'm missing something but I can't see the implementation for this method
in ssl_error_classification.cc?

[radhikabhar, 2014/07/21 16:05:11]

My bad. I had this function previously then deleted it as I no longer needed it.
Forgot to delete it here. 

+                           bool overridable);
+
   // A method which calculates the severity score when the ssl error is
   // CERT_DATE_INVALID.
   float InvalidDateSeverityScore() const;
 
-  static void RecordUMAStatistics(bool overridable);
+  void RecordUMAStatistics(bool overridable);
+  void RecordUMAStatisticsCaptivePortals(bool overridable);

[meacer, 2014/07/19 00:06:45]

nit: Maybe rename to RecordCaptivePortalUMAStatistics?

[radhikabhar, 2014/07/21 16:05:11]

Done.

   base::TimeDelta TimePassedSinceExpiry() const;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(SSLErrorClassification, TestDateInvalidScore);
+  FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest, TestDateInvalidScore);
 
   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_;

[meacer, 2014/07/19 00:06:45]

I think you should make this a pointer. The majority of the code keeps
web_contents as a pointer.

   // This stores the current time.
   base::Time current_time_;
-
   // 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_