Commit 941a9593 authored by Charlie Harrison's avatar Charlie Harrison Committed by Commit Bot

SafeBrowsingTriggeredPopupBlocker: remove |ignore_sublist| mode

This mode was added as a way to hedge our feature against failing to
launch metadata on the SafeBrowsing API side. This feature has been
launched for a while and the parameter did not end up being used, so
just remove it and associated tests.

Bug: None
Change-Id: Ib2e7d633da2d98478cee17dc3cdffe4e0b998d1b
Reviewed-on: https://chromium-review.googlesource.com/1124763Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572294}
parent 627c36a4
......@@ -7,7 +7,6 @@
#include <utility>
#include "base/memory/ptr_util.h"
#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_macros.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/pref_names.h"
......@@ -24,8 +23,6 @@
namespace {
const char kIgnoreSublistsParam[] = "ignore_sublists";
void LogAction(SafeBrowsingTriggeredPopupBlocker::Action action) {
UMA_HISTOGRAM_ENUMERATION("ContentSettings.Popups.StrongBlockerActions",
action,
......@@ -111,11 +108,7 @@ SafeBrowsingTriggeredPopupBlocker::SafeBrowsingTriggeredPopupBlocker(
subresource_filter::SubresourceFilterObserverManager* observer_manager)
: content::WebContentsObserver(web_contents),
scoped_observer_(this),
current_page_data_(std::make_unique<PageData>()),
ignore_sublists_(
base::GetFieldTrialParamByFeatureAsBool(kAbusiveExperienceEnforce,
kIgnoreSublistsParam,
false /* default_value */)) {
current_page_data_(std::make_unique<PageData>()) {
DCHECK(observer_manager);
scoped_observer_.Add(observer_manager);
}
......@@ -161,11 +154,6 @@ void SafeBrowsingTriggeredPopupBlocker::OnSafeBrowsingChecksComplete(
for (const auto& result : results) {
if (result.threat_type ==
safe_browsing::SBThreatType::SB_THREAT_TYPE_SUBRESOURCE_FILTER) {
if (ignore_sublists_) {
// No warning for ignore_sublists mode.
level_for_next_committed_navigation_ = SubresourceFilterLevel::ENFORCE;
return;
}
auto abusive = result.threat_metadata.subresource_filter_match.find(
safe_browsing::SubresourceFilterType::ABUSIVE);
if (abusive != result.threat_metadata.subresource_filter_match.end()) {
......
......@@ -135,11 +135,6 @@ class SafeBrowsingTriggeredPopupBlocker
// Should never be nullptr.
std::unique_ptr<PageData> current_page_data_;
// Whether to ignore the threat pattern type. Useful for flexibility because
// we have to wait until metadata patterns reach Stable before using them
// without error. Governed by a variation param.
bool ignore_sublists_ = false;
DISALLOW_COPY_AND_ASSIGN(SafeBrowsingTriggeredPopupBlocker);
};
......
......@@ -200,18 +200,6 @@ class SafeBrowsingTriggeredPopupBlockerBrowserTest
DISALLOW_COPY_AND_ASSIGN(SafeBrowsingTriggeredPopupBlockerBrowserTest);
};
// The boolean parameter is whether to ignore sublists.
class SafeBrowsingTriggeredPopupBlockerParamBrowserTest
: public SafeBrowsingTriggeredPopupBlockerBrowserTest,
public testing::WithParamInterface<bool> {
void FinalizeFeatures() override {
std::string ignore_param = GetParam() ? "true" : "false";
scoped_feature_list_.InitAndEnableFeatureWithParameters(
kAbusiveExperienceEnforce, {{"ignore_sublists", ignore_param}});
}
base::test::ScopedFeatureList scoped_feature_list_;
};
class SafeBrowsingTriggeredPopupBlockerDisabledTest
: public SafeBrowsingTriggeredPopupBlockerBrowserTest {
void FinalizeFeatures() override {
......@@ -605,34 +593,6 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
open_popup_and_expect_block(false);
}
IN_PROC_BROWSER_TEST_P(SafeBrowsingTriggeredPopupBlockerParamBrowserTest,
IgnoreSublist) {
const char kWindowOpenPath[] = "/subresource_filter/window_open.html";
GURL url(embedded_test_server()->GetURL("a.com", kWindowOpenPath));
// Do not set the ABUSIVE bit.
safe_browsing::ThreatMetadata metadata;
database_helper()->AddFullHashToDbAndFullHashCache(
url, safe_browsing::GetUrlSubresourceFilterId(), metadata);
// Navigate to url, should not trigger the popup blocker.
ui_test_utils::NavigateToURL(browser(), url);
bool opened_window = false;
EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
web_contents(), "openWindow()", &opened_window));
bool ignoring_sublists = GetParam();
EXPECT_EQ(ignoring_sublists, !opened_window);
EXPECT_EQ(ignoring_sublists,
TabSpecificContentSettings::FromWebContents(web_contents())
->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS));
}
INSTANTIATE_TEST_CASE_P(/* no prefix */,
SafeBrowsingTriggeredPopupBlockerParamBrowserTest,
testing::Values(true, false));
IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
WarningDoNotBlockCreatingNewWindows_LogsToConsole) {
const char kWindowOpenPath[] = "/subresource_filter/window_open.html";
......
......@@ -143,34 +143,6 @@ class SafeBrowsingTriggeredPopupBlockerTest
DISALLOW_COPY_AND_ASSIGN(SafeBrowsingTriggeredPopupBlockerTest);
};
class IgnoreSublistSafeBrowsingTriggeredPopupBlockerTest
: public SafeBrowsingTriggeredPopupBlockerTest {
std::unique_ptr<base::test::ScopedFeatureList> DefaultFeatureList() override {
auto feature_list = std::make_unique<base::test::ScopedFeatureList>();
feature_list->InitAndEnableFeatureWithParameters(
kAbusiveExperienceEnforce, {{"ignore_sublists", "true"}});
return feature_list;
}
};
TEST_F(IgnoreSublistSafeBrowsingTriggeredPopupBlockerTest,
MatchNoSublist_BlocksPopup) {
const GURL url("https://example.test/");
fake_safe_browsing_database()->AddBlacklistedUrl(
url, safe_browsing::SB_THREAT_TYPE_SUBRESOURCE_FILTER);
NavigateAndCommit(url);
EXPECT_TRUE(popup_blocker()->ShouldApplyStrongPopupBlocker(nullptr));
}
// ignore_sublists should still block on URLs matching a sublist.
TEST_F(IgnoreSublistSafeBrowsingTriggeredPopupBlockerTest,
MatchSublist_BlocksPopup) {
const GURL url("https://example.test/");
MarkUrlAsAbusiveEnforce(url);
NavigateAndCommit(url);
EXPECT_TRUE(popup_blocker()->ShouldApplyStrongPopupBlocker(nullptr));
}
struct RedirectSamplesAndResults {
GURL initial_url;
GURL redirect_url;
......
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