Fall back to local page if online NTP fails to load.

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 5eee78fa9db8db3ec23ac03030c76569c0028b5e..605e563d1ae2848305fa68b220ed62be5bd8fea1 100644
--- a/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
+++ b/chrome/browser/ui/search/instant_extended_interactive_uitest.cc
@@ -29,6 +29,8 @@
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/omnibox/omnibox_view.h"
 #include "chrome/browser/ui/search/instant_commit_type.h"
 #include "chrome/browser/ui/search/instant_ntp.h"
@@ -85,8 +87,8 @@ class InstantExtendedTest : public InProcessBrowserTest,
         submit_count_(0) {
   }
  protected:
-  virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
-    chrome::EnableInstantExtendedAPIForTesting();
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    command_line->AppendSwitch(switches::kEnableInstantExtendedAPI);
     ASSERT_TRUE(https_test_server().Start());
     GURL instant_url = https_test_server().GetURL(
         "files/instant_extended.html?strk=1&");
@@ -153,8 +155,11 @@ class InstantPolicyTest : public ExtensionBrowserTest, public InstantTestBase {
   InstantPolicyTest() {}
 
  protected:
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    command_line->AppendSwitch(switches::kEnableInstantExtendedAPI);
+  }
+
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
-    chrome::EnableInstantExtendedAPIForTesting();
     ASSERT_TRUE(https_test_server().Start());
     GURL instant_url = https_test_server().GetURL(
         "files/instant_extended.html?strk=1&");
@@ -734,8 +739,12 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, PreloadedNTPDoesntSupportInstant) {
   ASSERT_NO_FATAL_FAILURE(SetupInstant(browser()));
   FocusOmniboxAndWaitForInstantOverlayAndNTPSupport();
 
-  // NTP contents should not be preloaded.
-  ASSERT_EQ(static_cast<InstantNTP*>(NULL), instant()->ntp());
+  // NTP contents should have fallen back to the local page.
+  ASSERT_TRUE(instant()->ntp_);

[samarth, 2013/05/03 04:39:17]

nit: just write:
ASSERT_NE(NULL, instant()->ntp());
EXPECT_TRUE(instant()->ntp()->IsLocal());

[David Black, 2013/05/03 06:14:44]

Done.

+  content::WebContents* ntp_contents = instant()->ntp_->contents();
+  EXPECT_TRUE(ntp_contents);
+  GURL ntp_url = ntp_contents->GetURL();
+  EXPECT_EQ(chrome::kChromeSearchLocalNtpUrl, ntp_url.spec()); //asdf
 
   // Open new tab. Should use local NTP.
   ui_test_utils::NavigateToURLWithDisposition(
@@ -1989,9 +1998,13 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, OverlayDoesntSupportInstant) {
   ASSERT_NO_FATAL_FAILURE(SetupInstant(browser()));
 
   // Focus the omnibox. When the support determination response comes back,
-  // Instant will destroy the non-Instant page.
+  // Instant will destroy the non-Instant page and fall back to the local page.
   FocusOmniboxAndWaitForInstantOverlayAndNTPSupport();
-  EXPECT_EQ(NULL, instant()->GetOverlayContents());
+  ASSERT_TRUE(instant()->overlay_);

[samarth, 2013/05/03 04:39:17]

Same here and elsewhere

[David Black, 2013/05/03 06:14:44]

Done.

+  content::WebContents* overlay_contents = instant()->overlay_->contents();
+  EXPECT_TRUE(overlay_contents);//asdf
+  GURL overlay_url = overlay_contents->GetURL();
+  EXPECT_EQ(chrome::kChromeSearchLocalNtpUrl, overlay_url.spec());
 
   // The local overlay is used on the next Update().
   SetOmniboxText("query");
@@ -2003,9 +2016,14 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, OverlayDoesntSupportInstant) {
   FocusOmnibox();
   EXPECT_FALSE(instant()->overlay()->IsLocal());
 
-  // Overlay destroyed again after determining support.
+  // Overlay falls back to local again after determining support.
   FocusOmniboxAndWaitForInstantOverlaySupport();
-  EXPECT_EQ(NULL, instant()->GetOverlayContents());
+  ASSERT_TRUE(instant()->overlay_);
+  overlay_contents = instant()->overlay_->contents();
+  EXPECT_TRUE(overlay_contents);//asdf
+  overlay_url = overlay_contents->GetURL();
+  EXPECT_EQ(chrome::kChromeSearchLocalNtpUrl, overlay_url.spec());
+
 }
 
 // Test that if Instant alters the input from URL to search, it's respected.
@@ -2049,3 +2067,61 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, InputChangedFromSearchToURL) {
   // Confirm that the Instant overlay was NOT committed.
   EXPECT_NE(overlay, browser()->tab_strip_model()->GetActiveWebContents());
 }
