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

Index: chrome/browser/ssl/ssl_error_classification.h
diff --git a/chrome/browser/ssl/ssl_error_classification.h b/chrome/browser/ssl/ssl_error_classification.h
index dec1ed295901132ded88faf46f12ea52300a663d..9ccbe52fca94176ec20c3ed1f80b001378b18c63 100644
--- a/chrome/browser/ssl/ssl_error_classification.h
+++ b/chrome/browser/ssl/ssl_error_classification.h
@@ -9,17 +9,25 @@
 #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 calculates the severity scores for the different type of SSL

[palmer, 2014/08/07 23:48:37]

Now that it extends |NotificationObserver|, this class is no longer that simple.
Document what else it does.

[radhikabhar, 2014/08/08 00:19:43]

Done.

 // errors.
-class SSLErrorClassification {
+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 +39,19 @@ class SSLErrorClassification {
   static bool IsWindowsVersionSP3OrLower();
 
   // A function which calculates the severity score when the ssl error is

[palmer, 2014/08/07 23:48:37]

As before, be more concise in these comments (for this function and for
|InvalidCommonNameSeverityScore|).

The sentences are not super grammatical English.

[radhikabhar, 2014/08/08 00:19:43]

Done.

-  // CERT_DATE_INVALID, returns a score between 0.0 and 1.0, higher values
+  // CERT_DATE_INVALID, calculates 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;
+  void InvalidDateSeverityScore() const;

[palmer, 2014/08/07 23:48:37]

How can these functions be all of: void, const, and meaningful?

[radhikabhar, 2014/08/08 00:19:43]

Done.

 
   // 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
+  // when the SSL error is |CERT_COMMON_NAME_INVALID|, calcualates a score

[palmer, 2014/08/07 23:48:37]

Typo: should be "calculates"

   // 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;
+  void InvalidCommonNameSeverityScore() const;
 
-  void RecordUMAStatistics(bool overridable, int cert_error);
+  void RecordUMAStatistics(bool overridable) const;
+  void RecordCaptivePortalUMAStatistics(bool overridable) const;
   base::TimeDelta TimePassedSinceExpiry() const;
 
  private:
@@ -84,8 +93,6 @@ class SSLErrorClassification {
   //     b.example.com ~ *.example.com --> false
   bool IsSubDomainOutsideWildcard(const Tokens& hostname) const;
 
-  float CalculateScoreTimePassedSinceExpiry() const;
-
   static std::vector<Tokens> GetTokenizedDNSNames(
       const std::vector<std::string>& dns_names);
 
@@ -103,13 +110,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_