Commit 95bfa165 authored by Joe DeBlasio's avatar Joe DeBlasio Committed by Commit Bot

Allow disabling of topsite lookalikes in Safety Tips.

This CL allows us to disable topsites-based lookalike matching in Safety
Tips via a "disable_topsites" feature parameter.  This feature param is
inverted from the other feature params (it disables instead of enables)
to ensure that the default no-param state is consistent before and after
this change.

Bug: 1015003
Change-Id: I2e7e94622dd20b8879e8157d588d39cf9383e790
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865415Reviewed-by: default avatarEmily Stark <estark@chromium.org>
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706927}
parent e7968456
...@@ -13,6 +13,10 @@ ...@@ -13,6 +13,10 @@
namespace { namespace {
using MatchType = LookalikeUrlInterstitialPage::MatchType;
const base::FeatureParam<bool> kEnableLookalikeTopSites{
&security_state::features::kSafetyTipUI, "topsites", false};
const base::FeatureParam<bool> kEnableLookalikeEditDistance{ const base::FeatureParam<bool> kEnableLookalikeEditDistance{
&security_state::features::kSafetyTipUI, "editdistance", false}; &security_state::features::kSafetyTipUI, "editdistance", false};
const base::FeatureParam<bool> kEnableLookalikeEditDistanceSiteEngagement{ const base::FeatureParam<bool> kEnableLookalikeEditDistanceSiteEngagement{
...@@ -29,7 +33,7 @@ bool ShouldTriggerSafetyTipFromLookalike( ...@@ -29,7 +33,7 @@ bool ShouldTriggerSafetyTipFromLookalike(
const std::vector<lookalikes::DomainInfo>& engaged_sites, const std::vector<lookalikes::DomainInfo>& engaged_sites,
GURL* safe_url) { GURL* safe_url) {
std::string matched_domain; std::string matched_domain;
LookalikeUrlInterstitialPage::MatchType match_type; MatchType match_type;
if (!lookalikes::LookalikeUrlNavigationThrottle::GetMatchingDomain( if (!lookalikes::LookalikeUrlNavigationThrottle::GetMatchingDomain(
navigated_domain, engaged_sites, &matched_domain, &match_type)) { navigated_domain, engaged_sites, &matched_domain, &match_type)) {
...@@ -44,16 +48,23 @@ bool ShouldTriggerSafetyTipFromLookalike( ...@@ -44,16 +48,23 @@ bool ShouldTriggerSafetyTipFromLookalike(
*safe_url = GURL(std::string(url::kHttpScheme) + *safe_url = GURL(std::string(url::kHttpScheme) +
url::kStandardSchemeSeparator + matched_domain); url::kStandardSchemeSeparator + matched_domain);
// Edit distance has higher false positives, so it gets its own feature param switch (match_type) {
if (match_type == LookalikeUrlInterstitialPage::MatchType::kEditDistance) { case MatchType::kTopSite:
return kEnableLookalikeEditDistance.Get(); return kEnableLookalikeTopSites.Get();
} case MatchType::kEditDistance:
if (match_type == return kEnableLookalikeEditDistance.Get();
LookalikeUrlInterstitialPage::MatchType::kEditDistanceSiteEngagement) { case MatchType::kEditDistanceSiteEngagement:
return kEnableLookalikeEditDistanceSiteEngagement.Get(); return kEnableLookalikeEditDistanceSiteEngagement.Get();
case MatchType::kSiteEngagement:
// ShouldDisplayInterstitial always returns true on kSiteEngagement (i.e.
// an interstitial is always shown for this form of lookalike), so no
// Safety Tip is ever shown.
case MatchType::kNone:
NOTREACHED();
} }
return true; NOTREACHED();
return false;
} }
bool ShouldTriggerSafetyTipFromKeywordInURL( bool ShouldTriggerSafetyTipFromKeywordInURL(
......
...@@ -55,7 +55,7 @@ namespace { ...@@ -55,7 +55,7 @@ namespace {
enum class UIStatus { enum class UIStatus {
kDisabled, kDisabled,
kEnabled, kEnabled,
kEnabledWithEditDistance, kEnabledWithAllFeatures,
}; };
// An engagement score above MEDIUM. // An engagement score above MEDIUM.
...@@ -170,10 +170,11 @@ class SafetyTipPageInfoBubbleViewBrowserTest ...@@ -170,10 +170,11 @@ class SafetyTipPageInfoBubbleViewBrowserTest
{{"topsites", "true"}}}}, {{"topsites", "true"}}}},
{}); {});
break; break;
case UIStatus::kEnabledWithEditDistance: case UIStatus::kEnabledWithAllFeatures:
feature_list_.InitWithFeaturesAndParameters( feature_list_.InitWithFeaturesAndParameters(
{{security_state::features::kSafetyTipUI, {{security_state::features::kSafetyTipUI,
{{"editdistance", "true"}, {{"topsites", "true"},
{"editdistance", "true"},
{"editdistance_siteengagement", "true"}}}, {"editdistance_siteengagement", "true"}}},
{features::kLookalikeUrlNavigationSuggestionsUI, {features::kLookalikeUrlNavigationSuggestionsUI,
{{"topsites", "true"}}}}, {{"topsites", "true"}}}},
...@@ -226,6 +227,11 @@ class SafetyTipPageInfoBubbleViewBrowserTest ...@@ -226,6 +227,11 @@ class SafetyTipPageInfoBubbleViewBrowserTest
return ui_status() == UIStatus::kDisabled ? true : IsUIShowing(); return ui_status() == UIStatus::kDisabled ? true : IsUIShowing();
} }
bool IsUIShowingOnlyIfFeaturesEnabled() {
return ui_status() == UIStatus::kEnabledWithAllFeatures ? IsUIShowing()
: !IsUIShowing();
}
void CheckPageInfoShowsSafetyTipInfo(Browser* browser) { void CheckPageInfoShowsSafetyTipInfo(Browser* browser) {
if (ui_status() == UIStatus::kDisabled) { if (ui_status() == UIStatus::kDisabled) {
return; return;
...@@ -258,7 +264,7 @@ INSTANTIATE_TEST_SUITE_P(, ...@@ -258,7 +264,7 @@ INSTANTIATE_TEST_SUITE_P(,
SafetyTipPageInfoBubbleViewBrowserTest, SafetyTipPageInfoBubbleViewBrowserTest,
::testing::Values(UIStatus::kDisabled, ::testing::Values(UIStatus::kDisabled,
UIStatus::kEnabled, UIStatus::kEnabled,
UIStatus::kEnabledWithEditDistance)); UIStatus::kEnabledWithAllFeatures));
// Ensure normal sites with low engagement are not blocked. // Ensure normal sites with low engagement are not blocked.
IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest, IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
...@@ -453,7 +459,7 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest, ...@@ -453,7 +459,7 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
const GURL kNavigatedUrl = GetURL("googlé.sk"); const GURL kNavigatedUrl = GetURL("googlé.sk");
SetEngagementScore(browser(), kNavigatedUrl, kLowEngagement); SetEngagementScore(browser(), kNavigatedUrl, kLowEngagement);
NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB); NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB);
EXPECT_TRUE(IsUIShowingIfEnabled()); EXPECT_TRUE(IsUIShowingOnlyIfFeaturesEnabled());
ASSERT_NO_FATAL_FAILURE(CheckPageInfoDoesNotShowSafetyTipInfo(browser())); ASSERT_NO_FATAL_FAILURE(CheckPageInfoDoesNotShowSafetyTipInfo(browser()));
} }
...@@ -467,7 +473,7 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest, ...@@ -467,7 +473,7 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
// Ensure a Safety Tip is triggered initially... // Ensure a Safety Tip is triggered initially...
SetEngagementScore(browser(), kNavigatedUrl, kLowEngagement); SetEngagementScore(browser(), kNavigatedUrl, kLowEngagement);
NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB); NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB);
EXPECT_TRUE(IsUIShowingIfEnabled()); EXPECT_TRUE(IsUIShowingOnlyIfFeaturesEnabled());
// ...but suppressed by the allowlist. // ...but suppressed by the allowlist.
SetSafetyTipAllowlistPatterns({"xn--googl-fsa.sk/"}); SetSafetyTipAllowlistPatterns({"xn--googl-fsa.sk/"});
...@@ -484,7 +490,7 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest, ...@@ -484,7 +490,7 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
const GURL kNavigatedUrl = GetURL("goooglé.com"); const GURL kNavigatedUrl = GetURL("goooglé.com");
SetEngagementScore(browser(), kNavigatedUrl, kLowEngagement); SetEngagementScore(browser(), kNavigatedUrl, kLowEngagement);
NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB); NavigateToURL(browser(), kNavigatedUrl, WindowOpenDisposition::CURRENT_TAB);
EXPECT_EQ(IsUIShowing(), ui_status() == UIStatus::kEnabledWithEditDistance); EXPECT_EQ(IsUIShowing(), ui_status() == UIStatus::kEnabledWithAllFeatures);
} }
// Tests that the SafetyTipShown histogram triggers correctly. // Tests that the SafetyTipShown histogram triggers correctly.
...@@ -508,8 +514,9 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest, ...@@ -508,8 +514,9 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
const GURL kLookalikeUrl = GetURL("googlé.sk"); const GURL kLookalikeUrl = GetURL("googlé.sk");
SetEngagementScore(browser(), kLookalikeUrl, kLowEngagement); SetEngagementScore(browser(), kLookalikeUrl, kLowEngagement);
NavigateToURL(browser(), kLookalikeUrl, WindowOpenDisposition::CURRENT_TAB); NavigateToURL(browser(), kLookalikeUrl, WindowOpenDisposition::CURRENT_TAB);
histograms.ExpectBucketCount(kHistogramName, histograms.ExpectBucketCount(
security_state::SafetyTipStatus::kLookalike, 1); kHistogramName, security_state::SafetyTipStatus::kLookalike,
ui_status() == UIStatus::kEnabledWithAllFeatures ? 1 : 0);
histograms.ExpectTotalCount(kHistogramName, 3); histograms.ExpectTotalCount(kHistogramName, 3);
} }
...@@ -536,7 +543,7 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest, ...@@ -536,7 +543,7 @@ IN_PROC_BROWSER_TEST_P(SafetyTipPageInfoBubbleViewBrowserTest,
// These histograms are only recorded when the UI feature is enabled, so bail // These histograms are only recorded when the UI feature is enabled, so bail
// out when disabled. // out when disabled.
if (ui_status() != UIStatus::kEnabledWithEditDistance) { if (ui_status() != UIStatus::kEnabledWithAllFeatures) {
return; return;
} }
......
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