Ensures the autocomplete_controller input is saved and restored when changing tabs.

chrome/browser/ui/omnibox/omnibox_edit_model.cc

 // Copyright 2012 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/ui/omnibox/omnibox_edit_model.h"
 
 #include <string>
 
 #include "base/auto_reset.h"
 #include "base/format_macros.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
-#include "chrome/browser/autocomplete/autocomplete_input.h"
 #include "chrome/browser/autocomplete/autocomplete_provider.h"
 #include "chrome/browser/autocomplete/extension_app_provider.h"
 #include "chrome/browser/autocomplete/history_url_provider.h"
 #include "chrome/browser/autocomplete/keyword_provider.h"
 #include "chrome/browser/autocomplete/search_provider.h"
 #include "chrome/browser/bookmarks/bookmark_stats.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/extensions/api/omnibox/omnibox_api.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
@@ skipping 137 matching lines @@
 
 // OmniboxEditModel::State ----------------------------------------------------
 
 OmniboxEditModel::State::State(bool user_input_in_progress,
                                const base::string16& user_text,
                                const base::string16& gray_text,
                                const base::string16& keyword,
                                bool is_keyword_hint,
                                bool url_replacement_enabled,
                                OmniboxFocusState focus_state,
-                               FocusSource focus_source)
+                               FocusSource focus_source,
+                               const AutocompleteInput& autocomplete_input)
     : user_input_in_progress(user_input_in_progress),
       user_text(user_text),
       gray_text(gray_text),
       keyword(keyword),
       is_keyword_hint(is_keyword_hint),
       url_replacement_enabled(url_replacement_enabled),
       focus_state(focus_state),
