diff options
author | sky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-01-05 23:40:44 +0000 |
---|---|---|
committer | sky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-01-05 23:40:44 +0000 |
commit | 70833268d3cfae8f2cf6b9cd69cc07c8508ab319 (patch) | |
tree | e15783c3c899bf6fe4335b53f063e561f0864001 | |
parent | 833005a107c6997c106aa545a2257a52dfecadfb (diff) | |
download | chromium_src-70833268d3cfae8f2cf6b9cd69cc07c8508ab319.zip chromium_src-70833268d3cfae8f2cf6b9cd69cc07c8508ab319.tar.gz chromium_src-70833268d3cfae8f2cf6b9cd69cc07c8508ab319.tar.bz2 |
Makes sure the description 'Google Search' is set on the first search
match.
BUG=64375
TEST=see bug
Review URL: http://codereview.chromium.org/6053010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@70551 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/autocomplete/search_provider.cc | 59 | ||||
-rw-r--r-- | chrome/browser/autocomplete/search_provider.h | 3 | ||||
-rw-r--r-- | chrome/browser/autocomplete/search_provider_unittest.cc | 78 |
3 files changed, 124 insertions, 16 deletions
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc index ff49c8a..3f15f38 100644 --- a/chrome/browser/autocomplete/search_provider.cc +++ b/chrome/browser/autocomplete/search_provider.cc @@ -87,6 +87,16 @@ void SearchProvider::FinalizeInstantQuery(const std::wstring& input_text, // destination_url for comparison as it varies depending upon the index passed // to TemplateURL::ReplaceSearchTerms. for (ACMatches::iterator i = matches_.begin(); i != matches_.end();) { + // Reset the description/description_class of all searches. We'll set the + // description of the new first match in the call to + // UpdateFirstSearchMatchDescription() below. + if ((i->type == AutocompleteMatch::SEARCH_HISTORY) || + (i->type == AutocompleteMatch::SEARCH_SUGGEST) || + (i->type == AutocompleteMatch::SEARCH_WHAT_YOU_TYPED)) { + i->description.clear(); + i->description_class.clear(); + } + if (((i->type == AutocompleteMatch::SEARCH_HISTORY) || (i->type == AutocompleteMatch::SEARCH_SUGGEST)) && (i->fill_into_edit == text)) { @@ -109,6 +119,10 @@ void SearchProvider::FinalizeInstantQuery(const std::wstring& input_text, input_.initial_prevent_inline_autocomplete(), &match_map); DCHECK_EQ(1u, match_map.size()); matches_.push_back(match_map.begin()->second); + // Sort the results so that UpdateFirstSearchDescription does the right thing. + std::sort(matches_.begin(), matches_.end(), &AutocompleteMatch::MoreRelevant); + + UpdateFirstSearchMatchDescription(); listener_->OnProviderUpdate(true); } @@ -165,13 +179,8 @@ void SearchProvider::Start(const AutocompleteInput& input, l10n_util::GetStringUTF16(IDS_EMPTY_KEYWORD_VALUE))); match.contents_class.push_back( ACMatchClassification(0, ACMatchClassification::NONE)); - match.description.assign(UTF16ToWideHack(l10n_util::GetStringFUTF16( - IDS_AUTOCOMPLETE_SEARCH_DESCRIPTION, - WideToUTF16Hack( - default_provider->AdjustedShortNameForLocaleDirection())))); - match.description_class.push_back( - ACMatchClassification(0, ACMatchClassification::DIM)); matches_.push_back(match); + UpdateFirstSearchMatchDescription(); } Stop(); return; @@ -549,6 +558,8 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { if (matches_.size() > max_total_matches) matches_.erase(matches_.begin() + max_total_matches, matches_.end()); + UpdateFirstSearchMatchDescription(); + UpdateStarredStateOfMatches(); UpdateDone(); @@ -692,11 +703,11 @@ void SearchProvider::AddMatchToMap(const std::wstring& query_string, std::vector<size_t> content_param_offsets; const TemplateURL& provider = is_keyword ? providers_.keyword_provider() : providers_.default_provider(); + match.contents.assign(query_string); // We do intra-string highlighting for suggestions - the suggested segment // will be highlighted, e.g. for input_text = "you" the suggestion may be // "youtube", so we'll bold the "tube" section: you*tube*. if (input_text != query_string) { - match.contents.assign(query_string); size_t input_position = match.contents.find(input_text); if (input_position == std::wstring::npos) { // The input text is not a substring of the query string, e.g. input @@ -727,15 +738,9 @@ void SearchProvider::AddMatchToMap(const std::wstring& query_string, } } else { // Otherwise, we're dealing with the "default search" result which has no - // completion, but has the search provider name as the description. - match.contents.assign(query_string); + // completion. match.contents_class.push_back( ACMatchClassification(0, ACMatchClassification::NONE)); - match.description.assign(UTF16ToWideHack(l10n_util::GetStringFUTF16( - IDS_AUTOCOMPLETE_SEARCH_DESCRIPTION, - WideToUTF16Hack(provider.AdjustedShortNameForLocaleDirection())))); - match.description_class.push_back( - ACMatchClassification(0, ACMatchClassification::DIM)); } // When the user forced a query, we need to make sure all the fill_into_edit @@ -827,3 +832,29 @@ void SearchProvider::UpdateDone() { done_ = ((suggest_results_pending_ == 0) && (instant_finalized_ || !InstantController::IsEnabled(profile_))); } + +void SearchProvider::UpdateFirstSearchMatchDescription() { + if (!providers_.valid_default_provider() || matches_.empty()) + return; + + for (ACMatches::iterator i = matches_.begin(); i != matches_.end(); ++i) { + AutocompleteMatch& match = *i; + switch (match.type) { + case AutocompleteMatch::SEARCH_WHAT_YOU_TYPED: + case AutocompleteMatch::SEARCH_HISTORY: + case AutocompleteMatch::SEARCH_SUGGEST: + match.description.assign( + UTF16ToWideHack(l10n_util::GetStringFUTF16( + IDS_AUTOCOMPLETE_SEARCH_DESCRIPTION, + WideToUTF16Hack(providers_.default_provider(). + AdjustedShortNameForLocaleDirection())))); + match.description_class.push_back( + ACMatchClassification(0, ACMatchClassification::DIM)); + // Only the first search match gets a description. + return; + + default: + break; + } + } +} diff --git a/chrome/browser/autocomplete/search_provider.h b/chrome/browser/autocomplete/search_provider.h index addd02e..2163336 100644 --- a/chrome/browser/autocomplete/search_provider.h +++ b/chrome/browser/autocomplete/search_provider.h @@ -268,6 +268,9 @@ class SearchProvider : public AutocompleteProvider, // Updates the value of |done_| from the internal state. void UpdateDone(); + // Updates the description/description_class of the first search match. + void UpdateFirstSearchMatchDescription(); + // Should we query for suggest results immediately? This is normally false, // but may be set to true during testing. static bool query_suggest_immediately_; diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index 6c20d55..7120a08 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -10,9 +10,11 @@ #include "chrome/browser/autocomplete/search_provider.h" #include "chrome/browser/browser_thread.h" #include "chrome/browser/history/history.h" +#include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/search_engines/template_url.h" #include "chrome/browser/search_engines/template_url_model.h" #include "chrome/common/net/test_url_fetcher_factory.h" +#include "chrome/common/pref_names.h" #include "chrome/test/testing_profile.h" #include "net/url_request/url_request_status.h" #include "testing/gtest/include/gtest/gtest.h" @@ -61,6 +63,11 @@ class SearchProviderTest : public testing::Test, void QueryForInput(const string16& text, bool prevent_inline_autocomplete); + // Notifies the URLFetcher for the suggest query corresponding to the default + // search provider that it's done. + // Be sure and wrap calls to this in ASSERT_NO_FATAL_FAILURE. + void FinishDefaultSuggestQuery(); + // See description above class for details of these fields. TemplateURL* default_t_url_; const string16 term1_; @@ -194,6 +201,17 @@ AutocompleteMatch SearchProviderTest::FindMatchWithDestination( return AutocompleteMatch(NULL, 1, false, AutocompleteMatch::HISTORY_URL); } +void SearchProviderTest::FinishDefaultSuggestQuery() { + TestURLFetcher* default_fetcher = test_factory_.GetFetcherByID( + SearchProvider::kDefaultProviderURLFetcherID); + ASSERT_TRUE(default_fetcher); + + // Tell the SearchProvider the default suggest query is done. + default_fetcher->delegate()->OnURLFetchComplete( + default_fetcher, GURL(), URLRequestStatus(), 200, ResponseCookies(), + std::string()); +} + // Tests ----------------------------------------------------------------------- // Make sure we query history for the default provider and a URLFetcher is @@ -224,8 +242,20 @@ TEST_F(SearchProviderTest, QueryDefaultProvider) { // The SearchProvider is done. Make sure it has a result for the history // term term1. - AutocompleteMatch match = FindMatchWithDestination(term1_url_); - ASSERT_TRUE(!match.destination_url.is_empty()); + AutocompleteMatch term1_match = FindMatchWithDestination(term1_url_); + EXPECT_TRUE(!term1_match.destination_url.is_empty()); + // Term1 should have a description. + EXPECT_FALSE(term1_match.description.empty()); + + GURL what_you_typed_url = GURL(default_t_url_->url()->ReplaceSearchTerms( + *default_t_url_, UTF16ToWide(term), 0, std::wstring())); + AutocompleteMatch what_you_typed_match = + FindMatchWithDestination(what_you_typed_url); + EXPECT_TRUE(!what_you_typed_match.destination_url.is_empty()); + EXPECT_TRUE(what_you_typed_match.description.empty()); + + // The match for term1 should be more relevant than the what you typed result. + EXPECT_GT(term1_match.relevance, what_you_typed_match.relevance); } TEST_F(SearchProviderTest, HonorPreventInlineAutocomplete) { @@ -314,3 +344,47 @@ TEST_F(SearchProviderTest, DontSendPrivateDataToSuggest) { RunTillProviderDone(); } } + +// Make sure FinalizeInstantQuery works. +TEST_F(SearchProviderTest, FinalizeInstantQuery) { + PrefService* service = profile_.GetPrefs(); + service->SetBoolean(prefs::kInstantEnabled, true); + + QueryForInput(ASCIIToUTF16("foo"), false); + + // Wait until history and the suggest query complete. + profile_.BlockUntilHistoryProcessesPendingRequests(); + ASSERT_NO_FATAL_FAILURE(FinishDefaultSuggestQuery()); + + // When instant is enabled the provider isn't done until it hears from + // instant. + EXPECT_FALSE(provider_->done()); + + // Tell the provider instant is done. + provider_->FinalizeInstantQuery(L"foo", L"bar"); + + // The provider should now be done. + EXPECT_TRUE(provider_->done()); + + // There should be two matches, one for what you typed, the other for + // 'foobar'. + EXPECT_EQ(2u, provider_->matches().size()); + GURL instant_url = GURL(default_t_url_->url()->ReplaceSearchTerms( + *default_t_url_, L"foobar", 0, std::wstring())); + AutocompleteMatch instant_match = FindMatchWithDestination(instant_url); + EXPECT_TRUE(!instant_match.destination_url.is_empty()); + + // And the 'foobar' match should have a description. + EXPECT_FALSE(instant_match.description.empty()); + + // Make sure the what you typed match has no description. + GURL what_you_typed_url = GURL(default_t_url_->url()->ReplaceSearchTerms( + *default_t_url_, L"foo", 0, std::wstring())); + AutocompleteMatch what_you_typed_match = + FindMatchWithDestination(what_you_typed_url); + EXPECT_TRUE(!what_you_typed_match.destination_url.is_empty()); + EXPECT_TRUE(what_you_typed_match.description.empty()); + + // The instant search should be more relevant. + EXPECT_GT(instant_match.relevance, what_you_typed_match.relevance); +} |