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

chrome/browser/ui/search/instant_controller.cc

Index: chrome/browser/ui/search/instant_controller.cc
diff --git a/chrome/browser/ui/search/instant_controller.cc b/chrome/browser/ui/search/instant_controller.cc
index f63aeab6c07470428821cdb5c2a6fbee6c264dc8..a402cbf327f88d43762555dddd32ea2e16aed8db 100644
--- a/chrome/browser/ui/search/instant_controller.cc
+++ b/chrome/browser/ui/search/instant_controller.cc
@@ -1074,6 +1074,29 @@ void InstantController::UndoAllMostVisitedDeletions() {
   top_sites->ClearBlacklistedURLs();
 }
 
+void InstantController::InstantPageLoadFailed(content::WebContents* contents) {

[samarth, 2013/05/01 06:26:47]

Please keep the functions in the same order as in the header.

[David Black, 2013/05/02 00:49:45]

Done.

+  GURL local_fallback_url = chrome::GetLocalInstantURL(browser_->profile());
+
+  if (instant_tab_ && IsContentsFrom(instant_tab(), contents)) {
+    if (instant_tab_->IsLocal())
+      return;
+    SearchTabHelper::FromWebContents(contents)->RedirectingToLocal();

[samarth, 2013/05/01 06:26:47]

It't not clear to me why this is necessary.  Do you really just want to call
ResetInstantTab after the Redirect?

[David Black, 2013/05/02 00:49:45]

Fixed as we discussed in person.

+    RedirectToLocalNTP(contents);

[samarth, 2013/05/01 06:26:47]

I can try this tomorrow but what if you have a search page open, lose internet
connection, and then reload?  I think you'll just get redirected to the local
NTP.

It's not really a theoretical concern: this can happen if you have session
restore and reopen Chrome when you had a bunch of search tabs open.

[David Black, 2013/05/02 00:49:45]

Good point.  Changed to fall back only if the URL exactly equals the base online
Instant URL.

+  }
+
+  if (ntp_ && IsContentsFrom(ntp(), contents)) {
+    if (ntp_->IsLocal())
+      return;
+    ResetNTP(false, true);

[samarth, 2013/05/01 06:26:47]

No, this will lead to an infinite loop of reloading (the true means use the
remote NTP again).  You should just delete ntp_.

Also, you're deleting ntp_ with ntp_ on the stack.  You need to do a
DeleteSoon(ntp_.release()).

[David Black, 2013/05/02 00:49:45]

Will not cause an infinite loop - second param is use_local, not use_server.

Added DeleteSoon.  

Kept in the ResetNTP call so the NTP still gets preloaded.  The speed difference
between preloading the local NTP and loading it once they create a new tab is
noticeable, so it's nice to do it.

+  }
+
+  if (overlay_ && IsContentsFrom(overlay(), contents)) {
+    if (overlay_->IsLocal())
+      return;
+    CreateOverlay(local_fallback_url.spec(), contents);

[samarth, 2013/05/01 06:26:47]

Update already handles the fallback, so you just need to delete overlay_.

[David Black, 2013/05/02 00:49:45]

Update is too late - they've already typed a character at that point, right? 
They'll then have to wait for the local page to fully load before seeing
anything, resulting in a latency hit.  Like with the ntp case above, it's much
better to preload the local overlay.

+  }
+}
+
 void InstantController::Observe(int type,
                                 const content::NotificationSource& source,
                                 const content::NotificationDetails& details) {
@@ -1719,3 +1742,12 @@ bool InstantController::UsingLocalPage() const {
   return (instant_tab_ && instant_tab_->IsLocal()) ||
       (!instant_tab_ && overlay_ && overlay_->IsLocal());
 }
+
+void InstantController::RedirectToLocalNTP(content::WebContents* contents) {
+  contents->GetController().LoadURL(
+    chrome::GetLocalInstantURL(browser_->profile()),
+    content::Referrer(),
+    content::PAGE_TRANSITION_SERVER_REDIRECT,
+    std::string());  // No extra headers.
+  // TODO(dcblack): Remove extraneous history entry.
+}