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

chrome/browser/instant/instant_controller.cc

 // Copyright (c) 2012 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 "chrome/browser/instant/instant_controller.h"
 
 #include "base/command_line.h"
 #include "base/i18n/case_conversion.h"
 #include "base/metrics/histogram.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
@@ skipping 34 matching lines @@
 
 // An artificial delay (in milliseconds) we introduce before telling the Instant
 // page about the new omnibox bounds, in cases where the bounds shrink. This is
 // to avoid the page jumping up/down very fast in response to bounds changes.
 const int kUpdateBoundsDelayMS = 1000;
 
 // The maximum number of times we'll load a non-Instant-supporting search engine
 // 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";
     case InstantController::SUGGEST:  return "_Suggest";
     case InstantController::HIDDEN:   return "_Hidden";
     case InstantController::SILENT:   return "_Silent";
   }
 
   NOTREACHED();
   return std::string();
@@ skipping 192 matching lines @@
   return !IsOutOfDate() && GetPreviewContents() && loader_->supports_instant();
 }
 
 TabContents* InstantController::CommitCurrentPreview(InstantCommitType type) {
   const TabContents* active_tab = delegate_->GetActiveTabContents();
   TabContents* preview = ReleasePreviewContents(type);
   AddSessionStorageHistogram(mode_, active_tab, preview);
   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;
 }
 
 TabContents* InstantController::ReleasePreviewContents(InstantCommitType type) {
   TabContents* preview = loader_->ReleasePreviewContents(type, last_full_text_);
 
   // If the preview page has navigated since the last Update(), we need to add
   // the navigation to history ourselves. Else, the page will navigate after
   // commit, and it will be added to history in the usual manner.
   scoped_refptr<history::HistoryAddPageArgs> last_navigation =
@@ skipping 40 matching lines @@
   // still be on the stack. So, schedule a destruction for later.
   MessageLoop::current()->DeleteSoon(FROM_HERE, loader_.release());
 
   // This call is here to hide the preview and reset view state. It won't
   // actually delete |loader_| because it was just released to DeleteSoon().
   DeleteLoader();
 
   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;
   }
 
 #if defined(TOOLKIT_VIEWS)
   // For views the top level widget is always focused. If the focus change
   // originated in views determine the child Widget from the view that is being
   // focused.
   views::Widget* widget =
       views::Widget::GetWidgetForNativeView(view_gaining_focus);
   if (widget) {
     views::FocusManager* focus_manager = widget->GetFocusManager();
     if (focus_manager && focus_manager->is_changing_focus() &&
         focus_manager->GetFocusedView() &&
         focus_manager->GetFocusedView()->GetWidget()) {
       view_gaining_focus =
           focus_manager->GetFocusedView()->GetWidget()->GetNativeView();
     }
   }
 #endif
 
   gfx::NativeView tab_view =
       GetPreviewContents()->web_contents()->GetNativeView();
 
   // Focus is going to the renderer.
   if (rwhv->GetNativeView() == view_gaining_focus ||
       tab_view == view_gaining_focus) {
 
     // 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;
   }
 
   // Walk up the view hierarchy. If the view gaining focus is a subview of the
   // WebContents view (such as a windowed plugin or http auth dialog), we want
   // to keep the preview contents. Otherwise, focus has gone somewhere else,
   // such as the JS inspector, and we want to cancel the preview.
   gfx::NativeView view_gaining_focus_ancestor = view_gaining_focus;
   while (view_gaining_focus_ancestor &&
          view_gaining_focus_ancestor != tab_view) {
     view_gaining_focus_ancestor =
         platform_util::GetParent(view_gaining_focus_ancestor);
   }
 
   if (view_gaining_focus_ancestor) {
     CommitCurrentPreview(INSTANT_COMMIT_FOCUS_LOST);
     return;
   }
 
   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 {
   return GetPreviewContents() && loader_->IsPointerDownFromActivate();
 }
 
 void InstantController::SetSuggestions(
     InstantLoader* loader,
     const std::vector<InstantSuggestion>& suggestions) {
   DCHECK_EQ(loader_.get(), loader);
@@ skipping 77 matching lines @@
     delegate_->InstantPreviewFocused();
 #endif
 }
 
 void InstantController::ResetLoader(const std::string& instant_url,
                                     const TabContents* active_tab) {
   if (GetPreviewContents() && loader_->instant_url() != 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();
   last_user_text_.clear();
   last_verbatim_ = false;
   last_suggestion_ = InstantSuggestion();
   last_transition_type_ = content::PAGE_TRANSITION_LINK;
   last_omnibox_bounds_ = gfx::Rect();
   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();
 }
 
 void InstantController::Show() {
   if (!is_showing_) {
     is_showing_ = true;
     delegate_->ShowInstant();
     AddPreviewUsageForHistogram(mode_, PREVIEW_SHOWED);
   }
 }
@@ skipping 52 matching lines @@
     return false;
   }
 
   return true;
 }
 
 bool InstantController::IsOutOfDate() const {
   return !last_active_tab_ ||
          last_active_tab_ != delegate_->GetActiveTabContents();
 }