Move error classification into the ssl_errors component

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/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
 #include "base/i18n/rtl.h"
 #include "base/i18n/time_formatting.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/interstitials/chrome_metrics_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_preferences_util.h"
 #include "chrome/browser/ssl/cert_report_helper.h"
+#include "chrome/browser/ssl/chrome_error_classification.h"
 #include "chrome/browser/ssl/ssl_cert_reporter.h"
-#include "chrome/browser/ssl/ssl_error_classification.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/google/core/browser/google_util.h"
+#include "components/ssl_errors/error_classification.h"

[estark, 2015/09/23 22:42:19]

Do you need to include both, or would chrome_error_classification.h be enough?
(same question on bad_clock_blocking_page.cc)

 #include "components/ssl_errors/error_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/cert_store.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/interstitial_page_delegate.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
@@ skipping 103 matching lines @@
   metrics_helper()->RecordUserDecision(
       security_interstitials::MetricsHelper::SHOW);
   metrics_helper()->RecordUserInteraction(
       security_interstitials::MetricsHelper::TOTAL_VISITS);
 
   cert_report_helper_.reset(new CertReportHelper(
       ssl_cert_reporter.Pass(), web_contents, request_url, ssl_info,
       certificate_reporting::ErrorReport::INTERSTITIAL_SSL, overridable_,
       metrics_helper()));
 
-  ssl_error_classification_.reset(new SSLErrorClassification(
-      web_contents,
-      time_triggered_,
-      request_url,
-      cert_error_,
-      *ssl_info_.cert.get()));
+  ssl_error_classification_.reset(
+      new ChromeErrorClassification(web_contents, time_triggered_, request_url,

[estark, 2015/09/23 22:42:19]

If you decide to stick with this design (ChromeEC inheriting from
ssl_errors::EC), maybe keep |ssl_error_classification_| as a
ErrorClassification, and define a virtual
ErrorClassification::RecordCleanupStatistics() method that ChromeEC overrides to
record the captive portal statistics. And call
ssl_error_classification_->RecordCleanupStatistics() in the SSLBlockingPage
destructor instead of calling RecordCaptivePortalStatistics directly. Mostly
suggesting this because I think you'll eventually have to do this anyway when
you componentize SSLBlockingPage.

+                                    cert_error_, *ssl_info_.cert.get()));
   ssl_error_classification_->RecordUMAStatistics(overridable_);
 
   // Creating an interstitial without showing (e.g. from chrome://interstitials)
   // it leaks memory, so don't create it here.
 }
 
 bool SSLBlockingPage::ShouldCreateNewNavigation() const {
   return true;
 }
 
@@ skipping 55 matching lines @@
         l10n_util::GetStringUTF16(IDS_SSL_OVERRIDABLE_SAFETY_BUTTON));
     load_time_data->SetString(
         "finalParagraph",
         l10n_util::GetStringFUTF16(IDS_SSL_OVERRIDABLE_PROCEED_PARAGRAPH, url));
   } else {
     load_time_data->SetBoolean("overridable", false);
 
     ssl_errors::ErrorInfo::ErrorType type =
         ssl_errors::ErrorInfo::NetErrorToErrorType(cert_error_);
     if (type == ssl_errors::ErrorInfo::CERT_INVALID &&
-        SSLErrorClassification::MaybeWindowsLacksSHA256Support()) {
+        ssl_errors::ErrorClassification::MaybeWindowsLacksSHA256Support()) {
       load_time_data->SetString(
           "explanationParagraph",
           l10n_util::GetStringFUTF16(IDS_SSL_NONOVERRIDABLE_MORE_INVALID_SP3,
                                      url));
     } else {
       load_time_data->SetString(
           "explanationParagraph",
           l10n_util::GetStringFUTF16(IDS_SSL_NONOVERRIDABLE_MORE, url));
     }
     load_time_data->SetString("primaryButtonText",
@@ skipping 205 matching lines @@
       !(options_mask & SSLBlockingPage::STRICT_ENFORCEMENT) &&
       profile->GetPrefs()->GetBoolean(prefs::kSSLErrorOverrideAllowed);
   return is_overridable;
 }
 
 // static
 bool SSLBlockingPage::DoesPolicyAllowDangerOverride(
     const Profile* const profile) {
   return profile->GetPrefs()->GetBoolean(prefs::kSSLErrorOverrideAllowed);
 }