Skip to content

Commit

Permalink
multiword: add a method to decide to enable/disable multiword by chec…
Browse files Browse the repository at this point in the history 
…king the URL against a domain denylist.

Currently, we only enable multiword if the current tab URL is the subdomain of any domain in the allowlist. We am trying to move away from the allowlist and towards the denylist to reach more websites and customers.

This CL implements a method called "ImeRulesConfig::IsMultiWordSuggestDisabled" which evaluates if the param GURL is a subdomain of any domain in the denylist.

Note: no op change, this CL implements towards b/265372129.

Change-Id: I57ebc905745ddef4fe4ca92e5a8667e6fdca5bd9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4190499
Commit-Queue: Chuong Ho <hdchuong@google.com>
Reviewed-by: Darren Shen <shend@chromium.org>
Reviewed-by: Curtis McMullan <curtismcmullan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1099103}
  • Loading branch information
Chuong Ho authored and Chromium LUCI CQ committed Jan 31, 2023
1 parent f7e803f commit 878bb83
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 28 deletions.
41 changes: 40 additions & 1 deletion chrome/browser/ash/input_method/ime_rules_config.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ const char kConfigRuleItemsKey[] = "items";
// globally denylisted domains for auto correct..
const char kAutocorrectDomainDenylistKey[] = "ac-domain-denylist";

const char* kDefaultMultiwordSuggestDomainAndPathDenylist[][2] = {
{"amazon", ""},
{"b.corp.google", ""},
{"buganizer.corp.google", ""},
{"cider.corp.google", ""},
{"classroom.google", ""},
{"desmos", ""},
{"docs.google", ""},
{"facebook", ""},
{"instagram", ""},
{"mail.google", "/mail"},
{"outlook.live", ""},
{"outlook.office", ""},
{"quizlet", ""},
{"whatsapp", ""},
{"youtube", ""},
};

} // namespace

