Skip to content

Commit

Permalink
Deprecate old Safe Browsing reporting opt-ins
Browse files Browse the repository at this point in the history
The old prefs are no longer needed; M39 should be a sufficiently long
transition time period.

BUG=384668
R=mattm@chromium.org

Review URL: https://codereview.chromium.org/512083002

Cr-Commit-Position: refs/heads/master@{#292974}
  • Loading branch information
adrifelt authored and Commit bot committed Sep 2, 2014
1 parent 00ba935 commit a55ae06
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 126 deletions.
8 changes: 0 additions & 8 deletions chrome/browser/profiles/profile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,6 @@ void Profile::RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
prefs::kSafeBrowsingExtendedReportingEnabled,
false,
user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
registry->RegisterBooleanPref(
prefs::kSafeBrowsingDownloadFeedbackEnabled,
false,
user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
registry->RegisterBooleanPref(
prefs::kSafeBrowsingReportingEnabled,
false,
user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
registry->RegisterBooleanPref(
prefs::kSafeBrowsingProceedAnywayDisabled,
false,
Expand Down
35 changes: 2 additions & 33 deletions chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage(
url_(unsafe_resources[0].url),
interstitial_page_(NULL),
has_expanded_see_more_section_(false),
reporting_checkbox_checked_(false),
create_view_(true),
num_visits_(-1) {
bool malware = false;
Expand Down Expand Up @@ -457,31 +456,11 @@ void SafeBrowsingBlockingPage::SetReportingPreference(bool report) {
PrefService* pref = profile->GetPrefs();
pref->SetBoolean(prefs::kSafeBrowsingExtendedReportingEnabled, report);
UMA_HISTOGRAM_BOOLEAN("SB2.SetExtendedReportingEnabled", report);
reporting_checkbox_checked_ = report;
pref->ClearPref(prefs::kSafeBrowsingReportingEnabled);
pref->ClearPref(prefs::kSafeBrowsingDownloadFeedbackEnabled);
}

// If the reporting checkbox was left checked on close, the new pref
// kSafeBrowsingExtendedReportingEnabled should be updated.
// TODO(felt): Remove this in M-39. crbug.com/384668
void SafeBrowsingBlockingPage::UpdateReportingPref() {
if (!reporting_checkbox_checked_)
return;
if (IsPrefEnabled(prefs::kSafeBrowsingExtendedReportingEnabled))
return;
Profile* profile = Profile::FromBrowserContext(
web_contents_->GetBrowserContext());
if (profile->GetPrefs()->HasPrefPath(
prefs::kSafeBrowsingExtendedReportingEnabled))
return;
SetReportingPreference(true);
}

void SafeBrowsingBlockingPage::OnProceed() {
proceeded_ = true;
RecordUserAction(PROCEED);
UpdateReportingPref();
// Send the malware details, if we opted to.
FinishMalwareDetails(malware_details_proceed_delay_ms_);

Expand Down Expand Up @@ -534,7 +513,6 @@ void SafeBrowsingBlockingPage::OnDontProceed() {
return;

RecordUserAction(DONT_PROCEED);
UpdateReportingPref();
// Send the malware details, if we opted to.
FinishMalwareDetails(0); // No delay

Expand Down Expand Up @@ -944,18 +922,9 @@ void SafeBrowsingBlockingPage::PopulateMalwareLoadTimeData(
"optInLink",
l10n_util::GetStringFUTF16(IDS_SAFE_BROWSING_MALWARE_REPORTING_AGREE,
base::UTF8ToUTF16(privacy_link)));
Profile* profile = Profile::FromBrowserContext(
web_contents_->GetBrowserContext());
if (profile->GetPrefs()->HasPrefPath(
prefs::kSafeBrowsingExtendedReportingEnabled)) {
reporting_checkbox_checked_ =
IsPrefEnabled(prefs::kSafeBrowsingExtendedReportingEnabled);
} else if (IsPrefEnabled(prefs::kSafeBrowsingReportingEnabled) ||
IsPrefEnabled(prefs::kSafeBrowsingDownloadFeedbackEnabled)) {
reporting_checkbox_checked_ = true;
}
load_time_data->SetBoolean(
kBoxChecked, reporting_checkbox_checked_);
kBoxChecked,
IsPrefEnabled(prefs::kSafeBrowsingExtendedReportingEnabled));
}
}

Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/safe_browsing/safe_browsing_blocking_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,6 @@ class SafeBrowsingBlockingPage : public content::InterstitialPageDelegate {
// during this interstitial page.
bool has_expanded_see_more_section_;

// Whether the user has left the reporting checkbox checked.
bool reporting_checkbox_checked_;

// Whether the interstitial should create a view.
bool create_view_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,72 +664,3 @@ TEST_F(SafeBrowsingBlockingPageTest, MalwareReportsToggling) {
EXPECT_FALSE(profile->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingExtendedReportingEnabled));
}

// Test that the transition from old to new preference works.
TEST_F(SafeBrowsingBlockingPageTest, MalwareReportsTransitionEnabled) {
// The old pref is enabled.
Profile* profile = Profile::FromBrowserContext(
web_contents()->GetBrowserContext());
profile->GetPrefs()->SetBoolean(prefs::kSafeBrowsingReportingEnabled, true);

// Start a load.
controller().LoadURL(GURL(kBadURL), content::Referrer(),
content::PAGE_TRANSITION_TYPED, std::string());

// Simulate the load causing a safe browsing interstitial to be shown.
ShowInterstitial(false, kBadURL);
SafeBrowsingBlockingPage* sb_interstitial =
GetSafeBrowsingBlockingPage();
ASSERT_TRUE(sb_interstitial);

base::RunLoop().RunUntilIdle();

// At this point, nothing should have changed yet.
EXPECT_FALSE(profile->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingExtendedReportingEnabled));

ProceedThroughInterstitial(sb_interstitial);

// Since the user has proceeded without changing the checkbox, the new pref
// has been updated.
EXPECT_TRUE(profile->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingExtendedReportingEnabled));
}

// Test that the transition from old to new preference still respects the
// user's checkbox preferences.
TEST_F(SafeBrowsingBlockingPageTest, MalwareReportsTransitionDisabled) {
// The old pref is enabled.
Profile* profile = Profile::FromBrowserContext(
web_contents()->GetBrowserContext());
profile->GetPrefs()->SetBoolean(prefs::kSafeBrowsingReportingEnabled, true);

// Start a load.
controller().LoadURL(GURL(kBadURL), content::Referrer(),
content::PAGE_TRANSITION_TYPED, std::string());

// Simulate the load causing a safe browsing interstitial to be shown.
ShowInterstitial(false, kBadURL);
SafeBrowsingBlockingPage* sb_interstitial =
GetSafeBrowsingBlockingPage();
ASSERT_TRUE(sb_interstitial);

base::RunLoop().RunUntilIdle();

// At this point, nothing should have changed yet.
EXPECT_FALSE(profile->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingExtendedReportingEnabled));

// Simulate the user uncheck the report agreement checkbox.
sb_interstitial->SetReportingPreference(false);

ProceedThroughInterstitial(sb_interstitial);

// The new pref is turned off.
EXPECT_FALSE(profile->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingExtendedReportingEnabled));
EXPECT_FALSE(profile->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingReportingEnabled));
EXPECT_FALSE(profile->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingDownloadFeedbackEnabled));
}
12 changes: 1 addition & 11 deletions chrome/common/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -323,20 +323,10 @@ const char kWebKitPluginsEnabled[] = "webkit.webprefs.plugins_enabled";
// Boolean that is true when SafeBrowsing is enabled.
const char kSafeBrowsingEnabled[] = "safebrowsing.enabled";

