Recreate the loader as soon as it is deleted and ensure that it does not become stale.

chrome/browser/instant/instant_controller.cc

Index: chrome/browser/instant/instant_controller.cc
diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc
index 6133e45715c52918198fc2d6094e374107683ec0..3e29ca30fc2d9092dbca3c2844619fe261da9ca4 100644
--- a/chrome/browser/instant/instant_controller.cc
+++ b/chrome/browser/instant/instant_controller.cc
@@ -52,6 +52,10 @@ const int kUpdateBoundsDelayMS = 1000;
 // before we give up and blacklist it for the rest of the browsing session.
 const int kMaxInstantSupportFailures = 10;
 
+// If an Instant page has not been used in this many milliseconds, it is

[sreeram, 2012/08/24 21:06:25]

this -> these

[Shishir, 2012/08/24 22:17:39]

Done.

+// reloaded so that the page does not become stale.
+const int kStaleLoaderTimeoutMS = 3 * 3600 * 1000;
+
 std::string ModeToString(InstantController::Mode mode) {
   switch (mode) {
     case InstantController::INSTANT:  return "_Instant";
@@ -264,6 +268,11 @@ TabContents* InstantController::CommitCurrentPreview(InstantCommitType type) {
   preview->web_contents()->GetController().CopyStateFromAndPrune(
       &active_tab->web_contents()->GetController());
   delegate_->CommitInstant(preview);
+
+  // Try to create another loader immediately so that it is ready for the next
+  // user interaction.
+  CreateDefaultLoader();
+
   return preview;
 }
 
@@ -324,24 +333,31 @@ TabContents* InstantController::ReleasePreviewContents(InstantCommitType type) {
   return preview;
 }
 
-// TODO(sreeram): Since we never delete the loader except when committing
-// Instant, the loader may have a very stale page. Reload it when stale.
 void InstantController::OnAutocompleteLostFocus(
     gfx::NativeView view_gaining_focus) {
   DCHECK(!is_showing_ || GetPreviewContents());
 
-  // If the preview is not showing, nothing to do.
-  if (!is_showing_ || !GetPreviewContents())
+  // If there is no preview, nothing to do.
+  if (!GetPreviewContents())
+    return;
+
+  // If the preview is not showing, only need to check for loader staleness.
+  if (!is_showing_) {
+    MaybeOnStaleLoader();
     return;
+  }
 
 #if defined(OS_MACOSX)
-  if (!loader_->IsPointerDownFromActivate())
+  if (!loader_->IsPointerDownFromActivate()) {
     Hide();
+    MaybeOnStaleLoader();
+  }
 #else
   content::RenderWidgetHostView* rwhv =
       GetPreviewContents()->web_contents()->GetRenderWidgetHostView();
   if (!view_gaining_focus || !rwhv) {
     Hide();
+    MaybeOnStaleLoader();
     return;
   }
 
@@ -371,8 +387,10 @@ void InstantController::OnAutocompleteLostFocus(
 
     // If the mouse is not down, focus is not going to the renderer. Someone
     // else moved focus and we shouldn't commit.
-    if (!loader_->IsPointerDownFromActivate())
+    if (!loader_->IsPointerDownFromActivate()) {
       Hide();
+      MaybeOnStaleLoader();
+    }
 
     return;
   }
@@ -394,27 +412,12 @@ void InstantController::OnAutocompleteLostFocus(
   }
 
   Hide();
+  MaybeOnStaleLoader();
 #endif
 }
 
 void InstantController::OnAutocompleteGotFocus() {
-  const TabContents* active_tab = delegate_->GetActiveTabContents();
-
-  // We could get here with no active tab if the Browser is closing.
-  if (!active_tab)
-    return;
-
-  // Since we don't have any autocomplete match to work with, we'll just use
-  // the default search provider's Instant URL.
-  const TemplateURL* template_url =
-      TemplateURLServiceFactory::GetForProfile(active_tab->profile())->
-                                 GetDefaultSearchProvider();
-
-  std::string instant_url;
-  if (!GetInstantURL(template_url, &instant_url))
-    return;
-
-  ResetLoader(instant_url, active_tab);
+  CreateDefaultLoader();
 }
 
 bool InstantController::commit_on_pointer_release() const {
@@ -512,12 +515,52 @@ void InstantController::ResetLoader(const std::string& instant_url,
     DeleteLoader();
 
   if (!GetPreviewContents()) {
+    DCHECK(!loader_.get());
     loader_.reset(new InstantLoader(this, instant_url, active_tab));
     loader_->Init();
     AddPreviewUsageForHistogram(mode_, PREVIEW_CREATED);
+
+    // Reset the loader timer.
+    stale_loader_timer_.Stop();
+    stale_loader_timer_.Start(
+        FROM_HERE,
+        base::TimeDelta::FromMilliseconds(kStaleLoaderTimeoutMS), this,
+        &InstantController::OnStaleLoader);
   }
 }
 
+void InstantController::CreateDefaultLoader() {
+  const TabContents* active_tab = delegate_->GetActiveTabContents();
+
+  // We could get here with no active tab if the Browser is closing.
+  if (!active_tab)
+    return;
+
+  const TemplateURL* template_url =
+      TemplateURLServiceFactory::GetForProfile(active_tab->profile())->
+                                 GetDefaultSearchProvider();
+  std::string instant_url;
+  if (!GetInstantURL(template_url, &instant_url))
+    return;
+
+  ResetLoader(instant_url, active_tab);
+}
+
+void InstantController::OnStaleLoader() {
+  // If the loader is showing, do not delete it. It will get deleted the next
+  // time the autocomplete looses focus.

[sreeram, 2012/08/24 21:06:25]

looses -> loses

[Shishir, 2012/08/24 22:17:39]

Done.

+  if (is_showing_)
+    return;
+
+  DeleteLoader();
+  CreateDefaultLoader();
+}
+
+void InstantController::MaybeOnStaleLoader() {
+  if (!stale_loader_timer_.IsRunning())
+    OnStaleLoader();
+}
+
 void InstantController::DeleteLoader() {
   Hide();
   last_full_text_.clear();
@@ -529,6 +572,7 @@ void InstantController::DeleteLoader() {
   url_for_history_ = GURL();
   if (GetPreviewContents())
     AddPreviewUsageForHistogram(mode_, PREVIEW_DELETED);
+  stale_loader_timer_.Stop();

[sreeram, 2012/08/24 21:06:25]

Don't do this. Consider this sequence of events:
1. User focuses the omnibox.
2. We create the loader, and start the timer.
3. The loader comes back and says "instant not supported".
4. We call DeleteLoader().
--> This now stops the timer.

Without this line, we'll at least try again after 3 hours to see if the page now
supports Instant.

If the user starts typing before 3 hours, of course, we'll try to load the page
also. But it seems that it's not a bad idea to try again after 3 hours, if the
user hasn't caused an explicit load by typing.

[Shishir, 2012/08/24 22:17:39]

Removed.

I think we need to handle the doenst support case better - depending on why the
doesn't support instant fails - like network error.

   loader_.reset();
 }