ImeRulesConfig::ImeRulesConfig() {
Expand Down Expand Up @@ -105,7 +123,20 @@ bool ImeRulesConfig::IsAutoCorrectDisabled(
return false;
}

bool ImeRulesConfig::IsSubDomain(const GURL& url, const std::string& domain) {
bool ImeRulesConfig::IsMultiWordSuggestDisabled(const GURL& url) {
// Check the default domain denylist rules
for (auto& domainAndPath : kDefaultMultiwordSuggestDomainAndPathDenylist) {
const base::StringPiece domain = domainAndPath[0];
const base::StringPiece path = domainAndPath[1];
if (IsSubDomainWithPathPrefix(url, domain, path)) {
return true;
}
}
return false;
}

bool ImeRulesConfig::IsSubDomain(const GURL& url,
const base::StringPiece& domain) {
const size_t registryLength =
net::registry_controlled_domains::GetRegistryLength(
url, net::registry_controlled_domains::EXCLUDE_UNKNOWN_REGISTRIES,
Expand All @@ -120,6 +151,14 @@ bool ImeRulesConfig::IsSubDomain(const GURL& url, const std::string& domain) {
return url::DomainIs(urlDomain, domain);
}

bool ImeRulesConfig::IsSubDomainWithPathPrefix(
const GURL& url,
const base::StringPiece& domain,
const base::StringPiece& path_prefix) {
return IsSubDomain(url, domain) && url.has_path() &&
base::StartsWith(url.path(), path_prefix);
}

// static
ImeRulesConfig* ImeRulesConfig::GetInstance() {
return base::Singleton<ImeRulesConfig>::get();
Expand Down
12 changes: 9 additions & 3 deletions chrome/browser/ash/input_method/ime_rules_config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,17 @@ class ImeRulesConfig {
// Runs the rule check against contextual info.
bool IsAutoCorrectDisabled(const TextFieldContextualInfo& info);

// Runs the rule check against url
bool IsMultiWordSuggestDisabled(const GURL& url);

// Checks if domain is a sub-domain of url
private:
bool IsSubDomain(const GURL& url, const std::string& domain);
bool IsSubDomain(const GURL& url, const base::StringPiece& domain);

// Checks if url belongs to domain and has the path_prefix.
bool IsSubDomainWithPathPrefix(const GURL& url,
const base::StringPiece& domain,
const base::StringPiece& path_prefix);

private:
ImeRulesConfig();
Expand All @@ -43,8 +51,6 @@ class ImeRulesConfig {
friend struct base::DefaultSingletonTraits<ImeRulesConfig>;

friend class ImeRulesConfigTest;
friend class ImeRulesConfigEnabledTest;
friend class ImeRulesConfigDisabledTest;

// Initializes the config from IME rules trial parameters. If there is
// no trial or parsing fails, the rules will be empty and as such always
Expand Down
110 changes: 86 additions & 24 deletions chrome/browser/ash/input_method/ime_rules_config_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class ImeRulesConfigTest : public testing::Test {
};

TEST_F(ImeRulesConfigTest, LoadRulesFromFieldTrial) {
auto feature_list = std::make_unique<base::test::ScopedFeatureList>();
feature_list->InitAndEnableFeatureWithParameters(
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters(
ash::features::kImeRuleConfig,
{{"json_rules", kNormalAutocorrectRulesParams}});

Expand All @@ -60,19 +60,16 @@ TEST_F(ImeRulesConfigTest, LoadRulesFromFieldTrial) {
UnorderedElementsAre("docs.google", "chromium", "example", "test"));
}

class ImeRulesConfigEnabledTest : public testing::TestWithParam<std::string> {
class ImeRulesConfigAutoCorrectDisabledTest
: public testing::TestWithParam<std::string> {
public:
ImeRulesConfigEnabledTest() = default;
~ImeRulesConfigEnabledTest() override = default;

std::vector<std::string> GetAutocorrectDomainDenylistForTest() {
return ImeRulesConfig::GetInstance()->rule_auto_correct_domain_denylist_;
}
ImeRulesConfigAutoCorrectDisabledTest() = default;
~ImeRulesConfigAutoCorrectDisabledTest() override = default;
};

INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
ImeRulesConfigEnabledTest,
ImeRulesConfigAutoCorrectDisabledTest,
testing::Values(
"https://amazon.com",
"https://b.corp.google.com",
Expand All @@ -98,9 +95,9 @@ INSTANTIATE_TEST_SUITE_P(
"http://smile.amazon.com",
"http://www.abc.smile.amazon.com.au/abc+com+au/some/other/text"));

TEST_P(ImeRulesConfigEnabledTest, IsAutoCorrectEnabled) {
auto feature_list = std::make_unique<base::test::ScopedFeatureList>();
feature_list->InitAndEnableFeatureWithParameters(
TEST_P(ImeRulesConfigAutoCorrectDisabledTest, IsAutoCorrectDisabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters(
ash::features::kImeRuleConfig,
{{"json_rules", kNormalAutocorrectRulesParams}});

Expand All @@ -109,19 +106,16 @@ TEST_P(ImeRulesConfigEnabledTest, IsAutoCorrectEnabled) {
FakeTextFieldContextualInfo(GURL(GetParam()))));
}

class ImeRulesConfigDisabledTest : public testing::TestWithParam<std::string> {
class ImeRulesConfigAutoCorrectEnabledTest
: public testing::TestWithParam<std::string> {
public:
ImeRulesConfigDisabledTest() = default;
~ImeRulesConfigDisabledTest() override = default;

std::vector<std::string> GetAutocorrectDomainDenylistForTest() {
return ImeRulesConfig::GetInstance()->rule_auto_correct_domain_denylist_;
}
ImeRulesConfigAutoCorrectEnabledTest() = default;
~ImeRulesConfigAutoCorrectEnabledTest() override = default;
};

INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
ImeRulesConfigDisabledTest,
ImeRulesConfigAutoCorrectEnabledTest,
testing::Values("",
"http://",
"http://abc.com",
Expand All @@ -136,9 +130,9 @@ INSTANTIATE_TEST_SUITE_P(
"http://not-amazon.com/test",
"http://.com/test"));

TEST_P(ImeRulesConfigDisabledTest, IsAutoCorrectDisabled) {
auto feature_list = std::make_unique<base::test::ScopedFeatureList>();
feature_list->InitAndEnableFeatureWithParameters(
TEST_P(ImeRulesConfigAutoCorrectEnabledTest, IsAutoCorrectEnabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters(
ash::features::kImeRuleConfig,
{{"json_rules", kNormalAutocorrectRulesParams}});

Expand All @@ -147,5 +141,73 @@ TEST_P(ImeRulesConfigDisabledTest, IsAutoCorrectDisabled) {
FakeTextFieldContextualInfo(GURL(GetParam()))));
}

class ImeRulesConfigMultiWordSuggestDisabledTest
: public testing::TestWithParam<std::string> {
public:
ImeRulesConfigMultiWordSuggestDisabledTest() = default;
~ImeRulesConfigMultiWordSuggestDisabledTest() override = default;
};

INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
ImeRulesConfigMultiWordSuggestDisabledTest,
testing::Values("https://amazon.com",
"https://b.corp.google.com",
"https://buganizer.corp.google.com",
"https://cider.corp.google.com",
"https://classroom.google.com",
"https://desmos.com",
"https://docs.google.com",
"https://facebook.com",
"https://instagram.com",
"https://mail.google.com/mail",
"https://outlook.live.com",
"https://outlook.office.com",
"https://quizlet.com",
"https://whatsapp.com"));

TEST_P(ImeRulesConfigMultiWordSuggestDisabledTest, IsMultiWordSuggestDisabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters(
ash::features::kImeRuleConfig,
{{"json_rules", kNormalAutocorrectRulesParams}});
auto* rules = ImeRulesConfig::GetInstance();
EXPECT_TRUE(rules->IsMultiWordSuggestDisabled(GURL(GetParam())));
}

class ImeRulesConfigMultiWordSuggestEnabledTest
: public testing::TestWithParam<std::string> {
public:
ImeRulesConfigMultiWordSuggestEnabledTest() = default;
~ImeRulesConfigMultiWordSuggestEnabledTest() override = default;
};

INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
ImeRulesConfigMultiWordSuggestEnabledTest,
testing::Values("",
"http://",
"http://abc.com",
"http://abc.com/amazon+com",
"http://amazon",
"http://amazon/com/test",
"http://amazon/test",
"http://amazon.domain.com",
"https://mail.google.com/chat",
"http://my.own.quizlet.uniquie.co.uk/testing",
"http://not-amazon.com/test",
"http://sites.google.com/view/e14s-test",
"http://smile.amazon.foo.com",
"http://.com/test"));

TEST_P(ImeRulesConfigMultiWordSuggestEnabledTest, IsMultiWordSuggestEnabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters(
ash::features::kImeRuleConfig,
{{"json_rules", kNormalAutocorrectRulesParams}});
auto* rules = ImeRulesConfig::GetInstance();
EXPECT_FALSE(rules->IsMultiWordSuggestDisabled(GURL(GetParam())));
}

} // namespace input_method
} // namespace ash

0 comments on commit 878bb83

Please sign in to comment.