diff options
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete.cc | 69 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete.h | 42 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_edit.cc | 112 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_edit.h | 69 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_popup.cc | 154 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_popup.h | 31 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_unittest.cc | 83 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/history_contents_provider_unittest.cc | 8 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/history_url_provider_unittest.cc | 3 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/keyword_provider.cc | 10 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/keyword_provider_unittest.cc | 3 |
11 files changed, 222 insertions, 362 deletions
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc index 27a3e7b..e6ab849 100644 --- a/chrome/browser/autocomplete/autocomplete.cc +++ b/chrome/browser/autocomplete/autocomplete.cc @@ -31,9 +31,11 @@ AutocompleteInput::AutocompleteInput(const std::wstring& text, const std::wstring& desired_tld, - bool prevent_inline_autocomplete) + bool prevent_inline_autocomplete, + bool prefer_keyword) : desired_tld_(desired_tld), - prevent_inline_autocomplete_(prevent_inline_autocomplete) { + prevent_inline_autocomplete_(prevent_inline_autocomplete), + prefer_keyword_(prefer_keyword) { // Trim whitespace from edges of input; don't inline autocomplete if there // was trailing whitespace. if (TrimWhitespace(text, TRIM_ALL, &text_) & TRIM_TRAILING) @@ -213,7 +215,8 @@ bool AutocompleteInput::Equals(const AutocompleteInput& other) const { (type_ == other.type_) && (desired_tld_ == other.desired_tld_) && (scheme_ == other.scheme_) && - (prevent_inline_autocomplete_ == other.prevent_inline_autocomplete_); + (prevent_inline_autocomplete_ == other.prevent_inline_autocomplete_) && + (prefer_keyword_ == other.prefer_keyword_); } void AutocompleteInput::Clear() { @@ -223,6 +226,7 @@ void AutocompleteInput::Clear() { scheme_.clear(); desired_tld_.clear(); prevent_inline_autocomplete_ = false; + prefer_keyword_ = false; } // AutocompleteMatch ---------------------------------------------------------- @@ -436,19 +440,23 @@ void AutocompleteResult::CopyFrom(const AutocompleteResult& rhs) { } void AutocompleteResult::AppendMatches(const ACMatches& matches) { - default_match_ = end(); std::copy(matches.begin(), matches.end(), std::back_inserter(matches_)); + default_match_ = end(); } void AutocompleteResult::AddMatch(const AutocompleteMatch& match) { - default_match_ = end(); - matches_.insert(std::upper_bound(matches_.begin(), matches_.end(), match, - &AutocompleteMatch::MoreRelevant), match); + DCHECK(default_match_ != end()); + ACMatches::iterator insertion_point = + std::upper_bound(begin(), end(), match, &AutocompleteMatch::MoreRelevant); + ACMatches::iterator::difference_type default_offset = + default_match_ - begin(); + if ((insertion_point - begin()) <= default_offset) + ++default_offset; + matches_.insert(insertion_point, match); + default_match_ = begin() + default_offset; } void AutocompleteResult::SortAndCull() { - default_match_ = end(); - // Remove duplicates. std::sort(matches_.begin(), matches_.end(), &AutocompleteMatch::DestinationSortFunc); @@ -475,45 +483,7 @@ void AutocompleteResult::SortAndCull() { // Now put the final result set in order. std::sort(matches_.begin(), matches_.end(), &AutocompleteMatch::MoreRelevant); -} - -bool AutocompleteResult::SetDefaultMatch(const Selection& selection) { - default_match_ = end(); - - // Look for the best match. - for (const_iterator i(begin()); i != end(); ++i) { - // If we have an exact match, return immediately. - if (selection.is_history_what_you_typed_match ? - i->is_history_what_you_typed_match : - (!selection.destination_url.empty() && - (i->destination_url == selection.destination_url))) { - default_match_ = i; - return true; - } - - // Otherwise, see if this match is closer to the desired selection than the - // existing one. - if (default_match_ == end()) { - // No match at all yet, pick the first one we see. - default_match_ = i; - } else if (selection.provider_affinity == NULL) { - // No provider desired, choose solely based on relevance. - if (AutocompleteMatch::MoreRelevant(*i, *default_match_)) - default_match_ = i; - } else { - // Desired provider trumps any undesired provider; otherwise choose based - // on relevance. - const bool providers_match = - (i->provider == selection.provider_affinity); - const bool default_provider_doesnt_match = - (default_match_->provider != selection.provider_affinity); - if ((providers_match && default_provider_doesnt_match) || - ((providers_match || default_provider_doesnt_match) && - AutocompleteMatch::MoreRelevant(*i, *default_match_))) - default_match_ = i; - } - } - return false; + default_match_ = begin(); } std::wstring AutocompleteResult::GetAlternateNavURL( @@ -548,8 +518,7 @@ AutocompleteController::AutocompleteController(ACControllerListener* listener, : listener_(listener) { providers_.push_back(new SearchProvider(this, profile)); providers_.push_back(new HistoryURLProvider(this, profile)); - keyword_provider_ = new KeywordProvider(this, profile); - providers_.push_back(keyword_provider_); + providers_.push_back(new KeywordProvider(this, profile)); if (listener) { // These providers are async-only, so there's no need to create them when // we'll only be doing synchronous queries. diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h index 35fb790..cbf4faf 100644 --- a/chrome/browser/autocomplete/autocomplete.h +++ b/chrome/browser/autocomplete/autocomplete.h @@ -42,7 +42,7 @@ // // UNKNOWN input type: // --------------------------------------------------------------------|----- -// Keyword (non-substituting, exact match) | 1500 +// Keyword (non-substituting or in keyword UI mode, exact match) | 1500 // HistoryURL (exact or inline autocomplete match) | 1400 // Search (what you typed) | 1300 // HistoryURL (what you typed) | 1200 @@ -59,7 +59,7 @@ // // REQUESTED_URL input type: // --------------------------------------------------------------------|----- -// Keyword (non-substituting, exact match) | 1500 +// Keyword (non-substituting or in keyword UI mode, exact match) | 1500 // HistoryURL (exact or inline autocomplete match) | 1400 // HistoryURL (what you typed) | 1300 // Search (what you typed) | 1200 @@ -76,7 +76,7 @@ // // URL input type: // --------------------------------------------------------------------|----- -// Keyword (non-substituting, exact match) | 1500 +// Keyword (non-substituting or in keyword UI mode, exact match) | 1500 // HistoryURL (exact or inline autocomplete match) | 1400 // HistoryURL (what you typed) | 1200 // Keyword (substituting, exact match) | 1100 @@ -89,7 +89,7 @@ // // QUERY input type: // --------------------------------------------------------------------|----- -// Keyword (non-substituting, exact match) | 1500 +// Keyword (non-substituting or in keyword UI mode, exact match) | 1500 // Keyword (substituting, exact match) | 1400 // Search (what you typed) | 1300 // Search (past query in history) | 1250-- @@ -149,10 +149,16 @@ class AutocompleteInput { FORCED_QUERY, // Input forced to be a query by an initial '?' }; - AutocompleteInput() : type_(INVALID), prevent_inline_autocomplete_(false) {} + AutocompleteInput() + : type_(INVALID), + prevent_inline_autocomplete_(false), + prefer_keyword_(false) { + } + AutocompleteInput(const std::wstring& text, const std::wstring& desired_tld, - bool prevent_inline_autocomplete); + bool prevent_inline_autocomplete, + bool prefer_keyword); // Parses |text| and returns the type of input this will be interpreted as. // The components of the input are stored in the output parameter |parts|. @@ -187,6 +193,10 @@ class AutocompleteInput { return prevent_inline_autocomplete_; } + // Returns whether, given an input string consisting solely of a substituting + // keyword, we should score it like a non-substituting keyword. + const bool prefer_keyword() const { return prefer_keyword_; } + // operator==() by another name. bool Equals(const AutocompleteInput& other) const; @@ -203,6 +213,7 @@ class AutocompleteInput { std::wstring scheme_; std::wstring desired_tld_; bool prevent_inline_autocomplete_; + bool prefer_keyword_; }; // AutocompleteMatch ---------------------------------------------------------- @@ -571,14 +582,14 @@ class AutocompleteResult { // Adds a single match. The match is inserted at the appropriate position // based on relevancy and display order. This is ONLY for use after - // SortAndCull has been invoked. + // SortAndCull() has been invoked, and preserves default_match_. void AddMatch(const AutocompleteMatch& match); - // Adds a new set of matches to the set of results. + // Adds a new set of matches to the set of results. Does not re-sort. void AppendMatches(const ACMatches& matches); // Removes duplicates, puts the list in sorted order and culls to leave only - // the best kMaxMatches results. + // the best kMaxMatches results. Sets the default match to the best match. void SortAndCull(); // Vector-style accessors/operators. @@ -599,14 +610,6 @@ class AutocompleteResult { // end() if there is no default match. const_iterator default_match() const { return default_match_; } - // Sets the default match to the best result. When a particular URL is - // desired, we return that if available; otherwise, if a provider affinity is - // specified, we pick the most relevant match from that provider; otherwise, - // we return the best match overall. - // Returns true if the selection specified an exact match and we were able to - // find and use it. - bool SetDefaultMatch(const Selection& selection); - // Given some input and a particular match in this result set, returns the // "alternate navigation URL", if any, for that match. This is a URL to try // offering as a navigational option in case the user didn't actually mean to @@ -679,7 +682,6 @@ class AutocompleteController : public ACProviderListener { const ACProviders& providers) : listener_(listener), providers_(providers), - keyword_provider_(NULL), history_contents_provider_(NULL) { } #endif @@ -721,8 +723,6 @@ class AutocompleteController : public ACProviderListener { // From AutocompleteProvider::Listener virtual void OnProviderUpdate(bool updated_matches); - KeywordProvider* keyword_provider() const { return keyword_provider_; } - private: // Returns true if all providers have finished processing the query. bool QueryComplete() const; @@ -744,8 +744,6 @@ class AutocompleteController : public ACProviderListener { // A list of all providers. ACProviders providers_; - KeywordProvider* keyword_provider_; - HistoryContentsProvider* history_contents_provider_; // Input passed to Start. diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc index a48d4e8..5a36169 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.cc +++ b/chrome/browser/autocomplete/autocomplete_edit.cc @@ -65,7 +65,7 @@ AutocompleteEditModel::AutocompleteEditModel( paste_state_(NONE), control_key_state_(UP), is_keyword_hint_(false), - disable_keyword_ui_(false), + keyword_ui_state_(NORMAL), show_search_hint_(true), profile_(profile) { if (++paste_and_go_controller_refcount == 1) { @@ -95,9 +95,8 @@ const AutocompleteEditModel::State if (user_input_in_progress_) InternalSetUserText(UserTextFromDisplayText(view_->GetText())); - return State(user_input_in_progress_, user_text_, - popup_->manually_selected_match_, keyword_, is_keyword_hint_, - disable_keyword_ui_, show_search_hint_); + return State(user_input_in_progress_, user_text_, keyword_, is_keyword_hint_, + keyword_ui_state_, show_search_hint_); } void AutocompleteEditModel::RestoreState(const State& state) { @@ -107,11 +106,10 @@ void AutocompleteEditModel::RestoreState(const State& state) { // DisplayTextFromUserText(), as its result depends upon this state. keyword_ = state.keyword; is_keyword_hint_ = state.is_keyword_hint; - disable_keyword_ui_ = state.disable_keyword_ui; + keyword_ui_state_ = state.keyword_ui_state; show_search_hint_ = state.show_search_hint; view_->SetUserText(state.user_text, DisplayTextFromUserText(state.user_text), false); - popup_->manually_selected_match_ = state.manually_selected_match; } } @@ -134,7 +132,6 @@ void AutocompleteEditModel::SetUserText(const std::wstring& text) { InternalSetUserText(text); paste_state_ = NONE; has_temporary_text_ = false; - popup_->manually_selected_match_.Clear(); } void AutocompleteEditModel::GetDataForURLExport(GURL* url, @@ -192,7 +189,7 @@ void AutocompleteEditModel::Revert() { InternalSetUserText(std::wstring()); keyword_.clear(); is_keyword_hint_ = false; - disable_keyword_ui_ = false; + keyword_ui_state_ = NORMAL; show_search_hint_ = permanent_text_.empty(); has_temporary_text_ = false; view_->SetWindowTextAndCaretPos(permanent_text_, @@ -203,7 +200,7 @@ void AutocompleteEditModel::StartAutocomplete( bool prevent_inline_autocomplete) const { popup_->StartAutocomplete(user_text_, GetDesiredTLD(), prevent_inline_autocomplete || just_deleted_text_ || - (paste_state_ != NONE)); + (paste_state_ != NONE), keyword_ui_state_ == KEYWORD); } bool AutocompleteEditModel::CanPasteAndGo(const std::wstring& text) const { @@ -213,7 +210,7 @@ bool AutocompleteEditModel::CanPasteAndGo(const std::wstring& text) const { paste_and_go_alternate_nav_url_.clear(); // See if the clipboard text can be parsed. - const AutocompleteInput input(text, std::wstring(), true); + const AutocompleteInput input(text, std::wstring(), true, false); if (input.type() == AutocompleteInput::INVALID) return false; @@ -231,7 +228,6 @@ bool AutocompleteEditModel::CanPasteAndGo(const std::wstring& text) const { return false; // Set local state based on the default action for this input. - result.SetDefaultMatch(AutocompleteResult::Selection()); const AutocompleteResult::const_iterator match(result.default_match()); DCHECK(match != result.end()); paste_and_go_url_ = match->destination_url; @@ -316,11 +312,8 @@ void AutocompleteEditModel::SendOpenNotification(size_t selected_line, void AutocompleteEditModel::AcceptKeyword() { view_->OnBeforePossibleChange(); view_->SetWindowText(L""); - popup_->manually_selected_match_.Clear(); - popup_->manually_selected_match_.provider_affinity = - popup_->autocomplete_controller()->keyword_provider(); is_keyword_hint_ = false; - disable_keyword_ui_ = false; + keyword_ui_state_ = KEYWORD; view_->OnAfterPossibleChange(); just_deleted_text_ = false; // OnAfterPossibleChange() erroneously sets this // since the edit contents have disappeared. It @@ -333,8 +326,8 @@ void AutocompleteEditModel::ClearKeyword(const std::wstring& visible_text) { view_->OnBeforePossibleChange(); const std::wstring window_text(keyword_ + visible_text); view_->SetWindowTextAndCaretPos(window_text.c_str(), keyword_.length()); - popup_->manually_selected_match_.Clear(); keyword_.clear(); + keyword_ui_state_ = NORMAL; view_->OnAfterPossibleChange(); just_deleted_text_ = true; // OnAfterPossibleChange() fails to clear this // since the edit contents have actually grown @@ -386,8 +379,9 @@ bool AutocompleteEditModel::OnEscapeKeyPressed() { // NOTE: This purposefully does not reset paste_state_. just_deleted_text_ = false; has_temporary_text_ = false; - popup_->manually_selected_match_ = original_selected_match_; - view_->OnRevertTemporaryText(user_text_ + inline_autocomplete_text_); + keyword_ui_state_ = original_keyword_ui_state_; + popup_->ResetToDefaultMatch(); + view_->OnRevertTemporaryText(); return true; } @@ -406,7 +400,7 @@ void AutocompleteEditModel::OnControlKeyChanged(bool pressed) { void AutocompleteEditModel::OnUpOrDownKeyPressed(int count) { // NOTE: This purposefully don't trigger any code that resets paste_state_. - disable_keyword_ui_ = false; + if (!popup_->is_open()) { if (!popup_->query_in_progress()) { // The popup is neither open nor working on a query already. So, start an @@ -419,34 +413,27 @@ void AutocompleteEditModel::OnUpOrDownKeyPressed(int count) { InternalSetUserText(permanent_text_); view_->UpdatePopup(); } - - // Now go ahead and force the popup to open, and copy the text of the - // default item into the edit. We ignore |count|, since without the popup - // open, the user doesn't really know what they're interacting with. Since - // the user hit an arrow key to explicitly open the popup, we assume that - // they prefer the temporary text of the default item to their own text, - // like we do when they arrow around an already-open popup. In many cases - // the existing text in the edit and the new text will be the same, and the - // only visible effect will be to cancel any selection and place the cursor - // at the end of the edit. - popup_->Move(0); } else { // The popup is open, so the user should be able to interact with it // normally. popup_->Move(count); } + + // NOTE: We need to reset the keyword_ui_state_ after the popup updates, since + // Move() will eventually call back to OnPopupDataChanged(), which needs to + // save off the current keyword_ui_state_. + keyword_ui_state_ = NORMAL; } void AutocompleteEditModel::OnPopupDataChanged( const std::wstring& text, bool is_temporary_text, - const AutocompleteResult::Selection& previous_selected_match, const std::wstring& keyword, bool is_keyword_hint, bool can_show_search_hint) { // We don't want to show the search hint if we're showing a keyword hint or // selected keyword, or (subtle!) if we would be showing a selected keyword - // but for disable_keyword_ui_. + // but for keyword_ui_state_ == NO_KEYWORD. can_show_search_hint &= keyword.empty(); // Update keyword/hint-related local state. @@ -466,7 +453,7 @@ void AutocompleteEditModel::OnPopupDataChanged( // Save the original selection and URL so it can be reverted later. has_temporary_text_ = true; original_url_ = popup_->URLsForCurrentSelection(NULL, NULL, NULL); - original_selected_match_ = previous_selected_match; + original_keyword_ui_state_ = keyword_ui_state_; } view_->OnTemporaryTextMaybeChanged(DisplayTextFromUserText(text), save_original_selection); @@ -494,7 +481,6 @@ void AutocompleteEditModel::OnPopupDataChanged( bool AutocompleteEditModel::OnAfterPossibleChange(const std::wstring& new_text, bool selection_differs, - bool select_all_before_change, bool text_differs, bool just_deleted_text, bool at_end_of_edit) { @@ -515,51 +501,37 @@ bool AutocompleteEditModel::OnAfterPossibleChange(const std::wstring& new_text, if (!text_differs && !popup_->is_open()) return false; // Don't open the popup for no reason. } else if (!text_differs && - (inline_autocomplete_text_.empty() || !selection_differs)) { + (inline_autocomplete_text_.empty() || !selection_differs)) { return false; } - const bool had_keyword = !is_keyword_hint_ && !keyword_.empty(); + const bool had_keyword = (keyword_ui_state_ != NO_KEYWORD) && + !is_keyword_hint_ && !keyword_.empty(); // Modifying the selection counts as accepting the autocompleted text. InternalSetUserText(UserTextFromDisplayText(new_text)); has_temporary_text_ = false; - if (text_differs) { - // When the user has deleted text, don't allow inline autocomplete. - just_deleted_text_ = just_deleted_text; - - // When the user doesn't have a selected keyword, deleting text or replacing - // all of it with something else should reset the provider affinity. The - // typical use case for deleting is that the user starts typing, sees that - // some entry is close to what he wants, arrows to it, and then deletes some - // unnecessary bit from the end of the string. In this case the user didn't - // actually want "provider X", he wanted the string from that entry for - // editing purposes, and he's no longer looking at the popup to notice that, - // despite deleting some text, the action we'll take on enter hasn't changed - // at all. - if (!had_keyword && (just_deleted_text_ || select_all_before_change)) { - popup_->manually_selected_match_.Clear(); - } - } + // Track when the user has deleted text so we won't allow inline autocomplete. + just_deleted_text_ = just_deleted_text; // Disable the fancy keyword UI if the user didn't already have a visible // keyword and is not at the end of the edit. This prevents us from showing // the fancy UI (and interrupting the user's editing) if the user happens to // have a keyword for 'a', types 'ab' then puts a space between the 'a' and // the 'b'. - disable_keyword_ui_ = (is_keyword_hint_ || keyword_.empty()) && - !at_end_of_edit; + if (!had_keyword) + keyword_ui_state_ = at_end_of_edit ? NORMAL : NO_KEYWORD; view_->UpdatePopup(); - if (!had_keyword && !is_keyword_hint_ && !keyword_.empty()) { - // Went from no selected keyword to a selected keyword. Set the affinity to - // the keyword provider. This forces the selected keyword to persist even - // if the user deletes all the text. - popup_->manually_selected_match_.Clear(); - popup_->manually_selected_match_.provider_affinity = - popup_->autocomplete_controller()->keyword_provider(); + if (had_keyword) { + if (is_keyword_hint_ || keyword_.empty()) + keyword_ui_state_ = NORMAL; + } else if ((keyword_ui_state_ != NO_KEYWORD) && !is_keyword_hint_ && + !keyword_.empty()) { + // Went from no selected keyword to a selected keyword. + keyword_ui_state_ = KEYWORD; } return true; @@ -573,13 +545,15 @@ void AutocompleteEditModel::InternalSetUserText(const std::wstring& text) { std::wstring AutocompleteEditModel::DisplayTextFromUserText( const std::wstring& text) const { - return (is_keyword_hint_ || keyword_.empty()) ? + return ((keyword_ui_state_ == NO_KEYWORD) || is_keyword_hint_ || + keyword_.empty()) ? text : KeywordProvider::SplitReplacementStringFromInput(text); } std::wstring AutocompleteEditModel::UserTextFromDisplayText( const std::wstring& text) const { - return (is_keyword_hint_ || keyword_.empty()) ? + return ((keyword_ui_state_ == NO_KEYWORD) || is_keyword_hint_ || + keyword_.empty()) ? text : (keyword_ + L" " + text); } @@ -1080,11 +1054,8 @@ bool AutocompleteEditView::OnInlineAutocompleteTextMaybeChanged( return true; } -void AutocompleteEditView::OnRevertTemporaryText(const std::wstring& text) { - ScopedFreeze freeze(this, GetTextObjectModel()); - SetWindowText(text.c_str()); +void AutocompleteEditView::OnRevertTemporaryText() { SetSelectionRange(original_selection_); - UpdatePopup(); TextChanged(); } @@ -1092,7 +1063,6 @@ void AutocompleteEditView::OnBeforePossibleChange() { // Record our state. text_before_change_ = GetText(); GetSelection(sel_before_change_); - select_all_before_change_ = IsSelectAll(sel_before_change_); } bool AutocompleteEditView::OnAfterPossibleChange() { @@ -1130,8 +1100,7 @@ bool AutocompleteEditView::OnAfterPossibleChange() { const bool something_changed = model_->OnAfterPossibleChange(new_text, - selection_differs, select_all_before_change_, text_differs, - just_deleted_text, at_end_of_edit); + selection_differs, text_differs, just_deleted_text, at_end_of_edit); if (something_changed && text_differs) TextChanged(); @@ -1150,6 +1119,7 @@ bool AutocompleteEditView::OverrideAccelerator( if ((accelerator.GetKeyCode() != VK_ESCAPE) || accelerator.IsAltDown()) return false; + ScopedFreeze freeze(this, GetTextObjectModel()); return model_->OnEscapeKeyPressed(); } diff --git a/chrome/browser/autocomplete/autocomplete_edit.h b/chrome/browser/autocomplete/autocomplete_edit.h index 832297d..bdde105b 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.h +++ b/chrome/browser/autocomplete/autocomplete_edit.h @@ -70,29 +70,37 @@ class AutocompleteEditController { class AutocompleteEditModel { public: + enum KeywordUIState { + NORMAL, // The user is typing normally. + NO_KEYWORD, // The user is editing in the middle of the input string. Even + // if the input looks like a keyword, don't display the keyword + // UI, so as not to interfere with the user's editing. + KEYWORD, // The user has triggered the keyword UI. Until it disappears, + // bias autocomplete results so that input strings of the + // keyword alone default to the keyword provider, not a normal + // navigation or search. + }; + struct State { State(bool user_input_in_progress, const std::wstring& user_text, - const AutocompleteResult::Selection& manually_selected_match, const std::wstring& keyword, bool is_keyword_hint, - bool disable_keyword_ui, + KeywordUIState keyword_ui_state, bool show_search_hint) : user_input_in_progress(user_input_in_progress), user_text(user_text), - manually_selected_match(manually_selected_match), keyword(keyword), is_keyword_hint(is_keyword_hint), - disable_keyword_ui(disable_keyword_ui), + keyword_ui_state(keyword_ui_state), show_search_hint(show_search_hint) { } bool user_input_in_progress; const std::wstring user_text; - AutocompleteResult::Selection manually_selected_match; const std::wstring keyword; const bool is_keyword_hint; - const bool disable_keyword_ui; + const KeywordUIState keyword_ui_state; const bool show_search_hint; }; @@ -197,8 +205,7 @@ class AutocompleteEditModel { // Accessors for keyword-related state (see comments on keyword_ and // is_keyword_hint_). std::wstring keyword() const { - return ((is_keyword_hint_ && has_focus_) || - (!is_keyword_hint_ && !disable_keyword_ui_)) ? + return (is_keyword_hint_ ? has_focus_ : (keyword_ui_state_ != NO_KEYWORD)) ? keyword_ : std::wstring(); } bool is_keyword_hint() const { return is_keyword_hint_; } @@ -266,7 +273,6 @@ class AutocompleteEditModel { void OnPopupDataChanged( const std::wstring& text, bool is_temporary_text, - const AutocompleteResult::Selection& previous_selected_match, const std::wstring& keyword, bool is_keyword_hint, bool can_show_search_hint); @@ -276,12 +282,23 @@ class AutocompleteEditModel { // popup if necessary, and returns true if any significant changes occurred. bool OnAfterPossibleChange(const std::wstring& new_text, bool selection_differs, - bool select_all_before_change, bool text_differs, bool just_deleted_text, bool at_end_of_edit); private: + enum PasteState { + NONE, // Most recent edit was not a paste that replaced all text. + REPLACED_ALL, // Most recent edit was a paste that replaced all text. + REPLACING_ALL, // In the middle of doing a paste that replaces all + // text. We need this intermediate state because OnPaste() + // does the actual detection of such pastes, but + // OnAfterPossibleChange() has to update the paste state + // for every edit. If OnPaste() set the state directly to + // REPLACED_ALL, OnAfterPossibleChange() wouldn't know + // whether that represented the current edit or a past one. + }; + enum ControlKeyState { UP, // The control key is not depressed. DOWN_WITHOUT_CHANGE, // The control key is depressed, and the edit's @@ -296,18 +313,6 @@ class AutocompleteEditModel { // he intended to hit "ctrl-enter". }; - enum PasteState { - NONE, // Most recent edit was not a paste that replaced all text. - REPLACED_ALL, // Most recent edit was a paste that replaced all text. - REPLACING_ALL, // In the middle of doing a paste that replaces all - // text. We need this intermediate state because OnPaste() - // does the actual detection of such pastes, but - // OnAfterPossibleChange() has to update the paste state - // for every edit. If OnPaste() set the state directly to - // REPLACED_ALL, OnAfterPossibleChange() wouldn't know - // whether that represented the current edit or a past one. - }; - // Called whenever user_text_ should change. void InternalSetUserText(const std::wstring& text); @@ -392,14 +397,9 @@ class AutocompleteEditModel { // the unique identifier of the originally selected item. Thus, if the user // arrows to a different item with the same text, we can still distinguish // them and not revert all the way to the permanent_text_. - // - // original_selected_match_, which is valid in the same cases, is the manually - // selected match to revert the popup to, if any. This can be non-empty when - // the user has selected a keyword (by hitting <tab> when applicable), or when - // the user has manually selected a match and then continued to edit it. bool has_temporary_text_; std::wstring original_url_; - AutocompleteResult::Selection original_selected_match_; + KeywordUIState original_keyword_ui_state_; // When the user's last action was to paste and replace all the text, we // disallow inline autocomplete (on the theory that the user is trying to @@ -423,10 +423,8 @@ class AutocompleteEditModel { // keyword_ to show a "Press <tab> to search" sort of hint. bool is_keyword_hint_; - // In some cases, such as when the user is editing in the middle of the input - // string, the input might look like a keyword, but we don't want to display - // the keyword UI, so as not to interfere with the user's editing. - bool disable_keyword_ui_; + // See KeywordUIState enum. + KeywordUIState keyword_ui_state_; // True when it's safe to show a "Type to search" hint to the user (when the // edit is empty, or the user is in the process of searching). @@ -567,9 +565,9 @@ class AutocompleteEditView bool OnInlineAutocompleteTextMaybeChanged(const std::wstring& display_text, size_t user_text_length); - // Called when the temporary text has been reverted by the user. |text| is - // the text that should now be displayed. - void OnRevertTemporaryText(const std::wstring& text); + // Called when the temporary text has been reverted by the user. This will + // reset the user's original selection. + void OnRevertTemporaryText(); // Every piece of code that can change the edit should call these functions // before and after the change. These functions determine if anything @@ -851,7 +849,6 @@ class AutocompleteEditView // Variables for tracking state before and after a possible change. std::wstring text_before_change_; CHARRANGE sel_before_change_; - bool select_all_before_change_; // Set at the same time the model's original_* members are set, and valid in // the same cases. diff --git a/chrome/browser/autocomplete/autocomplete_popup.cc b/chrome/browser/autocomplete/autocomplete_popup.cc index c7cb50e..9886339 100644 --- a/chrome/browser/autocomplete/autocomplete_popup.cc +++ b/chrome/browser/autocomplete/autocomplete_popup.cc @@ -244,7 +244,7 @@ void AutocompletePopupView::OnHoverEnabledOrDisabled(bool disabled) { void AutocompletePopupView::OnLButtonDown(UINT keys, const CPoint& point) { const size_t new_hovered_line = PixelToLine(point.y); model_->SetHoveredLine(new_hovered_line); - model_->SetSelectedLine(new_hovered_line); + model_->SetSelectedLine(new_hovered_line, false); } void AutocompletePopupView::OnMButtonDown(UINT keys, const CPoint& point) { @@ -291,7 +291,7 @@ void AutocompletePopupView::OnMouseMove(UINT keys, const CPoint& point) { // When the user has the left button down, update their selection // immediately (don't wait for mouseup). if (keys & MK_LBUTTON) - model_->SetSelectedLine(new_hovered_line); + model_->SetSelectedLine(new_hovered_line, false); } } @@ -750,35 +750,22 @@ void AutocompletePopupModel::SetProfile(Profile* profile) { void AutocompletePopupModel::StartAutocomplete( const std::wstring& text, const std::wstring& desired_tld, - bool prevent_inline_autocomplete) { + bool prevent_inline_autocomplete, + bool prefer_keyword) { // The user is interacting with the edit, so stop tracking hover. SetHoveredLine(kNoMatch); // See if we can avoid rerunning autocomplete when the query hasn't changed - // much. If the popup isn't open, we threw the past results away, so no - // shortcuts are possible. - const AutocompleteInput input(text, desired_tld, prevent_inline_autocomplete); - bool minimal_changes = false; - if (is_open()) { - // When the user hits escape with a temporary selection, the edit asks us - // to update, but the text it supplies hasn't changed since the last query. - // Instead of stopping or rerunning the last query, just do an immediate - // repaint with the new (probably NULL) provider affinity. - if (input_.Equals(input)) { - SetDefaultMatchAndUpdate(true); - return; - } - - // When the user presses or releases the ctrl key, the desired_tld changes, - // and when the user finishes an IME composition, inline autocomplete may - // no longer be prevented. In both these cases the text itself hasn't - // changed since the last query, and some providers can do much less work - // (and get results back more quickly). Taking advantage of this reduces - // flicker. - if (input_.text() == text) - minimal_changes = true; - } - input_ = input; + // much. When the user presses or releases the ctrl key, the desired_tld + // changes, and when the user finishes an IME composition, inline autocomplete + // may no longer be prevented. In both these cases the text itself hasn't + // changed since the last query, and some providers can do much less work (and + // get results back more quickly). Taking advantage of this reduces flicker. + // If the popup isn't open, on the other hand, we threw the past results away, + // so no shortcuts are possible. + const bool minimal_changes = is_open() && (input_.text() == text); + input_ = AutocompleteInput(text, desired_tld, prevent_inline_autocomplete, + prefer_keyword); // If we're starting a brand new query, stop caring about any old query. if (!minimal_changes && query_in_progress_) { @@ -787,6 +774,7 @@ void AutocompletePopupModel::StartAutocomplete( } // Start the new query. + manually_selected_match_.Clear(); query_in_progress_ = !controller_->Start(input_, minimal_changes, false); controller_->GetResult(&latest_result_); @@ -801,23 +789,17 @@ void AutocompletePopupModel::StartAutocomplete( void AutocompletePopupModel::StopAutocomplete() { // Close any old query. - if (query_in_progress_) { - controller_->Stop(); - query_in_progress_ = false; - update_pending_ = false; - } + StopQuery(); // Reset results. This will force the popup to close. latest_result_.Reset(); CommitLatestResults(true); - // Clear input_ and manually_selected_match_ to make sure we don't try and use - // any of these results for the next query we receive. Strictly speaking this - // isn't necessary, since the popup isn't open, but it keeps our internal - // state consistent and serves as future-proofing in case the code in - // StartAutocomplete() changes. + // Clear input_ to make sure we don't try and use any of these results for the + // next query we receive. Strictly speaking this isn't necessary, since the + // popup isn't open, but it keeps our internal state consistent and serves as + // future-proofing in case the code in StartAutocomplete() changes. input_.Clear(); - manually_selected_match_.Clear(); } void AutocompletePopupModel::SetHoveredLine(size_t line) { @@ -842,28 +824,36 @@ void AutocompletePopupModel::SetHoveredLine(size_t line) { view_->OnHoverEnabledOrDisabled(is_disabling); } -void AutocompletePopupModel::SetSelectedLine(size_t line) { +void AutocompletePopupModel::SetSelectedLine(size_t line, + bool reset_to_default) { DCHECK(line < result_.size()); if (result_.empty()) return; + // Cancel the query so the results don't change on the user. + StopQuery(); + + const AutocompleteMatch& match = result_.match_at(line); + if (reset_to_default) { + manually_selected_match_.Clear(); + } else { + // Track the user's selection until they cancel it. + manually_selected_match_.destination_url = match.destination_url; + manually_selected_match_.provider_affinity = match.provider; + manually_selected_match_.is_history_what_you_typed_match = + match.is_history_what_you_typed_match; + } + if (line == selected_line_) - return; // Nothing to do + return; // Nothing else to do. // Update the edit with the new data for this match. - const AutocompleteMatch& match = result_.match_at(line); std::wstring keyword; const bool is_keyword_hint = GetKeywordForMatch(match, &keyword); - edit_model_->OnPopupDataChanged(match.fill_into_edit, true, - manually_selected_match_, keyword, - is_keyword_hint, - (match.type == AutocompleteMatch::SEARCH)); - - // Track the user's selection until they cancel it. - manually_selected_match_.destination_url = match.destination_url; - manually_selected_match_.provider_affinity = match.provider; - manually_selected_match_.is_history_what_you_typed_match = - match.is_history_what_you_typed_match; + edit_model_->OnPopupDataChanged( + reset_to_default ? std::wstring() : match.fill_into_edit, + !reset_to_default, keyword, is_keyword_hint, + (match.type == AutocompleteMatch::SEARCH)); // Repaint old and new selected lines immediately, so that the edit doesn't // appear to update [much] faster than the popup. We must not update @@ -897,7 +887,7 @@ std::wstring AutocompletePopupModel::URLsForCurrentSelection( if (update_pending_) { // The default match on the latest result should be up-to-date. If the user // changed the selection since that result was generated using the arrow - // keys, Move() will have force updated the popup. + // keys, SetSelectedLine() will have force updated the popup. result = &latest_result_; match = result->default_match(); } else { @@ -920,11 +910,13 @@ std::wstring AutocompletePopupModel::URLsForDefaultMatch( PageTransition::Type* transition, bool* is_history_what_you_typed_match, std::wstring* alternate_nav_url) { - // Cancel any existing query. - StopAutocomplete(); + // We had better not already be doing anything, or this call will blow it + // away. + DCHECK(!is_open()); + DCHECK(!query_in_progress_); // Run the new query and get only the synchronously available results. - const AutocompleteInput input(text, desired_tld, true); + const AutocompleteInput input(text, desired_tld, true, false); const bool done = controller_->Start(input, false, true); DCHECK(done); controller_->GetResult(&result_); @@ -932,7 +924,6 @@ std::wstring AutocompletePopupModel::URLsForDefaultMatch( return std::wstring(); // Get the URLs for the default match. - result_.SetDefaultMatch(manually_selected_match_); const AutocompleteResult::const_iterator match = result_.default_match(); const std::wstring url(match->destination_url); // Need to copy since we // reset result_ below. @@ -940,7 +931,7 @@ std::wstring AutocompletePopupModel::URLsForDefaultMatch( *transition = match->transition; if (is_history_what_you_typed_match) *is_history_what_you_typed_match = match->is_history_what_you_typed_match; - if (alternate_nav_url && manually_selected_match_.empty()) + if (alternate_nav_url) *alternate_nav_url = result_.GetAlternateNavURL(input, match); result_.Reset(); @@ -984,20 +975,17 @@ AutocompleteLog* AutocompletePopupModel::GetAutocompleteLog() { } void AutocompletePopupModel::Move(int count) { + if (result_.empty()) + return; + // The user is using the keyboard to change the selection, so stop tracking // hover. SetHoveredLine(kNoMatch); - // Force the popup to open/update, so the user is interacting with the - // latest results. - CommitLatestResults(false); - if (result_.empty()) - return; - // Clamp the new line to [0, result_.count() - 1]. const size_t new_line = selected_line_ + count; - SetSelectedLine(((count < 0) && (new_line >= selected_line_)) ? - 0 : std::min(new_line, result_.size() - 1)); + SetSelectedLine((((count < 0) && (new_line >= selected_line_)) ? + 0 : std::min(new_line, result_.size() - 1)), false); } void AutocompletePopupModel::TryDeletingCurrentItem() { @@ -1014,18 +1002,8 @@ void AutocompletePopupModel::TryDeletingCurrentItem() { // to OnAutocompleteUpdate() CommitLatestResults(false); if (!result_.empty()) { - // Move the selection to the next choice after the deleted one, but clear - // the manual selection so this choice doesn't act "sticky". - // - // It might also be correct to only call Clear() here when - // manually_selected_match_ didn't already have a provider() (i.e. when - // there was no existing manual selection). It's not clear what the user - // wants when they shift-delete something they've arrowed to. If they - // arrowed down just to shift-delete it, we should probably Clear(); if - // they arrowed to do something else, then got a bad match while typing, - // we probably shouldn't. - SetSelectedLine(std::min(result_.size() - 1, selected_line)); - manually_selected_match_.Clear(); + // Move the selection to the next choice after the deleted one. + SetSelectedLine(std::min(result_.size() - 1, selected_line), false); } } } @@ -1044,16 +1022,17 @@ void AutocompletePopupModel::Run() { CommitLatestResults(false); } -void AutocompletePopupModel::SetDefaultMatchAndUpdate(bool immediately) { - if (!latest_result_.SetDefaultMatch(manually_selected_match_) && - !query_in_progress_) { - // We don't clear the provider affinity because the user didn't do - // something to indicate that they want a different provider, we just - // couldn't find the specific match they wanted. - manually_selected_match_.destination_url.clear(); - manually_selected_match_.is_history_what_you_typed_match = false; +void AutocompletePopupModel::StopQuery() { + if (query_in_progress_) { + controller_->Stop(); + query_in_progress_ = false; + update_pending_ = false; + latest_result_.CopyFrom(result_); // Not strictly necessary, but keeps + // internal state consistent. } +} +void AutocompletePopupModel::SetDefaultMatchAndUpdate(bool immediately) { if (immediately) { CommitLatestResults(true); } else if (!update_pending_) { @@ -1088,9 +1067,8 @@ void AutocompletePopupModel::SetDefaultMatchAndUpdate(bool immediately) { is_keyword_hint = GetKeywordForMatch(*match, &keyword); can_show_search_hint = (match->type == AutocompleteMatch::SEARCH); } - edit_model_->OnPopupDataChanged(inline_autocomplete_text, false, - manually_selected_match_ /* ignored */, keyword, is_keyword_hint, - can_show_search_hint); + edit_model_->OnPopupDataChanged(inline_autocomplete_text, false, keyword, + is_keyword_hint, can_show_search_hint); } void AutocompletePopupModel::CommitLatestResults(bool force) { diff --git a/chrome/browser/autocomplete/autocomplete_popup.h b/chrome/browser/autocomplete/autocomplete_popup.h index 522ca1c..9d01f84 100644 --- a/chrome/browser/autocomplete/autocomplete_popup.h +++ b/chrome/browser/autocomplete/autocomplete_popup.h @@ -222,12 +222,17 @@ class AutocompletePopupModel : public ACControllerListener, public Task { // not require inline autocomplete for the default match. This is difficult // to explain in the abstract; the practical use case is that after the user // deletes text in the edit, the HistoryURLProvider should make sure not to - //promote a match requiring inline autocomplete too highly. + // promote a match requiring inline autocomplete too highly. + // + // |prefer_keyword| should be true when the keyword UI is onscreen; this will + // bias the autocomplete results toward the keyword provider when the input + // string is a bare keyword. void StartAutocomplete(const std::wstring& text, const std::wstring& desired_tld, - bool prevent_inline_autocomplete); + bool prevent_inline_autocomplete, + bool prefer_keyword); - // Closes the window and cancels any pending asynchronous queries + // Closes the window and cancels any pending asynchronous queries. void StopAutocomplete(); // Returns true if the popup is currently open. @@ -264,9 +269,18 @@ class AutocompletePopupModel : public ACControllerListener, public Task { // Call to change the selected line. This will update all state and repaint // the necessary parts of the window, as well as updating the edit with the // new temporary text. |line| should be within the range of valid lines. + // |reset_to_default| is true when the selection is being reset back to the + // default match, and thus there is no temporary text (and no + // |manually_selected_match_|). // NOTE: This assumes the popup is open, and thus both old and new values for // the selected line should not be kNoMatch. - void SetSelectedLine(size_t line); + void SetSelectedLine(size_t line, bool reset_to_default); + + // Called when the user hits escape after arrowing around the popup. This + // will change the selected line back to the default match and redraw. + void ResetToDefaultMatch() { + SetSelectedLine(result_.default_match() - result_.begin(), true); + } // Returns the URL for the selected match. If an update is in progress, // "selected" means "default in the latest results". If there are no @@ -339,14 +353,14 @@ class AutocompletePopupModel : public ACControllerListener, public Task { // Task - called when either timer fires. Calls CommitLatestResults(). virtual void Run(); - // The match the user has manually chosen, if any. - AutocompleteResult::Selection manually_selected_match_; - // The token value for selected_line_, hover_line_ and functions dealing with // a "line number" that indicates "no line". static const size_t kNoMatch = -1; private: + // Stops an existing query but doesn't close the popup. + void StopQuery(); + // Sets the correct default match in latest_result_, then updates the popup // appearance to match. If |immediately| is true this update happens // synchronously; otherwise, it's deferred until the next scheduled update. @@ -402,6 +416,9 @@ class AutocompletePopupModel : public ACControllerListener, public Task { // which should only be true when the popup is closed. size_t selected_line_; + // The match the user has manually chosen, if any. + AutocompleteResult::Selection manually_selected_match_; + DISALLOW_COPY_AND_ASSIGN(AutocompletePopupModel); }; diff --git a/chrome/browser/autocomplete/autocomplete_unittest.cc b/chrome/browser/autocomplete/autocomplete_unittest.cc index cadda8d..af96555 100644 --- a/chrome/browser/autocomplete/autocomplete_unittest.cc +++ b/chrome/browser/autocomplete/autocomplete_unittest.cc @@ -150,7 +150,7 @@ void AutocompleteProviderTest::OnAutocompleteUpdate(bool updated_result, void AutocompleteProviderTest::RunTest() { result_.Reset(); - const AutocompleteInput input(L"a", std::wstring(), true); + const AutocompleteInput input(L"a", std::wstring(), true, false); EXPECT_FALSE(controller_->Start(input, false, false)); // The message loop will terminate when all autocomplete input has been @@ -169,86 +169,11 @@ std::ostream& operator<<(std::ostream& os, TEST_F(AutocompleteProviderTest, Query) { RunTest(); - // Make sure the default match gets set to the highest relevance match when - // we have no preference. The highest relevance matches should come from - // the second provider. - AutocompleteResult::Selection selection; - result_.SetDefaultMatch(selection); + // Make sure the default match gets set to the highest relevance match. The + // highest relevance matches should come from the second provider. EXPECT_EQ(num_results_per_provider * 2, result_.size()); // two providers ASSERT_NE(result_.end(), result_.default_match()); EXPECT_EQ(providers_[1], result_.default_match()->provider); - - // Change provider affinity. - selection.provider_affinity = providers_[0]; - result_.SetDefaultMatch(selection); - ASSERT_NE(result_.end(), result_.default_match()); - EXPECT_EQ(providers_[0], result_.default_match()->provider); -} - -TEST_F(AutocompleteProviderTest, UpdateSelection) { - RunTest(); - - // An empty selection should simply result in the default match overall. - AutocompleteResult::Selection selection; - result_.SetDefaultMatch(selection); - ASSERT_NE(result_.end(), result_.default_match()); - EXPECT_EQ(providers_[1], result_.default_match()->provider); - - // ...As should specifying a provider that didn't return results. - scoped_refptr<TestProvider> test_provider = new TestProvider(0, L""); - selection.provider_affinity = test_provider.get(); - result_.SetDefaultMatch(selection); - ASSERT_NE(result_.end(), result_.default_match()); - EXPECT_EQ(providers_[1], result_.default_match()->provider); - selection.provider_affinity = NULL; - test_provider = NULL; - - // ...As should specifying a destination URL that doesn't exist, and no - // provider. - selection.destination_url = L"garbage"; - selection.provider_affinity = NULL; - result_.SetDefaultMatch(selection); - ASSERT_NE(result_.end(), result_.default_match()); - EXPECT_EQ(providers_[1], result_.default_match()->provider); - delete selection.provider_affinity; - - // Specifying a valid provider should result in the default match from that - // provider. - selection.destination_url.clear(); - selection.provider_affinity = providers_[0]; - result_.SetDefaultMatch(selection); - ASSERT_NE(result_.end(), result_.default_match()); - EXPECT_EQ(providers_[0], result_.default_match()->provider); - - // ...And nothing should change if we specify a destination that doesn't - // exist. - selection.destination_url = L"garbage"; - result_.SetDefaultMatch(selection); - ASSERT_NE(result_.end(), result_.default_match()); - EXPECT_EQ(providers_[0], result_.default_match()->provider); - - // Specifying a particular URL should match that URL. - std::wstring non_default_url_from_provider_0(L"a2"); - selection.destination_url = non_default_url_from_provider_0; - selection.provider_affinity = providers_[0]; - result_.SetDefaultMatch(selection); - ASSERT_NE(result_.end(), result_.default_match()); - EXPECT_EQ(non_default_url_from_provider_0, - result_.default_match()->destination_url); - - // ...Even when we ask for a different provider. - selection.provider_affinity = providers_[1]; - result_.SetDefaultMatch(selection); - ASSERT_NE(result_.end(), result_.default_match()); - EXPECT_EQ(non_default_url_from_provider_0, - result_.default_match()->destination_url); - - // ...Or when we don't ask for a provider at all. - selection.provider_affinity = NULL; - result_.SetDefaultMatch(selection); - ASSERT_NE(result_.end(), result_.default_match()); - EXPECT_EQ(non_default_url_from_provider_0, - result_.default_match()->destination_url); } TEST_F(AutocompleteProviderTest, RemoveDuplicates) { @@ -303,7 +228,7 @@ TEST(AutocompleteTest, InputType) { }; for (int i = 0; i < arraysize(input_cases); ++i) { - AutocompleteInput input(input_cases[i].input, std::wstring(), true); + AutocompleteInput input(input_cases[i].input, std::wstring(), true, false); EXPECT_EQ(input_cases[i].type, input.type()) << "Input: " << input_cases[i].input; } diff --git a/chrome/browser/autocomplete/history_contents_provider_unittest.cc b/chrome/browser/autocomplete/history_contents_provider_unittest.cc index 5122071..dabdcfa 100644 --- a/chrome/browser/autocomplete/history_contents_provider_unittest.cc +++ b/chrome/browser/autocomplete/history_contents_provider_unittest.cc @@ -101,7 +101,7 @@ class HistoryContentsProviderTest : public testing::Test, } // namespace TEST_F(HistoryContentsProviderTest, Body) { - AutocompleteInput input(L"FOO", std::wstring(), true); + AutocompleteInput input(L"FOO", std::wstring(), true, false); RunQuery(input, false, false); // The results should be the first two pages, in decreasing order. @@ -114,7 +114,7 @@ TEST_F(HistoryContentsProviderTest, Body) { } TEST_F(HistoryContentsProviderTest, Title) { - AutocompleteInput input(L"PAGEONE", std::wstring(), true); + AutocompleteInput input(L"PAGEONE", std::wstring(), true, false); RunQuery(input, false, false); // The results should be the first two pages. @@ -128,7 +128,7 @@ TEST_F(HistoryContentsProviderTest, Title) { // The "minimal changes" flag should mean that we don't re-query the DB. TEST_F(HistoryContentsProviderTest, MinimalChanges) { - AutocompleteInput input(L"PAGEONE", std::wstring(), true); + AutocompleteInput input(L"PAGEONE", std::wstring(), true, false); // A minimal changes request when there have been no real queries should // give us no results. @@ -157,7 +157,7 @@ TEST_F(HistoryContentsProviderTest, Bookmarks) { GURL bookmark_url("http://www.google.com/4"); profile()->GetBookmarkModel()->SetURLStarred(bookmark_url, L"bar", true); - AutocompleteInput input(L"bar", std::wstring(), true); + AutocompleteInput input(L"bar", std::wstring(), true, false); // Ask for synchronous. This should only get the bookmark. RunQuery(input, false, true); diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc index b2663e6..2b863db 100644 --- a/chrome/browser/autocomplete/history_url_provider_unittest.cc +++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc @@ -155,7 +155,8 @@ void HistoryURLProviderTest::RunTest(const std::wstring text, bool prevent_inline_autocomplete, const std::wstring* expected_urls, int num_results) { - AutocompleteInput input(text, desired_tld, prevent_inline_autocomplete); + AutocompleteInput input(text, desired_tld, prevent_inline_autocomplete, + false); autocomplete_->Start(input, false, false); if (!autocomplete_->done()) MessageLoop::current()->Run(); diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc index b9eae17..f389a4a 100644 --- a/chrome/browser/autocomplete/keyword_provider.cc +++ b/chrome/browser/autocomplete/keyword_provider.cc @@ -206,8 +206,8 @@ void KeywordProvider::FillInURLAndContents( // static int KeywordProvider::CalculateRelevance(AutocompleteInput::Type type, bool complete, - bool is_bookmark_keyword) { - if (complete && is_bookmark_keyword) + bool no_query_text_needed) { + if (complete && no_query_text_needed) return 1500; switch (type) { @@ -244,7 +244,11 @@ AutocompleteMatch KeywordProvider::CreateAutocompleteMatch( // choice and immediately begin typing in query input. const bool keyword_complete = (prefix_length == keyword.length()); AutocompleteMatch result(this, - CalculateRelevance(input.type(), keyword_complete, !supports_replacement), + CalculateRelevance(input.type(), keyword_complete, + // When the user wants keyword matches to take + // preference, score them highly regardless of whether + // the input provides query text. + input.prefer_keyword() || !supports_replacement), false); result.type = AutocompleteMatch::KEYWORD; result.fill_into_edit.assign(keyword); diff --git a/chrome/browser/autocomplete/keyword_provider_unittest.cc b/chrome/browser/autocomplete/keyword_provider_unittest.cc index 789667e..f4b50c6 100644 --- a/chrome/browser/autocomplete/keyword_provider_unittest.cc +++ b/chrome/browser/autocomplete/keyword_provider_unittest.cc @@ -58,7 +58,8 @@ void KeywordProviderTest::RunTest( std::wstring AutocompleteMatch::* member) { ACMatches matches; for (int i = 0; i < num_cases; ++i) { - AutocompleteInput input(keyword_cases[i].input, std::wstring(), true); + AutocompleteInput input(keyword_cases[i].input, std::wstring(), true, + false); kw_provider_->Start(input, false, false); EXPECT_TRUE(kw_provider_->done()); matches = kw_provider_->matches(); |