Handle Esc key press event in Local NTP.

chrome/browser/ui/search/instant_extended_interactive_uitest.cc

Index: chrome/browser/ui/search/instant_extended_interactive_uitest.cc
diff --git a/chrome/browser/ui/search/instant_extended_interactive_uitest.cc b/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
index 4147a2895aac16151d0ccba3ac517fc8e486eab5..8df2d2e9fd9e9c21edeedc9f2de5733ad0d5d09e 100644
--- a/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
+++ b/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
@@ -1930,10 +1930,18 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, EmptyAutocompleteResults) {
 }
 
 // Test that hitting Esc to clear the omnibox works. http://crbug.com/231744.
-// TODO(sreeram): reenable once ESC bug is actually fixed.
-IN_PROC_BROWSER_TEST_F(InstantExtendedTest, DISABLED_EscapeClearsOmnibox) {
+IN_PROC_BROWSER_TEST_F(InstantExtendedTest, EscapeClearsOmnibox) {
+  // Use only the local overlay.

[samarth, 2013/05/05 23:49:30]

This shouldn't be necessary given the call to SetInstantEnabled below.  Also,
why force the local only page at all?  We can't really test (or want to test)
its behavior in this test.

[kmadhusu, 2013/05/06 00:11:16]

hmm. No specific reason to force this test for local only page. I will
investigate more and update this test.

+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableLocalOnlyInstantExtendedAPI);
+  ASSERT_TRUE(chrome::IsLocalOnlyInstantExtendedAPIEnabled());
   ASSERT_NO_FATAL_FAILURE(SetupInstant(browser()));
-  FocusOmniboxAndWaitForInstantOverlayAndNTPSupport();
+
+  // The second argument indicates to use only the local overlay and NTP.
+  instant()->SetInstantEnabled(true, true);
+
+  // Focus omnibox and confirm overlay isn't shown.
+  FocusOmniboxAndWaitForInstantOverlaySupport();
 
   // Navigate to the Instant NTP, and wait for it to be recognized.
   content::WindowedNotificationObserver instant_tab_observer(
@@ -1945,29 +1953,13 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, DISABLED_EscapeClearsOmnibox) {
                                               ui_test_utils::BROWSER_TEST_NONE);
   instant_tab_observer.Wait();
 
-  content::WebContents* contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-
-  // Type a query. Verify that the query is seen by the page.
+  // Type a query. Verify that the query is shown in the omnibox.
   SetOmniboxText("mojo");
-  std::string query;
-  EXPECT_TRUE(GetStringFromJS(contents, "chrome.embeddedSearch.searchBox.value",
-                              &query));
-  EXPECT_EQ("mojo", query);
+  EXPECT_EQ("mojo", GetOmniboxText());
 
-  EXPECT_TRUE(content::ExecuteScript(contents,
-                                     "onChangeCalls = submitCount = 0;"));
-
-  // Hit Escape, and verify that the page sees that the query is cleared.
+  // Hit Escape, and verify that the omnibox text is cleared.
   SendEscape();
-  EXPECT_TRUE(GetStringFromJS(contents, "chrome.embeddedSearch.searchBox.value",
-                              &query));
-  EXPECT_EQ("", query);
   EXPECT_EQ("", GetOmniboxText());

[samarth, 2013/05/05 23:49:30]

This should be true, but isn't really testing everything.  We should test that
we sent a keypress event.  Generally we do this in tests by counting the IPCs in
the test js file.  See UpdateSearchState() and the test instant file
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/i...).

[kmadhusu, 2013/05/06 00:11:16]

I completely agree. This test can be improved. I will refer to
UpdateSearchState() and will update the test.

-
-  EXPECT_TRUE(UpdateSearchState(contents));
-  EXPECT_LT(0, on_change_calls_);
-  EXPECT_EQ(0, submit_count_);
 }
 
 IN_PROC_BROWSER_TEST_F(InstantExtendedTest, OnDefaultSearchProviderChanged) {