Fixing visibility transitions for Instant.

chrome/browser/instant/instant_browsertest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/command_line.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_edit.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/instant/instant_controller.h"
@@ skipping 41 matching lines @@
 #define MAYBE_NonSearchToSearch DISABLED_NonSearchToSearch
 #define MAYBE_SearchToNonSearch DISABLED_SearchToNonSearch
 #define MAYBE_ValidHeight DISABLED_ValidHeight
 #define MAYBE_OnSubmitEvent DISABLED_OnSubmitEvent
 #define MAYBE_OnCancelEvent DISABLED_OnCancelEvent
 #define MAYBE_InstantCompleteNever DISABLED_InstantCompleteNever
 #define MAYBE_InstantCompleteDelayed DISABLED_InstantCompleteDelayed
 #define MAYBE_DontCrashOnBlockedJS DISABLED_DontCrashOnBlockedJS
 #define MAYBE_DontPersistSearchbox DISABLED_DontPersistSearchbox
 #define MAYBE_PreloadsInstant DISABLED_PreloadsInstant
+#define MAYBE_PageVisibilityTest DISABLED_PageVisibilityTest
 #define MAYBE_ExperimentEnabled DISABLED_ExperimentEnabled
 #else
 #define MAYBE_OnChangeEvent OnChangeEvent
 #define MAYBE_SetSuggestionsArrayOfStrings SetSuggestionsArrayOfStrings
 #define MAYBE_SetSuggestionsEmptyArray SetSuggestionsEmptyArray
 #define MAYBE_SetSuggestionsValidJson SetSuggestionsValidJson
 #define MAYBE_SetSuggestionsInvalidSuggestions SetSuggestionsInvalidSuggestions
 #define MAYBE_SetSuggestionsEmptyJson SetSuggestionsEmptyJson
 #define MAYBE_SetSuggestionsEmptySuggestions SetSuggestionsEmptySuggestions
 #define MAYBE_SetSuggestionsEmptySuggestion SetSuggestionsEmptySuggestion
 #define MAYBE_ShowPreviewNonSearch ShowPreviewNonSearch
 #define MAYBE_NonSearchToSearch NonSearchToSearch
 #define MAYBE_SearchToNonSearch SearchToNonSearch
 #define MAYBE_ValidHeight ValidHeight
 #define MAYBE_OnSubmitEvent OnSubmitEvent
 #define MAYBE_OnCancelEvent OnCancelEvent
 #define MAYBE_InstantCompleteNever InstantCompleteNever
 #define MAYBE_InstantCompleteDelayed InstantCompleteDelayed
 #define MAYBE_DontCrashOnBlockedJS DontCrashOnBlockedJS
 #define MAYBE_DontPersistSearchbox DontPersistSearchbox
 #define MAYBE_PreloadsInstant PreloadsInstant
+#define MAYBE_PageVisibilityTest PageVisibilityTest
 #define MAYBE_ExperimentEnabled ExperimentEnabled
 #endif
 
 #if defined(OS_MACOSX) || defined(OS_LINUX)
 // Showing as flaky on Mac and Linux.
 // http://crbug.com/70860
 #define MAYBE_SearchServerDoesntSupportInstant \
     DISABLED_SearchServerDoesntSupportInstant
 #define MAYBE_NonSearchToSearchDoesntSupportInstant \
     DISABLED_NonSearchToSearchDoesntSupportInstant
@@ skipping 155 matching lines @@
         "window.domAutomationController.send(%s)", function.c_str());
     return ui_test_utils::ExecuteJavaScriptAndExtractBool(
         tab_contents->render_view_host(),
         std::wstring(), UTF8ToWide(script), result);
   }
 
   bool HasPreview() {
     return browser()->instant()->GetPreviewContents() != NULL;
   }
 
+  bool IsTabContentsPageVisibiltyVisible(TabContents* tab_contents) {

[sreeram, 2011/10/25 23:26:48]

Typo in "Visibilty". In any case, I'd recommend giving a simpler name:
    bool IsTabContentsVisible(...)
or even just
    bool IsVisible(...)

[Shishir, 2011/10/25 23:46:26]

Done.

+    std::string visibility;
+    if (!GetStringFromJavascript(tab_contents, "document.webkitVisibilityState",
+            &visibility))
+      return false;
+    return visibility == "visible";
+  }
+
   // Returns the state of the search box as a string. This consists of the
   // following:
   // window.chrome.sv
   // window.onsubmitcalls
   // window.oncancelcalls
   // window.onchangecalls
   // 'true' if window.onresizecalls has been sent, otherwise false.
   // window.beforeLoadSearchBox.value
   // window.beforeLoadSearchBox.verbatim
   // window.chrome.searchBox.value
@@ skipping 613 matching lines @@
   FindLocationBar();
   location_bar_->FocusLocation(false);
   TabContentsWrapper* tab_contents = browser()->instant()->GetPreviewContents();
   EXPECT_TRUE(tab_contents);
 
   instant_support_observer.Wait();
 
   // Instant should have a preview, but not display it.
   EXPECT_TRUE(HasPreview());
   EXPECT_FALSE(browser()->instant()->is_displayable());
