Omnibox refactor, moved OnResultChanged to OmniboxController

chrome/browser/ui/omnibox/omnibox_edit_model.cc

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 78cd2e34a6f8975e6f2c1021b84f10003d25539e..92f8ded590168c2722866248af445e5223d7561e 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -120,7 +120,6 @@ OmniboxEditModel::OmniboxEditModel(OmniboxView* view,
                                    OmniboxEditController* controller,
                                    Profile* profile)
     : view_(view),
-      popup_(NULL),
       controller_(controller),
       focus_state_(OMNIBOX_FOCUS_NONE),
       user_input_in_progress_(false),
@@ -233,7 +232,7 @@ void OmniboxEditModel::FinalizeInstantQuery(const string16& input_text,
     view_->SetWindowTextAndCaretPos(final_text, final_text.length(), false,
         false);
     view_->OnAfterPossibleChange();
-  } else if (popup_->IsOpen()) {
+  } else if (popup_model()->IsOpen()) {
     SearchProvider* search_provider =
         autocomplete_controller()->search_provider();
     // There may be no providers during testing; guard against that.
@@ -254,7 +253,10 @@ void OmniboxEditModel::SetInstantSuggestion(
     case INSTANT_COMPLETE_NEVER: {
       DCHECK_EQ(INSTANT_SUGGESTION_SEARCH, suggestion.type);
       view_->SetInstantSuggestion(suggestion.text);
-      autocomplete_controller()->search_provider()->ClearInstantSuggestion();
+      SearchProvider* search_provider =
+          autocomplete_controller()->search_provider();
+      if (search_provider)
+        search_provider->ClearInstantSuggestion();
       break;
     }
 
@@ -342,7 +344,7 @@ void OmniboxEditModel::OnChanged() {
         delegate_->DoPrerender(current_match);
       break;
     case AutocompleteActionPredictor::ACTION_PRECONNECT:
-      DoPreconnect(current_match);
+      omnibox_controller_->DoPreconnect(current_match);
       break;
     case AutocompleteActionPredictor::ACTION_NONE:
       break;
@@ -381,7 +383,7 @@ bool OmniboxEditModel::UseVerbatimInstant() {
   // we use a separated widget for displaying the Instant suggest (so it doesn't
   // interfere with #2). So, we don't need to care about the value of
   // input.prevent_inline_autocomplete() here.
-  return view_->DeleteAtEndPressed() || popup_->selected_line() != 0 ||
+  return view_->DeleteAtEndPressed() || popup_model()->selected_line() != 0 ||
       just_deleted_text_;
 }
 
@@ -494,10 +496,10 @@ void OmniboxEditModel::Revert() {
 void OmniboxEditModel::StartAutocomplete(
     bool has_selected_text,
     bool prevent_inline_autocomplete) const {
-  ClearPopupKeywordMode();
+  omnibox_controller_->ClearPopupKeywordMode();
 
   bool keyword_is_selected = KeywordIsSelected();
-  popup_->SetHoveredLine(OmniboxPopupModel::kNoMatch);
+  popup_model()->SetHoveredLine(OmniboxPopupModel::kNoMatch);
 
   size_t cursor_position;
   if (inline_autocomplete_text_.empty()) {
@@ -639,14 +641,14 @@ void OmniboxEditModel::OpenMatch(const AutocompleteMatch& match,
                                  size_t index) {
   // We only care about cases where there is a selection (i.e. the popup is
   // open).
-  if (popup_->IsOpen()) {
+  if (popup_model()->IsOpen()) {
     const base::TimeTicks& now(base::TimeTicks::Now());
     // TODO(sreeram): Handle is_temporary_text_set_by_instant_ correctly.
     AutocompleteLog log(
         autocomplete_controller()->input().text(),
         just_deleted_text_,
         autocomplete_controller()->input().type(),
-        popup_->selected_line(),
+        popup_model()->selected_line(),
         -1,  // don't yet know tab ID; set later if appropriate
         delegate_->CurrentPageExists() ? ClassifyPage(delegate_->GetURL()) :
             metrics::OmniboxEventProto_PageClassification_OTHER,
@@ -765,8 +767,8 @@ bool OmniboxEditModel::AcceptKeyword(EnteredKeywordModeMethod entered_method) {
   autocomplete_controller()->Stop(false);
   is_keyword_hint_ = false;
 
-  if (popup_->IsOpen())
-    popup_->SetSelectedLineState(OmniboxPopupModel::KEYWORD);
+  if (popup_model()->IsOpen())
+    popup_model()->SetSelectedLineState(OmniboxPopupModel::KEYWORD);
   else
     StartAutocomplete(false, true);
 
@@ -788,15 +790,23 @@ bool OmniboxEditModel::AcceptKeyword(EnteredKeywordModeMethod entered_method) {
   return true;
 }
 
+void OmniboxEditModel::AcceptTemporaryTextAsUserText() {
+  InternalSetUserText(UserTextFromDisplayText(view_->GetText()));
+  has_temporary_text_ = false;
+  is_temporary_text_set_by_instant_ = false;
+  OnPopupBoundsChanged(gfx::Rect());
+  delegate_->NotifySearchTabHelper(user_input_in_progress_, !in_revert_);
+}
+
 void OmniboxEditModel::ClearKeyword(const string16& visible_text) {
   autocomplete_controller()->Stop(false);
-  ClearPopupKeywordMode();
+  omnibox_controller_->ClearPopupKeywordMode();
 
   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()) {
+  if (just_deleted_text_ || !visible_text.empty() || !popup_model()->IsOpen()) {
     view_->OnBeforePossibleChange();
     view_->SetWindowTextAndCaretPos(window_text.c_str(), keyword_.length(),
         false, false);
@@ -813,10 +823,6 @@ void OmniboxEditModel::ClearKeyword(const string16& visible_text) {
   }
 }
 
-const AutocompleteResult& OmniboxEditModel::result() const {
-  return autocomplete_controller()->result();
-}
-
 void OmniboxEditModel::OnSetFocus(bool control_down) {
   // If the omnibox lost focus while the caret was hidden and then regained
   // focus, OnSetFocus() is called and should restore visibility. Note that
@@ -918,7 +924,7 @@ void OmniboxEditModel::OnControlKeyChanged(bool pressed) {
       is_temporary_text_set_by_instant_ = false;
       is_instant_temporary_text_a_search_query_ = false;
     }
-    if ((old_state != DOWN_WITH_CHANGE) && popup_->IsOpen()) {
+    if ((old_state != DOWN_WITH_CHANGE) && popup_model()->IsOpen()) {
       // Autocomplete history provider results may change, so refresh the
       // popup.  This will force user_input_in_progress_ to true, but if the
       // popup is open, that should have already been the case.
@@ -929,7 +935,7 @@ void OmniboxEditModel::OnControlKeyChanged(bool pressed) {
 
 void OmniboxEditModel::OnUpOrDownKeyPressed(int count) {
   // NOTE: This purposefully doesn't trigger any code that resets paste_state_.
-  if (!popup_->IsOpen()) {
+  if (!popup_model()->IsOpen()) {
     if (!query_in_progress()) {
       // The popup is neither open nor working on a query already.  So, start an
       // autocomplete query for the current text.  This also sets
@@ -957,7 +963,7 @@ void OmniboxEditModel::OnUpOrDownKeyPressed(int count) {
     } else {
       // The popup is open, so the user should be able to interact with it
       // normally.
-      popup_->Move(count);
+      popup_model()->Move(count);
     }
   }
 }
@@ -967,6 +973,7 @@ void OmniboxEditModel::OnPopupDataChanged(
     GURL* destination_for_temporary_text_change,
     const string16& keyword,
     bool is_keyword_hint) {
+  DLOG(ERROR) << "A";

[Mathieu, 2013/05/03 19:09:02]

remove

[beaudoin, 2013/05/04 03:06:51]

/facepalm. :)

Done.

   // Update keyword/hint-related local state.
   bool keyword_state_changed = (keyword_ != keyword) ||
       ((is_keyword_hint_ != is_keyword_hint) && !keyword.empty());
@@ -978,6 +985,7 @@ void OmniboxEditModel::OnPopupDataChanged(
     DCHECK(!keyword_.empty() || !is_keyword_hint_);
   }
 
+  DLOG(ERROR) << "B";
   // Handle changes to temporary text.
   if (destination_for_temporary_text_change != NULL) {
     const bool save_original_selection = !has_temporary_text_;
@@ -989,6 +997,7 @@ void OmniboxEditModel::OnPopupDataChanged(
       original_url_ = *destination_for_temporary_text_change;
       inline_autocomplete_text_.clear();
     }
+    DLOG(ERROR) << "C";
     if (control_key_state_ == DOWN_WITHOUT_CHANGE) {
       // Arrowing around the popup cancels control-enter.
       control_key_state_ = DOWN_WITH_CHANGE;
@@ -999,10 +1008,12 @@ void OmniboxEditModel::OnPopupDataChanged(
       // pressed, even though maybe it isn't any more.  There is no obvious
       // right answer here :(
     }
+    DLOG(ERROR) << "D";
     view_->OnTemporaryTextMaybeChanged(DisplayTextFromUserText(text),
                                        save_original_selection, true);
     return;
   }
+  DLOG(ERROR) << "E: " << keyword_state_changed;
 
   bool call_controller_onchanged = true;
   inline_autocomplete_text_ = text;
@@ -1023,11 +1034,13 @@ void OmniboxEditModel::OnPopupDataChanged(
     // temporary text back to a default match that's a keyword search, but in
     // that case the RevertTemporaryText() call below will reset the caret or
     // selection correctly so the caret positioning we do here won't matter.
+    DLOG(ERROR) << "F";
     view_->SetWindowTextAndCaretPos(DisplayTextFromUserText(user_text_), 0,
                                                             false, false);
   } else if (view_->OnInlineAutocompleteTextMaybeChanged(
              DisplayTextFromUserText(user_text_ + inline_autocomplete_text_),
              DisplayTextFromUserText(user_text_).length())) {
+    DLOG(ERROR) << "G";
     call_controller_onchanged = false;
   }
 
@@ -1036,11 +1049,13 @@ void OmniboxEditModel::OnPopupDataChanged(
   // non-NULL). This can happen when deleting the selected item in the popup.
   // In this case, we've already reverted the popup to the default match, so we
   // need to revert ourselves as well.
+  DLOG(ERROR) << "H";
   if (has_temporary_text_) {
     RevertTemporaryText(false);
     call_controller_onchanged = false;
   }
 
+  DLOG(ERROR) << "I";
   // We need to invoke OnChanged in case the destination url changed (as could
   // happen when control is toggled).
   if (call_controller_onchanged)
@@ -1078,7 +1093,7 @@ bool OmniboxEditModel::OnAfterPossibleChange(const string16& old_text,
   if ((text_differs || selection_differs) &&
       (control_key_state_ == DOWN_WITHOUT_CHANGE)) {
     control_key_state_ = DOWN_WITH_CHANGE;
-    if (!text_differs && !popup_->IsOpen())
+    if (!text_differs && !popup_model()->IsOpen())
       return false;  // Don't open the popup for no reason.
   } else if (!user_text_changed) {
     return false;
@@ -1150,56 +1165,6 @@ void OmniboxEditModel::OnPopupBoundsChanged(const gfx::Rect& bounds) {
 }
 
 void OmniboxEditModel::OnResultChanged(bool default_match_changed) {
-  const bool was_open = popup_->IsOpen();
-  if (default_match_changed) {
-    string16 inline_autocomplete_text;
-    string16 keyword;
-    bool is_keyword_hint = false;
-    const AutocompleteResult& result = this->result();
-    const AutocompleteResult::const_iterator match(result.default_match());
-    if (match != result.end()) {
-      if ((match->inline_autocomplete_offset != string16::npos) &&
-          (match->inline_autocomplete_offset <
-           match->fill_into_edit.length())) {
-        inline_autocomplete_text =
-            match->fill_into_edit.substr(match->inline_autocomplete_offset);
-      }
-
-      if (!prerender::IsOmniboxEnabled(profile_))
-        DoPreconnect(*match);
-
-      // We could prefetch the alternate nav URL, if any, but because there
-      // 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.
-
-      match->GetKeywordUIState(profile_, &keyword, &is_keyword_hint);
-    }
-
-    popup_->OnResultChanged();
-    OnPopupDataChanged(inline_autocomplete_text, NULL, keyword,
-                       is_keyword_hint);
-  } else {
-    popup_->OnResultChanged();
-  }
-
-  if (popup_->IsOpen()) {
-    OnPopupBoundsChanged(popup_->view()->GetTargetBounds());
-
-    InstantController* instant = controller_->GetInstant();
-    if (instant && !in_revert_) {
-      instant->HandleAutocompleteResults(
-          *autocomplete_controller()->providers());
-    }
-  } else if (was_open) {
-    // Accepts the temporary text as the user text, because it makes little
-    // sense to have temporary text when the popup is closed.
-    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_);
-  }
 }
 
 bool OmniboxEditModel::query_in_progress() const {
@@ -1216,12 +1181,6 @@ bool OmniboxEditModel::KeywordIsSelected() const {
   return !is_keyword_hint_ && !keyword_.empty();
 }
 
-void OmniboxEditModel::ClearPopupKeywordMode() const {
-  if (popup_->IsOpen() &&
-      popup_->selected_line_state() == OmniboxPopupModel::KEYWORD)
-    popup_->SetSelectedLineState(OmniboxPopupModel::NORMAL);
-}
-
 string16 OmniboxEditModel::DisplayTextFromUserText(const string16& text) const {
   return KeywordIsSelected() ?
       KeywordProvider::SplitReplacementStringFromInput(text, false) : text;
@@ -1231,74 +1190,12 @@ string16 OmniboxEditModel::UserTextFromDisplayText(const string16& text) const {
   return KeywordIsSelected() ? (keyword_ + char16(' ') + text) : text;
 }
 
-void OmniboxEditModel::InfoForCurrentSelection(AutocompleteMatch* match,
-                                               GURL* alternate_nav_url) const {
-  DCHECK(match != NULL);
-  const AutocompleteResult& result = this->result();
-  if (!autocomplete_controller()->done()) {
-    // It's technically possible for |result| to be empty if no provider returns
-    // a synchronous result but the query has not completed synchronously;
-    // pratically, however, that should never actually happen.
-    if (result.empty())
-      return;
-    // The user cannot have manually selected a match, or the query would have
-    // stopped.  So the default match must be the desired selection.
-    *match = *result.default_match();
-  } else {
-    CHECK(popup_->IsOpen());
-    // If there are no results, the popup should be closed (so we should have
-    // failed the CHECK above), and URLsForDefaultMatch() should have been
-    // called instead.
-    CHECK(!result.empty());
-    CHECK(popup_->selected_line() < result.size());
-    *match = result.match_at(popup_->selected_line());
-  }
-  if (alternate_nav_url && popup_->manually_selected_match().empty())
-    *alternate_nav_url = result.alternate_nav_url();
-}
-
 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 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();
-    AutocompleteInput input(text, string16::npos, string16(), GURL(), false,
-                            false, false, AutocompleteInput::BEST_MATCH);
-    // Only the destination_url and the transition of the match will be be used
-    // (to either navigate to the URL or 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.
-    if (is_instant_temporary_text_a_search_query_) {
-      const TemplateURL* default_provider =
-          TemplateURLServiceFactory::GetForProfile(profile_)->
-              GetDefaultSearchProvider();
-      if (default_provider && default_provider->SupportsReplacement()) {
-        *match = SearchProvider::CreateSearchSuggestion(profile_,
-            autocomplete_controller()->search_provider(), input, text, text, 0,
-            AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
-            TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, false,
-            default_provider->keyword());
-      } else {
-        // Can't create a new search match. Leave |match| as is, with an
-        // invalid destination_url. This shouldn't ever happen. For example,
-        // even if a group policy update in the midst of interacting with
-        // Instant leaves us without a valid search provider, Instant should've
-        // observed the update and reset |is_temporary_text_set_by_instant_|,
-        // so we still shouldn't get here. However, as protection against the
-        // unknowns and Instant regressions, we simply return an invalid match
-        // instead of crashing (hence no DCHECK).
-      }
-    } else {
-      *match = HistoryURLProvider::SuggestExactInput(
-        autocomplete_controller()->history_url_provider(), input, false);
-    }
-  } else if (popup_->IsOpen() || query_in_progress()) {
+  // in the popup model, so classify the text anew.
+  if ((popup_model()->IsOpen() || query_in_progress()) &&
+      !is_temporary_text_set_by_instant_) {
     InfoForCurrentSelection(match, alternate_nav_url);
   } else {
     AutocompleteClassifierFactory::GetForProfile(profile_)->Classify(
@@ -1336,7 +1233,7 @@ void OmniboxEditModel::RevertTemporaryText(bool revert_popup) {
                       user_text_ + inline_autocomplete_text_);
   }
   if (revert_popup)
-    popup_->ResetToDefaultMatch();
+    popup_model()->ResetToDefaultMatch();
   view_->OnRevertTemporaryText();
 }
 
@@ -1407,26 +1304,10 @@ bool OmniboxEditModel::DoInstant(const AutocompleteMatch& match) {
   view_->GetSelectionBounds(&start, &end);
 
   return instant->Update(match, user_text, full_text, start, end,
-      UseVerbatimInstant(), user_input_in_progress_, popup_->IsOpen(),
+      UseVerbatimInstant(), user_input_in_progress_, popup_model()->IsOpen(),
       in_escape_handler_, KeywordIsSelected());
 }
 
-void OmniboxEditModel::DoPreconnect(const AutocompleteMatch& match) {
-  if (!match.destination_url.SchemeIs(extensions::kExtensionScheme)) {
-    // Warm up DNS Prefetch cache, or preconnect to a search service.
-    UMA_HISTOGRAM_ENUMERATION("Autocomplete.MatchType", match.type,
-                              AutocompleteMatch::NUM_TYPES);
-    if (profile_->GetNetworkPredictor()) {
-      profile_->GetNetworkPredictor()->AnticipateOmniboxUrl(
-          match.destination_url,
-          AutocompleteActionPredictor::IsPreconnectable(match));
-    }
-    // We could prefetch the alternate nav URL, if any, but because there
-    // 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.
-  }
-}
-
 //  static
 bool OmniboxEditModel::IsSpaceCharForAcceptingKeyword(wchar_t c) {
   switch (c) {