summaryrefslogtreecommitdiffstats
path: root/components
diff options
context:
space:
mode:
authormpearson <mpearson@chromium.org>2015-11-12 22:44:28 -0800
committerCommit bot <commit-bot@chromium.org>2015-11-13 06:45:18 +0000
commit6456fb606e045e77b4efa9e8058dcd86209d992a (patch)
treea3d5ab92547e8c42bd03052c16e7e5782bf8538e /components
parent6dd2ec859c1bdf1ccae51720aec88552169c5dd1 (diff)
downloadchromium_src-6456fb606e045e77b4efa9e8058dcd86209d992a.zip
chromium_src-6456fb606e045e77b4efa9e8058dcd86209d992a.tar.gz
chromium_src-6456fb606e045e77b4efa9e8058dcd86209d992a.tar.bz2
Omnibox: Make Keyword Provide More Generous with Matching
Adds fields to TemplateURLService to keep a map from keyword's domain to a TemplateURL so that users don't have to type the beginning of the keyword in order to match the keyword. Also adds a notion of "effective keyword length" that allows us to decide that it's okay if the user doesn't type the full keyword but instead types the important part of it. e.g., this allows ignoring .com in keywords. Adds an omnibox field trial that controls both of these behaviors. The field trial also has a parameter to control the score of "nearly complete" matches. BUG=488901 Review URL: https://codereview.chromium.org/1411543011 Cr-Commit-Position: refs/heads/master@{#359505}
Diffstat (limited to 'components')
-rw-r--r--components/omnibox/browser/keyword_provider.cc93
-rw-r--r--components/omnibox/browser/keyword_provider.h10
-rw-r--r--components/omnibox/browser/keyword_provider_unittest.cc156
-rw-r--r--components/omnibox/browser/omnibox_field_trial.cc35
-rw-r--r--components/omnibox/browser/omnibox_field_trial.h25
-rw-r--r--components/omnibox/browser/search_provider.cc148
-rw-r--r--components/omnibox/browser/search_provider.h15
-rw-r--r--components/search_engines/DEPS2
-rw-r--r--components/search_engines/template_url_service.cc238
-rw-r--r--components/search_engines/template_url_service.h101
10 files changed, 615 insertions, 208 deletions
diff --git a/components/omnibox/browser/keyword_provider.cc b/components/omnibox/browser/keyword_provider.cc
index 8992f02..360d94b 100644
--- a/components/omnibox/browser/keyword_provider.cc
+++ b/components/omnibox/browser/keyword_provider.cc
@@ -15,6 +15,8 @@
#include "components/omnibox/browser/autocomplete_provider_client.h"
#include "components/omnibox/browser/autocomplete_provider_listener.h"
#include "components/omnibox/browser/keyword_extensions_delegate.h"
+#include "components/omnibox/browser/omnibox_field_trial.h"
+#include "components/omnibox/browser/search_provider.h"
#include "components/search_engines/template_url.h"
#include "components/search_engines/template_url_service.h"
#include "grit/components_strings.h"
@@ -27,15 +29,18 @@ namespace {
// Helper functor for Start(), for sorting keyword matches by quality.
class CompareQuality {
public:
- // A keyword is of higher quality when a greater fraction of it has been
- // typed, that is, when it is shorter.
+ // A keyword is of higher quality when a greater fraction of the important
+ // part of it has been typed, that is, when the meaningful keyword length is
+ // shorter.
//
// TODO(pkasting): Most recent and most frequent keywords are probably
// better rankings than the fraction of the keyword typed. We should
// always put any exact matches first no matter what, since the code in
// Start() assumes this (and it makes sense).
- bool operator()(const TemplateURL* t_url1, const TemplateURL* t_url2) const {
- return t_url1->keyword().length() < t_url2->keyword().length();
+ bool operator()(
+ const TemplateURLService::TURLAndMeaningfulLength t_url_match1,
+ const TemplateURLService::TURLAndMeaningfulLength t_url_match2) const {
+ return t_url_match1.second < t_url_match2.second;
}
};
@@ -196,8 +201,9 @@ AutocompleteMatch KeywordProvider::CreateVerbatimMatch(
const AutocompleteInput& input) {
// A verbatim match is allowed to be the default match.
return CreateAutocompleteMatch(
- GetTemplateURLService()->GetTemplateURLForKeyword(keyword), input,
- keyword.length(), SplitReplacementStringFromInput(text, true), true, 0);
+ GetTemplateURLService()->GetTemplateURLForKeyword(keyword),
+ keyword.length(), input, keyword.length(),
+ SplitReplacementStringFromInput(text, true), true, 0);
}
void KeywordProvider::Start(const AutocompleteInput& input,
@@ -243,13 +249,17 @@ void KeywordProvider::Start(const AutocompleteInput& input,
// |minimal_changes| case, but since we'd still have to recalculate their
// relevances and we can just recreate the results synchronously anyway, we
// don't bother.
- TemplateURLService::TemplateURLVector matches;
- GetTemplateURLService()->FindMatchingKeywords(
+ TemplateURLService::TURLsAndMeaningfulLengths matches;
+ GetTemplateURLService()->AddMatchingKeywords(
keyword, !remaining_input.empty(), &matches);
+ if (!OmniboxFieldTrial::KeywordRequiresPrefixMatch()) {
+ GetTemplateURLService()->AddMatchingDomainKeywords(
+ keyword, !remaining_input.empty(), &matches);
+ }
- for (TemplateURLService::TemplateURLVector::iterator i(matches.begin());
- i != matches.end(); ) {
- const TemplateURL* template_url = *i;
+ for (TemplateURLService::TURLsAndMeaningfulLengths::iterator
+ i(matches.begin()); i != matches.end(); ) {
+ const TemplateURL* template_url = i->first;
// Prune any extension keywords that are disallowed in incognito mode (if
// we're incognito), or disabled.
@@ -280,8 +290,9 @@ void KeywordProvider::Start(const AutocompleteInput& input,
// in the autocomplete popup.
// Any exact match is going to be the highest quality match, and thus at the
// front of our vector.
- if (matches.front()->keyword() == keyword) {
- const TemplateURL* template_url = matches.front();
+ if (matches.front().first->keyword() == keyword) {
+ const TemplateURL* template_url = matches.front().first;
+ const size_t meaningful_keyword_length = matches.front().second;
const bool is_extension_keyword =
template_url->GetType() == TemplateURL::OMNIBOX_API_EXTENSION;
@@ -300,7 +311,8 @@ void KeywordProvider::Start(const AutocompleteInput& input,
// remaining query or an extension keyword, possibly with remaining
// input), allow the match to be the default match.
matches_.push_back(CreateAutocompleteMatch(
- template_url, input, keyword.length(), remaining_input, true, -1));
+ template_url, meaningful_keyword_length, input, keyword.length(),
+ remaining_input, true, -1));
if (is_extension_keyword && extensions_delegate_) {
if (extensions_delegate_->Start(input, minimal_changes, template_url,
@@ -308,12 +320,23 @@ void KeywordProvider::Start(const AutocompleteInput& input,
keyword_mode_toggle.StayInKeywordMode();
}
} else {
- if (matches.size() > kMaxMatches)
- matches.erase(matches.begin() + kMaxMatches, matches.end());
- for (TemplateURLService::TemplateURLVector::const_iterator i(
- matches.begin()); i != matches.end(); ++i) {
- matches_.push_back(CreateAutocompleteMatch(
- *i, input, keyword.length(), remaining_input, false, -1));
+ for (TemplateURLService::TURLsAndMeaningfulLengths::const_iterator i(
+ matches.begin());
+ (i != matches.end()) && (matches_.size() < kMaxMatches); ++i) {
+ // Skip keywords that we've already added. It's possible we may have
+ // retrieved the same keyword twice. For example, the keyword
+ // "abc.abc.com" may be retrieved for the input "abc" from the full
+ // keyword matching and the domain matching passes.
+ ACMatches::const_iterator duplicate = std::find_if(
+ matches_.begin(), matches_.end(),
+ [&i] (const AutocompleteMatch& m) {
+ return m.keyword == i->first->keyword();
+ });
+ if (duplicate == matches_.end()) {
+ matches_.push_back(CreateAutocompleteMatch(
+ i->first, i->second, input, keyword.length(), remaining_input,
+ false, -1));
+ }
}
}
}
@@ -346,29 +369,28 @@ bool KeywordProvider::ExtractKeywordFromInput(const AutocompleteInput& input,
// static
int KeywordProvider::CalculateRelevance(metrics::OmniboxInputType::Type type,
bool complete,
+ bool sufficiently_complete,
bool supports_replacement,
bool prefer_keyword,
bool allow_exact_keyword_match) {
- // This function is responsible for scoring suggestions of keywords
- // themselves and the suggestion of the verbatim query on an
- // extension keyword. SearchProvider::CalculateRelevanceForKeywordVerbatim()
- // scores verbatim query suggestions for non-extension keywords.
- // These two functions are currently in sync, but there's no reason
- // we couldn't decide in the future to score verbatim matches
- // differently for extension and non-extension keywords. If you
- // make such a change, however, you should update this comment to
- // describe it, so it's clear why the functions diverge.
- if (!complete)
+ if (!complete) {
+ const int sufficiently_complete_score =
+ OmniboxFieldTrial::KeywordScoreForSufficientlyCompleteMatch();
+ // If we have a special score to apply for sufficiently-complete matches,
+ // do so.
+ if (sufficiently_complete && (sufficiently_complete_score > -1))
+ return sufficiently_complete_score;
return (type == metrics::OmniboxInputType::URL) ? 700 : 450;
- if (!supports_replacement || (allow_exact_keyword_match && prefer_keyword))
+ }
+ if (!supports_replacement)
return 1500;
- return (allow_exact_keyword_match &&
- (type == metrics::OmniboxInputType::QUERY)) ?
- 1450 : 1100;
+ return SearchProvider::CalculateRelevanceForKeywordVerbatim(
+ type, allow_exact_keyword_match, prefer_keyword);
}
AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
const TemplateURL* template_url,
+ const size_t meaningful_keyword_length,
const AutocompleteInput& input,
size_t prefix_length,
const base::string16& remaining_input,
@@ -384,9 +406,12 @@ AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
// choice and immediately begin typing in query input.
const base::string16& keyword = template_url->keyword();
const bool keyword_complete = (prefix_length == keyword.length());
+ const bool sufficiently_complete =
+ (prefix_length >= meaningful_keyword_length);
if (relevance < 0) {
relevance =
CalculateRelevance(input.type(), keyword_complete,
+ sufficiently_complete,
// When the user wants keyword matches to take
// preference, score them highly regardless of
// whether the input provides query text.
diff --git a/components/omnibox/browser/keyword_provider.h b/components/omnibox/browser/keyword_provider.h
index 3be6313..e82e6cd 100644
--- a/components/omnibox/browser/keyword_provider.h
+++ b/components/omnibox/browser/keyword_provider.h
@@ -111,12 +111,13 @@ class KeywordProvider : public AutocompleteProvider {
base::string16* remaining_input);
// Determines the relevance for some input, given its type, whether the user
- // typed the complete keyword, and whether the user is in "prefer keyword
- // matches" mode, and whether the keyword supports replacement.
- // If |allow_exact_keyword_match| is false, the relevance for complete
- // keywords that support replacements is degraded.
+ // typed the complete keyword (or close to it), and whether the user is in
+ // "prefer keyword matches" mode, and whether the keyword supports
+ // replacement. If |allow_exact_keyword_match| is false, the relevance for
+ // complete keywords that support replacements is degraded.
static int CalculateRelevance(metrics::OmniboxInputType::Type type,
bool complete,
+ bool sufficiently_complete,
bool support_replacement,
bool prefer_keyword,
bool allow_exact_keyword_match);
@@ -125,6 +126,7 @@ class KeywordProvider : public AutocompleteProvider {
// If |relevance| is negative, calculate a relevance based on heuristics.
AutocompleteMatch CreateAutocompleteMatch(
const TemplateURL* template_url,
+ const size_t meaningful_keyword_length,
const AutocompleteInput& input,
size_t prefix_length,
const base::string16& remaining_input,
diff --git a/components/omnibox/browser/keyword_provider_unittest.cc b/components/omnibox/browser/keyword_provider_unittest.cc
index 15be17c..ff66f15 100644
--- a/components/omnibox/browser/keyword_provider_unittest.cc
+++ b/components/omnibox/browser/keyword_provider_unittest.cc
@@ -4,15 +4,19 @@
#include "base/command_line.h"
#include "base/message_loop/message_loop.h"
+#include "base/metrics/field_trial.h"
#include "base/strings/utf_string_conversions.h"
#include "components/metrics/proto/omnibox_event.pb.h"
#include "components/omnibox/browser/autocomplete_match.h"
#include "components/omnibox/browser/autocomplete_scheme_classifier.h"
#include "components/omnibox/browser/keyword_provider.h"
#include "components/omnibox/browser/mock_autocomplete_provider_client.h"
+#include "components/omnibox/browser/omnibox_field_trial.h"
#include "components/search_engines/search_engines_switches.h"
#include "components/search_engines/template_url.h"
#include "components/search_engines/template_url_service.h"
+#include "components/variations/entropy_provider.h"
+#include "components/variations/variations_associated_data.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
@@ -48,10 +52,22 @@ class KeywordProviderTest : public testing::Test {
const MatchType<ResultType> output[3];
};
- KeywordProviderTest() : kw_provider_(NULL) { }
+ KeywordProviderTest() : kw_provider_(NULL) {
+ // Destroy the existing FieldTrialList before creating a new one to avoid
+ // a DCHECK.
+ field_trial_list_.reset();
+ field_trial_list_.reset(new base::FieldTrialList(
+ new metrics::SHA1EntropyProvider("foo")));
+ variations::testing::ClearAllVariationParams();
+ }
~KeywordProviderTest() override {}
- void SetUp() override;
+ // Should be called at least once during a test case. This is a separate
+ // function from SetUp() because the client may want to set parameters
+ // (e.g., field trials) before initializing TemplateURLService and the
+ // related internal variables here.
+ void SetUpClientAndKeywordProvider();
+
void TearDown() override;
template<class ResultType>
@@ -62,6 +78,7 @@ class KeywordProviderTest : public testing::Test {
protected:
static const TemplateURLService::Initializer kTestData[];
+ scoped_ptr<base::FieldTrialList> field_trial_list_;
scoped_refptr<KeywordProvider> kw_provider_;
scoped_ptr<MockAutocompleteProviderClient> client_;
};
@@ -76,9 +93,18 @@ const TemplateURLService::Initializer KeywordProviderTest::kTestData[] = {
{ "www", " +%2B?={searchTerms}foo ", "www" },
{ "nonsub", "http://nonsubstituting-keyword.com/", "nonsub" },
{ "z", "{searchTerms}=z", "z" },
+ { "host.site.com", "http://host.site.com/?q={searchTerms}", "host.site.com" },
+ { "ignoremelong.domain.com",
+ "http://ignoremelong.domain.com/?q={searchTerms}",
+ "ignoremelong.domain.com" },
+ { "ignoreme.domain2.com",
+ "http://ignoreme.domain2.com/?q={searchTerms}",
+ "ignoreme.domain2.com" },
+ { "fooshort.com", "http://fooshort.com/?q={searchTerms}", "fooshort.com" },
+ { "foolong.co.uk", "http://foolong.co.uk/?q={searchTerms}", "foolong.co.uk" },
};
-void KeywordProviderTest::SetUp() {
+void KeywordProviderTest::SetUpClientAndKeywordProvider() {
scoped_ptr<TemplateURLService> template_url_service(
new TemplateURLService(kTestData, arraysize(kTestData)));
client_.reset(new MockAutocompleteProviderClient());
@@ -151,6 +177,10 @@ TEST_F(KeywordProviderTest, Edit) {
{ { ASCIIToUTF16("aa "), false },
{ ASCIIToUTF16("ab "), false },
{ ASCIIToUTF16("aaaa "), false } } },
+ { ASCIIToUTF16("foo hello"), 2,
+ { { ASCIIToUTF16("fooshort.com hello"), false },
+ { ASCIIToUTF16("foolong.co.uk hello"), false },
+ kEmptyMatch } },
// Exact matches should prevent returning inexact matches. Also, the
// verbatim query for this keyword match should not be returned. (It's
// returned by SearchProvider.)
@@ -159,6 +189,17 @@ TEST_F(KeywordProviderTest, Edit) {
{ ASCIIToUTF16("www.aaaa foo"), 0,
{ kEmptyMatch, kEmptyMatch, kEmptyMatch } },
+ // Matches should be retrieved by typing the prefix of the keyword, not the
+ // domain name.
+ { ASCIIToUTF16("host foo"), 1,
+ { { ASCIIToUTF16("host.site.com foo"), false },
+ kEmptyMatch, kEmptyMatch } },
+ { ASCIIToUTF16("host.site foo"), 1,
+ { { ASCIIToUTF16("host.site.com foo"), false },
+ kEmptyMatch, kEmptyMatch } },
+ { ASCIIToUTF16("site foo"), 0,
+ { kEmptyMatch, kEmptyMatch, kEmptyMatch } },
+
// Clean up keyword input properly. "http" and "https" are the only
// allowed schemes.
{ ASCIIToUTF16("www"), 1,
@@ -190,8 +231,105 @@ TEST_F(KeywordProviderTest, Edit) {
{ { ASCIIToUTF16("nonsub"), true }, kEmptyMatch, kEmptyMatch } },
};
+ SetUpClientAndKeywordProvider();
+ RunTest<base::string16>(edit_cases, arraysize(edit_cases),
+ &AutocompleteMatch::fill_into_edit);
+}
+
+TEST_F(KeywordProviderTest, DomainMatches) {
+ const MatchType<base::string16> kEmptyMatch = { base::string16(), false };
+ TestData<base::string16> edit_cases[] = {
+ // Searching for a nonexistent prefix should give nothing.
+ { ASCIIToUTF16("Not Found"), 0,
+ { kEmptyMatch, kEmptyMatch, kEmptyMatch } },
+ { ASCIIToUTF16("aaaaaNot Found"), 0,
+ { kEmptyMatch, kEmptyMatch, kEmptyMatch } },
+
+ // Matches should be limited to three and sorted in quality order.
+ // This order depends on whether we're using the pre-domain-name text
+ // for matching--when matching the domain, we sort by the length of the
+ // domain, not the length of the whole keyword.
+ { ASCIIToUTF16("ignore foo"), 2,
+ { { ASCIIToUTF16("ignoreme.domain2.com foo"), false },
+ { ASCIIToUTF16("ignoremelong.domain.com foo"), false },
+ kEmptyMatch } },
+ { ASCIIToUTF16("dom foo"), 2,
+ { { ASCIIToUTF16("ignoremelong.domain.com foo"), false },
+ { ASCIIToUTF16("ignoreme.domain2.com foo"), false },
+ kEmptyMatch } },
+
+ // Matches should be retrieved by typing the domain name, not only
+ // a prefix to the keyword.
+ { ASCIIToUTF16("host foo"), 1,
+ { { ASCIIToUTF16("host.site.com foo"), false },
+ kEmptyMatch, kEmptyMatch } },
+ { ASCIIToUTF16("host.site foo"), 1,
+ { { ASCIIToUTF16("host.site.com foo"), false },
+ kEmptyMatch, kEmptyMatch } },
+ { ASCIIToUTF16("site foo"), 1,
+ { { ASCIIToUTF16("host.site.com foo"), false },
+ kEmptyMatch, kEmptyMatch } },
+ };
+
+ // Add a rule enabling matching in the domain name of keywords (i.e.,
+ // non-prefix matching).
+ {
+ std::map<std::string, std::string> params;
+ params[OmniboxFieldTrial::kKeywordRequiresPrefixMatchRule] = "false";
+ ASSERT_TRUE(variations::AssociateVariationParams(
+ OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A", params));
+ }
+ base::FieldTrialList::CreateFieldTrial(
+ OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A");
+
+ SetUpClientAndKeywordProvider();
+ RunTest<base::string16>(edit_cases, arraysize(edit_cases),
+ &AutocompleteMatch::fill_into_edit);
+}
+
+TEST_F(KeywordProviderTest, IgnoreRegistryForScoring) {
+ const MatchType<base::string16> kEmptyMatch = { base::string16(), false };
+ TestData<base::string16> edit_cases[] = {
+ // Matches should be limited to three and sorted in quality order.
+ // When ignoring the registry length, this order of suggestions should
+ // result (sorted by keyword length sans registry). The "Edit" test case
+ // has this exact test for when not ignoring the registry to check that
+ // the other order (shorter full keyword) results there.
+ { ASCIIToUTF16("foo hello"), 2,
+ { { ASCIIToUTF16("foolong.co.uk hello"), false },
+ { ASCIIToUTF16("fooshort.com hello"), false },
+ kEmptyMatch } },
+
+ // Keywords that don't have full hostnames should keep the same order
+ // as normal.
+ { ASCIIToUTF16("aaa"), 2,
+ { { ASCIIToUTF16("aaaa "), false },
+ { ASCIIToUTF16("aaaaa "), false },
+ kEmptyMatch } },
+ { ASCIIToUTF16("a 1 2 3"), 3,
+ { { ASCIIToUTF16("aa 1 2 3"), false },
+ { ASCIIToUTF16("ab 1 2 3"), false },
+ { ASCIIToUTF16("aaaa 1 2 3"), false } } },
+ { ASCIIToUTF16("www.a"), 3,
+ { { ASCIIToUTF16("aa "), false },
+ { ASCIIToUTF16("ab "), false },
+ { ASCIIToUTF16("aaaa "), false } } },
+ };
+
+ // Add a rule to make matching in the registry portion of a keyword
+ // unimportant.
+ {
+ std::map<std::string, std::string> params;
+ params[OmniboxFieldTrial::kKeywordRequiresRegistryRule] = "false";
+ ASSERT_TRUE(variations::AssociateVariationParams(
+ OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A", params));
+ }
+ base::FieldTrialList::CreateFieldTrial(
+ OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A");
+
+ SetUpClientAndKeywordProvider();
RunTest<base::string16>(edit_cases, arraysize(edit_cases),
- &AutocompleteMatch::fill_into_edit);
+ &AutocompleteMatch::fill_into_edit);
}
TEST_F(KeywordProviderTest, URL) {
@@ -225,6 +363,7 @@ TEST_F(KeywordProviderTest, URL) {
kEmptyMatch } },
};
+ SetUpClientAndKeywordProvider();
RunTest<GURL>(url_cases, arraysize(url_cases),
&AutocompleteMatch::destination_url);
}
@@ -268,11 +407,13 @@ TEST_F(KeywordProviderTest, Contents) {
{ ASCIIToUTF16("Search aaaa for 1 2+ 3"), false } } },
};
+ SetUpClientAndKeywordProvider();
RunTest<base::string16>(contents_cases, arraysize(contents_cases),
- &AutocompleteMatch::contents);
+ &AutocompleteMatch::contents);
}
TEST_F(KeywordProviderTest, AddKeyword) {
+ SetUpClientAndKeywordProvider();
TemplateURLData data;
data.SetShortName(ASCIIToUTF16("Test"));
base::string16 keyword(ASCIIToUTF16("foo"));
@@ -286,6 +427,7 @@ TEST_F(KeywordProviderTest, AddKeyword) {
}
TEST_F(KeywordProviderTest, RemoveKeyword) {
+ SetUpClientAndKeywordProvider();
TemplateURLService* template_url_service = client_->GetTemplateURLService();
base::string16 url(ASCIIToUTF16("http://aaaa/?aaaa=1&b={searchTerms}&c"));
template_url_service->Remove(
@@ -295,6 +437,7 @@ TEST_F(KeywordProviderTest, RemoveKeyword) {
}
TEST_F(KeywordProviderTest, GetKeywordForInput) {
+ SetUpClientAndKeywordProvider();
EXPECT_EQ(ASCIIToUTF16("aa"),
kw_provider_->GetKeywordForText(ASCIIToUTF16("aa")));
EXPECT_EQ(base::string16(),
@@ -344,6 +487,7 @@ TEST_F(KeywordProviderTest, GetSubstitutingTemplateURLForInput) {
{ "aa foo", base::string16::npos, false, "", "aa foo",
base::string16::npos },
};
+ SetUpClientAndKeywordProvider();
for (size_t i = 0; i < arraysize(cases); i++) {
AutocompleteInput input(ASCIIToUTF16(cases[i].text),
cases[i].cursor_position, std::string(), GURL(),
@@ -375,11 +519,13 @@ TEST_F(KeywordProviderTest, ExtraQueryParams) {
{ GURL("http://aaaa/?aaaa=1&b=1+2+3&c"), false } } },
};
+ SetUpClientAndKeywordProvider();
RunTest<GURL>(url_cases, arraysize(url_cases),
&AutocompleteMatch::destination_url);
}
TEST_F(KeywordProviderTest, DoesNotProvideMatchesOnFocus) {
+ SetUpClientAndKeywordProvider();
AutocompleteInput input(ASCIIToUTF16("aaa"), base::string16::npos,
std::string(), GURL(),
metrics::OmniboxEventProto::INVALID_SPEC, true, false,
diff --git a/components/omnibox/browser/omnibox_field_trial.cc b/components/omnibox/browser/omnibox_field_trial.cc
index b45f0b2..b07ce44 100644
--- a/components/omnibox/browser/omnibox_field_trial.cc
+++ b/components/omnibox/browser/omnibox_field_trial.cc
@@ -440,6 +440,35 @@ bool OmniboxFieldTrial::PreventUWYTDefaultForNonURLInputs() {
kPreventUWYTDefaultForNonURLInputsRule) == "true";
}
+bool OmniboxFieldTrial::KeywordRequiresRegistry() {
+ const std::string& value = variations::GetVariationParamValue(
+ kBundledExperimentFieldTrialName,
+ kKeywordRequiresRegistryRule);
+ return value.empty() || (value == "true");
+}
+
+bool OmniboxFieldTrial::KeywordRequiresPrefixMatch() {
+ const std::string& value = variations::GetVariationParamValue(
+ kBundledExperimentFieldTrialName,
+ kKeywordRequiresPrefixMatchRule);
+ return value.empty() || (value == "true");
+}
+
+int OmniboxFieldTrial::KeywordScoreForSufficientlyCompleteMatch() {
+ std::string value_str = variations::GetVariationParamValue(
+ kBundledExperimentFieldTrialName,
+ kKeywordScoreForSufficientlyCompleteMatchRule);
+ if (value_str.empty())
+ return -1;
+ // This is a best-effort conversion; we trust the hand-crafted parameters
+ // downloaded from the server to be perfect. There's no need for handle
+ // errors smartly.
+ int value;
+ base::StringToInt(value_str, &value);
+ return value;
+}
+
+
const char OmniboxFieldTrial::kBundledExperimentFieldTrialName[] =
"OmniboxBundledExperimentV1";
const char OmniboxFieldTrial::kDisableProvidersRule[] = "DisableProviders";
@@ -469,6 +498,12 @@ const char OmniboxFieldTrial::kHQPAlsoDoHUPLikeScoringRule[] =
"HQPAlsoDoHUPLikeScoring";
const char OmniboxFieldTrial::kPreventUWYTDefaultForNonURLInputsRule[] =
"PreventUWYTDefaultForNonURLInputs";
+const char OmniboxFieldTrial::kKeywordRequiresRegistryRule[] =
+ "KeywordRequiresRegistry";
+const char OmniboxFieldTrial::kKeywordRequiresPrefixMatchRule[] =
+ "KeywordRequiresPrefixMatch";
+const char OmniboxFieldTrial::kKeywordScoreForSufficientlyCompleteMatchRule[] =
+ "KeywordScoreForSufficientlyCompleteMatch";
const char OmniboxFieldTrial::kHUPNewScoringEnabledParam[] =
"HUPExperimentalScoringEnabled";
diff --git a/components/omnibox/browser/omnibox_field_trial.h b/components/omnibox/browser/omnibox_field_trial.h
index b6e3549..674170f 100644
--- a/components/omnibox/browser/omnibox_field_trial.h
+++ b/components/omnibox/browser/omnibox_field_trial.h
@@ -320,6 +320,28 @@ class OmniboxFieldTrial {
static bool PreventUWYTDefaultForNonURLInputs();
// ---------------------------------------------------------
+ // For the aggressive keyword matching experiment that's part of the bundled
+ // omnibox field trial.
+
+ // Returns whether KeywordProvider should consider the registry portion
+ // (e.g., co.uk) of keywords that look like hostnames as an important part of
+ // the keyword name for matching purposes. Returns true if the experiment
+ // isn't active.
+ static bool KeywordRequiresRegistry();
+
+ // For keywords that look like hostnames, returns whether KeywordProvider
+ // should require users to type a prefix of the hostname to match against
+ // them, rather than just the domain name portion. In other words, returns
+ // whether the prefix before the domain name should be considered important
+ // for matching purposes. Returns true if the experiment isn't active.
+ static bool KeywordRequiresPrefixMatch();
+
+ // Returns the relevance score that KeywordProvider should assign to keywords
+ // with sufficiently-complete matches, i.e., the user has typed all of the
+ // important part of the keyword. Returns -1 if the experiment isn't active.
+ static int KeywordScoreForSufficientlyCompleteMatch();
+
+ // ---------------------------------------------------------
// Exposed publicly for the sake of unittests.
static const char kBundledExperimentFieldTrialName[];
// Rule names used by the bundled experiment.
@@ -341,6 +363,9 @@ class OmniboxFieldTrial {
static const char kHQPNumTitleWordsRule[];
static const char kHQPAlsoDoHUPLikeScoringRule[];
static const char kPreventUWYTDefaultForNonURLInputsRule[];
+ static const char kKeywordRequiresRegistryRule[];
+ static const char kKeywordRequiresPrefixMatchRule[];
+ static const char kKeywordScoreForSufficientlyCompleteMatchRule[];
// Parameter names used by the HUP new scoring experiments.
static const char kHUPNewScoringEnabledParam[];
diff --git a/components/omnibox/browser/search_provider.cc b/components/omnibox/browser/search_provider.cc
index 914aeb5..c3c9a25 100644
--- a/components/omnibox/browser/search_provider.cc
+++ b/components/omnibox/browser/search_provider.cc
@@ -137,73 +137,49 @@ std::string SearchProvider::GetSuggestMetadata(const AutocompleteMatch& match) {
return match.GetAdditionalInfo(kSuggestMetadataKey);
}
-void SearchProvider::ResetSession() {
- set_field_trial_triggered_in_session(false);
-}
-
-void SearchProvider::OnTemplateURLServiceChanged() {
- // Only update matches at this time if we haven't already claimed we're done
- // processing the query.
- if (done_)
+void SearchProvider::RegisterDisplayedAnswers(
+ const AutocompleteResult& result) {
+ if (result.empty())
return;
- // Check that the engines we're using weren't renamed or deleted. (In short,
- // require that an engine still exists with the keywords in use.) For each
- // deleted engine, cancel the in-flight request if any, drop its suggestions,
- // and, in the case when the default provider was affected, point the cached
- // default provider keyword name at the new name for the default provider.
-
- // Get...ProviderURL() looks up the provider using the cached keyword name
- // stored in |providers_|.
- const TemplateURL* template_url = providers_.GetDefaultProviderURL();
- if (!template_url) {
- CancelFetcher(&default_fetcher_);
- default_results_.Clear();
- providers_.set(client()
- ->GetTemplateURLService()
- ->GetDefaultSearchProvider()
- ->keyword(),
- providers_.keyword_provider());
- }
- template_url = providers_.GetKeywordProviderURL();
- if (!providers_.keyword_provider().empty() && !template_url) {
- CancelFetcher(&keyword_fetcher_);
- keyword_results_.Clear();
- providers_.set(providers_.default_provider(), base::string16());
- }
- // It's possible the template URL changed without changing associated keyword.
- // Hence, it's always necessary to update matches to use the new template
- // URL. (One could cache the template URL and only call UpdateMatches() and
- // OnProviderUpdate() if a keyword was deleted/renamed or the template URL
- // was changed. That would save extra calls to these functions. However,
- // this is uncommon and not likely to be worth the extra work.)
- UpdateMatches();
- listener_->OnProviderUpdate(true); // always pretend something changed
-}
+ // The answer must be in the first or second slot to be considered. It should
+ // only be in the second slot if AutocompleteController ranked a local search
+ // history or a verbatim item higher than the answer.
+ AutocompleteResult::const_iterator match = result.begin();
+ if (match->answer_contents.empty() && result.size() > 1)
+ ++match;
+ if (match->answer_contents.empty() || match->answer_type.empty() ||
+ match->fill_into_edit.empty())
+ return;
-SearchProvider::~SearchProvider() {
- TemplateURLService* template_url_service = client()->GetTemplateURLService();
- if (template_url_service)
- template_url_service->RemoveObserver(this);
+ // Valid answer encountered, cache it for further queries.
+ answers_cache_.UpdateRecentAnswers(match->fill_into_edit, match->answer_type);
}
// static
int SearchProvider::CalculateRelevanceForKeywordVerbatim(
metrics::OmniboxInputType::Type type,
+ bool allow_exact_keyword_match,
bool prefer_keyword) {
// This function is responsible for scoring verbatim query matches
- // for non-extension keywords. KeywordProvider::CalculateRelevance()
- // scores verbatim query matches for extension keywords, as well as
- // for keyword matches (i.e., suggestions of a keyword itself, not a
- // suggestion of a query on a keyword search engine). These two
- // functions are currently in sync, but there's no reason we
- // couldn't decide in the future to score verbatim matches
- // differently for extension and non-extension keywords. If you
- // make such a change, however, you should update this comment to
- // describe it, so it's clear why the functions diverge.
- if (prefer_keyword)
+ // for non-extension substituting keywords.
+ // KeywordProvider::CalculateRelevance() scores all other types of
+ // keyword verbatim matches.
+ if (allow_exact_keyword_match && prefer_keyword)
return 1500;
- return (type == metrics::OmniboxInputType::QUERY) ? 1450 : 1100;
+ return (allow_exact_keyword_match &&
+ (type == metrics::OmniboxInputType::QUERY)) ?
+ 1450 : 1100;
+}
+
+void SearchProvider::ResetSession() {
+ set_field_trial_triggered_in_session(false);
+}
+
+SearchProvider::~SearchProvider() {
+ TemplateURLService* template_url_service = client()->GetTemplateURLService();
+ if (template_url_service)
+ template_url_service->RemoveObserver(this);
}
// static
@@ -368,6 +344,46 @@ void SearchProvider::RecordDeletionResult(bool success) {
}
}
+void SearchProvider::OnTemplateURLServiceChanged() {
+ // Only update matches at this time if we haven't already claimed we're done
+ // processing the query.
+ if (done_)
+ return;
+
+ // Check that the engines we're using weren't renamed or deleted. (In short,
+ // require that an engine still exists with the keywords in use.) For each
+ // deleted engine, cancel the in-flight request if any, drop its suggestions,
+ // and, in the case when the default provider was affected, point the cached
+ // default provider keyword name at the new name for the default provider.
+
+ // Get...ProviderURL() looks up the provider using the cached keyword name
+ // stored in |providers_|.
+ const TemplateURL* template_url = providers_.GetDefaultProviderURL();
+ if (!template_url) {
+ CancelFetcher(&default_fetcher_);
+ default_results_.Clear();
+ providers_.set(client()
+ ->GetTemplateURLService()
+ ->GetDefaultSearchProvider()
+ ->keyword(),
+ providers_.keyword_provider());
+ }
+ template_url = providers_.GetKeywordProviderURL();
+ if (!providers_.keyword_provider().empty() && !template_url) {
+ CancelFetcher(&keyword_fetcher_);
+ keyword_results_.Clear();
+ providers_.set(providers_.default_provider(), base::string16());
+ }
+ // It's possible the template URL changed without changing associated keyword.
+ // Hence, it's always necessary to update matches to use the new template
+ // URL. (One could cache the template URL and only call UpdateMatches() and
+ // OnProviderUpdate() if a keyword was deleted/renamed or the template URL
+ // was changed. That would save extra calls to these functions. However,
+ // this is uncommon and not likely to be worth the extra work.)
+ UpdateMatches();
+ listener_->OnProviderUpdate(true); // always pretend something changed
+}
+
void SearchProvider::OnURLFetchComplete(const net::URLFetcher* source) {
DCHECK(!done_);
const bool is_keyword = source == keyword_fetcher_.get();
@@ -1324,6 +1340,7 @@ int SearchProvider::GetKeywordVerbatimRelevance(
return use_server_relevance ?
keyword_results_.verbatim_relevance :
CalculateRelevanceForKeywordVerbatim(keyword_input_.type(),
+ true,
keyword_input_.prefer_keyword());
}
@@ -1472,25 +1489,6 @@ std::string SearchProvider::GetSessionToken() {
return current_token_;
}
-void SearchProvider::RegisterDisplayedAnswers(
- const AutocompleteResult& result) {
- if (result.empty())
- return;
-
- // The answer must be in the first or second slot to be considered. It should
- // only be in the second slot if AutocompleteController ranked a local search
- // history or a verbatim item higher than the answer.
- AutocompleteResult::const_iterator match = result.begin();
- if (match->answer_contents.empty() && result.size() > 1)
- ++match;
- if (match->answer_contents.empty() || match->answer_type.empty() ||
- match->fill_into_edit.empty())
- return;
-
- // Valid answer encountered, cache it for further queries.
- answers_cache_.UpdateRecentAnswers(match->fill_into_edit, match->answer_type);
-}
-
AnswersQueryData SearchProvider::FindAnswersPrefetchData() {
// Retrieve the top entry from scored history results.
MatchMap map;
diff --git a/components/omnibox/browser/search_provider.h b/components/omnibox/browser/search_provider.h
index 2680630..259e7c6 100644
--- a/components/omnibox/browser/search_provider.h
+++ b/components/omnibox/browser/search_provider.h
@@ -66,6 +66,15 @@ class SearchProvider : public BaseSearchProvider,
// match for Autocomplete and registers the contained answer data, if any.
void RegisterDisplayedAnswers(const AutocompleteResult& result);
+ // Calculates the relevance score for the keyword verbatim result (if the
+ // input matches one of the profile's keywords). If
+ // |allow_exact_keyword_match| is false, the relevance for complete
+ // keywords that support replacements is degraded.
+ static int CalculateRelevanceForKeywordVerbatim(
+ metrics::OmniboxInputType::Type type,
+ bool allow_exact_keyword_match,
+ bool prefer_keyword);
+
// AutocompleteProvider:
void ResetSession() override;
@@ -142,12 +151,6 @@ class SearchProvider : public BaseSearchProvider,
typedef std::vector<history::KeywordSearchTermVisit> HistoryResults;
- // Calculates the relevance score for the keyword verbatim result (if the
- // input matches one of the profile's keyword).
- static int CalculateRelevanceForKeywordVerbatim(
- metrics::OmniboxInputType::Type type,
- bool prefer_keyword);
-
// A helper function for UpdateAllOldResults().
static void UpdateOldResults(bool minimal_changes,
SearchSuggestionParser::Results* results);
diff --git a/components/search_engines/DEPS b/components/search_engines/DEPS
index 91a1059..aef4d1b 100644
--- a/components/search_engines/DEPS
+++ b/components/search_engines/DEPS
@@ -3,6 +3,8 @@ include_rules = [
"+components/history/core",
"+components/keyed_service/core",
"+components/metrics/proto",
+ # TODO(mpearson): for experiment; remove after crbug.com/488901 is launched.
+ "+components/omnibox/browser/omnibox_field_trial.h",
"+components/policy/core",
"+components/pref_registry",
"+components/rappor",
diff --git a/components/search_engines/template_url_service.cc b/components/search_engines/template_url_service.cc
index 3402ce3..908f7d9 100644
--- a/components/search_engines/template_url_service.cc
+++ b/components/search_engines/template_url_service.cc
@@ -22,6 +22,7 @@
#include "base/strings/utf_string_conversions.h"
#include "base/time/default_clock.h"
#include "base/time/time.h"
+#include "components/omnibox/browser/omnibox_field_trial.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/rappor/rappor_service.h"
#include "components/search_engines/search_engines_pref_names.h"
@@ -149,8 +150,42 @@ void LogDuplicatesHistogram(
UMA_HISTOGRAM_COUNTS_100("Search.SearchEngineDuplicateCounts", num_dupes);
}
-} // namespace
+// Returns the length of the registry portion of a hostname. For example,
+// www.google.co.uk will return 5 (the length of co.uk).
+size_t GetRegistryLength(const base::string16& host) {
+ return net::registry_controlled_domains::GetRegistryLength(
+ base::UTF16ToUTF8(host),
+ net::registry_controlled_domains::EXCLUDE_UNKNOWN_REGISTRIES,
+ net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES);
+}
+
+// Returns the domain name (including registry) of a hostname. For example,
+// www.google.co.uk will return google.co.uk.
+base::string16 GetDomainAndRegistry(const base::string16& host) {
+ return base::UTF8ToUTF16(
+ net::registry_controlled_domains::GetDomainAndRegistry(
+ base::UTF16ToUTF8(host),
+ net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES));
+}
+// Returns the length of the important part of the |keyword|, assumed to be
+// associated with the TemplateURL. For instance, for the keyword
+// google.co.uk, this can return 6 (the length of "google").
+size_t GetMeaningfulKeywordLength(const base::string16& keyword,
+ const TemplateURL* turl) {
+ if (OmniboxFieldTrial::KeywordRequiresRegistry())
+ return keyword.length();
+ const size_t registry_length = GetRegistryLength(keyword);
+ if (registry_length == std::string::npos)
+ return keyword.length();
+ DCHECK_LT(registry_length, keyword.length());
+ // The meaningful keyword length is the length of any portion before the
+ // registry ("co.uk") and its preceding dot.
+ return keyword.length() - (registry_length ? (registry_length + 1) : 0);
+
+}
+
+} // namespace
// TemplateURLService::LessWithPrefix -----------------------------------------
@@ -168,9 +203,10 @@ class TemplateURLService::LessWithPrefix {
// Unfortunately the calling convention is not "prefix and element" but
// rather "two elements", so we pass the prefix as a fake "element" which has
// a NULL KeywordDataElement pointer.
- bool operator()(const KeywordToTemplateMap::value_type& elem1,
- const KeywordToTemplateMap::value_type& elem2) const {
- return (elem1.second == NULL) ?
+ bool operator()(
+ const KeywordToTURLAndMeaningfulLength::value_type& elem1,
+ const KeywordToTURLAndMeaningfulLength::value_type& elem2) const {
+ return (elem1.second.first == NULL) ?
(elem2.first.compare(0, elem1.first.length(), elem1.first) > 0) :
(elem1.first < elem2.first);
}
@@ -344,44 +380,29 @@ bool TemplateURLService::CanAddAutogeneratedKeyword(
CanAddAutogeneratedKeywordForHost(url.host());
}
-void TemplateURLService::FindMatchingKeywords(
+void TemplateURLService::AddMatchingKeywords(
const base::string16& prefix,
- bool support_replacement_only,
- TemplateURLVector* matches) {
- // Sanity check args.
- if (prefix.empty())
- return;
- DCHECK(matches != NULL);
- DCHECK(matches->empty()); // The code for exact matches assumes this.
-
- // Required for VS2010: http://connect.microsoft.com/VisualStudio/feedback/details/520043/error-converting-from-null-to-a-pointer-type-in-std-pair
- TemplateURL* const kNullTemplateURL = NULL;
+ bool supports_replacement_only,
+ TURLsAndMeaningfulLengths* matches) {
+ AddMatchingKeywordsHelper(
+ keyword_to_turl_and_length_, prefix, supports_replacement_only, matches);
+}
- // Find matching keyword range. Searches the element map for keywords
- // beginning with |prefix| and stores the endpoints of the resulting set in
- // |match_range|.
- const std::pair<KeywordToTemplateMap::const_iterator,
- KeywordToTemplateMap::const_iterator> match_range(
- std::equal_range(
- keyword_to_template_map_.begin(), keyword_to_template_map_.end(),
- KeywordToTemplateMap::value_type(prefix, kNullTemplateURL),
- LessWithPrefix()));
-
- // Return vector of matching keywords.
- for (KeywordToTemplateMap::const_iterator i(match_range.first);
- i != match_range.second; ++i) {
- if (!support_replacement_only ||
- i->second->url_ref().SupportsReplacement(search_terms_data()))
- matches->push_back(i->second);
- }
+void TemplateURLService::AddMatchingDomainKeywords(
+ const base::string16& prefix,
+ bool supports_replacement_only,
+ TURLsAndMeaningfulLengths* matches) {
+ AddMatchingKeywordsHelper(
+ keyword_domain_to_turl_and_length_, prefix, supports_replacement_only,
+ matches);
}
TemplateURL* TemplateURLService::GetTemplateURLForKeyword(
const base::string16& keyword) {
- KeywordToTemplateMap::const_iterator elem(
- keyword_to_template_map_.find(keyword));
- if (elem != keyword_to_template_map_.end())
- return elem->second;
+ KeywordToTURLAndMeaningfulLength::const_iterator elem(
+ keyword_to_turl_and_length_.find(keyword));
+ if (elem != keyword_to_turl_and_length_.end())
+ return elem->second.first;
return (!loaded_ &&
initial_default_search_provider_.get() &&
(initial_default_search_provider_->keyword() == keyword)) ?
@@ -390,8 +411,8 @@ TemplateURL* TemplateURLService::GetTemplateURLForKeyword(
TemplateURL* TemplateURLService::GetTemplateURLForGUID(
const std::string& sync_guid) {
- GUIDToTemplateMap::const_iterator elem(guid_to_template_map_.find(sync_guid));
- if (elem != guid_to_template_map_.end())
+ GUIDToTURL::const_iterator elem(guid_to_turl_.find(sync_guid));
+ if (elem != guid_to_turl_.end())
return elem->second;
return (!loaded_ &&
initial_default_search_provider_.get() &&
@@ -1413,8 +1434,8 @@ void TemplateURLService::Init(const Initializer* initializers,
void TemplateURLService::RemoveFromMaps(TemplateURL* template_url) {
const base::string16& keyword = template_url->keyword();
- DCHECK_NE(0U, keyword_to_template_map_.count(keyword));
- if (keyword_to_template_map_[keyword] == template_url) {
+ DCHECK_NE(0U, keyword_to_turl_and_length_.count(keyword));
+ if (keyword_to_turl_and_length_[keyword].first == template_url) {
// We need to check whether the keyword can now be provided by another
// TemplateURL. See the comments in AddToMaps() for more information on
// extension keywords and how they can coexist with non-extension keywords.
@@ -1434,17 +1455,20 @@ void TemplateURLService::RemoveFromMaps(TemplateURL* template_url) {
(turl->id() > best_fallback->id()))))
best_fallback = turl;
}
- if (best_fallback)
- keyword_to_template_map_[keyword] = best_fallback;
- else
- keyword_to_template_map_.erase(keyword);
+ RemoveFromDomainMap(template_url);
+ if (best_fallback) {
+ AddToMap(best_fallback);
+ AddToDomainMap(best_fallback);
+ } else {
+ keyword_to_turl_and_length_.erase(keyword);
+ }
}
if (template_url->GetType() == TemplateURL::OMNIBOX_API_EXTENSION)
return;
if (!template_url->sync_guid().empty())
- guid_to_template_map_.erase(template_url->sync_guid());
+ guid_to_turl_.erase(template_url->sync_guid());
// |provider_map_| is only initialized after loading has completed.
if (loaded_) {
provider_map_->Remove(template_url);
@@ -1455,12 +1479,13 @@ void TemplateURLService::AddToMaps(TemplateURL* template_url) {
bool template_url_is_omnibox_api =
template_url->GetType() == TemplateURL::OMNIBOX_API_EXTENSION;
const base::string16& keyword = template_url->keyword();
- KeywordToTemplateMap::const_iterator i =
- keyword_to_template_map_.find(keyword);
- if (i == keyword_to_template_map_.end()) {
- keyword_to_template_map_[keyword] = template_url;
+ KeywordToTURLAndMeaningfulLength::const_iterator i =
+ keyword_to_turl_and_length_.find(keyword);
+ if (i == keyword_to_turl_and_length_.end()) {
+ AddToMap(template_url);
+ AddToDomainMap(template_url);
} else {
- const TemplateURL* existing_url = i->second;
+ const TemplateURL* existing_url = i->second.first;
// We should only have overlapping keywords when at least one comes from
// an extension. In that case, the ranking order is:
// Manually-modified keywords > extension keywords > replaceable keywords
@@ -1469,20 +1494,57 @@ void TemplateURLService::AddToMaps(TemplateURL* template_url) {
existing_url->GetType() == TemplateURL::OMNIBOX_API_EXTENSION;
DCHECK(existing_url_is_omnibox_api || template_url_is_omnibox_api);
if (existing_url_is_omnibox_api ?
- !CanReplace(template_url) : CanReplace(existing_url))
- keyword_to_template_map_[keyword] = template_url;
+ !CanReplace(template_url) : CanReplace(existing_url)) {
+ RemoveFromDomainMap(existing_url);
+ AddToMap(template_url);
+ AddToDomainMap(template_url);
+ }
}
if (template_url_is_omnibox_api)
return;
if (!template_url->sync_guid().empty())
- guid_to_template_map_[template_url->sync_guid()] = template_url;
+ guid_to_turl_[template_url->sync_guid()] = template_url;
// |provider_map_| is only initialized after loading has completed.
if (loaded_)
provider_map_->Add(template_url, search_terms_data());
}
+void TemplateURLService::RemoveFromDomainMap(const TemplateURL* template_url) {
+ const base::string16 domain = GetDomainAndRegistry(template_url->keyword());
+ if (domain.empty())
+ return;
+
+ const auto match_range(
+ keyword_domain_to_turl_and_length_.equal_range(domain));
+ for (auto it(match_range.first); it != match_range.second; ) {
+ if (it->second.first == template_url)
+ it = keyword_domain_to_turl_and_length_.erase(it);
+ else
+ ++it;
+ }
+}
+
+void TemplateURLService::AddToDomainMap(TemplateURL* template_url) {
+ const base::string16 domain = GetDomainAndRegistry(template_url->keyword());
+ // Only bother adding an entry to the domain map if its key in the domain
+ // map would be different from the key in the regular map.
+ if (domain != template_url->keyword()) {
+ keyword_domain_to_turl_and_length_.insert(std::make_pair(
+ domain,
+ TURLAndMeaningfulLength(
+ template_url, GetMeaningfulKeywordLength(domain, template_url))));
+ }
+}
+
+void TemplateURLService::AddToMap(TemplateURL* template_url) {
+ const base::string16& keyword = template_url->keyword();
+ keyword_to_turl_and_length_[keyword] =
+ TURLAndMeaningfulLength(
+ template_url, GetMeaningfulKeywordLength(keyword, template_url));
+}
+
// Helper for partition() call in next function.
bool HasValidID(TemplateURL* t_url) {
return t_url->id() != kInvalidTemplateURLID;
@@ -1593,9 +1655,10 @@ bool TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
DCHECK_NE(TemplateURL::OMNIBOX_API_EXTENSION, existing_turl->GetType());
base::string16 old_keyword(existing_turl->keyword());
- keyword_to_template_map_.erase(old_keyword);
+ keyword_to_turl_and_length_.erase(old_keyword);
+ RemoveFromDomainMap(existing_turl);
if (!existing_turl->sync_guid().empty())
- guid_to_template_map_.erase(existing_turl->sync_guid());
+ guid_to_turl_.erase(existing_turl->sync_guid());
// |provider_map_| is only initialized after loading has completed.
if (loaded_)
@@ -1610,10 +1673,11 @@ bool TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
}
const base::string16& keyword = existing_turl->keyword();
- KeywordToTemplateMap::const_iterator i =
- keyword_to_template_map_.find(keyword);
- if (i == keyword_to_template_map_.end()) {
- keyword_to_template_map_[keyword] = existing_turl;
+ KeywordToTURLAndMeaningfulLength::const_iterator i =
+ keyword_to_turl_and_length_.find(keyword);
+ if (i == keyword_to_turl_and_length_.end()) {
+ AddToMap(existing_turl);
+ AddToDomainMap(existing_turl);
} else {
// We can theoretically reach here in two cases:
// * There is an existing extension keyword and sync brings in a rename of
@@ -1623,21 +1687,24 @@ bool TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
// at load time causes it to conflict with an existing keyword. In this
// case we delete the existing keyword if it's replaceable, or else undo
// the change in keyword for |existing_turl|.
- TemplateURL* existing_keyword_turl = i->second;
+ TemplateURL* existing_keyword_turl = i->second.first;
if (existing_keyword_turl->GetType() != TemplateURL::NORMAL) {
- if (!CanReplace(existing_turl))
- keyword_to_template_map_[keyword] = existing_turl;
+ if (!CanReplace(existing_turl)) {
+ AddToMap(existing_turl);
+ AddToDomainMap(existing_turl);
+ }
} else {
if (CanReplace(existing_keyword_turl)) {
RemoveNoNotify(existing_keyword_turl);
} else {
existing_turl->data_.SetKeyword(old_keyword);
- keyword_to_template_map_[old_keyword] = existing_turl;
+ AddToMap(existing_turl);
+ AddToDomainMap(existing_turl);
}
}
}
if (!existing_turl->sync_guid().empty())
- guid_to_template_map_[existing_turl->sync_guid()] = existing_turl;
+ guid_to_turl_[existing_turl->sync_guid()] = existing_turl;
if (web_data_service_.get())
web_data_service_->UpdateKeyword(existing_turl->data());
@@ -1754,18 +1821,18 @@ void TemplateURLService::GoogleBaseURLChanged() {
if (t_url->HasGoogleBaseURLs(search_terms_data())) {
TemplateURL updated_turl(t_url->data());
updated_turl.ResetKeywordIfNecessary(search_terms_data(), false);
- KeywordToTemplateMap::const_iterator existing_entry =
- keyword_to_template_map_.find(updated_turl.keyword());
- if ((existing_entry != keyword_to_template_map_.end()) &&
- (existing_entry->second != t_url)) {
+ KeywordToTURLAndMeaningfulLength::const_iterator existing_entry =
+ keyword_to_turl_and_length_.find(updated_turl.keyword());
+ if ((existing_entry != keyword_to_turl_and_length_.end()) &&
+ (existing_entry->second.first != t_url)) {
// The new autogenerated keyword conflicts with another TemplateURL.
// Overwrite it if it's replaceable; otherwise, leave |t_url| using its
// current keyword. (This will not prevent |t_url| from auto-updating
// the keyword in the future if the conflicting TemplateURL disappears.)
// Note that we must still update |t_url| in this case, or the
// |provider_map_| will not be updated correctly.
- if (CanReplace(existing_entry->second))
- RemoveNoNotify(existing_entry->second);
+ if (CanReplace(existing_entry->second.first))
+ RemoveNoNotify(existing_entry->second.first);
else
updated_turl.data_.SetKeyword(t_url->keyword());
}
@@ -1964,7 +2031,7 @@ bool TemplateURLService::AddNoNotify(TemplateURL* template_url,
web_data_service_->AddKeyword(template_url->data());
// Inform sync of the addition. Note that this will assign a GUID to
- // template_url and add it to the guid_to_template_map_.
+ // template_url and add it to the guid_to_turl_.
ProcessTemplateURLChange(FROM_HERE,
template_url,
syncer::SyncChange::ACTION_ADD);
@@ -2280,6 +2347,35 @@ void TemplateURLService::OnSyncedDefaultSearchProviderGUIDChanged() {
default_search_manager_.SetUserSelectedDefaultSearchEngine(turl->data());
}
+template <typename Container>
+void TemplateURLService::AddMatchingKeywordsHelper(
+ const Container& keyword_to_turl_and_length,
+ const base::string16& prefix,
+ bool supports_replacement_only,
+ TURLsAndMeaningfulLengths* matches) {
+ // Sanity check args.
+ if (prefix.empty())
+ return;
+ DCHECK(matches);
+
+ // Find matching keyword range. Searches the element map for keywords
+ // beginning with |prefix| and stores the endpoints of the resulting set in
+ // |match_range|.
+ const auto match_range(std::equal_range(
+ keyword_to_turl_and_length.begin(), keyword_to_turl_and_length.end(),
+ typename Container::value_type(prefix,
+ TURLAndMeaningfulLength(nullptr, 0)),
+ LessWithPrefix()));
+
+ // Add to vector of matching keywords.
+ for (typename Container::const_iterator i(match_range.first);
+ i != match_range.second; ++i) {
+ if (!supports_replacement_only ||
+ i->second.first->url_ref().SupportsReplacement(search_terms_data()))
+ matches->push_back(i->second);
+ }
+}
+
TemplateURL* TemplateURLService::FindPrepopulatedTemplateURL(
int prepopulated_id) {
for (TemplateURLVector::const_iterator i = template_urls_.begin();
diff --git a/components/search_engines/template_url_service.h b/components/search_engines/template_url_service.h
index f77879c..2bac422 100644
--- a/components/search_engines/template_url_service.h
+++ b/components/search_engines/template_url_service.h
@@ -72,10 +72,18 @@ class TemplateURLService : public WebDataServiceConsumer,
public KeyedService,
public syncer::SyncableService {
public:
- typedef std::map<std::string, std::string> QueryTerms;
- typedef std::vector<TemplateURL*> TemplateURLVector;
- typedef std::map<std::string, syncer::SyncData> SyncDataMap;
- typedef base::CallbackList<void(void)>::Subscription Subscription;
+ using QueryTerms = std::map<std::string, std::string>;
+ using TemplateURLVector = std::vector<TemplateURL*>;
+ using SyncDataMap = std::map<std::string, syncer::SyncData>;
+ using Subscription = base::CallbackList<void(void)>::Subscription;
+
+ // We may want to treat the keyword in a TemplateURL as being a different
+ // length than it actually is. For example, for keywords that end in a
+ // registry, e.g., '.com', we want to consider the registry characters as not
+ // a meaningful part of the keyword and not penalize for the user not typing
+ // those.)
+ using TURLAndMeaningfulLength = std::pair<TemplateURL*, size_t>;
+ using TURLsAndMeaningfulLengths = std::vector<TURLAndMeaningfulLength>;
// Struct used for initializing the data store with fake data.
// Each initializer is mapped to a TemplateURL.
@@ -120,12 +128,23 @@ class TemplateURLService : public WebDataServiceConsumer,
const GURL& url,
TemplateURL** template_url_to_replace);
- // Returns (in |matches|) all TemplateURLs whose keywords begin with |prefix|,
- // sorted shortest keyword-first. If |support_replacement_only| is true, only
+ // Adds to |matches| all TemplateURLs whose keywords begin with |prefix|,
+ // sorted shortest-keyword-first. If |supports_replacement_only| is true, only
// TemplateURLs that support replacement are returned.
- void FindMatchingKeywords(const base::string16& prefix,
- bool support_replacement_only,
- TemplateURLVector* matches);
+ void AddMatchingKeywords(const base::string16& prefix,
+ bool supports_replacement_only,
+ TURLsAndMeaningfulLengths* matches);
+
+ // Adds to |matches| all TemplateURLs for search engines with the domain
+ // name part of the keyword starts with |prefix|, sorted
+ // shortest-domain-name-first. If |supports_replacement_only| is true, only
+ // TemplateURLs that support replacement are returned. Does not bother
+ // searching/returning keywords that would've been found with an identical
+ // call to FindMatchingKeywords(); i.e., doesn't search keywords for which
+ // the domain name is the keyword.
+ void AddMatchingDomainKeywords(const base::string16& prefix,
+ bool supports_replacement_only,
+ TURLsAndMeaningfulLengths* matches);
// Looks up |keyword| and returns the element it maps to. Returns NULL if
// the keyword was not found.
@@ -382,8 +401,22 @@ class TemplateURLService : public WebDataServiceConsumer,
friend class InstantUnitTestBase;
friend class TemplateURLServiceTestUtil;
- typedef std::map<base::string16, TemplateURL*> KeywordToTemplateMap;
- typedef std::map<std::string, TemplateURL*> GUIDToTemplateMap;
+ using GUIDToTURL = std::map<std::string, TemplateURL*>;
+
+ // A mapping from keywords to the corresponding TemplateURLs and their
+ // meaningful keyword lengths. A keyword can appear only once here because
+ // there can be only one active TemplateURL associated with a given keyword.
+ using KeywordToTURLAndMeaningfulLength =
+ std::map<base::string16, TURLAndMeaningfulLength>;
+
+ // A mapping from domain names to corresponding TemplateURLs and their
+ // meaningful keyword lengths. Specifically, for a keyword that is a
+ // hostname containing more than just a domain name, e.g., 'abc.def.com',
+ // the keyword is added to this map under the domain key 'def.com'. This
+ // means multiple keywords from the same domain share the same key, so this
+ // must be a multimap.
+ using KeywordDomainToTURLAndMeaningfulLength =
+ std::multimap<base::string16, TURLAndMeaningfulLength>;
// Declaration of values to be used in an enumerated histogram to tally
// changes to the default search provider from various entry points. In
@@ -420,10 +453,30 @@ class TemplateURLService : public WebDataServiceConsumer,
void Init(const Initializer* initializers, int num_initializers);
+ // Removes |template_url| from various internal maps
+ // (|keyword_to_turl_and_length_|, |keyword_domain_to_turl_and_length_|,
+ // |guid_to_turl_|, |provider_map_|).
void RemoveFromMaps(TemplateURL* template_url);
+ // Adds |template_url| to various internal maps
+ // (|keyword_to_turl_and_length_|, |keyword_domain_to_turl_and_length_|,
+ // |guid_to_turl_|, |provider_map_|) if appropriate. (It might not be
+ // appropriate if, for instance, |template_url|'s keyword conflicts with
+ // the keyword of a custom search engine already existing in the maps that
+ // is not allowed to be replaced.)
void AddToMaps(TemplateURL* template_url);
+ // Helper function for removing an element from
+ // |keyword_domain_to_turl_and_length_|.
+ void RemoveFromDomainMap(const TemplateURL* template_url);
+
+ // Helper fuction for adding an element to
+ // |keyword_domain_to_turl_and_length_| if appropriate.
+ void AddToDomainMap(TemplateURL* template_url);
+
+ // Helper function for adding an element to |keyword_to_turl_and_length_|.
+ void AddToMap(TemplateURL* template_url);
+
// Sets the keywords. This is used once the keywords have been loaded.
// This does NOT notify the delegate or the database.
//
@@ -608,6 +661,17 @@ class TemplateURLService : public WebDataServiceConsumer,
// |template_urls| on exit.
void AddTemplateURLs(TemplateURLVector* template_urls);
+ // Adds to |matches| all TemplateURLs stored in |keyword_to_turl_and_length|
+ // whose keywords begin with |prefix|, sorted shortest-keyword-first. If
+ // |supports_replacement_only| is true, only TemplateURLs that support
+ // replacement are returned.
+ template <typename Container>
+ void AddMatchingKeywordsHelper(
+ const Container& keyword_to_turl_and_length,
+ const base::string16& prefix,
+ bool supports_replacement_only,
+ TURLsAndMeaningfulLengths* matches);
+
// Returns the TemplateURL corresponding to |prepopulated_id|, if any.
TemplateURL* FindPrepopulatedTemplateURL(int prepopulated_id);
@@ -649,10 +713,21 @@ class TemplateURLService : public WebDataServiceConsumer,
PrefChangeRegistrar pref_change_registrar_;
// Mapping from keyword to the TemplateURL.
- KeywordToTemplateMap keyword_to_template_map_;
+ KeywordToTURLAndMeaningfulLength keyword_to_turl_and_length_;
+
+ // Mapping from keyword domain to the TemplateURL.
+ // Entries are only allowed here if there is a corresponding entry in
+ // |keyword_to_turl_and_length_|, i.e., if a template URL doesn't have an
+ // entry in |keyword_to_turl_and_length_| because it's subsumed by another
+ // template URL with an identical keyword, the template URL will not have an
+ // entry in this map either. This map will also not bother including entries
+ // for keywords in which the keyword is the domain name, with no subdomain
+ // before the domain name. (The ordinary |keyword_to_turl_and_length|
+ // suffices for that.)
+ KeywordDomainToTURLAndMeaningfulLength keyword_domain_to_turl_and_length_;
// Mapping from Sync GUIDs to the TemplateURL.
- GUIDToTemplateMap guid_to_template_map_;
+ GUIDToTURL guid_to_turl_;
TemplateURLVector template_urls_;