diff options
Diffstat (limited to 'chrome/browser/autocomplete')
| -rw-r--r-- | chrome/browser/autocomplete/search_provider.cc | 173 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/search_provider.h | 42 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/search_provider_unittest.cc | 190 |
3 files changed, 5 insertions, 400 deletions
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc index 8727247..eb4f27d 100644 --- a/chrome/browser/autocomplete/search_provider.cc +++ b/chrome/browser/autocomplete/search_provider.cc @@ -7,7 +7,6 @@ #include <algorithm> #include <cmath> -#include "base/auto_reset.h" #include "base/callback.h" #include "base/i18n/break_iterator.h" #include "base/i18n/case_conversion.h" @@ -250,10 +249,8 @@ SearchProvider::SearchProvider(AutocompleteProviderListener* listener, AutocompleteProvider::TYPE_SEARCH), providers_(TemplateURLServiceFactory::GetForProfile(profile)), suggest_results_pending_(0), - instant_finalized_(false), field_trial_triggered_(false), field_trial_triggered_in_session_(false), - suppress_search_suggestions_(false), omnibox_start_margin_(-1) { } @@ -373,98 +370,10 @@ void SearchProvider::ResetSession() { field_trial_triggered_in_session_ = false; } -void SearchProvider::FinalizeInstantQuery(const string16& input_text, - const InstantSuggestion& suggestion) { - if (done_ || instant_finalized_) - return; - - instant_finalized_ = true; - UpdateDone(); - - if (input_text.empty()) { - // We only need to update the listener if we're actually done. - if (done_) - listener_->OnProviderUpdate(false); - return; - } - - default_provider_suggestion_ = suggestion; - - string16 adjusted_input_text(input_text); - AutocompleteInput::RemoveForcedQueryStringIfNecessary(input_.type(), - &adjusted_input_text); - - const string16 text = adjusted_input_text + suggestion.text; - bool results_updated = false; - // Remove any matches that are identical to |text|. We don't use the - // destination_url for comparison as it varies depending upon the index passed - // to TemplateURL::ReplaceSearchTerms. - for (ACMatches::iterator i = matches_.begin(); i != matches_.end();) { - if (((i->type == AutocompleteMatchType::SEARCH_HISTORY) || - (i->type == AutocompleteMatchType::SEARCH_SUGGEST)) && - (i->fill_into_edit == text)) { - i = matches_.erase(i); - results_updated = true; - } else { - ++i; - } - } - - // Add the new Instant suggest result. - if (suggestion.type == INSTANT_SUGGESTION_SEARCH) { - // Instant has a query suggestion. Rank it higher than SEARCH_WHAT_YOU_TYPED - // so that it gets autocompleted. - bool relevance_from_server; - const int verbatim_relevance = GetVerbatimRelevance(&relevance_from_server); - int did_not_accept_default_suggestion = - default_results_.suggest_results.empty() ? - TemplateURLRef::NO_SUGGESTIONS_AVAILABLE : - TemplateURLRef::NO_SUGGESTION_CHOSEN; - MatchMap match_map; - AddMatchToMap(text, adjusted_input_text, verbatim_relevance + 1, - relevance_from_server, AutocompleteMatchType::SEARCH_SUGGEST, - did_not_accept_default_suggestion, false, &match_map); - if (!match_map.empty()) { - matches_.push_back(match_map.begin()->second); - results_updated = true; - } - } else { - // Instant has a URL suggestion. Rank it higher than other providers would - // rank URL_WHAT_YOU_TYPED so it gets autocompleted; use - // kNonURLVerbatimRelevance rather than |verbatim_relevance| so that the - // score does not change if the user keeps typing and the input changes from - // type UNKNOWN to URL. - matches_.push_back(NavigationToMatch(NavigationResult( - *this, GURL(UTF16ToUTF8(suggestion.text)), string16(), false, - kNonURLVerbatimRelevance + 1, false))); - results_updated = true; - } - - if (results_updated || done_) - listener_->OnProviderUpdate(results_updated); -} - -void SearchProvider::ClearInstantSuggestion() { - default_provider_suggestion_ = InstantSuggestion(); - if (done_ || instant_finalized_) - return; - instant_finalized_ = true; - UpdateMatches(); - listener_->OnProviderUpdate(true); -} - -void SearchProvider::SuppressSearchSuggestions() { - suppress_search_suggestions_ = true; -} - void SearchProvider::SetOmniboxStartMargin(int omnibox_start_margin) { omnibox_start_margin_ = omnibox_start_margin; } -bool SearchProvider::IsNonInstantSearchDone() const { - return !timer_.IsRunning() && (suggest_results_pending_ == 0); -} - SearchProvider::~SearchProvider() { } @@ -542,9 +451,6 @@ int SearchProvider::CalculateRelevanceForKeywordVerbatim( void SearchProvider::Start(const AutocompleteInput& input, bool minimal_changes) { - const bool suppress_search_suggestions = suppress_search_suggestions_; - suppress_search_suggestions_ = false; - // Do our best to load the model as early as possible. This will reduce // odds of having the model not ready when really needed (a non-empty input). TemplateURLService* model = providers_.template_url_service(); @@ -554,9 +460,6 @@ void SearchProvider::Start(const AutocompleteInput& input, matches_.clear(); field_trial_triggered_ = false; - instant_finalized_ = - (input.matches_requested() != AutocompleteInput::ALL_MATCHES); - // Can't return search/suggest results for bogus input or without a profile. if (!profile_ || (input.type() == AutocompleteInput::INVALID)) { Stop(false); @@ -593,23 +496,9 @@ void SearchProvider::Start(const AutocompleteInput& input, keyword_provider->keyword() : string16()); if (!minimal_changes || !providers_.equal(default_provider_keyword, keyword_provider_keyword)) { - // If Instant has not come back with a suggestion, adjust the previous - // suggestion if possible. If |instant_finalized| is true, we are looking - // for synchronous matches only, so the suggestion is cleared. - if (instant_finalized_) - default_provider_suggestion_ = InstantSuggestion(); - else - AdjustDefaultProviderSuggestion(input_.text(), input.text()); - // Cancel any in-flight suggest requests. - if (!done_) { - // The Stop(false) call below clears |default_provider_suggestion_|, but - // in this instance we do not want to clear cached results, so we - // restore it. - base::AutoReset<InstantSuggestion> reset(&default_provider_suggestion_, - InstantSuggestion()); + if (!done_) Stop(false); - } } providers_.set(default_provider_keyword, keyword_provider_keyword); @@ -632,17 +521,14 @@ void SearchProvider::Start(const AutocompleteInput& input, input_ = input; - if (!suppress_search_suggestions) { - DoHistoryQuery(minimal_changes); - StartOrStopSuggestQuery(minimal_changes); - } + DoHistoryQuery(minimal_changes); + StartOrStopSuggestQuery(minimal_changes); UpdateMatches(); } void SearchProvider::Stop(bool clear_cached_results) { StopSuggest(); done_ = true; - default_provider_suggestion_ = InstantSuggestion(); if (clear_cached_results) ClearAllResults(); @@ -905,39 +791,6 @@ void SearchProvider::RemoveAllStaleResults() { } } -void SearchProvider::AdjustDefaultProviderSuggestion( - const string16& previous_input, - const string16& current_input) { - if (default_provider_suggestion_.type == INSTANT_SUGGESTION_URL) { - // Description and relevance do not matter in the check for staleness. - NavigationResult result(*this, GURL(default_provider_suggestion_.text), - string16(), false, 100, false); - // If navigation suggestion is stale, clear |default_provider_suggestion_|. - if (!result.IsInlineable(current_input)) - default_provider_suggestion_ = InstantSuggestion(); - } else { - DCHECK(default_provider_suggestion_.type == INSTANT_SUGGESTION_SEARCH); - // InstantSuggestion of type SEARCH contain only the suggested text, and not - // the full text of the query. This looks at the current and previous input - // to determine if the user is typing forward, and if the new input is - // contained in |default_provider_suggestion_|. If so, the suggestion is - // adjusted and can be kept. Otherwise, it is reset. - if (!previous_input.empty() && - StartsWith(current_input, previous_input, false)) { - // User is typing forward; verify if new input is part of the suggestion. - const string16 new_text = string16(current_input, previous_input.size()); - if (StartsWith(default_provider_suggestion_.text, new_text, false)) { - // New input is a prefix to the previous suggestion, adjust the - // suggestion to strip the prefix. - default_provider_suggestion_.text.erase(0, new_text.size()); - return; - } - } - // If we are here, the search suggestion is stale; reset it. - default_provider_suggestion_ = InstantSuggestion(); - } -} - void SearchProvider::ApplyCalculatedRelevance() { ApplyCalculatedSuggestRelevance(&keyword_results_.suggest_results); ApplyCalculatedSuggestRelevance(&default_results_.suggest_results); @@ -1131,14 +984,6 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { } } } - if (!default_provider_suggestion_.text.empty() && - default_provider_suggestion_.type == INSTANT_SUGGESTION_SEARCH && - !input_.prevent_inline_autocomplete()) - AddMatchToMap(input_.text() + default_provider_suggestion_.text, - input_.text(), verbatim_relevance + 1, relevance_from_server, - AutocompleteMatchType::SEARCH_SUGGEST, - did_not_accept_default_suggestion, false, &map); - AddHistoryResultsToMap(keyword_history_results_, true, did_not_accept_keyword_suggestion, &map); AddHistoryResultsToMap(default_history_results_, false, @@ -1151,15 +996,6 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { for (MatchMap::const_iterator i(map.begin()); i != map.end(); ++i) matches.push_back(i->second); - if (!default_provider_suggestion_.text.empty() && - default_provider_suggestion_.type == INSTANT_SUGGESTION_URL && - !input_.prevent_inline_autocomplete()) { - // See comment in FinalizeInstantQuery() for why we don't use - // |verbatim_relevance| here. - matches.push_back(NavigationToMatch(NavigationResult( - *this, GURL(UTF16ToUTF8(default_provider_suggestion_.text)), string16(), - false, kNonURLVerbatimRelevance + 1, false))); - } AddNavigationResultsToMatches(keyword_results_.navigation_results, &matches); AddNavigationResultsToMatches(default_results_.navigation_results, &matches); @@ -1673,6 +1509,5 @@ void SearchProvider::DemoteKeywordNavigationMatchesPastTopQuery() { void SearchProvider::UpdateDone() { // We're done when the timer isn't running, there are no suggest queries // pending, and we're not waiting on Instant. - done_ = IsNonInstantSearchDone() && - (instant_finalized_ || !chrome::IsInstantEnabled(profile_)); + done_ = !timer_.IsRunning() && (suggest_results_pending_ == 0); } diff --git a/chrome/browser/autocomplete/search_provider.h b/chrome/browser/autocomplete/search_provider.h index 2bf17f5..03bf26c 100644 --- a/chrome/browser/autocomplete/search_provider.h +++ b/chrome/browser/autocomplete/search_provider.h @@ -25,7 +25,6 @@ #include "chrome/browser/autocomplete/autocomplete_provider.h" #include "chrome/browser/history/history_types.h" #include "chrome/browser/search_engines/template_url.h" -#include "chrome/common/instant_types.h" #include "net/url_request/url_fetcher_delegate.h" class Profile; @@ -84,34 +83,9 @@ class SearchProvider : public AutocompleteProvider, virtual void AddProviderInfo(ProvidersInfo* provider_info) const OVERRIDE; virtual void ResetSession() OVERRIDE; - // Marks the instant query as done. If |input_text| is non-empty this changes - // the 'search what you typed' results text to |input_text| + - // |suggestion.text|. |input_text| is the text the user input into the edit. - // |input_text| differs from |input_.text()| if the input contained - // whitespace. - // - // This method also marks the search provider as no longer needing to wait for - // the instant result. - void FinalizeInstantQuery(const string16& input_text, - const InstantSuggestion& suggestion); - void ClearInstantSuggestion(); - - // If called, SearchProvider will not fetch any search suggestions for the - // next call to Start(). Used with InstantExtended where Instant fetches its - // own search suggestions. - // - // Note that this only applies to the next call to Start() and so this must be - // called repeatedly before Start() if you wish to continually suppress search - // suggestions. - void SuppressSearchSuggestions(); - // Update the omnibox start margin used to generate search suggestion URLs. void SetOmniboxStartMargin(int omnibox_start_margin); - // Returns whether the provider is done processing the query with the - // exception of waiting for Instant to finish. - bool IsNonInstantSearchDone() const; - bool field_trial_triggered_in_session() const { return field_trial_triggered_in_session_; } @@ -370,12 +344,6 @@ class SearchProvider : public AutocompleteProvider, // on RemoveStaleResults(). void RemoveAllStaleResults(); - // If |default_provider_suggestion_| (which was suggested for - // |previous_input|) is still applicable given the |current_input|, adjusts it - // so it can be reused. Otherwise, clears it. - void AdjustDefaultProviderSuggestion(const string16& previous_input, - const string16& current_input); - // Apply calculated relevance scores to the current results. void ApplyCalculatedRelevance(); void ApplyCalculatedSuggestRelevance(SuggestResults* list); @@ -533,12 +501,6 @@ class SearchProvider : public AutocompleteProvider, Results default_results_; Results keyword_results_; - // Has FinalizeInstantQuery been invoked since the last |Start|? - bool instant_finalized_; - - // The |suggestion| parameter passed to FinalizeInstantQuery. - InstantSuggestion default_provider_suggestion_; - // Whether a field trial, if any, has triggered in the most recent // autocomplete query. This field is set to false in Start() and may be set // to true if either the default provider or keyword provider has completed @@ -551,10 +513,6 @@ class SearchProvider : public AutocompleteProvider, // session. bool field_trial_triggered_in_session_; - // If true, suppress search suggestions. Reset to false in Start(). - // See comments for SuppressSearchSuggestions(). - bool suppress_search_suggestions_; - // Start margin of the omnibox. Used to construct search URLs. int omnibox_start_margin_; diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index c7d9e6c..aa8e4be 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -25,7 +25,6 @@ #include "chrome/browser/search_engines/template_url.h" #include "chrome/browser/search_engines/template_url_service.h" #include "chrome/browser/search_engines/template_url_service_factory.h" -#include "chrome/common/instant_types.h" #include "chrome/common/metrics/entropy_provider.h" #include "chrome/common/pref_names.h" #include "chrome/test/base/testing_browser_process.h" @@ -286,7 +285,6 @@ void SearchProviderTest::QueryForInputAndSetWYTMatch( QueryForInput(text, false, false); profile_.BlockUntilHistoryProcessesPendingRequests(); ASSERT_NO_FATAL_FAILURE(FinishDefaultSuggestQuery()); - EXPECT_NE(chrome::IsInstantExtendedAPIEnabled(), provider_->done()); if (!wyt_match) return; ASSERT_GE(provider_->matches().size(), 1u); @@ -510,190 +508,6 @@ TEST_F(SearchProviderTest, DontSendPrivateDataToSuggest) { } } -// Make sure FinalizeInstantQuery works. -TEST_F(SearchProviderTest, FinalizeInstantQuery) { - chrome::EnableInstantExtendedAPIForTesting(); - - ASSERT_NO_FATAL_FAILURE(QueryForInputAndSetWYTMatch(ASCIIToUTF16("foo"), - NULL)); - - // Tell the provider Instant is done. - provider_->FinalizeInstantQuery(ASCIIToUTF16("foo"), - InstantSuggestion(ASCIIToUTF16("bar"), - INSTANT_COMPLETE_NOW, - INSTANT_SUGGESTION_SEARCH, - string16(), - kNoMatchIndex)); - - // 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(default_t_url_->url_ref().ReplaceSearchTerms( - TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("foobar")))); - AutocompleteMatch instant_match; - EXPECT_TRUE(FindMatchWithDestination(instant_url, &instant_match)); - - // And the 'foobar' match should not have a description, it'll be set later. - EXPECT_TRUE(instant_match.description.empty()); - - // Make sure the what you typed match has no description. - AutocompleteMatch wyt_match; - EXPECT_TRUE(FindMatchWithDestination( - GURL(default_t_url_->url_ref().ReplaceSearchTerms( - TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("foo")))), - &wyt_match)); - EXPECT_TRUE(wyt_match.description.empty()); - - // Instant search suggestions should be ranked above verbatim matches. - EXPECT_GT(instant_match.relevance, wyt_match.relevance); -} - -// Make sure FinalizeInstantQuery works with URL suggestions. -TEST_F(SearchProviderTest, FinalizeInstantURL) { - chrome::EnableInstantExtendedAPIForTesting(); - - ASSERT_NO_FATAL_FAILURE(QueryForInputAndSetWYTMatch(ASCIIToUTF16("ex"), - NULL)); - - // Tell the provider Instant is done. - provider_->FinalizeInstantQuery(ASCIIToUTF16("ex"), - InstantSuggestion( - ASCIIToUTF16("http://example.com/"), - INSTANT_COMPLETE_NOW, - INSTANT_SUGGESTION_URL, - string16(), - kNoMatchIndex)); - - // The provider should now be done. - EXPECT_TRUE(provider_->done()); - - // There should be two matches, one for what you typed, the other for - // "http://example.com/". - EXPECT_EQ(2u, provider_->matches().size()); - GURL instant_url("http://example.com"); - AutocompleteMatch instant_match; - EXPECT_TRUE(FindMatchWithDestination(instant_url, &instant_match)); - - // The Instant match should not have a description, it'll be set later. - EXPECT_TRUE(instant_match.description.empty()); - - // Make sure the what you typed match has no description. - AutocompleteMatch wyt_match; - EXPECT_TRUE(FindMatchWithDestination( - GURL(default_t_url_->url_ref().ReplaceSearchTerms( - TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("ex")))), - &wyt_match)); - EXPECT_TRUE(wyt_match.description.empty()); - - // The Instant URL should be more relevant. - EXPECT_GT(instant_match.relevance, wyt_match.relevance); -} - -// An Instant URL suggestion should behave the same way whether the input text -// is classified as UNKNOWN or as an URL. Otherwise if the user types -// "example.co" url-what-you-typed will displace the Instant suggestion for -// "example.com". -TEST_F(SearchProviderTest, FinalizeInstantURLWithURLText) { - chrome::EnableInstantExtendedAPIForTesting(); - - ASSERT_NO_FATAL_FAILURE(QueryForInputAndSetWYTMatch( - ASCIIToUTF16("example.co"), NULL)); - - // Tell the provider Instant is done. - provider_->FinalizeInstantQuery(ASCIIToUTF16("example.co"), - InstantSuggestion( - ASCIIToUTF16("http://example.com/"), - INSTANT_COMPLETE_NOW, - INSTANT_SUGGESTION_URL, - string16(), - kNoMatchIndex)); - - // The provider should now be done. - EXPECT_TRUE(provider_->done()); - - // There should be two matches, one for what you typed, the other for - // "http://example.com/". - EXPECT_EQ(2u, provider_->matches().size()); - GURL instant_url("http://example.com"); - AutocompleteMatch instant_match; - EXPECT_TRUE(FindMatchWithDestination(instant_url, &instant_match)); - - // The Instant match should not have a description, it'll be set later. - EXPECT_TRUE(instant_match.description.empty()); - - // The Instant URL should be more relevant than a URL_WHAT_YOU_TYPED match. - EXPECT_GT(instant_match.relevance, - HistoryURLProvider::kScoreForWhatYouTypedResult); -} - -// Make sure that if FinalizeInstantQuery is invoked before suggest results -// return, the suggest text from FinalizeInstantQuery is remembered. -TEST_F(SearchProviderTest, RememberInstantQuery) { - chrome::EnableInstantExtendedAPIForTesting(); - - QueryForInput(ASCIIToUTF16("foo"), false, false); - - // Finalize the Instant query immediately. - provider_->FinalizeInstantQuery(ASCIIToUTF16("foo"), - InstantSuggestion(ASCIIToUTF16("bar"), - INSTANT_COMPLETE_NOW, - INSTANT_SUGGESTION_SEARCH, - string16(), - kNoMatchIndex)); - - // There should be two matches, one for what you typed, the other for - // 'foobar'. - EXPECT_EQ(2u, provider_->matches().size()); - GURL instant_url(default_t_url_->url_ref().ReplaceSearchTerms( - TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("foobar")))); - AutocompleteMatch instant_match; - EXPECT_TRUE(FindMatchWithDestination(instant_url, &instant_match)); - - // Wait until history and the suggest query complete. - profile_.BlockUntilHistoryProcessesPendingRequests(); - ASSERT_NO_FATAL_FAILURE(FinishDefaultSuggestQuery()); - - // Provider should 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()); - EXPECT_TRUE(FindMatchWithDestination(instant_url, &instant_match)); - - // And the 'foobar' match should not have a description, it'll be set later. - EXPECT_TRUE(instant_match.description.empty()); -} - -// Make sure that if trailing whitespace is added to the text supplied to -// AutocompleteInput the default suggest text is cleared. -TEST_F(SearchProviderTest, DifferingText) { - chrome::EnableInstantExtendedAPIForTesting(); - - ASSERT_NO_FATAL_FAILURE(QueryForInputAndSetWYTMatch(ASCIIToUTF16("foo"), - NULL)); - - // Finalize the Instant query immediately. - provider_->FinalizeInstantQuery(ASCIIToUTF16("foo"), - InstantSuggestion(ASCIIToUTF16("bar"), - INSTANT_COMPLETE_NOW, - INSTANT_SUGGESTION_SEARCH, - string16(), - kNoMatchIndex)); - - // Query with the same input text, but trailing whitespace. - AutocompleteMatch instant_match; - ASSERT_NO_FATAL_FAILURE(QueryForInputAndSetWYTMatch(ASCIIToUTF16("foo "), - &instant_match)); - - // There should only one match, for what you typed. - EXPECT_EQ(1u, provider_->matches().size()); - EXPECT_FALSE(instant_match.destination_url.is_empty()); -} - TEST_F(SearchProviderTest, DontAutocompleteURLLikeTerms) { AutocompleteClassifierFactory::GetInstance()->SetTestingFactoryAndUse( &profile_, &AutocompleteClassifierFactory::BuildInstanceFor); @@ -1658,8 +1472,7 @@ TEST_F(SearchProviderTest, KeywordFetcherSuggestRelevance) { TEST_F(SearchProviderTest, LocalAndRemoteRelevances) { // Enable Instant Extended in order to allow an increased number of - // suggestions. Unfortunately this requires us to call FinalizeInstantQuery() - // every time. + // suggestions. chrome::EnableInstantExtendedAPIForTesting(); // We hardcode the string "term1" below, so ensure that the search term that @@ -1737,7 +1550,6 @@ TEST_F(SearchProviderTest, LocalAndRemoteRelevances) { for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) { QueryForInput(cases[i].input, false, false); - provider_->FinalizeInstantQuery(string16(), InstantSuggestion()); net::TestURLFetcher* fetcher = WaitUntilURLFetcherIsReady( SearchProvider::kDefaultProviderURLFetcherID); ASSERT_TRUE(fetcher); |