+
+class InstantExtendedOnlineTest : public InProcessBrowserTest,

[samarth, 2013/05/03 04:39:17]

nit: how about InstantExtendedFirstTabTest or something

[David Black, 2013/05/03 06:14:44]

Done.

+                                  public InstantTestBase {
+ public:
+  InstantExtendedOnlineTest() {}
+ protected:
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    command_line->AppendSwitch(switches::kEnableInstantExtendedAPI);
+    command_line->AppendSwitch(switches::kEnableLocalFirstLoadNTP);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(
+    InstantExtendedOnlineTest, RedirectToLocalOnLoadFailure) {
+  // Create a new window to test the first NTP load.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      GURL(chrome::kChromeUINewTabURL),
+      NEW_WINDOW,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_BROWSER);
+
+  const BrowserList* native_browser_list = BrowserList::GetInstance(
+      chrome::HOST_DESKTOP_TYPE_NATIVE);
+  ASSERT_EQ(2u, native_browser_list->size());
+  set_browser(native_browser_list->get(1));
+
+  FocusOmniboxAndWaitForInstantOverlayAndNTPSupport();
+
+  // Also make sure our instant_tab_ is loaded.
+  if (!instant()->instant_tab_) {
+    content::WindowedNotificationObserver instant_tab_observer(
+        chrome::NOTIFICATION_INSTANT_TAB_SUPPORT_DETERMINED,
+        content::NotificationService::AllSources());
+    instant_tab_observer.Wait();
+  }
+
+  // NTP contents should be preloaded.

[samarth, 2013/05/03 04:39:17]

So you're relying on the fact that initially we try to load google by default
which will fail because these tests can't reach the internet?   Seems somewhat
brittle but I don't have any better ideas.  Perhaps Sreeram will.

[David Black, 2013/05/03 06:14:44]

Yep.  I can't use the normal way of overriding the URL used in other tests as
the browser object for the new window goes ahead and navigates before I get a
chance to set anything on it.  I could override by setting --instant-url, but I
actually sort of like the test as-is, as it's exactly this loading-google
behavior that caught the bug I mentioned to you earlier.

[sreeram, 2013/05/05 08:40:39]

I think the right way would be to set the template URL data to point instant_url
to a non-existent URL. However, it's okay. I won't sweat this test.

+  ASSERT_NE(static_cast<InstantNTP*>(NULL), instant()->ntp());
+  content::WebContents* ntp_contents = instant()->ntp_->contents();
+  EXPECT_TRUE(ntp_contents);
+  GURL ntp_url = ntp_contents->GetURL();
+  EXPECT_EQ(chrome::kChromeSearchLocalGoogleNtpUrl, ntp_url.spec());
+
+  // Overlay contents should be preloaded.
+  ASSERT_NE(static_cast<InstantOverlay*>(NULL), instant()->overlay());
+  content::WebContents* overlay_contents = instant()->overlay_->contents();
+  EXPECT_TRUE(overlay_contents);
+  GURL overlay_url = overlay_contents->GetURL();
+  EXPECT_EQ(chrome::kChromeSearchLocalGoogleNtpUrl, overlay_url.spec());
+
+  // Instant tab contents should be preloaded.
+  ASSERT_NE(static_cast<InstantTab*>(NULL), instant()->instant_tab());
+  content::WebContents* instant_tab_contents =
+      instant()->instant_tab_->contents();
+  EXPECT_TRUE(instant_tab_contents);
+  GURL instant_tab_url = instant_tab_contents->GetURL();
+  EXPECT_EQ(chrome::kChromeSearchLocalGoogleNtpUrl, instant_tab_url.spec());
+}