+  EXPECT_FALSE(IsTabContentsPageVisibiltyVisible(tab_contents->tab_contents()));
 
   // Adding a new tab shouldn't delete (or recreate) the TabContentsWrapper.
   AddBlankTabAndShow(browser());
   EXPECT_EQ(tab_contents, browser()->instant()->GetPreviewContents());
 
   // Doing a search should still use the same loader for the preview.
   SetLocationBarText("def");
   EXPECT_EQ(tab_contents, browser()->instant()->GetPreviewContents());
 
   // Verify that the preview is in fact showing instant search.
   EXPECT_TRUE(HasPreview());
   EXPECT_TRUE(browser()->instant()->is_displayable());
   EXPECT_TRUE(browser()->instant()->IsCurrent());
 }
 
+// Tests that the instant search page's visibility is set correctly.
+IN_PROC_BROWSER_TEST_F(InstantTest, MAYBE_PageVisibilityTest) {
+  ASSERT_TRUE(test_server()->Start());
+  EnableInstant();
+  ASSERT_NO_FATAL_FAILURE(SetupInstantProvider("search.html"));
+
+  // Initially navigate to the empty page which should be visible.
+  ASSERT_NO_FATAL_FAILURE(FindLocationBar());
+  location_bar_->location_entry()->SetUserText(ASCIIToUTF16(
+      test_server()->GetURL("files/instant/empty.html").spec()));
+  TabContents* initial_contents = browser()->GetSelectedTabContents();
+  TestNavigationObserver observer(content::Source<NavigationController>(
+      &initial_contents->controller()), NULL, 1);
+  SendKey(ui::VKEY_RETURN);
+  observer.WaitForObservation();

[sreeram, 2011/10/25 23:26:48]

This seems like a lot of trouble. Why not this?

ASSERT_TRUE(test_server()->Start());
EnableInstant();
SetupInstantProvider("search.html");

ui_test_utils::NavigateToURL(browser(), test_server()->GetURL("/"));
TabContents* initial_contents = browser()->GetSelectedTabContents();
EXPECT_TRUE(IsVisible(initial_contents));

FindLocationBar();
location_bar_->FocusLocation(false);
SetupLocationBar();
...

[Shishir, 2011/10/25 23:46:26]

Done.

+  EXPECT_TRUE(IsTabContentsPageVisibiltyVisible(initial_contents));
+
+  // Type something for instant to trigger and wait for preview to navigate.
+  location_bar_->FocusLocation(false);
+  SetupLocationBar();
+  SetupPreview();
+  SetLocationBarText("def");
+  TabContents* preview_contents =
+      browser()->instant()->GetPreviewContents()->tab_contents();
+  EXPECT_TRUE(IsTabContentsPageVisibiltyVisible(preview_contents));
+  EXPECT_FALSE(IsTabContentsPageVisibiltyVisible(initial_contents));
+
+  // Delete the user text we should show the previous page.
+  location_bar_->location_entry()->SetUserText(UTF8ToUTF16(""));
+  EXPECT_FALSE(IsTabContentsPageVisibiltyVisible(preview_contents));
+  EXPECT_TRUE(IsTabContentsPageVisibiltyVisible(initial_contents));
+
+  // Set the user text back and we should see the preview again.
+  location_bar_->location_entry()->SetUserText(UTF8ToUTF16("def"));
+  EXPECT_TRUE(IsTabContentsPageVisibiltyVisible(preview_contents));

[sreeram, 2011/10/25 23:26:48]

Just FYI: At first glance, I wouldn't have expected this to work, since we need
to wait for the renderer to send back a setSuggestions call before we'll show
instant; but your visibility check actually sends a message and waits for the
renderer to respond, so I think we end up processing the message loop.

[Shishir, 2011/10/25 23:46:26]

Do you suggest something else here to increase robustness.

[sreeram, 2011/10/26 04:29:52]

I would normally have said to put in a call to
WaitForMessageToBeProcessedByRenderer(), so that if ever the visibility check
becomes something like a simple function call on the browser thread, instead of
needing to go around to the renderer and back, the test won't break. But this
(visibility check implementation changing) is unlikely to happen, so don't worry
about it.

+  EXPECT_FALSE(IsTabContentsPageVisibiltyVisible(initial_contents));
+
+  // Commit the preview.
+  SendKey(ui::VKEY_RETURN);
+  EXPECT_EQ(preview_contents, browser()->GetSelectedTabContents());
+  EXPECT_TRUE(IsTabContentsPageVisibiltyVisible(preview_contents));
+}
+
+
 // Tests the INSTANT experiment of the field trial.
 class InstantFieldTrialInstantTest : public InstantTest {
  public:
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     command_line->AppendSwitchASCII(switches::kInstantFieldTrial,
                                     switches::kInstantFieldTrialInstant);
   }
 };
 
 // Tests that instant is active, even without calling EnableInstant().
@@ skipping 79 matching lines @@
 
   // Wait for the preview to navigate.
   WaitForPreviewToNavigate();
   instant_support_observer.Wait();
 
   // Instant should be active, but not displayable or committable.
   EXPECT_TRUE(HasPreview());
   EXPECT_FALSE(browser()->instant()->is_displayable());
   EXPECT_FALSE(browser()->instant()->PrepareForCommit());
 }