Fix issue 21587, a regression of CL 196020.

This CL fixes issue 21587: REGRESSION: Omnibox does nothing when pressing enter after a single character of input.

It's a regression of my CL: http://codereview.chromium.org/196020, which reverts the text of omnibox in OnAfterPossibleChanges(), when enter key is pressed:

...
  // If the change is caused by an Enter key press event, and the event was not
  // handled by IME, then it's an unexpected change and shall be reverted here.
  // {Start|Finish}UpdatingHighlightedText() are called here to prevent the
  // PRIMARY selection from being changed.
  if (enter_was_pressed_ &&
      (char_inserted_ == '\n' || char_inserted_ == '\r')) {
    StartUpdatingHighlightedText();
    SetTextAndSelectedRange(text_before_change_, sel_before_change_);
    FinishUpdatingHighlightedText();
    return false;
  }
...

Unfortunately, this piece of code causes the char_inserted_ to be set to wrong value in HandleInsertText(), if the text has only one character. See the source code of HandleInsertText():

...
  // Filter out new line and tab characters.
  // |text| is guaranteed to be a valid UTF-8 string, so it's safe here to
  // filter byte by byte.
  //
  // If there was only a single character, then it might be generated by a key
  // event. In this case, we save the single character to help our
  // "key-press-event" signal handler distinguish if an Enter key event is
  // handled by IME or not.
  if (len == 1)
    char_inserted_ = text[0];
...

This CL uses a boolean |enter_was_inserted_| instead of char |char_inserted_| to record the enter key status, to make sure it won't be reset unexpectly.

The test has been updated to cover this case. An unexpected dns lookup issue in the test which causes a failure on Windows was also fixed.

BUG=21587: REGRESSION: Omnibox does nothing when pressing enter after a single character of input.
TEST=Input one character in omnibox and press enter, the default link matched with the character should be opened.

Review URL: http://codereview.chromium.org/200131

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

3 files changed, 43 insertions, 20 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc b/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
index c46cb50..1023b9c 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_browsertest.cc
@@ -50,6 +50,10 @@ const wchar_t kSearchTextKeys[] = {
   base::VKEY_A, base::VKEY_B, base::VKEY_C, 0
 };
 const char kSearchTextURL[] = "http://www.foo.com/search?q=abc";
+const wchar_t kSearchSingleChar[] = L"z";
+const wchar_t kSearchSingleCharKeys[] = { base::VKEY_Z, 0 };
+const char kSearchSingleCharURL[] = "http://www.foo.com/search?q=z";
+
 const char kHistoryPageURL[] = "chrome://history/#q=abc";
 
 const char kDesiredTLDHostname[] = "www.bar.com";
