diff options
38 files changed, 491 insertions, 993 deletions
@@ -160,4 +160,3 @@ François Beaufort <beaufort.francois@gmail.com> Eriq Augustine <eriq.augustine@gmail.com> Francois Kritzinger <francoisk777@gmail.com> Erik Hill <erikghill@gmail.com> -Aaron Randolph <aaron.randolph@gmail.com> diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc index 22dec0e..aa66c33 100644 --- a/chrome/browser/autocomplete/autocomplete.cc +++ b/chrome/browser/autocomplete/autocomplete.cc @@ -5,7 +5,6 @@ #include "chrome/browser/autocomplete/autocomplete.h" #include <algorithm> -#include <set> #include "base/basictypes.h" #include "base/command_line.h" @@ -25,16 +24,12 @@ #include "chrome/browser/autocomplete/search_provider.h" #include "chrome/browser/autocomplete/shortcuts_provider.h" #include "chrome/browser/bookmarks/bookmark_model.h" -#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/external_protocol/external_protocol_handler.h" #include "chrome/browser/instant/instant_field_trial.h" #include "chrome/browser/net/url_fixer_upper.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_io_data.h" -#include "chrome/browser/search_engines/template_url.h" -#include "chrome/browser/search_engines/template_url_service.h" -#include "chrome/browser/search_engines/template_url_service_factory.h" #include "chrome/browser/ui/webui/history_ui.h" #include "chrome/common/chrome_notification_types.h" #include "chrome/common/chrome_switches.h" @@ -688,9 +683,6 @@ void AutocompleteResult::AddMatch(const AutocompleteMatch& match) { } void AutocompleteResult::SortAndCull(const AutocompleteInput& input) { - for (ACMatches::iterator i = matches_.begin(); i != matches_.end(); ++i) - i->ComputeStrippedDestinationURL(); - // Remove duplicates. std::sort(matches_.begin(), matches_.end(), &AutocompleteMatch::DestinationSortFunc); @@ -999,7 +991,6 @@ void AutocompleteController::UpdateResult(bool is_synchronous_pass) { } UpdateKeywordDescriptions(&result_); - UpdateAssociatedKeywords(&result_); bool notify_default_match = is_synchronous_pass; if (!is_synchronous_pass) { @@ -1007,54 +998,19 @@ void AutocompleteController::UpdateResult(bool is_synchronous_pass) { last_result.default_match() != last_result.end(); const bool default_is_valid = result_.default_match() != result_.end(); // We've gotten async results. Send notification that the default match - // updated if fill_into_edit differs or associated_keyword differ. (The - // latter can change if we've just started Chrome and the keyword database - // finishes loading while processing this request.) We don't check the URL - // as that may change for the default match even though the fill into edit - // hasn't changed (see SearchProvider for one case of this). + // updated if fill_into_edit differs. We don't check the URL as that may + // change for the default match even though the fill into edit hasn't + // changed (see SearchProvider for one case of this). notify_default_match = (last_default_was_valid != default_is_valid) || (default_is_valid && - ((result_.default_match()->fill_into_edit != - last_result.default_match()->fill_into_edit) || - (result_.default_match()->associated_keyword.get() != - last_result.default_match()->associated_keyword.get()))); + (result_.default_match()->fill_into_edit != + last_result.default_match()->fill_into_edit)); } NotifyChanged(notify_default_match); } -void AutocompleteController::UpdateAssociatedKeywords( - AutocompleteResult* result) { - if (!keyword_provider_) - return; - - std::set<string16> keywords; - for (ACMatches::iterator match(result->begin()); match != result->end(); - ++match) { - if (!match->keyword.empty()) { - keywords.insert(match->keyword); - } else { - string16 keyword = match->associated_keyword.get() ? - match->associated_keyword->keyword : - keyword_provider_->GetKeywordForText(match->fill_into_edit); - - // Only add the keyword if the match does not have a duplicate keyword - // with a more relevant match. - if (!keyword.empty() && !keywords.count(keyword)) { - keywords.insert(keyword); - - if (!match->associated_keyword.get()) - match->associated_keyword.reset(new AutocompleteMatch( - keyword_provider_->CreateAutocompleteMatch(match->fill_into_edit, - keyword, input_))); - } else { - match->associated_keyword.reset(); - } - } - } -} - void AutocompleteController::UpdateKeywordDescriptions( AutocompleteResult* result) { const TemplateURL* last_template_url = NULL; diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h index 3e27177..9661404 100644 --- a/chrome/browser/autocomplete/autocomplete.h +++ b/chrome/browser/autocomplete/autocomplete.h @@ -10,7 +10,6 @@ #include <string> #include <vector> -#include "base/gtest_prod_util.h" #include "base/logging.h" #include "base/memory/ref_counted.h" #include "base/string16.h" @@ -679,12 +678,8 @@ class AutocompleteController : public ACProviderListener { void set_search_provider(SearchProvider* provider) { search_provider_ = provider; } - void set_keyword_provider(KeywordProvider* provider) { - keyword_provider_ = provider; - } #endif SearchProvider* search_provider() const { return search_provider_; } - KeywordProvider* keyword_provider() const { return keyword_provider_; } // Getters const AutocompleteInput& input() const { return input_; } @@ -696,20 +691,11 @@ class AutocompleteController : public ACProviderListener { virtual void OnProviderUpdate(bool updated_matches); private: - friend class AutocompleteProviderTest; - FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, - RedundantKeywordsIgnoredInResult); - // Updates |result_| to reflect the current provider state. Resets timers and // fires notifications as necessary. |is_synchronous_pass| is true only when // Start() is calling this to get the synchronous result. void UpdateResult(bool is_synchronous_pass); - // Updates |result| to populate each match's |associated_keyword| if that - // match can show a keyword hint. |result| should be sorted by - // relevance before this is called. - void UpdateAssociatedKeywords(AutocompleteResult* result); - // For each group of contiguous matches from the same TemplateURL, show the // provider name as a description on the first match in the group. void UpdateKeywordDescriptions(AutocompleteResult* result); diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc index 6c367c9..04e3c7b 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.cc +++ b/chrome/browser/autocomplete/autocomplete_edit.cc @@ -173,8 +173,7 @@ void AutocompleteEditModel::FinalizeInstantQuery( if (skip_inline_autocomplete) { const string16 final_text = input_text + suggest_text; view_->OnBeforePossibleChange(); - view_->SetWindowTextAndCaretPos(final_text, final_text.length(), false, - false); + view_->SetWindowTextAndCaretPos(final_text, final_text.length()); view_->OnAfterPossibleChange(); } else if (popup_->IsOpen()) { SearchProvider* search_provider = @@ -407,8 +406,7 @@ void AutocompleteEditModel::Revert() { is_keyword_hint_ = false; has_temporary_text_ = false; view_->SetWindowTextAndCaretPos(permanent_text_, - has_focus_ ? permanent_text_.length() : 0, - false, true); + has_focus_ ? permanent_text_.length() : 0); NetworkActionPredictor* network_action_predictor = NetworkActionPredictorFactory::GetForProfile(profile_); if (network_action_predictor) @@ -418,8 +416,6 @@ void AutocompleteEditModel::Revert() { void AutocompleteEditModel::StartAutocomplete( bool has_selected_text, bool prevent_inline_autocomplete) const { - ClearPopupKeywordMode(); - bool keyword_is_selected = KeywordIsSelected(); popup_->SetHoveredLine(AutocompletePopupModel::kNoMatch); // We don't explicitly clear AutocompletePopupModel::manually_selected_match, @@ -631,49 +627,28 @@ void AutocompleteEditModel::OpenMatch(const AutocompleteMatch& match, bool AutocompleteEditModel::AcceptKeyword() { DCHECK(is_keyword_hint_ && !keyword_.empty()); - autocomplete_controller_->Stop(false); + view_->OnBeforePossibleChange(); + view_->SetWindowTextAndCaretPos(string16(), 0); is_keyword_hint_ = false; - - if (popup_->IsOpen()) - popup_->SetSelectedLineState(AutocompletePopupModel::KEYWORD); - else - StartAutocomplete(false, true); - - // Ensure the current selection is saved before showing keyword mode - // so that moving to another line and then reverting the text will restore - // the current state properly. - view_->OnTemporaryTextMaybeChanged( - DisplayTextFromUserText(CurrentMatch().fill_into_edit), - !has_temporary_text_); - has_temporary_text_ = true; - + view_->OnAfterPossibleChange(); + just_deleted_text_ = false; // OnAfterPossibleChange() erroneously sets this + // since the edit contents have disappeared. It + // doesn't really matter, but we clear it to be + // consistent. content::RecordAction(UserMetricsAction("AcceptedKeywordHint")); return true; } void AutocompleteEditModel::ClearKeyword(const string16& visible_text) { - autocomplete_controller_->Stop(false); - ClearPopupKeywordMode(); - + view_->OnBeforePossibleChange(); const string16 window_text(keyword_ + visible_text); - - // Only reset the result if the edit text has changed since the - // keyword was accepted, or if the popup is closed. - if (just_deleted_text_ || !visible_text.empty() || !popup_->IsOpen()) { - view_->OnBeforePossibleChange(); - view_->SetWindowTextAndCaretPos(window_text.c_str(), keyword_.length(), - false, false); - keyword_.clear(); - is_keyword_hint_ = false; - view_->OnAfterPossibleChange(); - just_deleted_text_ = true; // OnAfterPossibleChange() fails to clear this - // since the edit contents have actually grown - // longer. - } else { - is_keyword_hint_ = true; - view_->SetWindowTextAndCaretPos(window_text.c_str(), keyword_.length(), - false, true); - } + view_->SetWindowTextAndCaretPos(window_text.c_str(), keyword_.length()); + keyword_.clear(); + is_keyword_hint_ = false; + view_->OnAfterPossibleChange(); + just_deleted_text_ = true; // OnAfterPossibleChange() fails to clear this + // since the edit contents have actually grown + // longer. } const AutocompleteResult& AutocompleteEditModel::result() const { @@ -936,9 +911,8 @@ void AutocompleteEditModel::OnResultChanged(bool default_match_changed) { // can be many of these as a user types an initial series of characters, // the OS DNS cache could suffer eviction problems for minimal gain. - is_keyword_hint = match->GetKeyword(&keyword); + is_keyword_hint = popup_->GetKeywordForMatch(*match, &keyword); } - popup_->OnResultChanged(); OnPopupDataChanged(inline_autocomplete_text, NULL, keyword, is_keyword_hint); @@ -971,12 +945,6 @@ bool AutocompleteEditModel::KeywordIsSelected() const { return !is_keyword_hint_ && !keyword_.empty(); } -void AutocompleteEditModel::ClearPopupKeywordMode() const { - if (popup_->IsOpen() && - popup_->selected_line_state() == AutocompletePopupModel::KEYWORD) - popup_->SetSelectedLineState(AutocompletePopupModel::NORMAL); -} - string16 AutocompleteEditModel::DisplayTextFromUserText( const string16& text) const { return KeywordIsSelected() ? @@ -1076,9 +1044,7 @@ bool AutocompleteEditModel::ShouldAllowExactKeywordMatch( TRIM_LEADING, &keyword); // Only allow exact keyword match if |keyword| represents a keyword hint. - return keyword.length() && - !autocomplete_controller_->keyword_provider()-> - GetKeywordForText(keyword).empty(); + return keyword.length() && popup_->GetKeywordForText(keyword, &keyword); } bool AutocompleteEditModel::DoInstant(const AutocompleteMatch& match, diff --git a/chrome/browser/autocomplete/autocomplete_edit.h b/chrome/browser/autocomplete/autocomplete_edit.h index 95bee74..3b046d3 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.h +++ b/chrome/browser/autocomplete/autocomplete_edit.h @@ -381,9 +381,6 @@ class AutocompleteEditModel : public AutocompleteControllerDelegate { // Returns true if a keyword is selected. bool KeywordIsSelected() const; - // Turns off keyword mode for the current match. - void ClearPopupKeywordMode() const; - // Conversion between user text and display text. User text is the text the // user has input. Display text is the text being shown in the edit. The // two are different if a keyword is selected. diff --git a/chrome/browser/autocomplete/autocomplete_edit_unittest.cc b/chrome/browser/autocomplete/autocomplete_edit_unittest.cc index f52bc4d..9771f94 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_unittest.cc +++ b/chrome/browser/autocomplete/autocomplete_edit_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -35,9 +35,7 @@ class TestingOmniboxView : public OmniboxView { const string16& display_text, bool update_popup) OVERRIDE {} virtual void SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) OVERRIDE {} + size_t caret_pos) OVERRIDE {} virtual void SetForcedQuery() OVERRIDE {} virtual bool IsSelectAll() OVERRIDE { return false; } virtual bool DeleteAtEndPressed() OVERRIDE { return false; } @@ -157,9 +155,9 @@ TEST(AutocompleteEditTest, AdjustTextForCopy) { TestingOmniboxView view; TestingAutocompleteEditController controller; TestingProfile profile; - profile.CreateTemplateURLService(); - profile.CreateAutocompleteClassifier(); AutocompleteEditModel model(&view, &controller, &profile); + profile.CreateAutocompleteClassifier(); + profile.CreateTemplateURLService(); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(input); ++i) { model.UpdatePermanentText(ASCIIToUTF16(input[i].perm_text)); diff --git a/chrome/browser/autocomplete/autocomplete_match.cc b/chrome/browser/autocomplete/autocomplete_match.cc index ac3c3fb..c01addb 100644 --- a/chrome/browser/autocomplete/autocomplete_match.cc +++ b/chrome/browser/autocomplete/autocomplete_match.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -46,61 +46,9 @@ AutocompleteMatch::AutocompleteMatch(AutocompleteProvider* provider, from_previous(false) { } -AutocompleteMatch::AutocompleteMatch(const AutocompleteMatch& match) - : provider(match.provider), - relevance(match.relevance), - deletable(match.deletable), - fill_into_edit(match.fill_into_edit), - inline_autocomplete_offset(match.inline_autocomplete_offset), - destination_url(match.destination_url), - stripped_destination_url(match.stripped_destination_url), - contents(match.contents), - contents_class(match.contents_class), - description(match.description), - description_class(match.description_class), - transition(match.transition), - is_history_what_you_typed_match(match.is_history_what_you_typed_match), - type(match.type), - keyword(match.keyword), - template_url(match.template_url), - starred(match.starred), - from_previous(match.from_previous) { - if (match.associated_keyword.get()) - associated_keyword.reset(new AutocompleteMatch(*match.associated_keyword)); -} - AutocompleteMatch::~AutocompleteMatch() { } -AutocompleteMatch& AutocompleteMatch::operator=( - const AutocompleteMatch& match) { - if (this == &match) - return *this; - - provider = match.provider; - relevance = match.relevance; - deletable = match.deletable; - fill_into_edit = match.fill_into_edit; - inline_autocomplete_offset = match.inline_autocomplete_offset; - destination_url = match.destination_url; - stripped_destination_url = match.stripped_destination_url; - contents = match.contents; - contents_class = match.contents_class; - description = match.description; - description_class = match.description_class; - transition = match.transition; - is_history_what_you_typed_match = match.is_history_what_you_typed_match; - type = match.type; - associated_keyword.reset(match.associated_keyword.get() ? - new AutocompleteMatch(*match.associated_keyword) : NULL); - keyword = match.keyword; - template_url = match.template_url; - starred = match.starred; - from_previous = match.from_previous; - - return *this; -} - // static std::string AutocompleteMatch::TypeToString(Type type) { const char* strings[] = { @@ -159,15 +107,15 @@ bool AutocompleteMatch::DestinationSortFunc(const AutocompleteMatch& elem1, // Sort identical destination_urls together. Place the most relevant matches // first, so that when we call std::unique(), these are the ones that get // preserved. - return (elem1.stripped_destination_url != elem2.stripped_destination_url) ? - (elem1.stripped_destination_url < elem2.stripped_destination_url) : + return (elem1.destination_url != elem2.destination_url) ? + (elem1.destination_url < elem2.destination_url) : MoreRelevant(elem1, elem2); } // static bool AutocompleteMatch::DestinationsEqual(const AutocompleteMatch& elem1, const AutocompleteMatch& elem2) { - return elem1.stripped_destination_url == elem2.stripped_destination_url; + return elem1.destination_url == elem2.destination_url; } // static @@ -227,28 +175,6 @@ string16 AutocompleteMatch::SanitizeString(const string16& text) { return result; } -void AutocompleteMatch::ComputeStrippedDestinationURL() { - static const char prefix[] = "www."; - static const size_t prefix_len = arraysize(prefix) - 1; - - std::string host = destination_url.host(); - if (destination_url.is_valid() && host.compare(0, prefix_len, prefix) == 0) { - host = host.substr(prefix_len); - GURL::Replacements replace_host; - replace_host.SetHostStr(host); - stripped_destination_url = destination_url.ReplaceComponents(replace_host); - } else { - stripped_destination_url = destination_url; - } -} - -bool AutocompleteMatch::GetKeyword(string16* keyword) const { - const bool is_keyword_hint = associated_keyword.get() != NULL; - keyword->assign(is_keyword_hint ? associated_keyword->keyword : - this->keyword); - return is_keyword_hint; -} - #ifndef NDEBUG void AutocompleteMatch::Validate() const { ValidateClassifications(contents, contents_class); diff --git a/chrome/browser/autocomplete/autocomplete_match.h b/chrome/browser/autocomplete/autocomplete_match.h index aff42cf..d1229a9 100644 --- a/chrome/browser/autocomplete/autocomplete_match.h +++ b/chrome/browser/autocomplete/autocomplete_match.h @@ -9,7 +9,6 @@ #include <vector> #include <string> -#include "base/memory/scoped_ptr.h" #include "content/public/common/page_transition_types.h" #include "googleurl/src/gurl.h" @@ -91,13 +90,9 @@ struct AutocompleteMatch { int relevance, bool deletable, Type type); - AutocompleteMatch(const AutocompleteMatch& match); ~AutocompleteMatch(); // Converts |type| to a string representation. Used in logging and debugging. - AutocompleteMatch& operator=(const AutocompleteMatch& match); - - // Converts |type| to a string representation. Used in logging. static std::string TypeToString(Type type); // Converts |type| to a resource identifier for the appropriate icon for this @@ -109,7 +104,6 @@ struct AutocompleteMatch { const AutocompleteMatch& elem2); // Comparison functions for removing matches with duplicate destinations. - // Destinations are compared using |stripped_destination_url|. static bool DestinationSortFunc(const AutocompleteMatch& elem1, const AutocompleteMatch& elem2); static bool DestinationsEqual(const AutocompleteMatch& elem1, @@ -139,18 +133,6 @@ struct AutocompleteMatch { // or |description|. static string16 SanitizeString(const string16& text); - // Copies the destination_url with "www." stripped off to - // |stripped_destination_url|. This method is invoked internally by the - // AutocompleteController and does not normally need to be invoked. - void ComputeStrippedDestinationURL(); - - // Gets the selected keyword or keyword hint for this match. Returns - // true if |keyword| represents a keyword hint, or false if |keyword| - // represents a selected keyword. (|keyword| will always be set [though - // possibly to the empty string], and you cannot have both a selected keyword - // and a keyword hint simultaneously.) - bool GetKeyword(string16* keyword) const; - // The provider of this match, used to remember which provider the user had // selected when the input changes. This may be NULL, in which case there is // no provider (or memory of the user's selection). @@ -183,9 +165,6 @@ struct AutocompleteMatch { // It may be empty if there is no possible navigation. GURL destination_url; - // The destination URL with "www." stripped off for better dupe finding. - GURL stripped_destination_url; - // The main text displayed in the address bar dropdown. string16 contents; ACMatchClassifications contents_class; @@ -206,22 +185,6 @@ struct AutocompleteMatch { // Type of this match. Type type; - // Set with a keyword provider match if this match can show a keyword hint. - // For example, if this is a SearchProvider match for "www.amazon.com", - // |associated_keyword| could be a KeywordProvider match for "amazon.com". - scoped_ptr<AutocompleteMatch> associated_keyword; - - // For matches that correspond to valid substituting keywords ("search - // engines" that aren't the default engine, or extension keywords), this - // is the keyword. If this is set, then when displaying this match, the - // edit will use the "keyword mode" UI that shows a blue - // "Search <engine name>" chit before the user's typing. This should be - // set for any match that's an |associated_keyword| of a match in the main - // result list, as well as any other matches in the main result list that - // are direct keyword matches (e.g. if the user types in a keyword name and - // some search terms directly). - string16 keyword; - // Indicates the TemplateURL the match originated from. This is set for // keywords as well as matches for the default search provider. const TemplateURL* template_url; diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.cc b/chrome/browser/autocomplete/autocomplete_popup_model.cc index 30198d5..d0d54f0 100644 --- a/chrome/browser/autocomplete/autocomplete_popup_model.cc +++ b/chrome/browser/autocomplete/autocomplete_popup_model.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -23,16 +23,13 @@ /////////////////////////////////////////////////////////////////////////////// // AutocompletePopupModel -const size_t AutocompletePopupModel::kNoMatch = -1; - AutocompletePopupModel::AutocompletePopupModel( AutocompletePopupView* popup_view, AutocompleteEditModel* edit_model) : view_(popup_view), edit_model_(edit_model), hovered_line_(kNoMatch), - selected_line_(kNoMatch), - selected_line_state_(NORMAL) { + selected_line_(kNoMatch) { edit_model->set_popup_model(this); } @@ -86,29 +83,24 @@ void AutocompletePopupModel::SetSelectedLine(size_t line, if (line == selected_line_ && !force) return; // Nothing else to do. - // We need to update |selected_line_state_| and |selected_line_| before - // calling InvalidateLine(), since it will check them to determine how to - // draw. We also need to update |selected_line_| before calling - // OnPopupDataChanged(), so that when the edit notifies its controller that - // something has changed, the controller can get the correct updated data. + // We need to update |selected_line_| before calling OnPopupDataChanged(), so + // that when the edit notifies its controller that something has changed, the + // controller can get the correct updated data. // // NOTE: We should never reach here with no selected line; the same code that // opened the popup and made it possible to get here should have also set a // selected line. CHECK(selected_line_ != kNoMatch); GURL current_destination(result.match_at(selected_line_).destination_url); - const size_t prev_selected_line = selected_line_; - selected_line_state_ = NORMAL; + view_->InvalidateLine(selected_line_); selected_line_ = line; - view_->InvalidateLine(prev_selected_line); view_->InvalidateLine(selected_line_); // Update the edit with the new data for this match. // TODO(pkasting): If |selected_line_| moves to the controller, this can be // eliminated and just become a call to the observer on the edit. string16 keyword; - const bool is_keyword_hint = match.GetKeyword(&keyword); - + const bool is_keyword_hint = GetKeywordForMatch(match, &keyword); if (reset_to_default) { string16 inline_autocomplete_text; if ((match.inline_autocomplete_offset != string16::npos) && @@ -135,6 +127,59 @@ void AutocompletePopupModel::ResetToDefaultMatch() { view_->OnDragCanceled(); } +bool AutocompletePopupModel::GetKeywordForMatch(const AutocompleteMatch& match, + string16* keyword) const { + // Assume we have no keyword until we find otherwise. + keyword->clear(); + + if (match.template_url && + TemplateURL::SupportsReplacement(match.template_url) && + match.transition == content::PAGE_TRANSITION_KEYWORD) { + // The current match is a keyword, return that as the selected keyword. + keyword->assign(match.template_url->keyword()); + return false; + } + + // See if the current match's fill_into_edit corresponds to a keyword. + return GetKeywordForText(match.fill_into_edit, keyword); +} + +bool AutocompletePopupModel::GetKeywordForText(const string16& text, + string16* keyword) const { + // Creates keyword_hint first in case |keyword| is a pointer to |text|. + const string16 keyword_hint(TemplateURLService::CleanUserInputKeyword(text)); + + // Assume we have no keyword until we find otherwise. + keyword->clear(); + + if (keyword_hint.empty()) + return false; + Profile* profile = edit_model_->profile(); + TemplateURLService* url_service = + TemplateURLServiceFactory::GetForProfile(profile); + if (!url_service) + return false; + url_service->Load(); + + // Don't provide a hint if this keyword doesn't support replacement. + const TemplateURL* const template_url = + url_service->GetTemplateURLForKeyword(keyword_hint); + if (!TemplateURL::SupportsReplacement(template_url)) + return false; + + // Don't provide a hint for inactive/disabled extension keywords. + if (template_url->IsExtensionKeyword()) { + const Extension* extension = profile->GetExtensionService()-> + GetExtensionById(template_url->GetExtensionId(), false); + if (!extension || (profile->IsOffTheRecord() && + !profile->GetExtensionService()->IsIncognitoEnabled(extension->id()))) + return false; + } + + keyword->assign(keyword_hint); + return true; +} + void AutocompletePopupModel::Move(int count) { const AutocompleteResult& result = this->result(); if (result.empty()) @@ -150,17 +195,6 @@ void AutocompletePopupModel::Move(int count) { false, false); } -void AutocompletePopupModel::SetSelectedLineState(LineState state) { - DCHECK(!result().empty()); - DCHECK_NE(kNoMatch, selected_line_); - - const AutocompleteMatch& match = result().match_at(selected_line_); - DCHECK(match.associated_keyword.get()); - - selected_line_state_ = state; - view_->InvalidateLine(selected_line_); -} - void AutocompletePopupModel::TryDeletingCurrentItem() { // We could use InfoForCurrentSelection() here, but it seems better to try // and shift-delete the actual selection, rather than any "in progress, not @@ -209,7 +243,6 @@ void AutocompletePopupModel::OnResultChanged() { // There had better not be a nonempty result set with no default match. CHECK((selected_line_ != kNoMatch) || result.empty()); manually_selected_match_.Clear(); - selected_line_state_ = NORMAL; // If we're going to trim the window size to no longer include the hovered // line, turn hover off. Practically, this shouldn't happen, but it // doesn't hurt to be defensive. diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.h b/chrome/browser/autocomplete/autocomplete_popup_model.h index 59266fd..173800a 100644 --- a/chrome/browser/autocomplete/autocomplete_popup_model.h +++ b/chrome/browser/autocomplete/autocomplete_popup_model.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -15,12 +15,6 @@ class SkBitmap; class AutocompletePopupModel { public: - // See selected_line_state_ for details. - enum LineState { - NORMAL = 0, - KEYWORD - }; - AutocompletePopupModel(AutocompletePopupView* popup_view, AutocompleteEditModel* edit_model); ~AutocompletePopupModel(); @@ -51,10 +45,6 @@ class AutocompletePopupModel { return selected_line_; } - LineState selected_line_state() const { - return selected_line_state_; - } - // 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| will be clamped to the range of valid lines. @@ -71,6 +61,19 @@ class AutocompletePopupModel { // will change the selected line back to the default match and redraw. void ResetToDefaultMatch(); + // Gets the selected keyword or keyword hint for the given match. If the match + // is already keyword, then the keyword will be returned directly. Otherwise, + // it returns GetKeywordForText(match.fill_into_edit, keyword). + bool GetKeywordForMatch(const AutocompleteMatch& match, + string16* keyword) const; + + // Gets the selected keyword or keyword hint for the given text. Returns + // true if |keyword| represents a keyword hint, or false if |keyword| + // represents a selected keyword. (|keyword| will always be set [though + // possibly to the empty string], and you cannot have both a selected keyword + // and a keyword hint simultaneously.) + bool GetKeywordForText(const string16& text, string16* keyword) const; + // Immediately updates and opens the popup if necessary, then moves the // current selection down (|count| > 0) or up (|count| < 0), clamping to the // first or last result if necessary. If |count| == 0, the selection will be @@ -78,11 +81,6 @@ class AutocompletePopupModel { // AutocompleteEditModel. void Move(int count); - // If the selected line has both a normal match and a keyword match, this can - // be used to choose which to select. It is an error to call this when the - // selected line does not have both matches (or there is no selection). - void SetSelectedLineState(LineState state); - // Called when the user hits shift-delete. This should determine if the item // can be removed from history, and if so, remove it and update the popup. void TryDeletingCurrentItem(); @@ -102,7 +100,7 @@ class AutocompletePopupModel { // The token value for selected_line_, hover_line_ and functions dealing with // a "line number" that indicates "no line". - static const size_t kNoMatch; + static const size_t kNoMatch = -1; private: AutocompletePopupView* view_; @@ -117,11 +115,6 @@ class AutocompletePopupModel { // which should only be true when the popup is closed. size_t selected_line_; - // If the selected line has both a normal match and a keyword match, this - // determines whether the normal match (if NORMAL) or the keyword match - // (if KEYWORD) is selected. - LineState selected_line_state_; - // The match the user has manually chosen, if any. AutocompleteResult::Selection manually_selected_match_; diff --git a/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc b/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc new file mode 100644 index 0000000..30988c0 --- /dev/null +++ b/chrome/browser/autocomplete/autocomplete_popup_model_unittest.cc @@ -0,0 +1,118 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/autocomplete/autocomplete_popup_model.h" + +#include "base/utf_string_conversions.h" +#include "chrome/browser/autocomplete/autocomplete_match.h" +#include "chrome/browser/search_engines/template_url.h" +#include "chrome/browser/search_engines/template_url_service.h" +#include "chrome/browser/search_engines/template_url_service_factory.h" +#include "chrome/test/base/testing_profile.h" +#include "testing/gtest/include/gtest/gtest.h" + +class AutoCompletePopupModelTest : public testing::Test { + public: + AutoCompletePopupModelTest() { + } + + virtual void SetUp(); + virtual void TearDown(); + + protected: + AutocompleteMatch CreateMatch(const string16& keyword, + const string16& query_string, + AutocompleteMatch::Type type); + void RunTest(const char* input, AutocompleteMatch::Type type, + const char* keyword); + + scoped_ptr<TestingProfile> profile_; + scoped_ptr<AutocompletePopupModel> model_; + scoped_ptr<AutocompleteEditModel> edit_model_; +}; + +void AutoCompletePopupModelTest::SetUp() { + profile_.reset(new TestingProfile()); + profile_->CreateTemplateURLService(); + edit_model_.reset(new AutocompleteEditModel(NULL, NULL, profile_.get())); + model_.reset(new AutocompletePopupModel(NULL, edit_model_.get())); + + TemplateURLService* turl_model = + TemplateURLServiceFactory::GetForProfile(profile_.get()); + + turl_model->Load(); + + // Reset the default TemplateURL. + TemplateURL* default_t_url = new TemplateURL(); + default_t_url->set_keyword(ASCIIToUTF16("t")); + default_t_url->SetURL("http://defaultturl/{searchTerms}", 0, 0); + turl_model->Add(default_t_url); + turl_model->SetDefaultSearchProvider(default_t_url); + ASSERT_NE(0, default_t_url->id()); + + // Create another TemplateURL for KeywordProvider. + TemplateURL* keyword_t_url = new TemplateURL(); + keyword_t_url->set_short_name(ASCIIToUTF16("k")); + keyword_t_url->set_keyword(ASCIIToUTF16("k")); + keyword_t_url->SetURL("http://keyword/{searchTerms}", 0, 0); + turl_model->Add(keyword_t_url); + ASSERT_NE(0, keyword_t_url->id()); +} + +void AutoCompletePopupModelTest::TearDown() { + profile_.reset(); + model_.reset(); + edit_model_.reset(); +} + +AutocompleteMatch AutoCompletePopupModelTest::CreateMatch( + const string16& keyword, + const string16& query_string, + AutocompleteMatch::Type type) { + AutocompleteMatch match(NULL, 0, false, type); + match.contents = query_string; + TemplateURLService* template_url_service = + TemplateURLServiceFactory::GetForProfile(profile_.get()); + if (!keyword.empty()) { + const TemplateURL* template_url = + template_url_service->GetTemplateURLForKeyword(keyword); + match.template_url = + TemplateURL::SupportsReplacement(template_url) ? template_url : NULL; + } + if (match.template_url) + match.fill_into_edit = match.template_url->keyword() + char16(' '); + else + match.template_url = template_url_service->GetDefaultSearchProvider(); + match.fill_into_edit.append(query_string); + match.transition = keyword.empty() ? + content::PAGE_TRANSITION_GENERATED : content::PAGE_TRANSITION_KEYWORD; + return match; +} + +void AutoCompletePopupModelTest::RunTest(const char* input, + AutocompleteMatch::Type type, + const char* keyword) { + string16 keyword16(ASCIIToUTF16(keyword)); + string16 detected_keyword; + EXPECT_FALSE(model_->GetKeywordForMatch( + CreateMatch(keyword16, ASCIIToUTF16(input), type), &detected_keyword)); + EXPECT_EQ(keyword16, detected_keyword); +} + +TEST_F(AutoCompletePopupModelTest, GetKeywordForMatch) { + string16 keyword; + + // Possible matches when the input is "tfoo" + RunTest("tfoo", AutocompleteMatch::SEARCH_WHAT_YOU_TYPED, ""); + RunTest("tfoo", AutocompleteMatch::SEARCH_HISTORY, ""); + RunTest("tfoo", AutocompleteMatch::SEARCH_SUGGEST, ""); + + // Possible matches when the input is "t foo" + RunTest("foo", AutocompleteMatch::SEARCH_HISTORY, "t"); + RunTest("foo", AutocompleteMatch::SEARCH_OTHER_ENGINE, "t"); + + // Possible matches when the input is "k foo" + RunTest("foo", AutocompleteMatch::SEARCH_HISTORY, "k"); + RunTest("foo", AutocompleteMatch::SEARCH_OTHER_ENGINE, "k"); +} diff --git a/chrome/browser/autocomplete/autocomplete_unittest.cc b/chrome/browser/autocomplete/autocomplete_unittest.cc index 8793e5b..f0c7dab 100644 --- a/chrome/browser/autocomplete/autocomplete_unittest.cc +++ b/chrome/browser/autocomplete/autocomplete_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -30,8 +30,8 @@ static std::ostream& operator<<(std::ostream& os, } namespace { -const size_t kResultsPerProvider = 3; -} + +const size_t num_results_per_provider = 3; // Autocomplete provider that provides known results. Note that this is // refcounted so that it can also be a task on the message loop. @@ -79,8 +79,8 @@ void TestProvider::Start(const AutocompleteInput& input, } void TestProvider::Run() { - DCHECK_GT(kResultsPerProvider, 0U); - AddResults(1, kResultsPerProvider); + DCHECK_GT(num_results_per_provider, 0U); + AddResults(1, num_results_per_provider); done_ = true; DCHECK(listener_); listener_->OnProviderUpdate(true); @@ -108,32 +108,19 @@ void TestProvider::AddResults(int start_at, int num) { class AutocompleteProviderTest : public testing::Test, public content::NotificationObserver { protected: - struct KeywordTestData { - const string16 fill_into_edit; - const string16 keyword; - const bool expected_keyword_result; - }; - - protected: void ResetControllerWithTestProviders(bool same_destinations); // Runs a query on the input "a", and makes sure both providers' input is // properly collected. void RunTest(); - void RunRedundantKeywordTest(const KeywordTestData* match_data, size_t size); - - void RunQuery(const string16 query); - void ResetControllerWithTestProvidersWithKeywordAndSearchProviders(); - void ResetControllerWithKeywordProvider(); void RunExactKeymatchTest(bool allow_exact_keyword_match); // These providers are owned by the controller once it's created. ACProviders providers_; AutocompleteResult result_; - scoped_ptr<AutocompleteController> controller_; private: // content::NotificationObserver @@ -142,6 +129,7 @@ class AutocompleteProviderTest : public testing::Test, const content::NotificationDetails& details); MessageLoopForUI message_loop_; + scoped_ptr<AutocompleteController> controller_; content::NotificationRegistrar registrar_; TestingProfile profile_; }; @@ -153,12 +141,12 @@ void AutocompleteProviderTest::ResetControllerWithTestProviders( providers_.clear(); // Construct two new providers, with either the same or different prefixes. - TestProvider* providerA = new TestProvider(kResultsPerProvider, + TestProvider* providerA = new TestProvider(num_results_per_provider, ASCIIToUTF16("http://a")); providerA->AddRef(); providers_.push_back(providerA); - TestProvider* providerB = new TestProvider(kResultsPerProvider * 2, + TestProvider* providerB = new TestProvider(num_results_per_provider * 2, same_destinations ? ASCIIToUTF16("http://a") : ASCIIToUTF16("http://b")); providerB->AddRef(); providers_.push_back(providerB); @@ -213,84 +201,19 @@ void AutocompleteProviderTest:: search_provider->AddRef(); providers_.push_back(search_provider); - controller_.reset(new AutocompleteController(providers_, &profile_)); -} - -void AutocompleteProviderTest:: - ResetControllerWithKeywordProvider() { - profile_.CreateTemplateURLService(); - - TemplateURLService* turl_model = - TemplateURLServiceFactory::GetForProfile(&profile_); - - // Create a TemplateURL for KeywordProvider. - TemplateURL* keyword_t_url = new TemplateURL(); - keyword_t_url->set_short_name(ASCIIToUTF16("foo.com")); - keyword_t_url->set_keyword(ASCIIToUTF16("foo.com")); - keyword_t_url->SetURL("http://foo.com/{searchTerms}", 0, 0); - turl_model->Add(keyword_t_url); - ASSERT_NE(0, keyword_t_url->id()); - - // Create another TemplateURL for KeywordProvider. - keyword_t_url = new TemplateURL(); - keyword_t_url->set_short_name(ASCIIToUTF16("bar.com")); - keyword_t_url->set_keyword(ASCIIToUTF16("bar.com")); - keyword_t_url->SetURL("http://bar.com/{searchTerms}", 0, 0); - turl_model->Add(keyword_t_url); - ASSERT_NE(0, keyword_t_url->id()); - - // Forget about any existing providers. The controller owns them and will - // Release() them below, when we delete it during the call to reset(). - providers_.clear(); - - // Create both a keyword and search provider, and add them in that order. - // (Order is important; see comments in RunExactKeymatchTest().) - KeywordProvider* keyword_provider = new KeywordProvider(NULL, - &profile_); - keyword_provider->AddRef(); - providers_.push_back(keyword_provider); - AutocompleteController* controller = new AutocompleteController(providers_, &profile_); - controller->set_keyword_provider(keyword_provider); controller_.reset(controller); } void AutocompleteProviderTest::RunTest() { - RunQuery(ASCIIToUTF16("a")); -} - -void AutocompleteProviderTest::RunRedundantKeywordTest( - const KeywordTestData* match_data, - size_t size) { - ACMatches matches; - for (size_t i = 0; i < size; ++i) { - AutocompleteMatch match; - match.fill_into_edit = match_data[i].fill_into_edit; - match.keyword = match_data[i].keyword; - matches.push_back(match); - } - - AutocompleteResult result; - result.AppendMatches(matches); - controller_->UpdateAssociatedKeywords(&result); - - for (size_t j = 0; j < result.size(); ++j) { - EXPECT_EQ(match_data[j].expected_keyword_result, - result.match_at(j).associated_keyword.get() != NULL); - } -} - - -void AutocompleteProviderTest::RunQuery(const string16 query) { result_.Reset(); - controller_->Start(query, string16(), true, false, true, - AutocompleteInput::ALL_MATCHES); + controller_->Start(ASCIIToUTF16("a"), string16(), true, false, true, + AutocompleteInput::ALL_MATCHES); - if (!controller_->done()) - // The message loop will terminate when all autocomplete input has been - // collected. - MessageLoop::current()->Run(); + // The message loop will terminate when all autocomplete input has been + // collected. + MessageLoop::current()->Run(); } void AutocompleteProviderTest::RunExactKeymatchTest( @@ -327,7 +250,7 @@ TEST_F(AutocompleteProviderTest, Query) { // Make sure the default match gets set to the highest relevance match. The // highest relevance matches should come from the second provider. - EXPECT_EQ(kResultsPerProvider * 2, result_.size()); // two providers + 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); } @@ -338,7 +261,7 @@ TEST_F(AutocompleteProviderTest, RemoveDuplicates) { // Make sure all the first provider's results were eliminated by the second // provider's. - EXPECT_EQ(kResultsPerProvider, result_.size()); + EXPECT_EQ(num_results_per_provider, result_.size()); for (AutocompleteResult::const_iterator i(result_.begin()); i != result_.end(); ++i) EXPECT_EQ(providers_[1], i->provider); @@ -350,48 +273,6 @@ TEST_F(AutocompleteProviderTest, AllowExactKeywordMatch) { RunExactKeymatchTest(false); } -// Test that redundant associated keywords are removed. -TEST_F(AutocompleteProviderTest, RedundantKeywordsIgnoredInResult) { - ResetControllerWithKeywordProvider(); - - // Get the controller's internal members in the correct state. - RunQuery(ASCIIToUTF16("fo")); - - { - KeywordTestData duplicate_url[] = { - { ASCIIToUTF16("fo"), string16(), false }, - { ASCIIToUTF16("foo.com"), string16(), true }, - { ASCIIToUTF16("foo.com"), string16(), false } - }; - - SCOPED_TRACE("Duplicate url"); - RunRedundantKeywordTest(duplicate_url, ARRAYSIZE_UNSAFE(duplicate_url)); - } - - { - KeywordTestData keyword_match[] = { - { ASCIIToUTF16("foo.com"), ASCIIToUTF16("foo.com"), false }, - { ASCIIToUTF16("foo.com"), string16(), false } - }; - - SCOPED_TRACE("Duplicate url with keyword match"); - RunRedundantKeywordTest(keyword_match, ARRAYSIZE_UNSAFE(keyword_match)); - } - - { - KeywordTestData multiple_keyword[] = { - { ASCIIToUTF16("fo"), string16(), false }, - { ASCIIToUTF16("foo.com"), string16(), true }, - { ASCIIToUTF16("foo.com"), string16(), false }, - { ASCIIToUTF16("bar.com"), string16(), true }, - }; - - SCOPED_TRACE("Duplicate url with multiple keywords"); - RunRedundantKeywordTest(multiple_keyword, - ARRAYSIZE_UNSAFE(multiple_keyword)); - } -} - typedef testing::Test AutocompleteTest; TEST_F(AutocompleteTest, InputType) { @@ -603,3 +484,5 @@ TEST(AutocompleteInput, ParseForEmphasizeComponent) { EXPECT_EQ(input_cases[i].host.len, host.len); } } + +} // namespace diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc index 75abc1d..97427dd 100644 --- a/chrome/browser/autocomplete/history_url_provider_unittest.cc +++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -232,8 +232,6 @@ void HistoryURLProviderTest::RunTest(const string16 text, matches_ = autocomplete_->matches(); if (sort_matches_) { - for (ACMatches::iterator i = matches_.begin(); i != matches_.end(); ++i) - i->ComputeStrippedDestinationURL(); std::sort(matches_.begin(), matches_.end(), &AutocompleteMatch::DestinationSortFunc); matches_.erase(std::unique(matches_.begin(), matches_.end(), diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc index f27609e..a88e97d 100644 --- a/chrome/browser/autocomplete/keyword_provider.cc +++ b/chrome/browser/autocomplete/keyword_provider.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -42,6 +42,21 @@ class KeywordProvider::ScopedEndExtensionKeywordMode { KeywordProvider* provider_; }; +// static +string16 KeywordProvider::SplitReplacementStringFromInput( + const string16& input, + bool trim_leading_whitespace) { + // The input may contain leading whitespace, strip it. + string16 trimmed_input; + TrimWhitespace(input, TRIM_LEADING, &trimmed_input); + + // And extract the replacement string. + string16 remaining_input; + SplitKeywordFromInput(trimmed_input, trim_leading_whitespace, + &remaining_input); + return remaining_input; +} + KeywordProvider::KeywordProvider(ACProviderListener* listener, Profile* profile) : AutocompleteProvider(listener, profile, "Keyword"), model_(NULL), @@ -92,45 +107,6 @@ static int global_input_uid_; } // namespace // static -string16 KeywordProvider::SplitKeywordFromInput( - const string16& input, - bool trim_leading_whitespace, - string16* remaining_input) { - // Find end of first token. The AutocompleteController has trimmed leading - // whitespace, so we need not skip over that. - const size_t first_white(input.find_first_of(kWhitespaceUTF16)); - DCHECK_NE(0U, first_white); - if (first_white == string16::npos) - return input; // Only one token provided. - - // Set |remaining_input| to everything after the first token. - DCHECK(remaining_input != NULL); - const size_t remaining_start = trim_leading_whitespace ? - input.find_first_not_of(kWhitespaceUTF16, first_white) : first_white + 1; - - if (remaining_start < input.length()) - remaining_input->assign(input.begin() + remaining_start, input.end()); - - // Return first token as keyword. - return input.substr(0, first_white); -} - -// static -string16 KeywordProvider::SplitReplacementStringFromInput( - const string16& input, - bool trim_leading_whitespace) { - // The input may contain leading whitespace, strip it. - string16 trimmed_input; - TrimWhitespace(input, TRIM_LEADING, &trimmed_input); - - // And extract the replacement string. - string16 remaining_input; - SplitKeywordFromInput(trimmed_input, trim_leading_whitespace, - &remaining_input); - return remaining_input; -} - -// static const TemplateURL* KeywordProvider::GetSubstitutingTemplateURLForInput( Profile* profile, const AutocompleteInput& input, @@ -152,44 +128,6 @@ const TemplateURL* KeywordProvider::GetSubstitutingTemplateURLForInput( return TemplateURL::SupportsReplacement(template_url) ? template_url : NULL; } -string16 KeywordProvider::GetKeywordForText( - const string16& text) const { - const string16 keyword(TemplateURLService::CleanUserInputKeyword(text)); - - if (keyword.empty()) - return keyword; - - TemplateURLService* url_service = GetTemplateURLService(); - if (!url_service) - return string16(); - - // Don't provide a keyword if it doesn't support replacement. - const TemplateURL* const template_url = - url_service->GetTemplateURLForKeyword(keyword); - if (!TemplateURL::SupportsReplacement(template_url)) - return string16(); - - // Don't provide a keyword for inactive/disabled extension keywords. - if (template_url->IsExtensionKeyword()) { - const Extension* extension = profile_->GetExtensionService()-> - GetExtensionById(template_url->GetExtensionId(), false); - if (!extension || - (profile_->IsOffTheRecord() && - !profile_->GetExtensionService()->IsIncognitoEnabled(extension->id()))) - return string16(); - } - - return keyword; -} - -AutocompleteMatch KeywordProvider::CreateAutocompleteMatch( - const string16& text, - const string16& keyword, - const AutocompleteInput& input) { - return CreateAutocompleteMatch(GetTemplateURLService(), keyword, input, - keyword.size(), SplitReplacementStringFromInput(text, true), 0); -} - void KeywordProvider::Start(const AutocompleteInput& input, bool minimal_changes) { // This object ensures we end keyword mode if we exit the function without @@ -223,7 +161,14 @@ void KeywordProvider::Start(const AutocompleteInput& input, if (!ExtractKeywordFromInput(input, &keyword, &remaining_input)) return; - TemplateURLService* model = GetTemplateURLService(); + // Make sure the model is loaded. This is cheap and quickly bails out if + // the model is already loaded. + TemplateURLService* model = + profile_ ? + TemplateURLServiceFactory::GetForProfile(profile_) : + model_; + DCHECK(model); + model->Load(); // Get the best matches for this keyword. // @@ -240,12 +185,11 @@ void KeywordProvider::Start(const AutocompleteInput& input, !remaining_input.empty(), &keyword_matches); + // Prune any extension keywords that are disallowed in incognito mode (if + // we're incognito), or disabled. for (std::vector<string16>::iterator i(keyword_matches.begin()); i != keyword_matches.end(); ) { const TemplateURL* template_url(model->GetTemplateURLForKeyword(*i)); - - // Prune any extension keywords that are disallowed in incognito mode (if - // we're incognito), or disabled. if (profile_ && input.matches_requested() == AutocompleteInput::ALL_MATCHES && template_url->IsExtensionKeyword()) { @@ -260,14 +204,6 @@ void KeywordProvider::Start(const AutocompleteInput& input, continue; } } - - // Prune any substituting keywords if there is no substitution. - if (TemplateURL::SupportsReplacement(template_url) && - remaining_input.empty() && !input.allow_exact_keyword_match()) { - i = keyword_matches.erase(i); - continue; - } - ++i; } if (keyword_matches.empty()) @@ -358,6 +294,30 @@ bool KeywordProvider::ExtractKeywordFromInput(const AutocompleteInput& input, } // static +string16 KeywordProvider::SplitKeywordFromInput( + const string16& input, + bool trim_leading_whitespace, + string16* remaining_input) { + // Find end of first token. The AutocompleteController has trimmed leading + // whitespace, so we need not skip over that. + const size_t first_white(input.find_first_of(kWhitespaceUTF16)); + DCHECK_NE(0U, first_white); + if (first_white == string16::npos) + return input; // Only one token provided. + + // Set |remaining_input| to everything after the first token. + DCHECK(remaining_input != NULL); + const size_t remaining_start = trim_leading_whitespace ? + input.find_first_not_of(kWhitespaceUTF16, first_white) : first_white + 1; + + if (remaining_start < input.length()) + remaining_input->assign(input.begin() + remaining_start, input.end()); + + // Return first token as keyword. + return input.substr(0, first_white); +} + +// static void KeywordProvider::FillInURLAndContents( Profile* profile, const string16& remaining_input, @@ -455,36 +415,36 @@ AutocompleteMatch KeywordProvider::CreateAutocompleteMatch( supports_replacement, input.prefer_keyword(), input.allow_exact_keyword_match()); } - AutocompleteMatch match(this, relevance, false, + AutocompleteMatch result(this, relevance, false, supports_replacement ? AutocompleteMatch::SEARCH_OTHER_ENGINE : AutocompleteMatch::HISTORY_KEYWORD); - match.fill_into_edit.assign(keyword); + result.fill_into_edit.assign(keyword); if (!remaining_input.empty() || !keyword_complete || supports_replacement) - match.fill_into_edit.push_back(L' '); - match.fill_into_edit.append(remaining_input); + result.fill_into_edit.push_back(L' '); + result.fill_into_edit.append(remaining_input); // If we wanted to set |result.inline_autocomplete_offset| correctly, we'd // need CleanUserInputKeyword() to return the amount of adjustment it's made // to the user's input. Because right now inexact keyword matches can't score // more highly than a "what you typed" match from one of the other providers, // we just don't bother to do this, and leave inline autocompletion off. - match.inline_autocomplete_offset = string16::npos; + result.inline_autocomplete_offset = string16::npos; // Create destination URL and popup entry content by substituting user input // into keyword templates. - FillInURLAndContents(profile_, remaining_input, element, &match); + FillInURLAndContents(profile_, remaining_input, element, &result); if (supports_replacement) - match.template_url = element; - match.keyword = keyword; - match.transition = content::PAGE_TRANSITION_KEYWORD; + result.template_url = element; + result.transition = content::PAGE_TRANSITION_KEYWORD; - return match; + return result; } void KeywordProvider::Observe(int type, const content::NotificationSource& source, const content::NotificationDetails& details) { - TemplateURLService* model = GetTemplateURLService(); + TemplateURLService* model = + profile_ ? TemplateURLServiceFactory::GetForProfile(profile_) : model_; const AutocompleteInput& input = extension_suggest_last_input_; switch (type) { @@ -561,16 +521,6 @@ void KeywordProvider::Observe(int type, } } -TemplateURLService* KeywordProvider::GetTemplateURLService() const { - TemplateURLService* service = profile_ ? - TemplateURLServiceFactory::GetForProfile(profile_) : model_; - // Make sure the model is loaded. This is cheap and quickly bails out if - // the model is already loaded. - DCHECK(service); - service->Load(); - return service; -} - void KeywordProvider::EnterExtensionKeywordMode( const std::string& extension_id) { DCHECK(current_keyword_extension_id_.empty()); diff --git a/chrome/browser/autocomplete/keyword_provider.h b/chrome/browser/autocomplete/keyword_provider.h index 21d7b2e..3867345 100644 --- a/chrome/browser/autocomplete/keyword_provider.h +++ b/chrome/browser/autocomplete/keyword_provider.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. // @@ -55,15 +55,6 @@ class KeywordProvider : public AutocompleteProvider, // For testing. KeywordProvider(ACProviderListener* listener, TemplateURLService* model); - // Extracts the next whitespace-delimited token from input and returns it. - // Sets |remaining_input| to everything after the first token (skipping over - // the first intervening whitespace). - // If |trim_leading_whitespace| is true then leading whitespace in - // |*remaining_input| will be trimmed. - static string16 SplitKeywordFromInput(const string16& input, - bool trim_leading_whitespace, - string16* remaining_input); - // Returns the replacement string from the user input. The replacement // string is the portion of the input that does not contain the keyword. // For example, the replacement string for "b blah" is blah. @@ -80,17 +71,6 @@ class KeywordProvider : public AutocompleteProvider, const AutocompleteInput& input, string16* remaining_input); - // If |text| corresponds (in the sense of - // TemplateURLModel::CleanUserInputKeyword()) to an enabled, substituting - // keyword, returns that keyword; returns the empty string otherwise. - string16 GetKeywordForText(const string16& text) const; - - // Creates a fully marked-up AutocompleteMatch for a specific keyword. - AutocompleteMatch CreateAutocompleteMatch( - const string16& text, - const string16& keyword, - const AutocompleteInput& input); - // AutocompleteProvider virtual void Start(const AutocompleteInput& input, bool minimal_changes) OVERRIDE; @@ -113,6 +93,15 @@ class KeywordProvider : public AutocompleteProvider, string16* keyword, string16* remaining_input); + // Extracts the next whitespace-delimited token from input and returns it. + // Sets |remaining_input| to everything after the first token (skipping over + // the first intervening whitespace). + // If |trim_leading_whitespace| is true then leading whitespace in + // |*remaining_input| will be trimmed. + static string16 SplitKeywordFromInput(const string16& input, + bool trim_leading_whitespace, + string16* remaining_input); + // Fills in the "destination_url" and "contents" fields of |match| with the // provided user input and keyword data. static void FillInURLAndContents( @@ -150,8 +139,6 @@ class KeywordProvider : public AutocompleteProvider, const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE; - TemplateURLService* GetTemplateURLService() const; - // Model for the keywords. This is only non-null when testing, otherwise the // TemplateURLService from the Profile is used. TemplateURLService* model_; diff --git a/chrome/browser/autocomplete/keyword_provider_unittest.cc b/chrome/browser/autocomplete/keyword_provider_unittest.cc index 6bd439a..7a02a97 100644 --- a/chrome/browser/autocomplete/keyword_provider_unittest.cc +++ b/chrome/browser/autocomplete/keyword_provider_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -216,13 +216,3 @@ TEST_F(KeywordProviderTest, RemoveKeyword) { model_->Remove(model_->GetTemplateURLForKeyword(ASCIIToUTF16("aaaa"))); ASSERT_TRUE(model_->GetTemplateURLForKeyword(ASCIIToUTF16("aaaa")) == NULL); } - -TEST_F(KeywordProviderTest, GetKeywordForInput) { - EXPECT_EQ(ASCIIToUTF16("aa"), - kw_provider_->GetKeywordForText(ASCIIToUTF16("aa"))); - EXPECT_EQ(string16(), - kw_provider_->GetKeywordForText(ASCIIToUTF16("aafoo"))); - EXPECT_EQ(string16(), - kw_provider_->GetKeywordForText(ASCIIToUTF16("aa foo"))); -} - diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc index 6280427..62d5b92 100644 --- a/chrome/browser/autocomplete/search_provider.cc +++ b/chrome/browser/autocomplete/search_provider.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -874,9 +874,7 @@ void SearchProvider::AddMatchToMap(const string16& query_string, if (is_keyword) { match.fill_into_edit.append( providers_.keyword_provider().keyword() + char16(' ')); - - match.keyword = providers_.keyword_provider().keyword(); - search_start += match.keyword.size() + 1; + search_start += providers_.keyword_provider().keyword().size() + 1; } match.fill_into_edit.append(query_string); // Not all suggestions start with the original input. diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm index 1a878ef..bc6dfc6 100644 --- a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm +++ b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm @@ -548,8 +548,10 @@ void OmniboxPopupViewMac::OpenURLForRow(int row, bool force_background) { // relevant match out to make sure it stays alive until the call // completes. AutocompleteMatch match = model_->result().match_at(row); + string16 keyword; + const bool is_keyword_hint = model_->GetKeywordForMatch(match, &keyword); omnibox_view_->OpenMatch(match, disposition, GURL(), row, - match.keyword); + is_keyword_hint ? string16() : keyword); } void OmniboxPopupViewMac::UserPressedOptIn(bool opt_in) { diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.h b/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.h index 0265f84..eddcd6d 100644 --- a/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.h +++ b/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -54,9 +54,7 @@ class OmniboxViewMac : public OmniboxView, const string16& display_text, bool update_popup) OVERRIDE; virtual void SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) OVERRIDE; + size_t caret_pos) OVERRIDE; virtual void SetForcedQuery() OVERRIDE; virtual bool IsSelectAll() OVERRIDE; virtual bool DeleteAtEndPressed() OVERRIDE; @@ -166,9 +164,6 @@ class OmniboxViewMac : public OmniboxView, // though here we cannot really do the in-place operation they do. void EmphasizeURLComponents(); - // Internally invoked whenever the text changes in some way. - void TextChanged(); - // Calculates text attributes according to |display_text| and applies them // to the given |as| object. void ApplyTextAttributes(const string16& display_text, diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm index 7e4cf9c..42592291 100644 --- a/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm +++ b/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -329,8 +329,11 @@ void OmniboxViewMac::SetUserText(const string16& text, model_->SetUserText(text); // TODO(shess): TODO below from gtk. // TODO(deanm): something about selection / focus change here. - SetWindowTextAndCaretPos(display_text, display_text.length(), update_popup, - true); + SetText(display_text); + if (update_popup) { + UpdatePopup(); + } + model_->OnChanged(); } NSRange OmniboxViewMac::GetSelectedRange() const { @@ -361,17 +364,9 @@ void OmniboxViewMac::SetSelectedRange(const NSRange range) { } void OmniboxViewMac::SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) { + size_t caret_pos) { DCHECK_LE(caret_pos, text.size()); SetTextAndSelectedRange(text, NSMakeRange(caret_pos, caret_pos)); - - if (update_popup) - UpdatePopup(); - - if (notify_text_changed) - TextChanged(); } void OmniboxViewMac::SetForcedQuery() { @@ -537,11 +532,6 @@ void OmniboxViewMac::EmphasizeURLComponents() { } } -void OmniboxViewMac::TextChanged() { - EmphasizeURLComponents(); - model_->OnChanged(); -} - void OmniboxViewMac::ApplyTextAttributes(const string16& display_text, NSMutableAttributedString* as) { NSUInteger as_length = [as length]; @@ -616,7 +606,7 @@ void OmniboxViewMac::OnTemporaryTextMaybeChanged(const string16& display_text, saved_temporary_selection_ = GetSelectedRange(); suggest_text_length_ = 0; - SetWindowTextAndCaretPos(display_text, display_text.size(), false, false); + SetWindowTextAndCaretPos(display_text, display_text.size()); model_->OnChanged(); [field_ clearUndoChain]; } @@ -708,7 +698,8 @@ bool OmniboxViewMac::OnAfterPossibleChange() { // Linux watches for something_changed && text_differs, but that // fails for us in case you copy the URL and paste the identical URL // back (we'll lose the styling). - TextChanged(); + EmphasizeURLComponents(); + model_->OnChanged(); delete_was_pressed_ = false; @@ -802,7 +793,7 @@ bool OmniboxViewMac::OnDoCommandBySelector(SEL cmd) { if (cmd == @selector(deleteForward:)) delete_was_pressed_ = true; - // Don't intercept up/down-arrow or backtab if the popup isn't open. + // Don't intercept up/down-arrow if the popup isn't open. if (popup_view_->IsOpen()) { if (cmd == @selector(moveDown:)) { model_->OnUpOrDownKeyPressed(1); @@ -813,13 +804,6 @@ bool OmniboxViewMac::OnDoCommandBySelector(SEL cmd) { model_->OnUpOrDownKeyPressed(-1); return true; } - - if (cmd == @selector(insertBacktab:) && - model_->popup_model()->selected_line_state() == - AutocompletePopupModel::KEYWORD) { - model_->ClearKeyword(GetText()); - return true; - } } if (cmd == @selector(moveRight:)) { @@ -845,10 +829,26 @@ bool OmniboxViewMac::OnDoCommandBySelector(SEL cmd) { return model_->OnEscapeKeyPressed(); } - if ((cmd == @selector(insertTab:) || - cmd == @selector(insertTabIgnoringFieldEditor:)) && - model_->is_keyword_hint()) { - return model_->AcceptKeyword(); + if (cmd == @selector(insertTab:) || + cmd == @selector(insertTabIgnoringFieldEditor:)) { + if (model_->is_keyword_hint()) + return model_->AcceptKeyword(); + + if (suggest_text_length_ > 0) { + model_->CommitSuggestedText(true); + return true; + } + + if (!IsCaretAtEnd()) { + PlaceCaretAt(GetTextLength()); + // OnDidChange() will not be triggered when setting selected range in this + // method, so we need to call it explicitly. + OnDidChange(); + return true; + } + + if (model_->AcceptCurrentInstantPreview()) + return true; } // |-noop:| is sent when the user presses Cmd+Return. Override the no-op diff --git a/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc b/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc index 59b2262..4dd49e4 100644 --- a/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc +++ b/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc @@ -479,8 +479,10 @@ void OmniboxPopupViewGtk::AcceptLine(size_t line, // extension, |match| and its contents. So copy the relevant match out to // make sure it stays alive until the call completes. AutocompleteMatch match = model_->result().match_at(line); + string16 keyword; + const bool is_keyword_hint = model_->GetKeywordForMatch(match, &keyword); omnibox_view_->OpenMatch(match, disposition, GURL(), line, - match.keyword); + is_keyword_hint ? string16() : keyword); } const gfx::Image* OmniboxPopupViewGtk::IconForMatch( diff --git a/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.cc b/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.cc index 9796292..2b75203 100644 --- a/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.cc +++ b/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.cc @@ -555,22 +555,16 @@ void OmniboxViewGtk::SetUserText(const string16& text, bool update_popup) { model_->SetUserText(text); // TODO(deanm): something about selection / focus change here. - SetWindowTextAndCaretPos(display_text, display_text.length(), update_popup, - true); + SetWindowTextAndCaretPos(display_text, display_text.length()); + if (update_popup) + UpdatePopup(); + TextChanged(); } void OmniboxViewGtk::SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) { + size_t caret_pos) { CharRange range(static_cast<int>(caret_pos), static_cast<int>(caret_pos)); SetTextAndSelectedRange(text, range); - - if (update_popup) - UpdatePopup(); - - if (notify_text_changed) - TextChanged(); } void OmniboxViewGtk::SetForcedQuery() { @@ -645,7 +639,7 @@ void OmniboxViewGtk::OnTemporaryTextMaybeChanged( saved_temporary_selection_ = GetSelection(); StartUpdatingHighlightedText(); - SetWindowTextAndCaretPos(display_text, display_text.length(), false, false); + SetWindowTextAndCaretPos(display_text, display_text.length()); FinishUpdatingHighlightedText(); TextChanged(); } @@ -1084,8 +1078,7 @@ gboolean OmniboxViewGtk::HandleKeyPress(GtkWidget* widget, GdkEventKey* event) { // if IME did not handle it then "move-focus" signal will be emitted by the // default signal handler of |text_view_|. So we can intercept "move-focus" // signal of |text_view_| to know if a Tab key press event was handled by IME, - // and trigger Tab to search or result traversal behavior when necessary in - // the signal handler. + // and trigger Tab to search behavior when necessary in the signal handler. // // But for Enter key, if IME did not handle the key event, the default signal // handler will delete current selection range and insert '\n' and always @@ -1127,9 +1120,7 @@ gboolean OmniboxViewGtk::HandleKeyPress(GtkWidget* widget, GdkEventKey* event) { tab_was_pressed_ = (event->keyval == GDK_Tab || event->keyval == GDK_ISO_Left_Tab || event->keyval == GDK_KP_Tab) && - !(event->state & GDK_CONTROL_MASK); - - shift_was_pressed_ = event->state & GDK_SHIFT_MASK; + !(event->state & (GDK_CONTROL_MASK | GDK_SHIFT_MASK)); delete_was_pressed_ = event->keyval == GDK_Delete || event->keyval == GDK_KP_Delete; @@ -1725,18 +1716,8 @@ void OmniboxViewGtk::HandleViewMoveFocus(GtkWidget* widget, bool handled = false; // Trigger Tab to search behavior only when Tab key is pressed. - if (model_->is_keyword_hint() && !shift_was_pressed_) { + if (model_->is_keyword_hint()) handled = model_->AcceptKeyword(); - } else if (model_->popup_model()->IsOpen()) { - if (shift_was_pressed_ && - model_->popup_model()->selected_line_state() == - AutocompletePopupModel::KEYWORD) - model_->ClearKeyword(GetText()); - else - model_->OnUpOrDownKeyPressed(shift_was_pressed_ ? -1 : 1); - - handled = true; - } if (supports_pre_edit_ && !handled && !pre_edit_.empty()) handled = true; @@ -1744,6 +1725,15 @@ void OmniboxViewGtk::HandleViewMoveFocus(GtkWidget* widget, if (!handled && gtk_widget_get_visible(instant_view_)) handled = model_->CommitSuggestedText(true); + if (!handled) { + if (!IsCaretAtEnd()) { + OnBeforePossibleChange(); + PlaceCaretAt(GetTextLength()); + OnAfterPossibleChange(); + handled = true; + } + } + if (!handled) handled = model_->AcceptCurrentInstantPreview(); diff --git a/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.h b/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.h index 0fb8d1b..3c578a7 100644 --- a/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.h +++ b/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.h @@ -106,9 +106,7 @@ class OmniboxViewGtk : public OmniboxView, const string16& display_text, bool update_popup) OVERRIDE; virtual void SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) OVERRIDE; + size_t caret_pos) OVERRIDE; virtual void SetForcedQuery() OVERRIDE; virtual bool IsSelectAll() OVERRIDE; virtual bool DeleteAtEndPressed() OVERRIDE; @@ -463,11 +461,6 @@ class OmniboxViewGtk : public OmniboxView, // during sync dispatch of "move-focus" signal. bool tab_was_pressed_; - // Indicates if Shift key was pressed. - // Used in conjunction with the Tab key to determine if either traversal - // needs to move up the results or if the keyword needs to be cleared. - bool shift_was_pressed_; - // Indicates that user requested to paste clipboard. // The actual paste clipboard action might be performed later if the // clipboard is not empty. diff --git a/chrome/browser/ui/omnibox/omnibox_view.h b/chrome/browser/ui/omnibox/omnibox_view.h index 234907f..a35f00e 100644 --- a/chrome/browser/ui/omnibox/omnibox_view.h +++ b/chrome/browser/ui/omnibox/omnibox_view.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -109,9 +109,7 @@ class OmniboxView { // Sets the window text and the caret position. virtual void SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) = 0; + size_t caret_pos) = 0; // Sets the edit to forced query mode. Practically speaking, this means that // if the edit is not in forced query mode, its text is set to "?" with the diff --git a/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc b/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc index dc18f5d..1782625 100644 --- a/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc +++ b/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc @@ -37,10 +37,6 @@ #include "ui/base/events.h" #include "ui/base/keycodes/keyboard_codes.h" -#if defined(TOOLKIT_GTK) -#include "chrome/browser/ui/gtk/browser_window_gtk.h" -#endif - #if defined(TOOLKIT_USES_GTK) #include <gdk/gdk.h> #include <gtk/gtk.h> @@ -161,15 +157,6 @@ class OmniboxViewTest : public InProcessBrowserTest, HistoryQuickProvider::set_disabled(true); } -#if defined(TOOLKIT_GTK) - virtual void OnBeforeShowBrowser(Browser* browser) { - // Disable the timer because it causes the browser to move, which results - // in the popup closing before results are verified. - static_cast<BrowserWindowGtk*>( - browser->window())->DisableDebounceTimerForTests(true); - } -#endif - virtual void SetUpOnMainThread() { ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser())); ASSERT_NO_FATAL_FAILURE(SetupComponents()); @@ -760,7 +747,7 @@ class OmniboxViewTest : public InProcessBrowserTest, // Keyword should also be accepted by typing an ideographic space. omnibox_view->OnBeforePossibleChange(); omnibox_view->SetWindowTextAndCaretPos(text + WideToUTF16(L"\x3000"), - text.length() + 1, false, false); + text.length() + 1); omnibox_view->OnAfterPossibleChange(); ASSERT_FALSE(omnibox_view->model()->is_keyword_hint()); ASSERT_EQ(text, omnibox_view->model()->keyword()); @@ -775,7 +762,7 @@ class OmniboxViewTest : public InProcessBrowserTest, // Keyword shouldn't be accepted by pressing space with a trailing // whitespace. omnibox_view->SetWindowTextAndCaretPos( - text + char16(' '), text.length() + 1, false, false); + text + char16(' '), text.length() + 1); ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_SPACE, 0)); ASSERT_TRUE(omnibox_view->model()->is_keyword_hint()); ASSERT_EQ(text, omnibox_view->model()->keyword()); @@ -812,7 +799,7 @@ class OmniboxViewTest : public InProcessBrowserTest, omnibox_view->OnBeforePossibleChange(); omnibox_view->model()->on_paste(); omnibox_view->SetWindowTextAndCaretPos(text + ASCIIToUTF16(" bar"), - text.length() + 4, false, false); + text.length() + 4); omnibox_view->OnAfterPossibleChange(); ASSERT_FALSE(omnibox_view->model()->is_keyword_hint()); ASSERT_TRUE(omnibox_view->model()->keyword().empty()); @@ -1026,137 +1013,59 @@ class OmniboxViewTest : public InProcessBrowserTest, ASSERT_TRUE(omnibox_view->IsSelectAll()); } - void TabAcceptKeyword() { + void TabMoveCursorToEndTest() { OmniboxView* omnibox_view = NULL; ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view)); - string16 text = ASCIIToUTF16(kSearchKeyword); - - // Trigger keyword hint mode. - ASSERT_NO_FATAL_FAILURE(SendKeySequence(kSearchKeywordKeys)); - ASSERT_TRUE(omnibox_view->model()->is_keyword_hint()); - ASSERT_EQ(text, omnibox_view->model()->keyword()); - ASSERT_EQ(text, omnibox_view->GetText()); - - // Trigger keyword mode by tab. - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, 0)); - ASSERT_FALSE(omnibox_view->model()->is_keyword_hint()); - ASSERT_EQ(text, omnibox_view->model()->keyword()); - ASSERT_TRUE(omnibox_view->GetText().empty()); - - // Revert to keyword hint mode. - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_BACK, 0)); - ASSERT_TRUE(omnibox_view->model()->is_keyword_hint()); - ASSERT_EQ(text, omnibox_view->model()->keyword()); - ASSERT_EQ(text, omnibox_view->GetText()); - - // The location bar should still have focus. - ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(), - location_bar_focus_view_id_)); - - // Trigger keyword mode by tab. - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, 0)); - ASSERT_FALSE(omnibox_view->model()->is_keyword_hint()); - ASSERT_EQ(text, omnibox_view->model()->keyword()); - ASSERT_TRUE(omnibox_view->GetText().empty()); + omnibox_view->SetUserText(ASCIIToUTF16("Hello world")); - // Revert to keyword hint mode with SHIFT+TAB. + // Move cursor to the beginning. #if defined(OS_MACOSX) - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_BACKTAB, 0)); + // Home doesn't work on Mac trybot. + ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_A, ui::EF_CONTROL_DOWN)); #else - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN)); + ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_HOME, 0)); #endif - ASSERT_TRUE(omnibox_view->model()->is_keyword_hint()); - ASSERT_EQ(text, omnibox_view->model()->keyword()); - ASSERT_EQ(text, omnibox_view->GetText()); - - ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(), - location_bar_focus_view_id_)); - } - - void TabTraverseResultsTest() { - OmniboxView* omnibox_view = NULL; - ASSERT_NO_FATAL_FAILURE(GetOmniboxView(&omnibox_view)); - AutocompletePopupModel* popup_model = omnibox_view->model()->popup_model(); - ASSERT_TRUE(popup_model); - // Input something to trigger results. - ASSERT_NO_FATAL_FAILURE(SendKeySequence(kDesiredTLDKeys)); - ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone()); - ASSERT_TRUE(popup_model->IsOpen()); - - size_t old_selected_line = popup_model->selected_line(); - EXPECT_EQ(0U, old_selected_line); - - // Move down the results. - for (size_t size = popup_model->result().size(); - popup_model->selected_line() < size - 1; - old_selected_line = popup_model->selected_line()) { - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, 0)); - ASSERT_LT(old_selected_line, popup_model->selected_line()); - } + size_t start, end; + omnibox_view->GetSelectionBounds(&start, &end); + EXPECT_EQ(0U, start); + EXPECT_EQ(0U, end); - // Don't move past the end. + // Pressing tab should move cursor to the end. ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, 0)); - ASSERT_EQ(old_selected_line, popup_model->selected_line()); - ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(), - location_bar_focus_view_id_)); - // Move back up the results. - for (; popup_model->selected_line() > 0U; - old_selected_line = popup_model->selected_line()) { - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN)); - ASSERT_GT(old_selected_line, popup_model->selected_line()); - } + omnibox_view->GetSelectionBounds(&start, &end); + EXPECT_EQ(omnibox_view->GetText().size(), start); + EXPECT_EQ(omnibox_view->GetText().size(), end); - // Don't move past the beginning. - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN)); - ASSERT_EQ(0U, popup_model->selected_line()); + // The location bar should still have focus. ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(), location_bar_focus_view_id_)); - const TestHistoryEntry kHistoryFoo = { - "http://foo/", "Page foo", kSearchText, 1, 1, false - }; - - // Add a history entry so "foo" gets multiple matches. - ASSERT_NO_FATAL_FAILURE( - AddHistoryEntry(kHistoryFoo, Time::Now() - TimeDelta::FromHours(1))); - - // Load results. - ASSERT_NO_FATAL_FAILURE(omnibox_view->SelectAll(false)); - ASSERT_NO_FATAL_FAILURE(SendKeySequence(kSearchKeywordKeys)); - ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone()); + // Select all text. + omnibox_view->SelectAll(true); + EXPECT_TRUE(omnibox_view->IsSelectAll()); + omnibox_view->GetSelectionBounds(&start, &end); + EXPECT_EQ(0U, start); + EXPECT_EQ(omnibox_view->GetText().size(), end); - // Trigger keyword mode by tab. - string16 text = ASCIIToUTF16(kSearchKeyword); + // Pressing tab should move cursor to the end. ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, 0)); - ASSERT_FALSE(omnibox_view->model()->is_keyword_hint()); - ASSERT_EQ(text, omnibox_view->model()->keyword()); - ASSERT_TRUE(omnibox_view->GetText().empty()); - - // The location bar should still have focus. - ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(), - location_bar_focus_view_id_)); - // Pressing tab again should move to the next result and clear keyword - // mode. - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, 0)); - ASSERT_EQ(1U, omnibox_view->model()->popup_model()->selected_line()); - ASSERT_FALSE(omnibox_view->model()->is_keyword_hint()); - ASSERT_NE(text, omnibox_view->model()->keyword()); + omnibox_view->GetSelectionBounds(&start, &end); + EXPECT_EQ(omnibox_view->GetText().size(), start); + EXPECT_EQ(omnibox_view->GetText().size(), end); // The location bar should still have focus. ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(), location_bar_focus_view_id_)); - // Moving back up should not show keyword mode. - ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN)); - ASSERT_TRUE(omnibox_view->model()->is_keyword_hint()); - ASSERT_EQ(text, omnibox_view->model()->keyword()); + // Pressing tab when cursor is at the end should change focus. + ASSERT_NO_FATAL_FAILURE(SendKey(ui::VKEY_TAB, 0)); - ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(), - location_bar_focus_view_id_)); + ASSERT_FALSE(ui_test_utils::IsViewFocused(browser(), + location_bar_focus_view_id_)); } void PersistKeywordModeOnTabSwitch() { @@ -1282,17 +1191,10 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest, DeleteItem) { DeleteItemTest(); } -IN_PROC_BROWSER_TEST_F(OmniboxViewTest, TabAcceptKeyword) { - TabAcceptKeyword(); +IN_PROC_BROWSER_TEST_F(OmniboxViewTest, TabMoveCursorToEnd) { + TabMoveCursorToEndTest(); } -#if !defined(OS_MACOSX) -// Mac intentionally does not support this behavior. -IN_PROC_BROWSER_TEST_F(OmniboxViewTest, TabTraverseResultsTest) { - TabTraverseResultsTest(); -} -#endif - IN_PROC_BROWSER_TEST_F(OmniboxViewTest, PersistKeywordModeOnTabSwitch) { PersistKeywordModeOnTabSwitch(); diff --git a/chrome/browser/ui/views/autocomplete/autocomplete_popup_contents_view.cc b/chrome/browser/ui/views/autocomplete/autocomplete_popup_contents_view.cc index 0f477fe..6ea92ad 100644 --- a/chrome/browser/ui/views/autocomplete/autocomplete_popup_contents_view.cc +++ b/chrome/browser/ui/views/autocomplete/autocomplete_popup_contents_view.cc @@ -53,7 +53,6 @@ namespace { const SkAlpha kGlassPopupAlpha = 240; const SkAlpha kOpaquePopupAlpha = 255; - // The size delta between the font used for the edit and the result rows. Passed // to gfx::Font::DeriveFont. #if defined(OS_CHROMEOS) @@ -237,13 +236,6 @@ AutocompletePopupContentsView::AutocompletePopupContentsView( set_border(bubble_border); // The contents is owned by the LocationBarView. set_parent_owned(false); - - for (size_t i = 0; i < AutocompleteResult::kMaxMatches; ++i) { - AutocompleteResultView* result_view = - CreateResultView(this, i, result_font_, result_bold_font_); - result_view->SetVisible(false); - AddChildViewAt(result_view, static_cast<int>(i)); - } } AutocompletePopupContentsView::~AutocompletePopupContentsView() { @@ -289,14 +281,7 @@ bool AutocompletePopupContentsView::IsOpen() const { } void AutocompletePopupContentsView::InvalidateLine(size_t line) { - AutocompleteResultView* result = static_cast<AutocompleteResultView*>( - child_at(static_cast<int>(line))); - result->Invalidate(); - - if (HasMatchAt(line) && GetMatchAtIndex(line).associated_keyword.get()) { - result->ShowKeyword(IsSelectedIndex(line) && - model_->selected_line_state() == AutocompletePopupModel::KEYWORD); - } + child_at(static_cast<int>(line))->SchedulePaint(); } void AutocompletePopupContentsView::UpdatePopupAppearance() { @@ -304,7 +289,6 @@ void AutocompletePopupContentsView::UpdatePopupAppearance() { // No matches, close any existing popup. if (popup_ != NULL) { size_animation_.Stop(); - // NOTE: Do NOT use CloseNow() here, as we may be deep in a callstack // triggered by the popup receiving a message (e.g. LBUTTONUP), and // destroying the popup would cause us to read garbage when we unwind back @@ -322,14 +306,19 @@ void AutocompletePopupContentsView::UpdatePopupAppearance() { DCHECK_GT(child_rv_count, 0u); child_rv_count--; } - const size_t result_size = model_->result().size(); - for (size_t i = 0; i < result_size; ++i) { - AutocompleteResultView* view = static_cast<AutocompleteResultView*>( - child_at(i)); - view->SetMatch(GetMatchAtIndex(i)); - view->SetVisible(true); + for (size_t i = 0; i < model_->result().size(); ++i) { + AutocompleteResultView* result_view; + if (i >= child_rv_count) { + result_view = + CreateResultView(this, i, result_font_, result_bold_font_); + AddChildViewAt(result_view, static_cast<int>(i)); + } else { + result_view = static_cast<AutocompleteResultView*>(child_at(i)); + result_view->SetVisible(true); + } + result_view->SetMatch(GetMatchAtIndex(i)); } - for (size_t i = result_size; i < child_rv_count; ++i) + for (size_t i = model_->result().size(); i < child_rv_count; ++i) child_at(i)->SetVisible(false); PromoCounter* counter = profile_->GetInstantPromoCounter(); @@ -407,11 +396,11 @@ void AutocompletePopupContentsView::OnDragCanceled() { // AutocompletePopupContentsView, AutocompleteResultViewModel implementation: bool AutocompletePopupContentsView::IsSelectedIndex(size_t index) const { - return index == model_->selected_line(); + return HasMatchAt(index) ? index == model_->selected_line() : false; } bool AutocompletePopupContentsView::IsHoveredIndex(size_t index) const { - return index == model_->hovered_line(); + return HasMatchAt(index) ? index == model_->hovered_line() : false; } const SkBitmap* AutocompletePopupContentsView::GetIconIfExtensionMatch( @@ -447,7 +436,7 @@ void AutocompletePopupContentsView::Layout() { views::View* AutocompletePopupContentsView::GetEventHandlerForPoint( const gfx::Point& point) { - // If there is no opt in view then we want all mouse events. Otherwise, let + // If there is no opt in view, then we want all mouse events. Otherwise let // any descendants of the opt-in view get mouse events. if (!opt_in_view_) return this; @@ -658,8 +647,10 @@ void AutocompletePopupContentsView::OpenIndex( // extension, |match| and its contents. So copy the relevant match out to // make sure it stays alive until the call completes. AutocompleteMatch match = model_->result().match_at(index); + string16 keyword; + const bool is_keyword_hint = model_->GetKeywordForMatch(match, &keyword); omnibox_view_->OpenMatch(match, disposition, GURL(), index, - match.keyword); + is_keyword_hint ? string16() : keyword); } size_t AutocompletePopupContentsView::GetIndexForPoint( diff --git a/chrome/browser/ui/views/autocomplete/autocomplete_result_view.cc b/chrome/browser/ui/views/autocomplete/autocomplete_result_view.cc index dc2ce4f..3009a1c 100644 --- a/chrome/browser/ui/views/autocomplete/autocomplete_result_view.cc +++ b/chrome/browser/ui/views/autocomplete/autocomplete_result_view.cc @@ -14,7 +14,6 @@ #include <algorithm> // NOLINT #include "base/i18n/bidi_line_iterator.h" -#include "chrome/browser/autocomplete/autocomplete_popup_model.h" #include "chrome/browser/ui/views/autocomplete/autocomplete_result_view_model.h" #include "chrome/browser/ui/views/location_bar/location_bar_view.h" #include "grit/generated_resources.h" @@ -113,20 +112,13 @@ AutocompleteResultView::AutocompleteResultView( normal_font_(font), bold_font_(bold_font), ellipsis_width_(font.GetStringWidth(string16(kEllipsis))), - mirroring_context_(new MirroringContext()), - keyword_icon_(new views::ImageView()), - ALLOW_THIS_IN_INITIALIZER_LIST( - animation_(new ui::SlideAnimation(this))) { + mirroring_context_(new MirroringContext()) { CHECK_GE(model_index, 0); if (default_icon_size_ == 0) { default_icon_size_ = ResourceBundle::GetSharedInstance().GetBitmapNamed( AutocompleteMatch::TypeToIcon(AutocompleteMatch::URL_WHAT_YOU_TYPED))-> width(); } - keyword_icon_->set_parent_owned(false); - keyword_icon_->EnableCanvasFlippingForRTLUI(true); - keyword_icon_->SetImage(GetKeywordIcon()); - keyword_icon_->SizeToPreferredSize(); } AutocompleteResultView::~AutocompleteResultView() { @@ -186,38 +178,9 @@ SkColor AutocompleteResultView::GetColor(ResultViewState state, void AutocompleteResultView::SetMatch(const AutocompleteMatch& match) { match_ = match; - animation_->Reset(); - - if (match.associated_keyword.get()) { - keyword_icon_->SetImage(GetKeywordIcon()); - - if (!keyword_icon_->parent()) - AddChildView(keyword_icon_.get()); - } else if (keyword_icon_->parent()) { - RemoveChildView(keyword_icon_.get()); - } - Layout(); } -void AutocompleteResultView::ShowKeyword(bool show_keyword) { - if (show_keyword) - animation_->Show(); - else - animation_->Hide(); -} - -void AutocompleteResultView::Invalidate() { - keyword_icon_->SetImage(GetKeywordIcon()); - SchedulePaint(); -} - -gfx::Size AutocompleteResultView::GetPreferredSize() { - return gfx::Size(0, std::max( - default_icon_size_ + (kMinimumIconVerticalPadding * 2), - GetTextHeight() + (kMinimumTextVerticalPadding * 2))); -} - //////////////////////////////////////////////////////////////////////////////// // AutocompleteResultView, protected: @@ -280,7 +243,7 @@ const SkBitmap* AutocompleteResultView::GetIcon() const { int icon = match_.starred ? IDR_OMNIBOX_STAR : AutocompleteMatch::TypeToIcon(match_.type); - if (GetState() == SELECTED) { + if (model_->IsSelectedIndex(model_index_)) { switch (icon) { case IDR_OMNIBOX_EXTENSION_APP: icon = IDR_OMNIBOX_EXTENSION_APP_SELECTED; @@ -305,13 +268,6 @@ const SkBitmap* AutocompleteResultView::GetIcon() const { return ResourceBundle::GetSharedInstance().GetBitmapNamed(icon); } -const SkBitmap* AutocompleteResultView::GetKeywordIcon() const { - // NOTE: If we ever begin returning icons of varying size, then callers need - // to ensure that |keyword_icon_| is resized each time its image is reset. - return ResourceBundle::GetSharedInstance().GetBitmapNamed( - (GetState() == SELECTED) ? IDR_OMNIBOX_TTS_SELECTED : IDR_OMNIBOX_TTS); -} - int AutocompleteResultView::DrawString( gfx::Canvas* canvas, const string16& text, @@ -542,9 +498,14 @@ void AutocompleteResultView::Elide(Runs* runs, int remaining_width) const { runs->clear(); } +gfx::Size AutocompleteResultView::GetPreferredSize() { + return gfx::Size(0, std::max( + default_icon_size_ + (kMinimumIconVerticalPadding * 2), + GetTextHeight() + (kMinimumTextVerticalPadding * 2))); +} + void AutocompleteResultView::Layout() { const SkBitmap* icon = GetIcon(); - icon_bounds_.SetRect(LocationBarView::kEdgeItemPadding + ((icon->width() == default_icon_size_) ? 0 : LocationBarView::kIconInternalPadding), @@ -553,34 +514,9 @@ void AutocompleteResultView::Layout() { int text_x = LocationBarView::kEdgeItemPadding + default_icon_size_ + LocationBarView::kItemPadding; int text_height = GetTextHeight(); - int text_width; - - if (match_.associated_keyword.get()) { - const int kw_collapsed_size = keyword_icon_->width() + - LocationBarView::kEdgeItemPadding; - const int max_kw_x = width() - kw_collapsed_size; - const int kw_x = animation_->CurrentValueBetween(max_kw_x, - LocationBarView::kEdgeItemPadding); - const int kw_text_x = kw_x + keyword_icon_->width() + - LocationBarView::kItemPadding; - - text_width = kw_x - text_x - LocationBarView::kItemPadding; - keyword_text_bounds_.SetRect(kw_text_x, 0, std::max( - width() - kw_text_x - LocationBarView::kEdgeItemPadding, 0), - text_height); - keyword_icon_->SetPosition(gfx::Point(kw_x, - (height() - keyword_icon_->height()) / 2)); - } else { - text_width = width() - text_x - LocationBarView::kEdgeItemPadding; - } - text_bounds_.SetRect(text_x, std::max(0, (height() - text_height) / 2), - std::max(text_width, 0), text_height); -} - -void AutocompleteResultView::OnBoundsChanged( - const gfx::Rect& previous_bounds) { - animation_->SetSlideDuration(width() / 4); + std::max(bounds().width() - text_x - LocationBarView::kEdgeItemPadding, + 0), text_height); } void AutocompleteResultView::OnPaint(gfx::Canvas* canvas) { @@ -588,29 +524,12 @@ void AutocompleteResultView::OnPaint(gfx::Canvas* canvas) { if (state != NORMAL) canvas->GetSkCanvas()->drawColor(GetColor(state, BACKGROUND)); - if (!match_.associated_keyword.get() || - keyword_icon_->x() > icon_bounds_.right()) { - // Paint the icon. - canvas->DrawBitmapInt(*GetIcon(), GetMirroredXForRect(icon_bounds_), - icon_bounds_.y()); - - // Paint the text. - int x = GetMirroredXForRect(text_bounds_); - mirroring_context_->Initialize(x, text_bounds_.width()); - PaintMatch(canvas, match_, x); - } - - if (match_.associated_keyword.get()) { - // Paint the keyword text. - int x = GetMirroredXForRect(keyword_text_bounds_); - mirroring_context_->Initialize(x, keyword_text_bounds_.width()); - PaintMatch(canvas, *match_.associated_keyword.get(), x); - } -} + // Paint the icon. + canvas->DrawBitmapInt(*GetIcon(), GetMirroredXForRect(icon_bounds_), + icon_bounds_.y()); -void AutocompleteResultView::AnimationProgressed( - const ui::Animation* animation) { - Layout(); - SchedulePaint(); + // Paint the text. + int x = GetMirroredXForRect(text_bounds_); + mirroring_context_->Initialize(x, text_bounds_.width()); + PaintMatch(canvas, match_, x); } - diff --git a/chrome/browser/ui/views/autocomplete/autocomplete_result_view.h b/chrome/browser/ui/views/autocomplete/autocomplete_result_view.h index 2f72497..2c405929 100644 --- a/chrome/browser/ui/views/autocomplete/autocomplete_result_view.h +++ b/chrome/browser/ui/views/autocomplete/autocomplete_result_view.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -8,21 +8,16 @@ #include "chrome/browser/autocomplete/autocomplete_match.h" #include "third_party/skia/include/core/SkColor.h" -#include "ui/base/animation/animation_delegate.h" -#include "ui/base/animation/slide_animation.h" #include "ui/gfx/font.h" #include "ui/gfx/rect.h" -#include "ui/views/controls/image_view.h" #include "ui/views/view.h" class AutocompleteResultViewModel; - namespace gfx { class Canvas; } -class AutocompleteResultView : public views::View, - private ui::AnimationDelegate { +class AutocompleteResultView : public views::View { public: enum ResultViewState { NORMAL = 0, @@ -52,13 +47,6 @@ class AutocompleteResultView : public views::View, // model has changed. void SetMatch(const AutocompleteMatch& match); - void ShowKeyword(bool show_keyword); - - void Invalidate(); - - // views::View: - virtual gfx::Size GetPreferredSize() OVERRIDE; - protected: virtual void PaintMatch(gfx::Canvas* canvas, const AutocompleteMatch& match, @@ -94,7 +82,6 @@ class AutocompleteResultView : public views::View, ResultViewState GetState() const; const SkBitmap* GetIcon() const; - const SkBitmap* GetKeywordIcon() const; // Elides |runs| to fit in |remaining_width|. The runs in |runs| should be in // logical order. @@ -113,13 +100,10 @@ class AutocompleteResultView : public views::View, void Elide(Runs* runs, int remaining_width) const; // views::View: + virtual gfx::Size GetPreferredSize() OVERRIDE; virtual void Layout() OVERRIDE; - virtual void OnBoundsChanged(const gfx::Rect& previous_bounds); virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE; - // ui::AnimationDelegate: - virtual void AnimationProgressed(const ui::Animation* animation) OVERRIDE; - static int default_icon_size_; // This row's model and model index. @@ -141,11 +125,6 @@ class AutocompleteResultView : public views::View, gfx::Rect text_bounds_; gfx::Rect icon_bounds_; - gfx::Rect keyword_text_bounds_; - scoped_ptr<views::ImageView> keyword_icon_; - - scoped_ptr<ui::SlideAnimation> animation_; - DISALLOW_COPY_AND_ASSIGN(AutocompleteResultView); }; diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc index 2f28e55..2a7fd19 100644 --- a/chrome/browser/ui/views/location_bar/location_bar_view.cc +++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc @@ -1017,9 +1017,10 @@ std::string LocationBarView::GetClassName() const { bool LocationBarView::SkipDefaultKeyEventProcessing( const views::KeyEvent& event) { #if defined(OS_WIN) + bool views_omnibox = views::Widget::IsPureViews(); if (views::FocusManager::IsTabTraversalKeyEvent(event)) { - if (location_entry_->model()->popup_model()->IsOpen()) { - // Return true so that the edit sees the tab and moves the selection. + if (HasValidSuggestText()) { + // Return true so that the edit sees the tab and commits the suggestion. return true; } if (keyword_hint_view_->visible() && !event.IsShiftDown()) { @@ -1027,6 +1028,12 @@ bool LocationBarView::SkipDefaultKeyEventProcessing( return true; } +#if !defined(USE_AURA) + // If the caret is not at the end, then tab moves the caret to the end. + if (!views_omnibox && !GetOmniboxViewWin()->IsCaretAtEnd()) + return true; +#endif + // Tab while showing instant commits instant immediately. // Return true so that focus traversal isn't attempted. The edit ends // up doing nothing in this case. @@ -1035,7 +1042,7 @@ bool LocationBarView::SkipDefaultKeyEventProcessing( } #if !defined(USE_AURA) - if (!views::Widget::IsPureViews()) + if (!views_omnibox) return GetOmniboxViewWin()->SkipDefaultKeyEventProcessing(event); #endif NOTIMPLEMENTED(); diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc index c711b86..3710bfd 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc +++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc @@ -264,18 +264,10 @@ bool OmniboxViewViews::HandleAfterKeyEvent(const views::KeyEvent& event, handled = true; } else if (!handled && event.key_code() == ui::VKEY_TAB && + !event.IsShiftDown() && !event.IsControlDown()) { - if (model_->is_keyword_hint() && !event.IsShiftDown()) { + if (model_->is_keyword_hint()) { handled = model_->AcceptKeyword(); - } else if (model_->popup_model()->IsOpen()) { - if (event.IsShiftDown() && - model_->popup_model()->selected_line_state() == - AutocompletePopupModel::KEYWORD) { - model_->ClearKeyword(GetText()); - } else { - model_->OnUpOrDownKeyPressed(event.IsShiftDown() ? -1 : 1); - } - handled = true; } else { string16::size_type start = 0; string16::size_type end = 0; @@ -469,14 +461,14 @@ void OmniboxViewViews::SetUserText(const string16& text, const string16& display_text, bool update_popup) { model_->SetUserText(text); - SetWindowTextAndCaretPos(display_text, display_text.length(), update_popup, - true); + SetWindowTextAndCaretPos(display_text, display_text.length()); + if (update_popup) + UpdatePopup(); + TextChanged(); } void OmniboxViewViews::SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) { + size_t caret_pos) { const ui::Range range(caret_pos, caret_pos); SetTextAndSelectedRange(text, range); } @@ -552,7 +544,8 @@ void OmniboxViewViews::OnTemporaryTextMaybeChanged( if (save_original_selection) textfield_->GetSelectedRange(&saved_temporary_selection_); - SetWindowTextAndCaretPos(display_text, display_text.length(), false, true); + SetWindowTextAndCaretPos(display_text, display_text.length()); + TextChanged(); } bool OmniboxViewViews::OnInlineAutocompleteTextMaybeChanged( diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.h b/chrome/browser/ui/views/omnibox/omnibox_view_views.h index 07a2625..e4a9b18 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_views.h +++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.h @@ -113,9 +113,7 @@ class OmniboxViewViews const string16& display_text, bool update_popup) OVERRIDE; virtual void SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) OVERRIDE; + size_t caret_pos) OVERRIDE; virtual void SetForcedQuery() OVERRIDE; virtual bool IsSelectAll() OVERRIDE; virtual bool DeleteAtEndPressed() OVERRIDE; diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc index d370a38..f0ab505 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc +++ b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc @@ -689,22 +689,16 @@ void OmniboxViewWin::SetUserText(const string16& text, ScopedFreeze freeze(this, GetTextObjectModel()); model_->SetUserText(text); saved_selection_for_focus_change_.cpMin = -1; - SetWindowTextAndCaretPos(display_text, display_text.length(), update_popup, - true); + SetWindowTextAndCaretPos(display_text, display_text.length()); + if (update_popup) + UpdatePopup(); + TextChanged(); } void OmniboxViewWin::SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) { + size_t caret_pos) { SetWindowText(text.c_str()); PlaceCaretAt(caret_pos); - - if (update_popup) - UpdatePopup(); - - if (notify_text_changed) - TextChanged(); } void OmniboxViewWin::SetForcedQuery() { @@ -744,8 +738,9 @@ void OmniboxViewWin::SelectAll(bool reversed) { void OmniboxViewWin::RevertAll() { ScopedFreeze freeze(this, GetTextObjectModel()); ClosePopup(); - saved_selection_for_focus_change_.cpMin = -1; model_->Revert(); + saved_selection_for_focus_change_.cpMin = -1; + TextChanged(); } void OmniboxViewWin::UpdatePopup() { @@ -840,7 +835,8 @@ void OmniboxViewWin::OnTemporaryTextMaybeChanged(const string16& display_text, // text and then arrowed to another entry with the same text, we'd still want // to move the caret. ScopedFreeze freeze(this, GetTextObjectModel()); - SetWindowTextAndCaretPos(display_text, display_text.length(), false, true); + SetWindowTextAndCaretPos(display_text, display_text.length()); + TextChanged(); } bool OmniboxViewWin::OnInlineAutocompleteTextMaybeChanged( @@ -2091,17 +2087,17 @@ bool OmniboxViewWin::OnKeyDownOnlyWritable(TCHAR key, } case VK_TAB: { - const bool shift_pressed = GetKeyState(VK_SHIFT) < 0; - if (model_->is_keyword_hint() && !shift_pressed) { + if (model_->is_keyword_hint()) { // Accept the keyword. ScopedFreeze freeze(this, GetTextObjectModel()); model_->AcceptKeyword(); - } else if (shift_pressed && - model_->popup_model()->selected_line_state() == - AutocompletePopupModel::KEYWORD) { - model_->ClearKeyword(GetText()); + } else if (!IsCaretAtEnd()) { + ScopedFreeze freeze(this, GetTextObjectModel()); + OnBeforePossibleChange(); + PlaceCaretAt(GetTextLength()); + OnAfterPossibleChange(); } else { - model_->OnUpOrDownKeyPressed(shift_pressed ? -count : count); + model_->CommitSuggestedText(true); } return true; } diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_win.h b/chrome/browser/ui/views/omnibox/omnibox_view_win.h index 27ac288..386179d 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_win.h +++ b/chrome/browser/ui/views/omnibox/omnibox_view_win.h @@ -110,9 +110,7 @@ class OmniboxViewWin const string16& display_text, bool update_popup) OVERRIDE; virtual void SetWindowTextAndCaretPos(const string16& text, - size_t caret_pos, - bool update_popup, - bool notify_text_changed) OVERRIDE; + size_t caret_pos) OVERRIDE; virtual void SetForcedQuery() OVERRIDE; virtual bool IsSelectAll() OVERRIDE; virtual bool DeleteAtEndPressed() OVERRIDE; diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 661711a..b4bcac5 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1245,6 +1245,7 @@ 'browser/accessibility/browser_accessibility_mac_unittest.mm', 'browser/app_controller_mac_unittest.mm', 'browser/autocomplete/autocomplete_edit_unittest.cc', + 'browser/autocomplete/autocomplete_popup_model_unittest.cc', 'browser/autocomplete/autocomplete_result_unittest.cc', 'browser/autocomplete/autocomplete_unittest.cc', 'browser/autocomplete/builtin_provider_unittest.cc', diff --git a/chrome/test/base/in_process_browser_test.cc b/chrome/test/base/in_process_browser_test.cc index 59111a4..89d0e56 100644 --- a/chrome/test/base/in_process_browser_test.cc +++ b/chrome/test/base/in_process_browser_test.cc @@ -242,7 +242,6 @@ Browser* InProcessBrowserTest::CreateBrowserForPopup(Profile* profile) { } void InProcessBrowserTest::AddBlankTabAndShow(Browser* browser) { - OnBeforeShowBrowser(browser); ui_test_utils::WindowedNotificationObserver observer( content::NOTIFICATION_LOAD_STOP, content::NotificationService::AllSources()); diff --git a/chrome/test/base/in_process_browser_test.h b/chrome/test/base/in_process_browser_test.h index 2b872e2..56983a6 100644 --- a/chrome/test/base/in_process_browser_test.h +++ b/chrome/test/base/in_process_browser_test.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -142,10 +142,6 @@ class InProcessBrowserTest : public BrowserTestBase { // for the tab to finish loading, and shows the browser. Browser* CreateBrowserForPopup(Profile* profile); - // Allows configuration of the browser object before it is shown and the - // message pump starts running. - virtual void OnBeforeShowBrowser(Browser* browser) {} - // Called from the various CreateBrowser methods to add a blank tab, wait for // the navigation to complete, and show the browser's window. void AddBlankTabAndShow(Browser* browser); diff --git a/ui/base/keycodes/keyboard_code_conversion_mac.mm b/ui/base/keycodes/keyboard_code_conversion_mac.mm index 3d9e0a6..c990255 100644 --- a/ui/base/keycodes/keyboard_code_conversion_mac.mm +++ b/ui/base/keycodes/keyboard_code_conversion_mac.mm @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -31,7 +31,6 @@ bool operator<(const KeyCodeMap& a, const KeyCodeMap& b) { const KeyCodeMap kKeyCodesMap[] = { { VKEY_BACK /* 0x08 */, kVK_Delete, kBackspaceCharCode }, { VKEY_TAB /* 0x09 */, kVK_Tab, kTabCharCode }, - { VKEY_BACKTAB /* 0x0A */, 0x21E4, '\031' }, { VKEY_CLEAR /* 0x0C */, kVK_ANSI_KeypadClear, kClearCharCode }, { VKEY_RETURN /* 0x0D */, kVK_Return, kReturnCharCode }, { VKEY_SHIFT /* 0x10 */, kVK_Shift, 0 }, diff --git a/ui/base/keycodes/keyboard_codes_posix.h b/ui/base/keycodes/keyboard_codes_posix.h index c8b3bfe..023701e 100644 --- a/ui/base/keycodes/keyboard_codes_posix.h +++ b/ui/base/keycodes/keyboard_codes_posix.h @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -37,7 +37,6 @@ namespace ui { typedef enum { VKEY_BACK = 0x08, VKEY_TAB = 0x09, - VKEY_BACKTAB = 0x0A, VKEY_CLEAR = 0x0C, VKEY_RETURN = 0x0D, VKEY_SHIFT = 0x10, |