summaryrefslogtreecommitdiffstats
path: root/chrome/browser
diff options
context:
space:
mode:
authorpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2008-09-19 22:36:33 +0000
committerpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2008-09-19 22:36:33 +0000
commit9fc8ebd5b437e78b32ac13d570e9d26c18f482b8 (patch)
treeea77edee33c575a8aa9db785a79ab4443f869e60 /chrome/browser
parent857218b6d36e1ead540c49c35330b50333085acb (diff)
downloadchromium_src-9fc8ebd5b437e78b32ac13d570e9d26c18f482b8.zip
chromium_src-9fc8ebd5b437e78b32ac13d570e9d26c18f482b8.tar.gz
chromium_src-9fc8ebd5b437e78b32ac13d570e9d26c18f482b8.tar.bz2
Stop exposing manually_selected_match_ outside the AutocompletePopupModel. The main goal of this is to be a first step towards divorcing the popup and the edit from each other.
To do this, I changed the behavior of manual selections. They now do not persist once the user types more characters, hits esc, etc. Our old behavior, which Brett and I designed long ago, turns out to have been a mistake; users who arrowed to an item and then typed more weren't expecting "stickiness" on their previous choice, and it led to user mistakes. This also required changing how we do the "keyword UI" persistence in the case where the user switches into keyword UI, but then deletes all his text. Previously, we used manually_selected_match_ with a provider affinity to the keyword provider in order to accomplish this. Now we stick another flag on the AutocompleteInput, which, when set, biases the keyword provider to return the best results. The user-visible effect of this is that when in keyword UI mode with no query string, the selected entry in the popup will be the first, rather than third, entry. This is a small win. While here I fixed the bug where editing a string and transforming it into a keyword search would avoid switching into keyword UI (as expected), but also delete the keyword off the visible string (oops). I also made us lock the popup once the user changes the manually_selected_match_, in order to give a little more stability to it. I'm sorry this makes so many behavioral changes at once. All this code is tangled together and untangling it is hard :( The keyword-related variables in the AutocompleteEditModel seem a mess. They are probably worse now than before this change; I think I need a followup change at some point to make them all more sane. It seems like we have three variables and complex conditionals where two, and simpler ones, would do. BUG=997976,1201974,1204173 Review URL: http://codereview.chromium.org/3172 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@2426 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r--chrome/browser/autocomplete/autocomplete.cc69
-rw-r--r--chrome/browser/autocomplete/autocomplete.h42
-rw-r--r--chrome/browser/autocomplete/autocomplete_edit.cc112
-rw-r--r--chrome/browser/autocomplete/autocomplete_edit.h69
-rw-r--r--chrome/browser/autocomplete/autocomplete_popup.cc154
-rw-r--r--chrome/browser/autocomplete/autocomplete_popup.h31
-rw-r--r--chrome/browser/autocomplete/autocomplete_unittest.cc83
-rw-r--r--chrome/browser/autocomplete/history_contents_provider_unittest.cc8
-rw-r--r--chrome/browser/autocomplete/history_url_provider_unittest.cc3
-rw-r--r--chrome/browser/autocomplete/keyword_provider.cc10
-rw-r--r--chrome/browser/autocomplete/keyword_provider_unittest.cc3
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();