// Boolean that tell us whether malicious download feedback is enabled.
// Boolean that tell us whether Safe Browsing extended reporting is enabled.
const char kSafeBrowsingExtendedReportingEnabled[] =
"safebrowsing.extended_reporting_enabled";

// Boolean that tell us whether malicious download feedback is enabled.
// TODO(felt): Deprecate. crbug.com/383866
const char kSafeBrowsingDownloadFeedbackEnabled[] =
"safebrowsing.download_feedback_enabled";

// Boolean that is true when SafeBrowsing Malware Report is enabled.
// TODO(felt): Deprecate. crbug.com/383866
const char kSafeBrowsingReportingEnabled[] =
"safebrowsing.reporting_enabled";

// Boolean that is true when the SafeBrowsing interstitial should not allow
// users to proceed anyway.
const char kSafeBrowsingProceedAnywayDisabled[] =
Expand Down
2 changes: 0 additions & 2 deletions chrome/common/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@ extern const char kWebKitPasswordEchoEnabled[];
#endif
extern const char kSafeBrowsingEnabled[];
extern const char kSafeBrowsingExtendedReportingEnabled[];
extern const char kSafeBrowsingDownloadFeedbackEnabled[];
extern const char kSafeBrowsingReportingEnabled[];
extern const char kSafeBrowsingProceedAnywayDisabled[];
extern const char kSafeBrowsingIncidentReportSent[];
extern const char kSafeBrowsingIncidentsSent[];
Expand Down

0 comments on commit a55ae06

Please sign in to comment.