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

 // Copyright (c) 2012 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/ssl_blocking_page.h"
 
 #include "base/build_time.h"
 #include "base/command_line.h"
 #include "base/i18n/rtl.h"
 #include "base/i18n/time_formatting.h"
@@ skipping 28 matching lines @@
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "net/base/hash_value.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #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(OS_WIN)
 #include "base/base_paths_win.h"
 #include "base/path_service.h"
 #include "base/strings/string16.h"
 #include "base/win/windows_version.h"
 #endif
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/chrome_pages.h"
@@ skipping 55 matching lines @@
   DONT_PROCEED_NAME,
   DONT_PROCEED_DATE,
   DONT_PROCEED_AUTHORITY,
   MORE,
   SHOW_UNDERSTAND,  // Used by the summer 2013 Finch trial. Deprecated.
   SHOW_INTERNAL_HOSTNAME,
   PROCEED_INTERNAL_HOSTNAME,
   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,
   UNUSED_BLOCKING_PAGE_EVENT,
 };
 
 void RecordSSLBlockingPageEventStats(SSLBlockingPageEvent event) {
   UMA_HISTOGRAM_ENUMERATION("interstitial.ssl",
                             event,
                             UNUSED_BLOCKING_PAGE_EVENT);
 }
 
 void RecordSSLBlockingPageDetailedStats(
     bool proceed,
     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);
     }
     // Overridable is false if the user didn't have any option except to turn
     // back. If that's the case, don't record some of the metrics.
     return;
   }
   if (num_visits == 0)
     RecordSSLBlockingPageEventStats(SHOW_NEW_SITE);
@@ skipping 123 matching lines @@
     const base::Callback<void(bool)>& callback)
     : callback_(callback),
       web_contents_(web_contents),
       cert_error_(cert_error),
       ssl_info_(ssl_info),
       request_url_(request_url),
       overridable_(overridable),
       strict_enforcement_(strict_enforcement),
       internal_(false),
       num_visits_(-1),
-      captive_portal_detection_enabled_(false),
-      captive_portal_probe_completed_(false),
-      captive_portal_no_response_(false),
-      captive_portal_detected_(false),
       trial_condition_(GetTrialCondition()) {
   Profile* profile = Profile::FromBrowserContext(
       web_contents->GetBrowserContext());
   // For UMA stats.
   if (net::IsHostnameNonUnique(request_url_.HostNoBrackets()))
     internal_ = true;
   RecordSSLBlockingPageEventStats(SHOW_ALL);
   if (overridable_ && !strict_enforcement_) {
     RecordSSLBlockingPageEventStats(SHOW_OVERRIDABLE);
     if (internal_)
       RecordSSLBlockingPageEventStats(SHOW_INTERNAL_HOSTNAME);
     HistoryService* history_service = HistoryServiceFactory::GetForProfile(
         profile, Profile::EXPLICIT_ACCESS);
     if (history_service) {
       history_service->GetVisibleVisitCountToHost(
           request_url_,
           base::Bind(&SSLBlockingPage::OnGotHistoryCount,
                      base::Unretained(this)),
           &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)
-  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_ && !strict_enforcement_);

[meacer, 2014/07/24 00:46:30]

Maybe you can add a public method to SSLErrorClassification such as RecordUMA
which calls RecordSSLUMAStatistics and RecordCaptivePortalUMAStatistics, and
then call ssl_error_classification.RecordUMA() here?

That way you don't have to check for ENABLE_CAPTIVE_PORTAL_DETECTION here, and
you can do the
SSLErrorInfo::NetErrorToErrorType(cert_error_)==SSLErrorInfo::CERT_DATE_INVALID
check in the error handler as well.

I don't have a strong preference though.

 #endif
 
   interstitial_page_ = InterstitialPage::Create(
       web_contents_, true, request_url, this);
   interstitial_page_->Show();
 }
 
 SSLBlockingPage::~SSLBlockingPage() {
   if (!callback_.is_null()) {
     RecordSSLBlockingPageDetailedStats(false,
                                        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();
   }
 }
 
 std::string SSLBlockingPage::GetHTMLContents() {
   if (trial_condition_ == kCondV1 || trial_condition_ == kCondV1LayoutV2Text)
     return GetHTMLContentsV1();
   return GetHTMLContentsV2();
@@ skipping 197 matching lines @@
     load_time_data.SetString(
         "primaryParagraph",
         l10n_util::GetStringFUTF16(IDS_SSL_V2_PRIMARY_PARAGRAPH, url));
   }
   load_time_data.SetString(
      "openDetails",
      l10n_util::GetStringUTF16(IDS_SSL_V2_OPEN_DETAILS_BUTTON));
   load_time_data.SetString(
      "closeDetails",
      l10n_util::GetStringUTF16(IDS_SSL_V2_CLOSE_DETAILS_BUTTON));
-  load_time_data.SetString("errorCode", net::ErrorToString(cert_error_));
 
   if (overridable_ && !strict_enforcement_) {  // Overridable.
     SSLErrorInfo error_info =
         SSLErrorInfo::CreateError(
             SSLErrorInfo::NetErrorToErrorType(cert_error_),
             ssl_info_.cert.get(),
             request_url_);
     load_time_data.SetString(
         "explanationParagraph", error_info.details());
     load_time_data.SetString(
@@ skipping 26 matching lines @@
         break;
       case SSLErrorInfo::CERT_INVALID:
         help_string = IDS_SSL_NONOVERRIDABLE_INVALID;
         break;
       default:
         if (strict_enforcement_)
           help_string = IDS_SSL_NONOVERRIDABLE_HSTS;
     }
     load_time_data.SetString(
         "finalParagraph", l10n_util::GetStringFUTF16(help_string, url));
+    load_time_data.SetString("errorCode", net::ErrorToString(cert_error_));
   }
 
   base::StringPiece html(
      ResourceBundle::GetSharedInstance().GetRawDataResource(
          IRD_SSL_INTERSTITIAL_V2_HTML));
   webui::UseVersion2 version;
   return webui::GetI18nTemplateHtml(html, &load_time_data);
 }
 
 void SSLBlockingPage::OverrideEntry(NavigationEntry* entry) {
@@ skipping 54 matching lines @@
   Profile* profile = Profile::FromBrowserContext(
       web_contents_->GetBrowserContext());
   renderer_preferences_util::UpdateFromSystemSettings(prefs, profile);
 }
 
 void SSLBlockingPage::OnProceed() {
   RecordSSLBlockingPageDetailedStats(true,
                                      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();
 }
 
 void SSLBlockingPage::OnDontProceed() {
   RecordSSLBlockingPageDetailedStats(false,
                                      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();
 }
 
 void SSLBlockingPage::NotifyDenyCertificate() {
   // It's possible that callback_ may not exist if the user clicks "Proceed"
   // followed by pressing the back button before the interstitial is hidden.
   // In that case the certificate will still be treated as allowed.
   if (callback_.is_null())
     return;
 
@@ skipping 23 matching lines @@
   for (; i < 5; i++) {
     strings->SetString(keys[i], std::string());
   }
 }
 
 void SSLBlockingPage::OnGotHistoryCount(bool success,
                                         int num_visits,
                                         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
-}