Fix spurious visibility events when committing Instant.

chrome/browser/instant/instant_browsertest.cc

Index: chrome/browser/instant/instant_browsertest.cc
diff --git a/chrome/browser/instant/instant_browsertest.cc b/chrome/browser/instant/instant_browsertest.cc
index 5948c7893f570948ba10d29436e1a031f416c2bd..576dc567e0c5a67920656d8c02c4e8bd5a73d6b4 100644
--- a/chrome/browser/instant/instant_browsertest.cc
+++ b/chrome/browser/instant/instant_browsertest.cc
@@ -21,6 +21,7 @@
 #include "chrome/browser/ui/omnibox/location_bar.h"
 #include "chrome/browser/ui/omnibox/omnibox_view.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -143,7 +144,8 @@ class InstantTest : public InProcessBrowserTest {
 
   bool UpdateSearchState(TabContents* tab) WARN_UNUSED_RESULT {
     content::RenderViewHost* rvh = tab->web_contents()->GetRenderViewHost();
-    return GetIntFromJS(rvh, "onchangecalls", &onchangecalls_) &&
+    return GetIntFromJS(rvh, "onvisibilitycalls", &onvisibilitycalls_) &&
+           GetIntFromJS(rvh, "onchangecalls", &onchangecalls_) &&
            GetIntFromJS(rvh, "onsubmitcalls", &onsubmitcalls_) &&
            GetIntFromJS(rvh, "oncancelcalls", &oncancelcalls_) &&
            GetIntFromJS(rvh, "onresizecalls", &onresizecalls_) &&
@@ -169,6 +171,7 @@ class InstantTest : public InProcessBrowserTest {
            actual == expected;
   }
 
+  int onvisibilitycalls_;
   int onchangecalls_;
   int onsubmitcalls_;
   int oncancelcalls_;
@@ -267,6 +270,7 @@ IN_PROC_BROWSER_TEST_F(InstantTest, OnSubmitEvent) {
   EXPECT_EQ("search", value_);
   EXPECT_FALSE(verbatim_);
   EXPECT_EQ(0, onsubmitcalls_);
+  EXPECT_EQ(1, onvisibilitycalls_);
 
   // Before the commit, the active tab is the NTP (i.e., not Instant).
   TabContents* active_tab = chrome::GetActiveTabContents(browser());
@@ -310,12 +314,14 @@ IN_PROC_BROWSER_TEST_F(InstantTest, OnSubmitEvent) {
   EXPECT_EQ("search", value_);
   EXPECT_TRUE(verbatim_);
   EXPECT_EQ(1, onsubmitcalls_);
+  EXPECT_EQ(1, onvisibilitycalls_);
 }
 
 // Test that the oncancel event is dispatched upon clicking on the preview.
 IN_PROC_BROWSER_TEST_F(InstantTest, OnCancelEvent) {
   ASSERT_NO_FATAL_FAILURE(SetupInstant("instant.html"));
   EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+  FocusOmniboxAndWaitForInstantSupport();
   SetOmniboxTextAndWaitForInstantToShow("search");
 
   // Stash a reference to the preview, so we can refer to it after commit.
@@ -327,6 +333,7 @@ IN_PROC_BROWSER_TEST_F(InstantTest, OnCancelEvent) {
   EXPECT_EQ("search", value_);
   EXPECT_FALSE(verbatim_);
   EXPECT_EQ(0, oncancelcalls_);
+  EXPECT_EQ(1, onvisibilitycalls_);
 
   // Before the commit, the active tab is the NTP (i.e., not Instant).
   TabContents* active_tab = chrome::GetActiveTabContents(browser());
@@ -370,6 +377,7 @@ IN_PROC_BROWSER_TEST_F(InstantTest, OnCancelEvent) {
   EXPECT_EQ("search", value_);
   EXPECT_TRUE(verbatim_);
   EXPECT_EQ(1, oncancelcalls_);
+  EXPECT_EQ(1, onvisibilitycalls_);
 }
 
 // Test that the onreisze event is dispatched upon typing in the omnibox.
@@ -873,6 +881,7 @@ IN_PROC_BROWSER_TEST_F(InstantTest, History) {
 IN_PROC_BROWSER_TEST_F(InstantTest, NewWindowDismissesInstant) {
   ASSERT_NO_FATAL_FAILURE(SetupInstant("instant.html"));
   EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+  FocusOmniboxAndWaitForInstantSupport();
   SetOmniboxTextAndWaitForInstantToShow("search");
 
   Browser* previous_window = browser();
@@ -972,3 +981,65 @@ IN_PROC_BROWSER_TEST_F(InstantTest, SuggestionsAreCaseInsensitive) {
   SetOmniboxTextAndWaitForInstantToShow(WideToUTF8(L"\u1e0b\u0323"));
   EXPECT_EQ(WideToUTF16(L"\u1e0b\u0323oh"), omnibox()->GetText());
 }
+
+// Test that the preview can be committed onto a new tab.
+IN_PROC_BROWSER_TEST_F(InstantTest, CommitInNewTab) {
+  ASSERT_NO_FATAL_FAILURE(SetupInstant("instant.html"));
+  FocusOmniboxAndWaitForInstantSupport();
+  SetOmniboxTextAndWaitForInstantToShow("search");
+
+  // Stash a reference to the preview, so we can refer to it after commit.
+  TabContents* preview_tab = instant()->GetPreviewContents();
+  EXPECT_TRUE(preview_tab);
+
+  // The state of the searchbox before the commit.
+  EXPECT_TRUE(UpdateSearchState(preview_tab));
+  EXPECT_EQ("search", value_);
+  EXPECT_FALSE(verbatim_);
+  EXPECT_EQ(0, onsubmitcalls_);
+  EXPECT_EQ(1, onvisibilitycalls_);
+
+  // Before the commit, the active tab is the NTP (i.e., not Instant).
+  TabContents* active_tab = chrome::GetActiveTabContents(browser());
+  EXPECT_NE(preview_tab, active_tab);
+  EXPECT_EQ(1, active_tab->web_contents()->GetController().GetEntryCount());
+  EXPECT_EQ(std::string(chrome::kAboutBlankURL),
+            omnibox()->model()->PermanentURL().spec());
+
+  // Commit the search by pressing Alt-Enter.
+  omnibox()->model()->AcceptInput(NEW_FOREGROUND_TAB, false);
+
+  // After the commit, Instant should not be showing.
+  EXPECT_FALSE(instant()->IsCurrent());
+  EXPECT_FALSE(instant()->model()->is_ready());
+
+  // The old loader is deleted and a new one is created.
+  EXPECT_TRUE(instant()->GetPreviewContents());
+  EXPECT_NE(instant()->GetPreviewContents(), preview_tab);
+
+  // Check that we have two tabs and that the active tab is indeed what was once
+  // the preview. The preview tab should have just one navigation entry, for the
+  // Instant search that was committed.
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  EXPECT_EQ(preview_tab, chrome::GetActiveTabContents(browser()));
+  EXPECT_EQ(1, preview_tab->web_contents()->GetController().GetEntryCount());
+
+  // Check that the omnibox contains the Instant URL we loaded.
+  std::string instant_url = TemplateURLServiceFactory::GetForProfile(
+      browser()->profile())->GetDefaultSearchProvider()->instant_url_ref().
+      ReplaceSearchTerms(TemplateURLRef::SearchTermsArgs(string16()));
+  EXPECT_EQ(instant_url, omnibox()->model()->PermanentURL().spec());
+
+  // Check that the searchbox API values have been reset.
+  std::string value;
+  EXPECT_TRUE(GetStringFromJS(preview_tab->web_contents()->GetRenderViewHost(),
+                              "chrome.searchBox.value", &value));
+  EXPECT_EQ("", value);
+
+  // However, the page should've correctly received the committed query.
+  EXPECT_TRUE(UpdateSearchState(preview_tab));
+  EXPECT_EQ("search", value_);
+  EXPECT_TRUE(verbatim_);
+  EXPECT_EQ(1, onsubmitcalls_);
+  EXPECT_EQ(1, onvisibilitycalls_);
+}