 Chromium Code Reviews
 Chromium Code Reviews Issue 13141002:
  Use Instant suggested match type for Instant temporary text.  (Closed) 
  Base URL: svn://svn.chromium.org/chrome/trunk/src
    
  
    Issue 13141002:
  Use Instant suggested match type for Instant temporary text.  (Closed) 
  Base URL: svn://svn.chromium.org/chrome/trunk/src| Index: chrome/browser/ui/omnibox/omnibox_edit_model.cc | 
| diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc | 
| index ea333289fd304a75704a4457a59cdeede09981ea..eec6b31aa22767f3249980a88b6144b939181779 100644 | 
| --- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc | 
| +++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc | 
| @@ -122,6 +122,7 @@ OmniboxEditModel::OmniboxEditModel(OmniboxView* view, | 
| just_deleted_text_(false), | 
| has_temporary_text_(false), | 
| is_temporary_text_set_by_instant_(false), | 
| + is_instant_temporary_text_a_search_query_(false), | 
| paste_state_(NONE), | 
| control_key_state_(UP), | 
| is_keyword_hint_(false), | 
| @@ -219,6 +220,7 @@ void OmniboxEditModel::SetUserText(const string16& text) { | 
| paste_state_ = NONE; | 
| has_temporary_text_ = false; | 
| is_temporary_text_set_by_instant_ = false; | 
| + is_instant_temporary_text_a_search_query_ = false; | 
| } | 
| void OmniboxEditModel::FinalizeInstantQuery(const string16& input_text, | 
| @@ -251,10 +253,7 @@ void OmniboxEditModel::SetInstantSuggestion( | 
| case INSTANT_COMPLETE_NEVER: { | 
| DCHECK_EQ(INSTANT_SUGGESTION_SEARCH, suggestion.type); | 
| view_->SetInstantSuggestion(suggestion.text); | 
| - SearchProvider* search_provider = | 
| - autocomplete_controller_->search_provider(); | 
| - if (search_provider) | 
| - search_provider->ClearInstantSuggestion(); | 
| + autocomplete_controller_->search_provider()->ClearInstantSuggestion(); | 
| break; | 
| } | 
| @@ -263,6 +262,8 @@ void OmniboxEditModel::SetInstantSuggestion( | 
| view_->SetInstantSuggestion(string16()); | 
| has_temporary_text_ = true; | 
| is_temporary_text_set_by_instant_ = true; | 
| + is_instant_temporary_text_a_search_query_ = | 
| + suggestion.type == INSTANT_SUGGESTION_SEARCH; | 
| // Instant suggestions are never a keyword. | 
| keyword_ = string16(); | 
| is_keyword_hint_ = false; | 
| @@ -477,6 +478,7 @@ void OmniboxEditModel::Revert() { | 
| is_keyword_hint_ = false; | 
| has_temporary_text_ = false; | 
| is_temporary_text_set_by_instant_ = false; | 
| + is_instant_temporary_text_a_search_query_ = false; | 
| view_->SetWindowTextAndCaretPos(permanent_text_, | 
| has_focus() ? permanent_text_.length() : 0, | 
| false, true); | 
| @@ -757,6 +759,7 @@ bool OmniboxEditModel::AcceptKeyword() { | 
| bool save_original_selection = !has_temporary_text_; | 
| has_temporary_text_ = true; | 
| is_temporary_text_set_by_instant_ = false; | 
| + is_instant_temporary_text_a_search_query_ = false; | 
| view_->OnTemporaryTextMaybeChanged( | 
| DisplayTextFromUserText(CurrentMatch().fill_into_edit), | 
| save_original_selection, true); | 
| @@ -893,6 +896,7 @@ void OmniboxEditModel::OnControlKeyChanged(bool pressed) { | 
| InternalSetUserText(UserTextFromDisplayText(view_->GetText())); | 
| has_temporary_text_ = false; | 
| is_temporary_text_set_by_instant_ = false; | 
| + is_instant_temporary_text_a_search_query_ = false; | 
| } | 
| if ((old_state != DOWN_WITH_CHANGE) && popup_->IsOpen()) { | 
| // Autocomplete history provider results may change, so refresh the | 
| @@ -961,6 +965,7 @@ void OmniboxEditModel::OnPopupDataChanged( | 
| // Save the original selection and URL so it can be reverted later. | 
| has_temporary_text_ = true; | 
| is_temporary_text_set_by_instant_ = false; | 
| + is_instant_temporary_text_a_search_query_ = false; | 
| original_url_ = *destination_for_temporary_text_change; | 
| inline_autocomplete_text_.clear(); | 
| } | 
| @@ -1066,6 +1071,7 @@ bool OmniboxEditModel::OnAfterPossibleChange(const string16& old_text, | 
| InternalSetUserText(UserTextFromDisplayText(new_text)); | 
| has_temporary_text_ = false; | 
| is_temporary_text_set_by_instant_ = false; | 
| + is_instant_temporary_text_a_search_query_ = false; | 
| // Track when the user has deleted text so we won't allow inline | 
| // autocomplete. | 
| @@ -1159,6 +1165,7 @@ void OmniboxEditModel::OnResultChanged(bool default_match_changed) { | 
| InternalSetUserText(UserTextFromDisplayText(view_->GetText())); | 
| has_temporary_text_ = false; | 
| is_temporary_text_set_by_instant_ = false; | 
| + is_instant_temporary_text_a_search_query_ = false; | 
| OnPopupBoundsChanged(gfx::Rect()); | 
| delegate_->NotifySearchTabHelper(user_input_in_progress_, !in_revert_); | 
| } | 
| @@ -1226,9 +1233,32 @@ void OmniboxEditModel::InfoForCurrentSelection(AutocompleteMatch* match, | 
| void OmniboxEditModel::GetInfoForCurrentText(AutocompleteMatch* match, | 
| GURL* alternate_nav_url) const { | 
| // If there's temporary text and it has been set by Instant, we won't find it | 
| - // in the popup model, so classify the text anew. | 
| - if ((popup_->IsOpen() || query_in_progress()) && | 
| - !is_temporary_text_set_by_instant_) { | 
| + // in the popup model, so create the match based on the type Instant told us | 
| + // (SWYT for queries and UWYT for URLs). We do this instead of classifying the | 
| + // text ourselves because the text may look like a URL, but Instant may expect | 
| + // it to be a search (e.g.: a query for "amazon.com"). | 
| + if (is_temporary_text_set_by_instant_) { | 
| + const string16& text = view_->GetText(); | 
| + if (is_instant_temporary_text_a_search_query_) { | 
| + // Only the transition and destination_url of the match will be used (to | 
| + // either navigate to the URL or to let Instant commit its preview). The | 
| + // match won't be used for logging, displaying in the dropdown, etc. So, | 
| + // it's okay to pass in mostly bogus params (such as relevance = 0). | 
| + // TODO(sreeram): Always using NO_SUGGESTIONS_AVAILABLE is wrong when | 
| + // Instant is using the local fallback overlay. Fix. | 
| + autocomplete_controller_->search_provider()->GetSearchWhatYouTypedMatch( | 
| + text, text, 0, TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, false, | 
| + match); | 
| + } else { | 
| + // Only the destination_url of the match will be used (to navigate to the | 
| 
Peter Kasting
2013/04/12 00:29:01
Nit: Factor something like this comment (but that
 
sreeram
2013/04/12 22:36:18
Done.
 | 
| + // URL). The match won't be used for logging, displaying in the dropdown, | 
| + // etc. So it's okay to pass in mostly bogus params. | 
| + AutocompleteInput input(text, string16::npos, string16(), GURL(), false, | 
| + false, false, AutocompleteInput::BEST_MATCH); | 
| + *match = HistoryURLProvider::SuggestExactInput( | 
| + autocomplete_controller_->history_url_provider(), input, false); | 
| + } | 
| + } else if (popup_->IsOpen() || query_in_progress()) { | 
| InfoForCurrentSelection(match, alternate_nav_url); | 
| } else { | 
| AutocompleteClassifierFactory::GetForProfile(profile_)->Classify( | 
| @@ -1245,6 +1275,7 @@ void OmniboxEditModel::RevertTemporaryText(bool revert_popup) { | 
| just_deleted_text_ = false; | 
| has_temporary_text_ = false; | 
| is_temporary_text_set_by_instant_ = false; | 
| + is_instant_temporary_text_a_search_query_ = false; | 
| InstantController* instant = controller_->GetInstant(); | 
| if (instant && notify_instant) { | 
| @@ -1315,9 +1346,9 @@ bool OmniboxEditModel::DoInstant(const AutocompleteMatch& match) { | 
| if (!instant || in_revert_) | 
| return false; | 
| - // Don't call Update() if the change is a result of a | 
| + // Don't call Update() if the change is the result of an | 
| // INSTANT_COMPLETE_REPLACE instant suggestion. | 
| - if (has_temporary_text_ && is_temporary_text_set_by_instant_) | 
| + if (is_temporary_text_set_by_instant_) | 
| return false; | 
| // The two pieces of text we want to send Instant, viz., what the user has |