InstantExtended: Fix improper calls to OnSubmit.

An invalid assumption in InstantController::Update() caused it to send
OnSubmit() for URLs during navigation. This caused the page to navigate
to search results during the browser's navigation, result being that
search results were shown instead of navigating to the URL.

Also, Update() was being called with the original query text during
navigation, possibly causing a flash; this is never correct and an
existing bool, in_revert_, was supposed to guard against it but did not.

BUG=165580


Review URL: https://chromiumcodereview.appspot.com/11558028

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

4 files changed, 23 insertions, 7 deletions

diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc
index f024191..303c5e9 100644
--- a/chrome/browser/instant/instant_controller.cc
+++ b/chrome/browser/instant/instant_controller.cc
@@ -170,7 +170,8 @@ bool InstantController::Update(const AutocompleteMatch& match,
                                size_t selection_end,
                                bool verbatim,
                                bool user_input_in_progress,
-                               bool omnibox_popup_is_open) {
+                               bool omnibox_popup_is_open,
+                               bool escape_pressed) {
   if (!extended_enabled_ && !instant_enabled_)
     return false;
 
@@ -178,7 +179,8 @@ bool InstantController::Update(const AutocompleteMatch& match,
            << " user_text='" << user_text << "' full_text='" << full_text << "'"
            << " selection_start=" << selection_start << " selection_end="
            << selection_end << " verbatim=" << verbatim << " typing="
-           << user_input_in_progress << " popup=" << omnibox_popup_is_open;
+           << user_input_in_progress << " popup=" << omnibox_popup_is_open
+           << " escape_pressed=" << escape_pressed;
 
   // If the popup is open, the user has to be typing.
   DCHECK(!omnibox_popup_is_open || user_input_in_progress);
@@ -229,8 +231,10 @@ bool InstantController::Update(const AutocompleteMatch& match,
           // be pointing to the previous tab (which was a search results page).
           // Ensure we don't send the omnibox text to a random webpage (the new
           // tab), by comparing the old and new WebContents.
-          if (instant_tab_->contents() == browser_->GetActiveWebContents())
+          if (escape_pressed &&
+              instant_tab_->contents() == browser_->GetActiveWebContents()) {
             instant_tab_->Submit(full_text);
+          }
         } else if (!full_text.empty()) {
           // If |full_text| is empty, the user is on the NTP. The preview may
           // be showing custom NTP content; hide only if that's not the case.
diff --git a/chrome/browser/instant/instant_controller.h b/chrome/browser/instant/instant_controller.h
index 3830fa9..1c1d804 100644
--- a/chrome/browser/instant/instant_controller.h
+++ b/chrome/browser/instant/instant_controller.h
@@ -61,7 +61,8 @@ class InstantController {
               size_t selection_end,
               bool verbatim,
               bool user_input_in_progress,
-              bool omnibox_popup_is_open);
+              bool omnibox_popup_is_open,
+              bool escape_pressed);
 
   // Sets the bounds of the omnibox dropdown, in screen coordinates.
   void SetOmniboxBounds(const gfx::Rect& bounds);
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.cc b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
index 698fba6..507cf65 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.cc
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.cc
@@ -6,6 +6,7 @@
 
 #include <string>
 
+#include "base/auto_reset.h"
 #include "base/format_macros.h"
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
@@ -101,6 +102,7 @@ OmniboxEditModel::OmniboxEditModel(OmniboxView* view,
       is_keyword_hint_(false),
       profile_(profile),
       in_revert_(false),
+      in_escape_handler_(false),
       allow_exact_keyword_match_(false) {
   // Use a restricted subset of the autocomplete providers if we're using the
   // Instant Extended API, as it doesn't support them all.
@@ -628,9 +630,8 @@ void OmniboxEditModel::OpenMatch(const AutocompleteMatch& match,
   }
 
   if (disposition != NEW_BACKGROUND_TAB) {
-    in_revert_ = true;
+    base::AutoReset<bool> tmp(&in_revert_, true);
     view_->RevertAll();  // Revert the box to its unedited state
-    in_revert_ = false;
   }
 
   if (match.type == AutocompleteMatch::EXTENSION_APP) {
@@ -652,6 +653,8 @@ void OmniboxEditModel::OpenMatch(const AutocompleteMatch& match,
       destination_url = GURL(template_url->url_ref().
                              ReplaceSearchTerms(search_terms_args));
     }
+    // This calls RevertAll again.
+    base::AutoReset<bool> tmp(&in_revert_, true);
     controller_->OnAutocompleteAccept(destination_url, disposition,
                                       match.transition, alternate_nav_url);
   }
@@ -794,7 +797,9 @@ bool OmniboxEditModel::OnEscapeKeyPressed() {
   if (!user_input_in_progress_ && view_->IsSelectAll())
     return false;
 
+  in_escape_handler_ = true;
   view_->RevertAll();
+  in_escape_handler_ = false;
   view_->SelectAll(true);
   return true;
 }
@@ -1229,7 +1234,8 @@ 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_->IsOpen(),
+      in_escape_handler_);
 }
 
 void OmniboxEditModel::DoPrerender(const AutocompleteMatch& match) {
diff --git a/chrome/browser/ui/omnibox/omnibox_edit_model.h b/chrome/browser/ui/omnibox/omnibox_edit_model.h
index 06f47a2..7952ef7 100644
--- a/chrome/browser/ui/omnibox/omnibox_edit_model.h
+++ b/chrome/browser/ui/omnibox/omnibox_edit_model.h
@@ -550,6 +550,11 @@ class OmniboxEditModel : public AutocompleteControllerDelegate {
   // in this case, so we use the flag to determine if this is happening.
   bool in_revert_;
 
+  // InstantController needs this in extended mode to distinguish the case in
+  // which it should instruct a committed search results page to revert to
+  // showing results for the original query.
+  bool in_escape_handler_;
+
   // Indicates if the upcoming autocomplete search is allowed to be treated as
   // an exact keyword match.  If this is true then keyword mode will be
   // triggered automatically if the input is "<keyword> <search string>".  We