summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-01-05 23:40:44 +0000
committersky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-01-05 23:40:44 +0000
commit70833268d3cfae8f2cf6b9cd69cc07c8508ab319 (patch)
treee15783c3c899bf6fe4335b53f063e561f0864001
parent833005a107c6997c106aa545a2257a52dfecadfb (diff)
downloadchromium_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.cc59
-rw-r--r--chrome/browser/autocomplete/search_provider.h3
-rw-r--r--chrome/browser/autocomplete/search_provider_unittest.cc78
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);
+}