Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(356)

Unified Diff: chrome/browser/ssl/ssl_blocking_page.cc

Issue 400323002: Refactor the captive portal code to move from the ssl_blocking_page class to the ssl_error_classific (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressed comments for the histogram.xml file Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 a9727e1f9ca24a1f9acd89143dc66ece755fcded..6ea05f9ae730fb9db04052d4acdaf465577d9d2d 100644
--- a/chrome/browser/ssl/ssl_blocking_page.cc
+++ b/chrome/browser/ssl/ssl_blocking_page.cc
@@ -46,11 +46,6 @@
#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(ENABLE_EXTENSIONS)
#include "chrome/browser/extensions/api/experience_sampling_private/experience_sampling.h"
#endif
@@ -106,14 +101,15 @@ 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,
+ // Captive Portal errors moved to ssl_error_classification.
+ DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED,
+ DEPRECATED_CAPTIVE_PORTAL_DETECTION_ENABLED_OVERRIDABLE,
+ DEPRECATED_CAPTIVE_PORTAL_PROBE_COMPLETED,
+ 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,
};
@@ -128,36 +124,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);
@@ -301,11 +270,7 @@ SSLBlockingPage::SSLBlockingPage(
strict_enforcement_(strict_enforcement),
interstitial_page_(NULL),
internal_(false),
- num_visits_(-1),
- captive_portal_detection_enabled_(false),
- captive_portal_probe_completed_(false),
- captive_portal_no_response_(false),
- captive_portal_detected_(false) {
+ num_visits_(-1) {
Profile* profile = Profile::FromBrowserContext(
web_contents->GetBrowserContext());
if (strict_enforcement_) overridable_ = false;
@@ -329,19 +294,15 @@ SSLBlockingPage::SSLBlockingPage(
}
SSLErrorClassification ssl_error_classification(
+ web_contents_,
base::Time::NowFromSystemTime(),
request_url_,
+ cert_error_,
*ssl_info_.cert.get());
- ssl_error_classification.RecordUMAStatistics(overridable_, cert_error_);
+ ssl_error_classification.RecordUMAStatistics(overridable_);
#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_);
#endif
#if defined(ENABLE_EXTENSIONS)
@@ -364,16 +325,35 @@ SSLBlockingPage::SSLBlockingPage(
}
SSLBlockingPage::~SSLBlockingPage() {
+ // InvalidCommonNameSeverityScore() and InvalidDateSeverityScore() are in the
+ // destructor because they depend on knowing whether captive portal detection
+ // happened before the user made a decision.
+ SSLErrorClassification ssl_error_classification(
+ web_contents_,
+ base::Time::NowFromSystemTime(),
+ request_url_,
+ cert_error_,
+ *ssl_info_.cert.get());
+ SSLErrorInfo::ErrorType type =
+ SSLErrorInfo::NetErrorToErrorType(cert_error_);
+ switch (type) {
+ case SSLErrorInfo::CERT_DATE_INVALID: {
+ ssl_error_classification.InvalidDateSeverityScore();
+ break;
+ }
+ case SSLErrorInfo::CERT_COMMON_NAME_INVALID: {
+ ssl_error_classification.InvalidCommonNameSeverityScore();
+ break;
+ }
+ default:
+ break;
+ }
if (!callback_.is_null()) {
RecordSSLBlockingPageDetailedStats(false,
cert_error_,
overridable_,
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();
@@ -558,16 +538,13 @@ void SSLBlockingPage::OnProceed() {
cert_error_,
overridable_,
internal_,
- num_visits_,
- captive_portal_detection_enabled_,
- captive_portal_probe_completed_,
- captive_portal_no_response_,
- captive_portal_detected_);
+ num_visits_);
#if defined(ENABLE_EXTENSIONS)
// ExperienceSampling: Notify that user decided to proceed.
if (sampling_event_.get())
sampling_event_->CreateUserDecisionEvent(ExperienceSamplingEvent::kProceed);
#endif
+
// Accepting the certificate resumes the loading of the page.
NotifyAllowCertificate();
}
@@ -577,11 +554,7 @@ void SSLBlockingPage::OnDontProceed() {
cert_error_,
overridable_,
internal_,
- num_visits_,
- captive_portal_detection_enabled_,
- captive_portal_probe_completed_,
- captive_portal_no_response_,
- captive_portal_detected_);
+ num_visits_);
#if defined(ENABLE_EXTENSIONS)
// ExperienceSampling: Notify that user decided to not proceed.
// This also occurs if the user navigates away or closes the tab.
@@ -631,35 +604,3 @@ void SSLBlockingPage::OnGotHistoryCount(bool success,
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
-}

Powered by Google App Engine
This is Rietveld 408576698