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

Failing to do so confuses the omnibox_edit_model. A visible bug appears when using ctrl+enter after having switched tab, which can cause the text of the previous tab to be used.

BUG=338448

Review URL: https://codereview.chromium.org/232023004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266385 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/autocomplete/autocomplete_controller.h

@@ -117,7 +117,6 @@ class AutocompleteController : public AutocompleteProviderListener {
   KeywordProvider* keyword_provider() const { return keyword_provider_; }
   SearchProvider* search_provider() const { return search_provider_; }
 
-  const AutocompleteInput& input() const { return input_; }
   const AutocompleteResult& result() const { return result_; }
   bool done() const { return done_; }
   const ACProviders* providers() const { return &providers_; }
@@ -132,6 +131,7 @@ class AutocompleteController : public AutocompleteProviderListener {
                            RedundantKeywordsIgnoredInResult);
   FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, UpdateAssistedQueryStats);
   FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, GetDestinationURL);
+  FRIEND_TEST_ALL_PREFIXES(OmniboxViewTest, DoesNotUpdateAutocompleteOnBlur);
 
   // Updates |result_| to reflect the current provider state and fires
   // notifications.  If |regenerate_result| then we clear the result

chrome/browser/ui/omnibox/omnibox_controller.cc

@@ -72,22 +72,13 @@ OmniboxController::~OmniboxController() {
 }
 
 void OmniboxController::StartAutocomplete(
-    base::string16 user_text,
-    size_t cursor_position,
-    const GURL& current_url,
-    AutocompleteInput::PageClassification current_page_classification,
-    bool prevent_inline_autocomplete,
-    bool prefer_keyword,
-    bool allow_exact_keyword_match) const {
+    const AutocompleteInput& input) const {
   ClearPopupKeywordMode();
   popup_->SetHoveredLine(OmniboxPopupModel::kNoMatch);
 
   // We don't explicitly clear OmniboxPopupModel::manually_selected_match, as
   // Start ends up invoking OmniboxPopupModel::OnResultChanged which clears it.
-  autocomplete_controller_->Start(AutocompleteInput(
-      user_text, cursor_position, base::string16(), current_url,
-      current_page_classification, prevent_inline_autocomplete,
-      prefer_keyword, allow_exact_keyword_match, true));
+  autocomplete_controller_->Start(input);
 }
 
 void OmniboxController::OnResultChanged(bool default_match_changed) {

chrome/browser/ui/omnibox/omnibox_controller.h

@@ -16,7 +16,6 @@
 
 struct AutocompleteMatch;
 class AutocompleteResult;
-class GURL;
 class InstantController;
 class OmniboxEditModel;
 class OmniboxPopupModel;
@@ -40,15 +39,8 @@ class OmniboxController : public AutocompleteControllerDelegate {
                     Profile* profile);
   virtual ~OmniboxController();
 
-  // |current_url| is only set for mobile ports.
-  void StartAutocomplete(
-      base::string16 user_text,
-      size_t cursor_position,
-      const GURL& current_url,
-      AutocompleteInput::PageClassification current_page_classification,
-      bool prevent_inline_autocomplete,
-      bool prefer_keyword,
-      bool allow_exact_keyword_match) const;
+  // The |current_url| field of input is only set for mobile ports.
+  void StartAutocomplete(const AutocompleteInput& input) const;
 
   // AutocompleteControllerDelegate:
   virtual void OnResultChanged(bool default_match_changed) OVERRIDE;

chrome/browser/ui/omnibox/omnibox_edit_model.cc

@@ -17,7 +17,6 @@
 #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"
@@ -175,7 +174,8 @@ OmniboxEditModel::State::State(bool user_input_in_progress,
                                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),
@@ -183,7 +183,8 @@ OmniboxEditModel::State::State(bool user_input_in_progress,
       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() {
@@ -240,7 +241,7 @@ const OmniboxEditModel::State OmniboxEditModel::GetStateForTabSwitch() {
       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) {
@@ -252,6 +253,9 @@ void OmniboxEditModel::RestoreState(const State* state) {
   // 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;
 
@@ -278,7 +282,7 @@ AutocompleteMatch OmniboxEditModel::CurrentMatch(
     GetInfoForCurrentText(&match, alternate_nav_url);
   } else if (alternate_nav_url) {
     *alternate_nav_url = AutocompleteResult::ComputeAlternateNavUrl(
-        autocomplete_controller()->input(), match);
+        input_, match);
   }
   return match;
 }
@@ -538,7 +542,7 @@ void OmniboxEditModel::Revert() {
 
 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.
@@ -568,16 +572,20 @@ void OmniboxEditModel::StartAutocomplete(
       (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() {
@@ -631,18 +639,18 @@ void OmniboxEditModel::AcceptInput(WindowOpenDisposition disposition,
     // 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.
@@ -724,7 +732,7 @@ void OmniboxEditModel::OpenMatch(AutocompleteMatch match,
     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(),

chrome/browser/ui/omnibox/omnibox_edit_model.h

@@ -11,6 +11,7 @@
 #include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "chrome/browser/autocomplete/autocomplete_controller_delegate.h"
+#include "chrome/browser/autocomplete/autocomplete_input.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/ui/omnibox/omnibox_controller.h"
 #include "chrome/common/instant_types.h"
@@ -60,7 +61,8 @@ class OmniboxEditModel {
           bool is_keyword_hint,
           bool url_replacement_enabled,
           OmniboxFocusState focus_state,
-          FocusSource focus_source);
+          FocusSource focus_source,
+          const AutocompleteInput& autocomplete_input);
     ~State();
 
     bool user_input_in_progress;
@@ -71,6 +73,7 @@ class OmniboxEditModel {
     bool url_replacement_enabled;
     OmniboxFocusState focus_state;
     FocusSource focus_source;
+    const AutocompleteInput autocomplete_input;
   };
 
   OmniboxEditModel(OmniboxView* view,
@@ -171,7 +174,7 @@ class OmniboxEditModel {
 
   // Directs the popup to start autocomplete.
   void StartAutocomplete(bool has_selected_text,
-                         bool prevent_inline_autocomplete) const;
+                         bool prevent_inline_autocomplete);
 
   // Closes the popup and cancels any pending asynchronous queries.
   void StopAutocomplete();
@@ -551,6 +554,11 @@ class OmniboxEditModel {
   // This has no effect if we're already in keyword mode.
   bool allow_exact_keyword_match_;
 
+  // The input that was sent to the AutocompleteController. Since no
+  // autocomplete query is started after a tab switch, it is possible for this
+  // |input_| to differ from the one currently stored in AutocompleteController.
+  AutocompleteInput input_;
+
   DISALLOW_COPY_AND_ASSIGN(OmniboxEditModel);
 };
 

chrome/browser/ui/omnibox/omnibox_view_browsertest.cc

@@ -1460,7 +1460,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest,
   omnibox_view->GetSelectionBounds(&start, &end);
   EXPECT_TRUE(start != end);
   base::string16 old_autocomplete_text =
-      omnibox_view->model()->autocomplete_controller()->input().text();
+      omnibox_view->model()->autocomplete_controller()->input_.text();
 
   // Unfocus the omnibox. This should clear the text field selection and
   // close the popup, but should not run autocomplete.
@@ -1471,7 +1471,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewTest,
   EXPECT_TRUE(start == end);
 
   EXPECT_EQ(old_autocomplete_text,
-      omnibox_view->model()->autocomplete_controller()->input().text());
+      omnibox_view->model()->autocomplete_controller()->input_.text());
 }
 
 IN_PROC_BROWSER_TEST_F(OmniboxViewTest, Paste) {

chrome/browser/ui/webui/omnibox/omnibox_ui_handler.cc

@@ -15,7 +15,6 @@
 #include "base/values.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
 #include "chrome/browser/autocomplete/autocomplete_controller.h"
-#include "chrome/browser/autocomplete/autocomplete_input.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/autocomplete/autocomplete_provider.h"
 #include "chrome/browser/history/history_service.h"
@@ -80,9 +79,9 @@ void OmniboxUIHandler::OnResultChanged(bool default_match_changed) {
   result_to_output.SetBoolean("done", controller_->done());
   result_to_output.SetInteger("time_since_omnibox_started_ms",
       (base::Time::Now() - time_omnibox_started_).InMilliseconds());
-  const base::string16& host = controller_->input().text().substr(
-      controller_->input().parts().host.begin,
-      controller_->input().parts().host.len);
+  const base::string16& host = input_.text().substr(
+      input_.parts().host.begin,
+      input_.parts().host.len);
   result_to_output.SetString("host", host);
   bool is_typed_host;
   if (LookupIsTypedHost(host, &is_typed_host)) {
@@ -198,7 +197,7 @@ void OmniboxUIHandler::StartOmniboxQuery(const base::ListValue* input) {
   // actual results to not depend on the state of the previous request.
   ResetController();
   time_omnibox_started_ = base::Time::Now();
-  controller_->Start(AutocompleteInput(
+  input_ = AutocompleteInput(
       input_string,
       cursor_position,
       base::string16(),  // user's desired tld (top-level domain)
@@ -208,7 +207,8 @@ void OmniboxUIHandler::StartOmniboxQuery(const base::ListValue* input) {
       prevent_inline_autocomplete,
       prefer_keyword,
       true,  // allow exact keyword matches
-      true));  // want all matches
+      true);
+  controller_->Start(input_);  // want all matches
 }
 
 void OmniboxUIHandler::ResetController() {

chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h

@@ -10,6 +10,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
 #include "chrome/browser/autocomplete/autocomplete_controller_delegate.h"
+#include "chrome/browser/autocomplete/autocomplete_input.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "content/public/browser/web_ui_message_handler.h"
 
@@ -85,6 +86,9 @@ class OmniboxUIHandler : public AutocompleteControllerDelegate,
   // hand back to the javascript.
   base::Time time_omnibox_started_;
 
+  // The input used when starting the AutocompleteController.
+  AutocompleteInput input_;
+
   // The Profile* handed to us in our constructor.
   Profile* profile_;