Commit 2521f338 authored by Varun Khaneja's avatar Varun Khaneja Committed by Commit Bot

Remove some unused metrics related to SBER1

histograms.xml updated in https://crrev.com/c/2225380

R=bdea

Bug: 1089426
Change-Id: Ib59d2596034a16bbccba18a750515915873d69ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225383
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Reviewed-by: default avatarBettina Dea <bdea@chromium.org>
Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Auto-Submit: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773930}
parent 9521d391
...@@ -138,10 +138,6 @@ void BaseBlockingPage::PopulateInterstitialStrings( ...@@ -138,10 +138,6 @@ void BaseBlockingPage::PopulateInterstitialStrings(
sb_error_ui_->PopulateStringsForHtml(load_time_data); sb_error_ui_->PopulateStringsForHtml(load_time_data);
} }
void BaseBlockingPage::OnInterstitialClosing() {
UpdateMetricsAfterSecurityInterstitial();
}
void BaseBlockingPage::FinishThreatDetails(const base::TimeDelta& delay, void BaseBlockingPage::FinishThreatDetails(const base::TimeDelta& delay,
bool did_proceed, bool did_proceed,
int num_visits) {} int num_visits) {}
......
...@@ -61,7 +61,7 @@ class BaseBlockingPage ...@@ -61,7 +61,7 @@ class BaseBlockingPage
bool ShouldCreateNewNavigation() const override; bool ShouldCreateNewNavigation() const override;
void PopulateInterstitialStrings( void PopulateInterstitialStrings(
base::DictionaryValue* load_time_data) override; base::DictionaryValue* load_time_data) override;
void OnInterstitialClosing() override; void OnInterstitialClosing() override {}
// Called when the interstitial is going away. Intentionally do nothing in // Called when the interstitial is going away. Intentionally do nothing in
// this base class. // this base class.
......
...@@ -19,16 +19,6 @@ ...@@ -19,16 +19,6 @@
namespace { namespace {
// The Extended Reporting pref that is currently active, used for UMA metrics.
// These values are written to logs. New enum values can be added, but
// existing enums must never be renumbered or deleted and reused.
enum ActiveExtendedReportingPref {
SBER1_PREF = 0,
SBER2_PREF = 1,
// New prefs must be added before MAX_SBER_PREF
MAX_SBER_PREF
};
// Update the correct UMA metric based on which pref was changed and which UI // Update the correct UMA metric based on which pref was changed and which UI
// the change was made on. // the change was made on.
void RecordExtendedReportingPrefChanged( void RecordExtendedReportingPrefChanged(
...@@ -249,6 +239,7 @@ void SetExtendedReportingPrefAndMetric( ...@@ -249,6 +239,7 @@ void SetExtendedReportingPrefAndMetric(
RecordExtendedReportingPrefChanged(*prefs, location); RecordExtendedReportingPrefChanged(*prefs, location);
} }
// Only used for tests.
void SetExtendedReportingPref(PrefService* prefs, bool value) { void SetExtendedReportingPref(PrefService* prefs, bool value) {
prefs->SetBoolean(prefs::kSafeBrowsingScoutReportingEnabled, value); prefs->SetBoolean(prefs::kSafeBrowsingScoutReportingEnabled, value);
} }
...@@ -260,67 +251,6 @@ void SetEnhancedProtectionPref(PrefService* prefs, bool value) { ...@@ -260,67 +251,6 @@ void SetEnhancedProtectionPref(PrefService* prefs, bool value) {
prefs->SetBoolean(prefs::kSafeBrowsingEnhanced, value); prefs->SetBoolean(prefs::kSafeBrowsingEnhanced, value);
} }
void UpdateMetricsAfterSecurityInterstitial(const PrefService& prefs,
bool on_show_pref_existed,
bool on_show_pref_value) {
const bool cur_pref_value = IsExtendedReportingEnabled(prefs);
if (!on_show_pref_existed) {
if (!ExtendedReportingPrefExists(prefs)) {
// User seeing pref for the first time, didn't touch the checkbox (left it
// unchecked).
UMA_HISTOGRAM_ENUMERATION(
"SafeBrowsing.Pref.Scout.Decision.First_LeftUnchecked", SBER2_PREF,
MAX_SBER_PREF);
return;
}
// Pref currently exists so user did something to the checkbox
if (cur_pref_value) {
// User turned the pref on.
UMA_HISTOGRAM_ENUMERATION(
"SafeBrowsing.Pref.Scout.Decision.First_Enabled", SBER2_PREF,
MAX_SBER_PREF);
return;
}
// Otherwise, user turned the pref off, but because it didn't exist when
// the interstitial was first shown, they must have turned it on and then
// off before the interstitial was closed.
UMA_HISTOGRAM_ENUMERATION("SafeBrowsing.Pref.Scout.Decision.First_Disabled",
SBER2_PREF, MAX_SBER_PREF);
return;
}
// At this point, the pref existed when the interstitial was shown so this is
// a repeat appearance of the opt-in. Existence can't be removed during an
// interstitial so no need to check whether the pref currently exists.
if (on_show_pref_value && cur_pref_value) {
// User left the pref on.
UMA_HISTOGRAM_ENUMERATION(
"SafeBrowsing.Pref.Scout.Decision.Repeat_LeftEnabled", SBER2_PREF,
MAX_SBER_PREF);
return;
} else if (on_show_pref_value && !cur_pref_value) {
// User turned the pref off.
UMA_HISTOGRAM_ENUMERATION(
"SafeBrowsing.Pref.Scout.Decision.Repeat_Disabled", SBER2_PREF,
MAX_SBER_PREF);
return;
} else if (!on_show_pref_value && cur_pref_value) {
// User turned the pref on.
UMA_HISTOGRAM_ENUMERATION("SafeBrowsing.Pref.Scout.Decision.Repeat_Enabled",
SBER2_PREF, MAX_SBER_PREF);
return;
} else {
// Both on_show and cur values are false - user left the pref off.
UMA_HISTOGRAM_ENUMERATION(
"SafeBrowsing.Pref.Scout.Decision.Repeat_LeftDisabled", SBER2_PREF,
MAX_SBER_PREF);
return;
}
}
void UpdatePrefsBeforeSecurityInterstitial(PrefService* prefs) { void UpdatePrefsBeforeSecurityInterstitial(PrefService* prefs) {
// Remember that this user saw an interstitial. // Remember that this user saw an interstitial.
prefs->SetBoolean(prefs::kSafeBrowsingSawInterstitialScoutReporting, true); prefs->SetBoolean(prefs::kSafeBrowsingSawInterstitialScoutReporting, true);
......
...@@ -85,14 +85,6 @@ SecurityInterstitialControllerClient* SecurityInterstitialPage::controller() ...@@ -85,14 +85,6 @@ SecurityInterstitialControllerClient* SecurityInterstitialPage::controller()
return controller_.get(); return controller_.get();
} }
void SecurityInterstitialPage::UpdateMetricsAfterSecurityInterstitial() {
if (controller_->GetPrefService()) {
safe_browsing::UpdateMetricsAfterSecurityInterstitial(
*controller_->GetPrefService(), on_show_extended_reporting_pref_exists_,
on_show_extended_reporting_pref_value_);
}
}
void SecurityInterstitialPage::SetUpMetrics() { void SecurityInterstitialPage::SetUpMetrics() {
// Remember the initial state of the extended reporting pref, to be compared // Remember the initial state of the extended reporting pref, to be compared
// to the same data when the interstitial is closed. // to the same data when the interstitial is closed.
......
...@@ -36,10 +36,8 @@ SSLBlockingPageBase::SSLBlockingPageBase( ...@@ -36,10 +36,8 @@ SSLBlockingPageBase::SSLBlockingPageBase(
SSLBlockingPageBase::~SSLBlockingPageBase() = default; SSLBlockingPageBase::~SSLBlockingPageBase() = default;
void SSLBlockingPageBase::OnInterstitialClosing() { void SSLBlockingPageBase::OnInterstitialClosing() {
UpdateMetricsAfterSecurityInterstitial();
cert_report_helper_->FinishCertCollection(); cert_report_helper_->FinishCertCollection();
} }
void SSLBlockingPageBase::SetSSLCertReporterForTesting( void SSLBlockingPageBase::SetSSLCertReporterForTesting(
std::unique_ptr<SSLCertReporter> ssl_cert_reporter) { std::unique_ptr<SSLCertReporter> ssl_cert_reporter) {
cert_report_helper_->SetSSLCertReporterForTesting( cert_report_helper_->SetSSLCertReporterForTesting(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment