diff options
| -rw-r--r-- | chrome/browser/interstitials/security_interstitial_metrics_helper.cc (renamed from chrome/browser/interstitials/security_interstitial_uma_helper.cc) | 60 | ||||
| -rw-r--r-- | chrome/browser/interstitials/security_interstitial_metrics_helper.h | 94 | ||||
| -rw-r--r-- | chrome/browser/interstitials/security_interstitial_uma_helper.h | 72 | ||||
| -rw-r--r-- | chrome/browser/safe_browsing/safe_browsing_blocking_page.cc | 43 | ||||
| -rw-r--r-- | chrome/browser/safe_browsing/safe_browsing_blocking_page.h | 6 | ||||
| -rw-r--r-- | chrome/browser/ssl/ssl_blocking_page.cc | 48 | ||||
| -rw-r--r-- | chrome/browser/ssl/ssl_blocking_page.h | 8 | ||||
| -rw-r--r-- | chrome/chrome_browser.gypi | 4 | ||||
| -rw-r--r-- | components/rappor/rappor_service.cc | 3 | ||||
| -rw-r--r-- | tools/metrics/rappor/rappor.xml | 32 |
10 files changed, 236 insertions, 134 deletions
diff --git a/chrome/browser/interstitials/security_interstitial_uma_helper.cc b/chrome/browser/interstitials/security_interstitial_metrics_helper.cc index aade7ff..322220e 100644 --- a/chrome/browser/interstitials/security_interstitial_uma_helper.cc +++ b/chrome/browser/interstitials/security_interstitial_metrics_helper.cc @@ -2,56 +2,88 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "chrome/browser/interstitials/security_interstitial_uma_helper.h" +#include "chrome/browser/interstitials/security_interstitial_metrics_helper.h" + +#include <string> #include "base/metrics/histogram.h" +#include "chrome/browser/browser_process.h" #include "chrome/browser/history/history_service.h" #include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/webdata/web_data_service_factory.h" +#include "components/rappor/rappor_service.h" #include "content/public/browser/web_contents.h" +#include "net/base/registry_controlled_domains/registry_controlled_domain.h" #if defined(ENABLE_EXTENSIONS) #include "chrome/browser/extensions/api/experience_sampling_private/experience_sampling.h" #endif -SecurityInterstitialUmaHelper::SecurityInterstitialUmaHelper( +SecurityInterstitialMetricsHelper::SecurityInterstitialMetricsHelper( content::WebContents* web_contents, const GURL& request_url, - const std::string& histogram_prefix, + const std::string& uma_prefix, + const std::string& rappor_prefix, + RapporReporting rappor_reporting, const std::string& sampling_event_name) : web_contents_(web_contents), request_url_(request_url), - histogram_prefix_(histogram_prefix), + uma_prefix_(uma_prefix), + rappor_prefix_(rappor_prefix), + rappor_reporting_(rappor_reporting), sampling_event_name_(sampling_event_name), num_visits_(-1) { + DCHECK(!uma_prefix_.empty()); + DCHECK(!rappor_prefix_.empty()); + DCHECK(!sampling_event_name_.empty()); HistoryService* history_service = HistoryServiceFactory::GetForProfile( Profile::FromBrowserContext(web_contents->GetBrowserContext()), ServiceAccessType::EXPLICIT_ACCESS); if (history_service) { history_service->GetVisibleVisitCountToHost( request_url_, - base::Bind(&SecurityInterstitialUmaHelper::OnGotHistoryCount, + base::Bind(&SecurityInterstitialMetricsHelper::OnGotHistoryCount, base::Unretained(this)), &request_tracker_); } } -SecurityInterstitialUmaHelper::~SecurityInterstitialUmaHelper() { +SecurityInterstitialMetricsHelper::~SecurityInterstitialMetricsHelper() { } -// Directly adds to the histograms, using the same properties as +// Directly adds to the UMA histograms, using the same properties as // UMA_HISTOGRAM_ENUMERATION, because the macro doesn't allow non-constant -// histogram names. -void SecurityInterstitialUmaHelper::RecordUserDecision( +// histogram names. Reports to Rappor for certain decisions. +void SecurityInterstitialMetricsHelper::RecordUserDecision( SecurityInterstitialDecision decision) { + // UMA std::string decision_histogram_name( - "interstitial." + histogram_prefix_ + ".decision"); + "interstitial." + uma_prefix_ + ".decision"); base::HistogramBase* decision_histogram = base::LinearHistogram::FactoryGet( decision_histogram_name, 1, MAX_DECISION, MAX_DECISION + 1, base::HistogramBase::kUmaTargetedHistogramFlag); decision_histogram->Add(decision); + // Rappor + rappor::RapporService* rappor_service = g_browser_process->rappor_service(); + if (rappor_service && rappor_reporting_ == REPORT_RAPPOR && + (decision == PROCEED || decision == DONT_PROCEED)) { + // |domain| will be empty for hosts w/o TLDs (localhost, ip addrs) + const std::string domain = + net::registry_controlled_domains::GetDomainAndRegistry( + request_url_, + net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES); + + // e.g. "interstitial.malware.domain" or "interstitial.ssl.domain" + const std::string metric_name = + "interstitial." + rappor_prefix_ + ".domain"; + rappor_service->RecordSample(metric_name, rappor::COARSE_RAPPOR_TYPE, + domain); + // TODO(nparker): Add reporting of (num_visits > 0) and decision + // once http://crbug.com/451647 is fixed. + } + #if defined(ENABLE_EXTENSIONS) if (!sampling_event_.get()) { sampling_event_.reset(new extensions::ExperienceSamplingEvent( @@ -81,7 +113,7 @@ void SecurityInterstitialUmaHelper::RecordUserDecision( if (num_visits_ < 1 || (decision != PROCEED && decision != DONT_PROCEED)) return; std::string history_histogram_name( - "interstitial." + histogram_prefix_ + ".decision.repeat_visit"); + "interstitial." + uma_prefix_ + ".decision.repeat_visit"); base::HistogramBase* history_histogram = base::LinearHistogram::FactoryGet( history_histogram_name, 1, MAX_DECISION, MAX_DECISION + 1, base::HistogramBase::kUmaTargetedHistogramFlag); @@ -89,10 +121,10 @@ void SecurityInterstitialUmaHelper::RecordUserDecision( history_histogram->Add(decision); } -void SecurityInterstitialUmaHelper::RecordUserInteraction( +void SecurityInterstitialMetricsHelper::RecordUserInteraction( SecurityInterstitialInteraction interaction) { std::string interaction_histogram_name( - "interstitial." + histogram_prefix_ + ".interaction"); + "interstitial." + uma_prefix_ + ".interaction"); base::HistogramBase* interaction_histogram = base::LinearHistogram::FactoryGet( interaction_histogram_name, 1, MAX_INTERACTION, MAX_INTERACTION + 1, @@ -125,7 +157,7 @@ void SecurityInterstitialUmaHelper::RecordUserInteraction( #endif } -void SecurityInterstitialUmaHelper::OnGotHistoryCount( +void SecurityInterstitialMetricsHelper::OnGotHistoryCount( bool success, int num_visits, base::Time first_visit) { diff --git a/chrome/browser/interstitials/security_interstitial_metrics_helper.h b/chrome/browser/interstitials/security_interstitial_metrics_helper.h new file mode 100644 index 0000000..1808852 --- /dev/null +++ b/chrome/browser/interstitials/security_interstitial_metrics_helper.h @@ -0,0 +1,94 @@ +// Copyright (c) 2014 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. + +#ifndef CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_METRICS_HELPER_H_ +#define CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_METRICS_HELPER_H_ + +#include <string> + +#include "base/task/cancelable_task_tracker.h" +#include "base/time/time.h" +#include "url/gurl.h" + +namespace content { +class WebContents; +} + +namespace extensions { +class ExperienceSamplingEvent; +} + +// Most of the security interstitials share a common layout and set of +// choices. SecurityInterstitialMetricsHelper is intended to help the security +// interstitials record user choices in a common way via METRICS histograms +// and RAPPOR metrics. +class SecurityInterstitialMetricsHelper { + public: + // These enums are used for histograms. Don't reorder, delete, or insert + // elements. New elements should be added at the end (right before the max). + enum SecurityInterstitialDecision { + SHOW, + PROCEED, + DONT_PROCEED, + PROCEEDING_DISABLED, + MAX_DECISION + }; + enum SecurityInterstitialInteraction { + TOTAL_VISITS, + SHOW_ADVANCED, + SHOW_PRIVACY_POLICY, + SHOW_DIAGNOSTIC, + SHOW_LEARN_MORE, + RELOAD, + OPEN_TIME_SETTINGS, + MAX_INTERACTION + }; + + enum RapporReporting { + REPORT_RAPPOR, + SKIP_RAPPOR, + }; + + // Args: + // url: URL of page that triggered the interstitial. Only origin is used. + // uma_prefix: Histogram prefix for UMA. + // examples: "phishing", "ssl_overridable" + // rappor_prefix: Metric prefix for Rappor. + // examples: "phishing", "ssl" + // rappor_reporting: Used to skip rappor rapporting if desired. + // sampling_event_name: Event name for Experience Sampling. + // e.g. "phishing_interstitial_" + SecurityInterstitialMetricsHelper(content::WebContents* web_contents, + const GURL& url, + const std::string& uma_prefix, + const std::string& rappor_prefix, + RapporReporting rappor_reporting, + const std::string& sampling_event_name); + ~SecurityInterstitialMetricsHelper(); + + // Record a user decision or interaction to the appropriate UMA histogram + // and potentially in a RAPPOR metric. + void RecordUserDecision(SecurityInterstitialDecision decision); + void RecordUserInteraction(SecurityInterstitialInteraction interaction); + + private: + // Used to query the HistoryService to see if the URL is in history. + void OnGotHistoryCount(bool success, int num_visits, base::Time first_visit); + + content::WebContents* web_contents_; + const GURL request_url_; + const std::string uma_prefix_; + const std::string rappor_prefix_; + const RapporReporting rappor_reporting_; + const std::string sampling_event_name_; + int num_visits_; + base::CancelableTaskTracker request_tracker_; +#if defined(ENABLE_EXTENSIONS) + scoped_ptr<extensions::ExperienceSamplingEvent> sampling_event_; +#endif + + DISALLOW_COPY_AND_ASSIGN(SecurityInterstitialMetricsHelper); +}; + +#endif // CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_METRICS_HELPER_H_ diff --git a/chrome/browser/interstitials/security_interstitial_uma_helper.h b/chrome/browser/interstitials/security_interstitial_uma_helper.h deleted file mode 100644 index ffb465d..0000000 --- a/chrome/browser/interstitials/security_interstitial_uma_helper.h +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright (c) 2014 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. - -#ifndef CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_UMA_HELPER_H_ -#define CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_UMA_HELPER_H_ - -#include "base/task/cancelable_task_tracker.h" -#include "base/time/time.h" -#include "url/gurl.h" - -namespace content { -class WebContents; -} - -namespace extensions { -class ExperienceSamplingEvent; -} - -// Most of the security interstitials share a common layout and set of -// choices. SecurityInterstitialUmaHelper is intended to help the security -// interstitials record user choices in a common way via UMA histograms. -class SecurityInterstitialUmaHelper { - public: - // These enums are used for histograms. Don't reorder, delete, or insert - // elements. New elements should be added at the end (right before the max). - enum SecurityInterstitialDecision { - SHOW, - PROCEED, - DONT_PROCEED, - PROCEEDING_DISABLED, - MAX_DECISION - }; - enum SecurityInterstitialInteraction { - TOTAL_VISITS, - SHOW_ADVANCED, - SHOW_PRIVACY_POLICY, - SHOW_DIAGNOSTIC, - SHOW_LEARN_MORE, - RELOAD, - OPEN_TIME_SETTINGS, - MAX_INTERACTION - }; - - SecurityInterstitialUmaHelper(content::WebContents* web_contents, - const GURL& url, - const std::string& histogram_prefix, - const std::string& sampling_event_name); - ~SecurityInterstitialUmaHelper(); - - // Record a user decision or interaction to the appropriate UMA histogram. - void RecordUserDecision(SecurityInterstitialDecision decision); - void RecordUserInteraction(SecurityInterstitialInteraction interaction); - - private: - // Used to query the HistoryService to see if the URL is in history. - void OnGotHistoryCount(bool success, int num_visits, base::Time first_visit); - - content::WebContents* web_contents_; - const GURL request_url_; - const std::string histogram_prefix_; - const std::string sampling_event_name_; - int num_visits_; - base::CancelableTaskTracker request_tracker_; -#if defined(ENABLE_EXTENSIONS) - scoped_ptr<extensions::ExperienceSamplingEvent> sampling_event_; -#endif - - DISALLOW_COPY_AND_ASSIGN(SecurityInterstitialUmaHelper); -}; - -#endif // CHROME_BROWSER_INTERSTITIALS_SECURITY_INTERSTITIAL_UMA_HELPER_H_ diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc index 0515ecd..67f2cc4 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc @@ -175,15 +175,17 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage( interstitial_reason_ = SB_REASON_PHISHING; // 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); + // Use same prefix for UMA as for Rappor. + metrics_helper_.reset(new SecurityInterstitialMetricsHelper( + web_contents, request_url(), GetMetricPrefix(), GetMetricPrefix(), + SecurityInterstitialMetricsHelper::REPORT_RAPPOR, + GetSamplingEventName())); + metrics_helper_->RecordUserDecision(SecurityInterstitialMetricsHelper::SHOW); + metrics_helper_->RecordUserInteraction( + SecurityInterstitialMetricsHelper::TOTAL_VISITS); if (IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled)) { - uma_helper_->RecordUserDecision( - SecurityInterstitialUmaHelper::PROCEEDING_DISABLED); + metrics_helper_->RecordUserDecision( + SecurityInterstitialMetricsHelper::PROCEEDING_DISABLED); } if (!is_main_frame_load_blocked_) { @@ -239,8 +241,8 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) { if (command == kLearnMoreCommand) { // User pressed "Learn more". - uma_helper_->RecordUserInteraction( - SecurityInterstitialUmaHelper::SHOW_LEARN_MORE); + metrics_helper_->RecordUserInteraction( + SecurityInterstitialMetricsHelper::SHOW_LEARN_MORE); GURL learn_more_url( interstitial_reason_ == SB_REASON_PHISHING ? kLearnMorePhishingUrlV2 : kLearnMoreMalwareUrlV2); @@ -257,8 +259,8 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) { if (command == kShowPrivacyCommand) { // User pressed "Safe Browsing privacy policy". - uma_helper_->RecordUserInteraction( - SecurityInterstitialUmaHelper::SHOW_PRIVACY_POLICY); + metrics_helper_->RecordUserInteraction( + SecurityInterstitialMetricsHelper::SHOW_PRIVACY_POLICY); GURL privacy_url( l10n_util::GetStringUTF8(IDS_SAFE_BROWSING_PRIVACY_POLICY_URL)); privacy_url = google_util::AppendGoogleLocaleParam( @@ -277,7 +279,8 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) { if (IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled)) { proceed_blocked = true; } else { - uma_helper_->RecordUserDecision(SecurityInterstitialUmaHelper::PROCEED); + metrics_helper_->RecordUserDecision( + SecurityInterstitialMetricsHelper::PROCEED); interstitial_page()->Proceed(); // |this| has been deleted after Proceed() returns. return; @@ -333,8 +336,8 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) { std::string bad_url_spec = unsafe_resources_[element_index].url.spec(); if (command == kShowDiagnosticCommand) { // We're going to take the user to Google's SafeBrowsing diagnostic page. - uma_helper_->RecordUserInteraction( - SecurityInterstitialUmaHelper::SHOW_DIAGNOSTIC); + metrics_helper_->RecordUserInteraction( + SecurityInterstitialMetricsHelper::SHOW_DIAGNOSTIC); std::string diagnostic = base::StringPrintf(kSbDiagnosticUrl, net::EscapeQueryParamValue(bad_url_spec, true).c_str()); @@ -355,8 +358,8 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) { } if (command == kExpandedSeeMoreCommand) { - uma_helper_->RecordUserInteraction( - SecurityInterstitialUmaHelper::SHOW_ADVANCED); + metrics_helper_->RecordUserInteraction( + SecurityInterstitialMetricsHelper::SHOW_ADVANCED); return; } @@ -421,8 +424,8 @@ void SafeBrowsingBlockingPage::OnDontProceed() { return; if (!IsPrefEnabled(prefs::kSafeBrowsingProceedAnywayDisabled)) { - uma_helper_->RecordUserDecision( - SecurityInterstitialUmaHelper::DONT_PROCEED); + metrics_helper_->RecordUserDecision( + SecurityInterstitialMetricsHelper::DONT_PROCEED); } // Send the malware details, if we opted to. @@ -555,7 +558,7 @@ bool SafeBrowsingBlockingPage::IsMainPageLoadBlocked( return unsafe_resources.size() == 1 && !unsafe_resources[0].is_subresource; } -std::string SafeBrowsingBlockingPage::GetHistogramPrefix() const { +std::string SafeBrowsingBlockingPage::GetMetricPrefix() const { switch (interstitial_reason_) { case SB_REASON_MALWARE: return "malware"; diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.h b/chrome/browser/safe_browsing/safe_browsing_blocking_page.h index 27ee2e6..424e516 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.h +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.h @@ -34,8 +34,8 @@ #include "base/gtest_prod_util.h" #include "base/task/cancelable_task_tracker.h" +#include "chrome/browser/interstitials/security_interstitial_metrics_helper.h" #include "chrome/browser/interstitials/security_interstitial_page.h" -#include "chrome/browser/interstitials/security_interstitial_uma_helper.h" #include "chrome/browser/safe_browsing/ui_manager.h" #include "url/gurl.h" @@ -191,10 +191,10 @@ class SafeBrowsingBlockingPage : public SecurityInterstitialPage { void PopulateHarmfulLoadTimeData(base::DictionaryValue* load_time_data); void PopulatePhishingLoadTimeData(base::DictionaryValue* load_time_data); - std::string GetHistogramPrefix() const; + std::string GetMetricPrefix() const; std::string GetSamplingEventName() const; - scoped_ptr<SecurityInterstitialUmaHelper> uma_helper_; + scoped_ptr<SecurityInterstitialMetricsHelper> metrics_helper_; DISALLOW_COPY_AND_ASSIGN(SafeBrowsingBlockingPage); }; diff --git a/chrome/browser/ssl/ssl_blocking_page.cc b/chrome/browser/ssl/ssl_blocking_page.cc index 214c035..7ef848d 100644 --- a/chrome/browser/ssl/ssl_blocking_page.cc +++ b/chrome/browser/ssl/ssl_blocking_page.cc @@ -87,6 +87,9 @@ enum SSLExpirationAndDecision { END_OF_SSL_EXPIRATION_AND_DECISION, }; +// Rappor prefix +const char kSSLRapporPrefix[] = "ssl"; + void RecordSSLExpirationPageEventState(bool expired_but_previously_allowed, bool proceed, bool overridable) { @@ -233,12 +236,19 @@ 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); + metrics_helper_.reset(new SecurityInterstitialMetricsHelper( + web_contents, request_url, GetUmaHistogramPrefix(), kSSLRapporPrefix, + (interstitial_reason_ == SSL_REASON_BAD_CLOCK + ? SecurityInterstitialMetricsHelper::SKIP_RAPPOR + : SecurityInterstitialMetricsHelper::REPORT_RAPPOR), + GetSamplingEventName())); + + metrics_helper_->RecordUserDecision(SecurityInterstitialMetricsHelper::SHOW); + metrics_helper_->RecordUserInteraction( + SecurityInterstitialMetricsHelper::TOTAL_VISITS); ssl_error_classification_.reset(new SSLErrorClassification( web_contents, @@ -267,8 +277,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 +461,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 +482,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 +502,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 +511,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 +536,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_) diff --git a/chrome/browser/ssl/ssl_blocking_page.h b/chrome/browser/ssl/ssl_blocking_page.h index 1a6b3da..dac5ab4 100644 --- a/chrome/browser/ssl/ssl_blocking_page.h +++ b/chrome/browser/ssl/ssl_blocking_page.h @@ -12,8 +12,8 @@ #include "base/strings/string16.h" #include "base/task/cancelable_task_tracker.h" #include "base/time/time.h" +#include "chrome/browser/interstitials/security_interstitial_metrics_helper.h" #include "chrome/browser/interstitials/security_interstitial_page.h" -#include "chrome/browser/interstitials/security_interstitial_uma_helper.h" #include "net/ssl/ssl_info.h" #include "url/gurl.h" @@ -95,7 +95,7 @@ class SSLBlockingPage : public SecurityInterstitialPage { void NotifyDenyCertificate(); void NotifyAllowCertificate(); - std::string GetHistogramPrefix() const; + std::string GetUmaHistogramPrefix() const; std::string GetSamplingEventName() const; base::Callback<void(bool)> callback_; @@ -117,9 +117,9 @@ class SSLBlockingPage : public SecurityInterstitialPage { // expired? const bool expired_but_previously_allowed_; scoped_ptr<SSLErrorClassification> ssl_error_classification_; - scoped_ptr<SecurityInterstitialUmaHelper> uma_helper_; + scoped_ptr<SecurityInterstitialMetricsHelper> metrics_helper_; - // Which type of Safe Browsing interstitial this is. + // Which type of interstitial this is. enum SSLInterstitialReason { SSL_REASON_SSL, SSL_REASON_BAD_CLOCK diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 934eb6c..c17d6ea 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -426,8 +426,8 @@ 'browser/internal_auth.h', 'browser/interstitials/security_interstitial_page.cc', 'browser/interstitials/security_interstitial_page.h', - 'browser/interstitials/security_interstitial_uma_helper.cc', - 'browser/interstitials/security_interstitial_uma_helper.h', + 'browser/interstitials/security_interstitial_metrics_helper.cc', + 'browser/interstitials/security_interstitial_metrics_helper.h', 'browser/intranet_redirect_detector.cc', 'browser/intranet_redirect_detector.h', 'browser/invalidation/invalidation_service_factory_android.cc', diff --git a/components/rappor/rappor_service.cc b/components/rappor/rappor_service.cc index 5b7b753..7fc4fce 100644 --- a/components/rappor/rappor_service.cc +++ b/components/rappor/rappor_service.cc @@ -126,7 +126,8 @@ void RapporService::Update(RecordingLevel recording_level, bool may_upload) { } } - DVLOG(1) << "RapporService may_upload=" << may_upload; + DVLOG(1) << "RapporService recording_level=" << recording_level_ + << " may_upload=" << may_upload; if (may_upload) { uploader_->Start(); } else { diff --git a/tools/metrics/rappor/rappor.xml b/tools/metrics/rappor/rappor.xml index 7fbe8ba..2f7cac2 100644 --- a/tools/metrics/rappor/rappor.xml +++ b/tools/metrics/rappor/rappor.xml @@ -140,6 +140,38 @@ components/rappor/rappor_service.cc. </summary> </rappor-metric> +<rappor-metric name="interstitial.malware.domain" type="COARSE_RAPPOR_TYPE"> + <owner>nparker@chromium.org</owner> + <summary> + The domain+registry of a URL that triggered a safe-browsing malware + interstitial. + </summary> +</rappor-metric> + +<rappor-metric name="interstitial.phishing.domain" type="COARSE_RAPPOR_TYPE"> + <owner>nparker@chromium.org</owner> + <summary> + The domain+registry of a URL that triggered a safe-browsing phishing + interstitial. + </summary> +</rappor-metric> + +<rappor-metric name="interstitial.harmful.domain" type="COARSE_RAPPOR_TYPE"> + <owner>nparker@chromium.org</owner> + <summary> + The domain+registry of a URL that triggered a safe-browsing UWS + interstitial. + </summary> +</rappor-metric> + +<rappor-metric name="interstitial.ssl.domain" type="COARSE_RAPPOR_TYPE"> + <owner>nparker@chromium.org</owner> + <summary> + The domain+registry of a URL that triggered an SSL interstitial. + Domains for bad-clock warnings are not reported. + </summary> +</rappor-metric> + </rappor-metrics> </rappor-configuration> |