-      focus_source(focus_source) {
+      focus_source(focus_source),
+      autocomplete_input(autocomplete_input) {
 }
 
 OmniboxEditModel::State::~State() {
 }
 
 
 // OmniboxEditModel -----------------------------------------------------------
 
 OmniboxEditModel::OmniboxEditModel(OmniboxView* view,
                                    OmniboxEditController* controller,
@@ skipping 13 matching lines @@
       in_revert_(false),
       allow_exact_keyword_match_(false) {
   omnibox_controller_.reset(new OmniboxController(this, profile));
   delegate_.reset(new OmniboxCurrentPageDelegateImpl(controller, profile));
 }
 
 OmniboxEditModel::~OmniboxEditModel() {
 }
 
 const OmniboxEditModel::State OmniboxEditModel::GetStateForTabSwitch() {
+  StopAutocomplete();

[beaudoin, 2014/04/24 19:52:20]

This is no longer strictly required, but it feels strange to leave the
AutocompleteController running after a tab switch.

Peter: What do you think?

[Peter Kasting, 2014/04/24 20:34:53]

I would stop the controller when the switch actually happens, i.e. in
RestoreState(), instead of here; as-is, it's perfectly legal for some interested
piece of code to just query this function without actually doing anything, in
which case we wouldn't want to stop the controller.

In turn, that stopping already happens via the call to
view_->RevertWithoutResettingSearchTermReplacement() in RestoreState().

Therefore, I'd remove this statement entirely.

[beaudoin, 2014/04/25 16:46:21]

Right, but the method's comment clearly says:
  // Returns the current state.  This assumes we are switching tabs, and changes
  // the internal state appropriately.

Done.

[Peter Kasting, 2014/04/25 17:59:07]

Hmm, you're right.  That's a reasonable argument for this, but it's probably an
even better argument for peering closely at both Save and Restore functions
together to see if they're duplicating work.  We shouldn't have reset/stop code
happening twice.

Because things work OK today I'd probably leave out the StopAutocomplete() call,
but if it bugs you you're welcome to look further at these functions separately.

+
   // Like typing, switching tabs "accepts" the temporary text as the user
   // text, because it makes little sense to have temporary text when the
   // popup is closed.
   if (user_input_in_progress_) {
     // Weird edge case to match other browsers: if the edit is empty, revert to
     // the permanent text (so the user can get it back easily) but select it (so
     // on switching back, typing will "just work").
     const base::string16 user_text(UserTextFromDisplayText(view_->GetText()));
     if (user_text.empty()) {
       base::AutoReset<bool> tmp(&in_revert_, true);
       view_->RevertAll();
       view_->SelectAll(true);
     } else {
       InternalSetUserText(user_text);
     }
   }
 
   UMA_HISTOGRAM_BOOLEAN("Omnibox.SaveStateForTabSwitch.UserInputInProgress",
                         user_input_in_progress_);
   return State(
       user_input_in_progress_, user_text_, view_->GetGrayTextAutocompletion(),
       keyword_, is_keyword_hint_,
       controller_->GetToolbarModel()->url_replacement_enabled(),
-      focus_state_, focus_source_);
+      focus_state_, focus_source_, input_);
 }
 
 void OmniboxEditModel::RestoreState(const State* state) {
   // We need to update the permanent text correctly and revert the view
   // regardless of whether there is saved state.
   controller_->GetToolbarModel()->set_url_replacement_enabled(
       !state || state->url_replacement_enabled);
   permanent_text_ = controller_->GetToolbarModel()->GetText();
   // Don't muck with the search term replacement state, as we've just set it
   // correctly.
   view_->RevertWithoutResettingSearchTermReplacement();
+  // Restore the autocomplete controller's input, or clear it if this is a new
+  // tab.
+  input_ = state ? state->autocomplete_input : AutocompleteInput();
   if (!state)
     return;
 
   SetFocusState(state->focus_state, OMNIBOX_FOCUS_CHANGE_TAB_SWITCH);
   focus_source_ = state->focus_source;
   // Restore any user editing.
   if (state->user_input_in_progress) {
     // NOTE: Be sure and set keyword-related state BEFORE invoking
     // DisplayTextFromUserText(), as its result depends upon this state.
     keyword_ = state->keyword;
     is_keyword_hint_ = state->is_keyword_hint;
     view_->SetUserText(state->user_text,
         DisplayTextFromUserText(state->user_text), false);
     view_->SetGrayTextAutocompletion(state->gray_text);
   }
 }
 
 AutocompleteMatch OmniboxEditModel::CurrentMatch(
     GURL* alternate_nav_url) const {
   // If we have a valid match use it. Otherwise get one for the current text.
   AutocompleteMatch match = omnibox_controller_->current_match();
 
   if (!match.destination_url.is_valid()) {
     GetInfoForCurrentText(&match, alternate_nav_url);
   } else if (alternate_nav_url) {
     *alternate_nav_url = AutocompleteResult::ComputeAlternateNavUrl(
-        autocomplete_controller()->input(), match);
+        input_, match);
   }
   return match;
 }
 
 bool OmniboxEditModel::UpdatePermanentText() {
   SearchProvider* search_provider =
       autocomplete_controller()->search_provider();
   if (search_provider && delegate_->CurrentPageExists())
     search_provider->set_current_page_url(delegate_->GetURL());
 
@@ skipping 238 matching lines @@
   view_->SetWindowTextAndCaretPos(permanent_text_,
                                   has_focus() ? permanent_text_.length() : 0,
                                   false, true);
   AutocompleteActionPredictor* action_predictor =
       predictors::AutocompleteActionPredictorFactory::GetForProfile(profile_);
   action_predictor->ClearTransitionalMatches();
 }
 
 void OmniboxEditModel::StartAutocomplete(
     bool has_selected_text,
-    bool prevent_inline_autocomplete) const {
+    bool prevent_inline_autocomplete) {
   size_t cursor_position;
   if (inline_autocomplete_text_.empty()) {
     // Cursor position is equivalent to the current selection's end.
     size_t start;
     view_->GetSelectionBounds(&start, &cursor_position);
     // Adjust cursor position taking into account possible keyword in the user
     // text.  We rely on DisplayTextFromUserText() method which is consistent
     // with keyword extraction done in KeywordProvider/SearchProvider.
     const size_t cursor_offset =
         user_text_.length() - DisplayTextFromUserText(user_text_).length();
     cursor_position += cursor_offset;
   } else {
     // There are some cases where StartAutocomplete() may be called
     // with non-empty |inline_autocomplete_text_|.  In such cases, we cannot
     // use the current selection, because it could result with the cursor
     // position past the last character from the user text.  Instead,
     // we assume that the cursor is simply at the end of input.
     // One example is when user presses Ctrl key while having a highlighted
     // inline autocomplete text.
     // TODO: Rethink how we are going to handle this case to avoid
     // inconsistent behavior when user presses Ctrl key.
     // See http://crbug.com/165961 and http://crbug.com/165968 for more details.
     cursor_position = user_text_.length();
   }
 
   GURL current_url =
       (delegate_->CurrentPageExists() && view_->IsIndicatingQueryRefinement()) ?
       delegate_->GetURL() : GURL();
   bool keyword_is_selected = KeywordIsSelected();
-  omnibox_controller_->StartAutocomplete(
+  input_ = AutocompleteInput(
       user_text_,
       cursor_position,
+      base::string16(),
       current_url,
       ClassifyPage(),
       prevent_inline_autocomplete || just_deleted_text_ ||
       (has_selected_text && inline_autocomplete_text_.empty()) ||
       (paste_state_ != NONE),
       keyword_is_selected,
-      keyword_is_selected || allow_exact_keyword_match_);
+      keyword_is_selected || allow_exact_keyword_match_,
+      true);
+
+  omnibox_controller_->StartAutocomplete(input_);
 }
 
 void OmniboxEditModel::StopAutocomplete() {
   autocomplete_controller()->Stop(true);
 }
 
 bool OmniboxEditModel::CanPasteAndGo(const base::string16& text) const {
   if (!view_->command_updater()->IsCommandEnabled(IDC_OPEN_CURRENT_URL))
     return false;
 
@@ skipping 33 matching lines @@
   if (control_key_state_ == DOWN_WITHOUT_CHANGE && !KeywordIsSelected() &&
       autocomplete_controller()->history_url_provider()) {
     // Generate a new AutocompleteInput, copying the latest one but using "com"
     // as the desired TLD. Then use this autocomplete input to generate a
     // URL_WHAT_YOU_TYPED AutocompleteMatch. Note that using the most recent
     // input instead of the currently visible text means we'll ignore any
     // visible inline autocompletion: if a user types "foo" and is autocompleted
     // to "foodnetwork.com", ctrl-enter will navigate to "foo.com", not
     // "foodnetwork.com".  At the time of writing, this behavior matches
     // Internet Explorer, but not Firefox.
-    const AutocompleteInput& old_input = autocomplete_controller()->input();
-    AutocompleteInput input(
+    input_ = AutocompleteInput(
       has_temporary_text_ ?
-          UserTextFromDisplayText(view_->GetText())  : old_input.text(),
-      old_input.cursor_position(), base::ASCIIToUTF16("com"),
-      GURL(), old_input.current_page_classification(),
-      old_input.prevent_inline_autocomplete(), old_input.prefer_keyword(),
-      old_input.allow_exact_keyword_match(),
-      old_input.want_asynchronous_matches());
+          UserTextFromDisplayText(view_->GetText())  : input_.text(),
+      input_.cursor_position(), base::ASCIIToUTF16("com"),
+      GURL(), input_.current_page_classification(),
+      input_.prevent_inline_autocomplete(), input_.prefer_keyword(),
+      input_.allow_exact_keyword_match(),
+      input_.want_asynchronous_matches());
     AutocompleteMatch url_match(
         autocomplete_controller()->history_url_provider()->SuggestExactInput(
-            input.text(), input.canonicalized_url(), false));
+            input_.text(), input_.canonicalized_url(), false));
+
 
     if (url_match.destination_url.is_valid()) {
       // We have a valid URL, we use this newly generated AutocompleteMatch.
       match = url_match;
       alternate_nav_url = GURL();
     }
   }
 
   if (!match.destination_url.is_valid())
     return;