@@ -64,7 +68,9 @@ const char *kBlockedHostnames[] = {
   "bar",
   "*.bar.com",
   "abc",
-  "*.abc.com"
+  "*.abc.com",
+  "history",
+  "z"
 };
 
 const struct TestHistoryEntry {
@@ -380,6 +386,7 @@ IN_PROC_BROWSER_TEST_F(AutocompleteEditViewTest, DISABLED_DesiredTLD) {
 }
 
 IN_PROC_BROWSER_TEST_F(AutocompleteEditViewTest, DISABLED_AltEnter) {
+  ASSERT_NO_FATAL_FAILURE(SetupHostResolver());
   browser()->FocusLocationBar();
 
   AutocompleteEditView* edit_view = NULL;
@@ -415,6 +422,23 @@ IN_PROC_BROWSER_TEST_F(AutocompleteEditViewTest, DISABLED_EnterToSearch) {
   ASSERT_NO_FATAL_FAILURE(SendKey(base::VKEY_RETURN, false, false, false));
   GURL url = browser()->GetSelectedTabContents()->GetURL();
   EXPECT_STREQ(kSearchTextURL, url.spec().c_str());
+
+  // Test that entering a single character then Enter performs a search.
+  browser()->FocusLocationBar();
+  EXPECT_TRUE(edit_view->IsSelectAll());
+  ASSERT_NO_FATAL_FAILURE(SendKeySequence(kSearchSingleCharKeys));
+  ASSERT_NO_FATAL_FAILURE(WaitForAutocompleteControllerDone());
+  ASSERT_TRUE(popup_model->IsOpen());
+  EXPECT_EQ(std::wstring(kSearchSingleChar), edit_view->GetText());
+
+  // Check if the default match result is Search Primary Provider.
+  ASSERT_EQ(AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
+            popup_model->result().default_match()->type);
+
+  // Open the default match.
+  ASSERT_NO_FATAL_FAILURE(SendKey(base::VKEY_RETURN, false, false, false));
+  url = browser()->GetSelectedTabContents()->GetURL();
+  EXPECT_STREQ(kSearchSingleCharURL, url.spec().c_str());
 }
 
 // See http://crbug.com/20934: Omnibox keyboard behavior wrong for
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
index 8e75865..80691a1 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
@@ -108,7 +108,8 @@ AutocompleteEditViewGtk::AutocompleteEditViewGtk(
 #endif
       enter_was_pressed_(false),
       tab_was_pressed_(false),
-      paste_clipboard_requested_(false) {
+      paste_clipboard_requested_(false),
+      enter_was_inserted_(false) {
   model_->SetPopupModel(popup_view_->GetModel());
 }
 
@@ -462,8 +463,7 @@ bool AutocompleteEditViewGtk::OnAfterPossibleChange() {
   // handled by IME, then it's an unexpected change and shall be reverted here.
   // {Start|Finish}UpdatingHighlightedText() are called here to prevent the
   // PRIMARY selection from being changed.
-  if (enter_was_pressed_ &&
-      (char_inserted_ == '\n' || char_inserted_ == '\r')) {
+  if (enter_was_pressed_ && enter_was_inserted_) {
     StartUpdatingHighlightedText();
     SetTextAndSelectedRange(text_before_change_, sel_before_change_);
     FinishUpdatingHighlightedText();
@@ -597,14 +597,14 @@ gboolean AutocompleteEditViewGtk::HandleKeyPress(GtkWidget* widget,
   // event and performing built-in operation. So in order to achieve our goal,
   // "insert-text" signal of |text_buffer_| object is intercepted, and
   // following actions are done in the signal handler:
-  // - If there is only one character in inserted text, save it in
-  //   char_inserted_.
+  // - If there is only one character in inserted text, and it's '\n' or '\r',
+  //   then set |enter_was_inserted_| to true.
   // - Filter out all new line and tab characters.
   //
-  // So if |char_inserted_| equals '\n' after calling |text_view_|'s
-  // default signal handler against an Enter key press event, then we know that
-  // the Enter key press event was handled by GtkTextView rather than IME, and
-  // can perform the special behavior for Enter key safely.
+  // So if |enter_was_inserted_| is true after calling |text_view_|'s default
+  // signal handler against an Enter key press event, then we know that the
+  // Enter key press event was handled by GtkTextView rather than IME, and can
+  // perform the special behavior for Enter key safely.
   //
   // Now the last thing is to prevent the content of omnibox from being changed
   // by GtkTextView when Enter key is pressed. As OnBeforePossibleChange() and
@@ -628,9 +628,9 @@ gboolean AutocompleteEditViewGtk::HandleKeyPress(GtkWidget* widget,
                        event->keyval == GDK_KP_Tab) &&
                       !(event->state & GDK_CONTROL_MASK));
 
-  // Reset |char_inserted_|, which may be set in the "insert-text" signal
+  // Reset |enter_was_inserted_|, which may be set in the "insert-text" signal
   // handler, so that we'll know if an Enter key event was handled by IME.
-  char_inserted_ = 0;
+  enter_was_inserted_ = false;
 
   // Reset |paste_clipboard_requested_| to make sure we won't misinterpret this
   // key input action as a paste action.
@@ -645,8 +645,7 @@ gboolean AutocompleteEditViewGtk::HandleKeyPress(GtkWidget* widget,
   // only be triggered by pressing Tab key.
   tab_was_pressed_ = false;
 
-  if (enter_was_pressed_ &&
-      (char_inserted_ == '\n' || char_inserted_ == '\r')) {
+  if (enter_was_pressed_ && enter_was_inserted_) {
     bool alt_held = (event->state & GDK_MOD1_MASK);
     model_->AcceptInput(alt_held ? NEW_FOREGROUND_TAB : CURRENT_TAB, false);
     result = TRUE;
@@ -940,8 +939,8 @@ void AutocompleteEditViewGtk::HandleInsertText(
   // event. In this case, we save the single character to help our
   // "key-press-event" signal handler distinguish if an Enter key event is
   // handled by IME or not.
-  if (len == 1)
-    char_inserted_ = text[0];
+  if (len == 1 && (text[0] == '\n' || text[0] == '\r'))
+    enter_was_inserted_ = true;
 
   for (gint i = 0; i < len; ++i) {
     gchar c = text[i];
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
index aff1159..3f5c87a 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
@@ -408,10 +408,10 @@ class AutocompleteEditViewGtk : public AutocompleteEditView,
   // clipboard is not empty.
   bool paste_clipboard_requested_;
 
-  // If a character is inserted, store it in this variable so that it can
-  // be used later in "key-press-event" signal handler to determine if a Tab or
-  // Enter key event is handled by IME or not.
-  char char_inserted_;
+  // Indicates if an Enter key press is inserted as text.
+  // It's used in the key press handler to determine if an Enter key event is
+  // handled by IME or not.
+  bool enter_was_inserted_;
 
   DISALLOW_COPY_AND_ASSIGN(AutocompleteEditViewGtk);
 };