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

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

Issue 872813003: Add Rappor metrics for Safe Browsing and SSL interstitials. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Actually fix nit Created 5 years, 11 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 214c035317d472aeb545d1b119b16fe900a7a83a..0349dde723f3ded797ded55f9be53ca2d51f23d3 100644
--- a/chrome/browser/ssl/ssl_blocking_page.cc
+++ b/chrome/browser/ssl/ssl_blocking_page.cc
@@ -233,12 +233,17 @@ SSLBlockingPage::SSLBlockingPage(content::WebContents* web_contents,
IsErrorDueToBadClock(base::Time::NowFromSystemTime(), cert_error_) ?
SSL_REASON_BAD_CLOCK : SSL_REASON_SSL;
+ // We collapse the Rappor metric name to just "ssl" so we don't leak
+ // the "overridable" bit. We skip Rappor altogether for bad clocks.
// This must be done after calculating |interstitial_reason_| above.
- uma_helper_.reset(new SecurityInterstitialUmaHelper(
- web_contents, request_url, GetHistogramPrefix(), GetSamplingEventName()));
- uma_helper_->RecordUserDecision(SecurityInterstitialUmaHelper::SHOW);
- uma_helper_->RecordUserInteraction(
- SecurityInterstitialUmaHelper::TOTAL_VISITS);
+ const std::string rappor_prefix(
+ interstitial_reason_ == SSL_REASON_BAD_CLOCK ? std::string() : "ssl");
+ metrics_helper_.reset(new SecurityInterstitialMetricsHelper(
+ web_contents, request_url, GetUmaHistogramPrefix(), rappor_prefix,
+ GetSamplingEventName()));
+ metrics_helper_->RecordUserDecision(SecurityInterstitialMetricsHelper::SHOW);
+ metrics_helper_->RecordUserInteraction(
+ SecurityInterstitialMetricsHelper::TOTAL_VISITS);
ssl_error_classification_.reset(new SSLErrorClassification(
web_contents,
@@ -267,8 +272,8 @@ SSLBlockingPage::~SSLBlockingPage() {
if (!callback_.is_null()) {
// The page is closed without the user having chosen what to do, default to
// deny.
- uma_helper_->RecordUserDecision(
- SecurityInterstitialUmaHelper::DONT_PROCEED);
+ metrics_helper_->RecordUserDecision(
+ SecurityInterstitialMetricsHelper::DONT_PROCEED);
RecordSSLExpirationPageEventState(
expired_but_previously_allowed_, false, overridable_);
NotifyDenyCertificate();
@@ -451,20 +456,20 @@ void SSLBlockingPage::CommandReceived(const std::string& command) {
break;
}
case CMD_MORE: {
- uma_helper_->RecordUserInteraction(
- SecurityInterstitialUmaHelper::SHOW_ADVANCED);
+ metrics_helper_->RecordUserInteraction(
+ SecurityInterstitialMetricsHelper::SHOW_ADVANCED);
break;
}
case CMD_RELOAD: {
- uma_helper_->RecordUserInteraction(
- SecurityInterstitialUmaHelper::RELOAD);
+ metrics_helper_->RecordUserInteraction(
+ SecurityInterstitialMetricsHelper::RELOAD);
// The interstitial can't refresh itself.
web_contents()->GetController().Reload(true);
break;
}
case CMD_HELP: {
- uma_helper_->RecordUserInteraction(
- SecurityInterstitialUmaHelper::SHOW_LEARN_MORE);
+ metrics_helper_->RecordUserInteraction(
+ SecurityInterstitialMetricsHelper::SHOW_LEARN_MORE);
content::NavigationController::LoadURLParams help_page_params(
google_util::AppendGoogleLocaleParam(
GURL(kHelpURL), g_browser_process->GetApplicationLocale()));
@@ -472,8 +477,8 @@ void SSLBlockingPage::CommandReceived(const std::string& command) {
break;
}
case CMD_CLOCK: {
- uma_helper_->RecordUserInteraction(
- SecurityInterstitialUmaHelper::OPEN_TIME_SETTINGS);
+ metrics_helper_->RecordUserInteraction(
+ SecurityInterstitialMetricsHelper::OPEN_TIME_SETTINGS);
LaunchDateAndTimeSettings();
break;
}
@@ -492,7 +497,8 @@ void SSLBlockingPage::OverrideRendererPrefs(
}
void SSLBlockingPage::OnProceed() {
- uma_helper_->RecordUserDecision(SecurityInterstitialUmaHelper::PROCEED);
+ metrics_helper_->RecordUserDecision(
+ SecurityInterstitialMetricsHelper::PROCEED);
RecordSSLExpirationPageEventState(
expired_but_previously_allowed_, true, overridable_);
// Accepting the certificate resumes the loading of the page.
@@ -500,7 +506,8 @@ void SSLBlockingPage::OnProceed() {
}
void SSLBlockingPage::OnDontProceed() {
- uma_helper_->RecordUserDecision(SecurityInterstitialUmaHelper::DONT_PROCEED);
+ metrics_helper_->RecordUserDecision(
+ SecurityInterstitialMetricsHelper::DONT_PROCEED);
RecordSSLExpirationPageEventState(
expired_but_previously_allowed_, false, overridable_);
NotifyDenyCertificate();
@@ -524,7 +531,7 @@ void SSLBlockingPage::NotifyAllowCertificate() {
callback_.Reset();
}
-std::string SSLBlockingPage::GetHistogramPrefix() const {
+std::string SSLBlockingPage::GetUmaHistogramPrefix() const {
switch (interstitial_reason_) {
case SSL_REASON_SSL:
if (overridable_)

Powered by Google App Engine
This is Rietveld 408576698