@@ skipping 61 matching lines @@
         (match.provider->type() == AutocompleteProvider::TYPE_ZERO_SUGGEST)) {
       elapsed_time_since_user_first_modified_omnibox =
           base::TimeDelta::FromMilliseconds(-1);
       elapsed_time_since_last_change_to_default_match =
           base::TimeDelta::FromMilliseconds(-1);
     }
     DCHECK_NE(OmniboxPopupModel::kNoMatch, index);
     OmniboxLog log(
         input_text,
         just_deleted_text_,
-        autocomplete_controller()->input().type(),
+        input_.type(),
         index,
         -1,  // don't yet know tab ID; set later if appropriate
         ClassifyPage(),
         elapsed_time_since_user_first_modified_omnibox,
         match.inline_autocompletion.length(),
         elapsed_time_since_last_change_to_default_match,
         result());
 
     DCHECK(user_input_in_progress_ || (match.provider &&
            (match.provider->type() == AutocompleteProvider::TYPE_ZERO_SUGGEST)))
@@ skipping 706 matching lines @@
   // Update state and notify view if the omnibox has focus and the caret
   // visibility changed.
   const bool was_caret_visible = is_caret_visible();
   focus_state_ = state;
   if (focus_state_ != OMNIBOX_FOCUS_NONE &&
       is_caret_visible() != was_caret_visible)
     view_->ApplyCaretVisibility();
 
   delegate_->OnFocusChanged(focus_state_, reason);
 }