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

chrome/browser/instant/instant_browsertest.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 "base/command_line.h"
 #include "base/stringprintf.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/history/history.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/instant/instant_controller.h"
@@ skipping 219 matching lines @@
   // Before the commit, the active tab is the NTP (i.e., not Instant).
   TabContents* active_tab = chrome::GetActiveTabContents(browser());
   EXPECT_NE(preview_tab, active_tab);
   EXPECT_EQ(1, active_tab->web_contents()->GetController().GetEntryCount());
   EXPECT_EQ(std::string(chrome::kAboutBlankURL),
             omnibox()->model()->PermanentURL().spec());
 
   // Commit the search by pressing Enter.
   browser()->window()->GetLocationBar()->AcceptInput();
 
-  // After the commit, Instant should not be showing, or even have a preview.
-  EXPECT_FALSE(instant()->GetPreviewContents());
+  // After the commit, Instant should not be showing.
   EXPECT_FALSE(instant()->IsCurrent());
   EXPECT_FALSE(instant()->is_showing());
 
+  // The old loader is deleted and a new one is created.
+  EXPECT_TRUE(instant()->GetPreviewContents());
+  EXPECT_NE(instant()->GetPreviewContents(), preview_tab);
+
   // Check that the current active tab is indeed what was once the preview.
   EXPECT_EQ(preview_tab, chrome::GetActiveTabContents(browser()));
 
   // We should have two navigation entries, one for the NTP, and one for the
   // Instant search that was committed.
   EXPECT_EQ(2, preview_tab->web_contents()->GetController().GetEntryCount());
 
   // Check that the omnibox contains the Instant URL we loaded.
   std::string instant_url = TemplateURLServiceFactory::GetForProfile(
       browser()->profile())->GetDefaultSearchProvider()->instant_url_ref().
@@ skipping 33 matching lines @@
   TabContents* active_tab = chrome::GetActiveTabContents(browser());
   EXPECT_NE(preview_tab, active_tab);
   EXPECT_EQ(1, active_tab->web_contents()->GetController().GetEntryCount());
   EXPECT_EQ(std::string(chrome::kAboutBlankURL),
             omnibox()->model()->PermanentURL().spec());
 
   // Commit the search by clicking on the preview.
   EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
   ui_test_utils::ClickOnView(browser(), VIEW_ID_TAB_CONTAINER);
 
-  // After the commit, Instant should not be showing, or even have a preview.
-  EXPECT_FALSE(instant()->GetPreviewContents());
+  // After the commit, Instant should not be showing.
   EXPECT_FALSE(instant()->IsCurrent());
   EXPECT_FALSE(instant()->is_showing());
 
+  // The old loader is deleted and a new one is created.
+  EXPECT_TRUE(instant()->GetPreviewContents());
+  EXPECT_NE(instant()->GetPreviewContents(), preview_tab);
+
   // Check that the current active tab is indeed what was once the preview.
   EXPECT_EQ(preview_tab, chrome::GetActiveTabContents(browser()));
 
   // We should have two navigation entries, one for the NTP, and one for the
   // Instant search that was committed.
   EXPECT_EQ(2, preview_tab->web_contents()->GetController().GetEntryCount());
 
   // Check that the omnibox contains the Instant URL we loaded.
   std::string instant_url = TemplateURLServiceFactory::GetForProfile(
       browser()->profile())->GetDefaultSearchProvider()->instant_url_ref().
@@ skipping 505 matching lines @@
   content::WindowedNotificationObserver observer(
       chrome::NOTIFICATION_INSTANT_CONTROLLER_HIDDEN,
       content::NotificationService::AllSources());
   chrome::NewEmptyWindow(browser()->profile());
   observer.Wait();
   EXPECT_TRUE(instant()->GetPreviewContents());
   EXPECT_FALSE(instant()->IsCurrent());
   EXPECT_FALSE(instant()->is_showing());
 }
 #endif
+
+// Tests that:
+// - Instant loader is recreated on OnStaleLoader call when it is hidden.
+// - Instant loader is not recreated on OnStaleLoader call when it is visible.
+IN_PROC_BROWSER_TEST_F(InstantTest, InstantLoaderRefresh) {
+  ASSERT_NO_FATAL_FAILURE(SetupInstant("instant.html"));
+  instant()->OnAutocompleteGotFocus();
+  WaitFor(chrome::NOTIFICATION_INSTANT_SUPPORT_DETERMINED);
+
+  // Instant is not showing, so a refresh should create a new preview contents.
+  instant()->OnStaleLoader();
+  WaitFor(chrome::NOTIFICATION_INSTANT_SUPPORT_DETERMINED);
+  // Can't compare the old and new loader pointers here since the same address
+  // may be resued when the new loader is created.

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

I just realized that we _can_ test something here (and so you can remove this
comment about not being able to compare stuff):

// Instant is not showing, so a refresh should create a new preview contents.
EXPECT_TRUE(instant()->loader()->supports_instant());
instant()->OnStaleLoader();
EXPECT_FALSE(instant()->loader()->supports_instant());

WaitFor(chrome::NOTIFICATION_INSTANT_SUPPORT_DETERMINED);
...

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

Done.

Although this works, it feels like we are using a hack to check that the loader
has changed. Maybe we should add a creation time to the loader or number of
loaders created to instant controller to better check this.

+
+  // Show Instant.
+  SetOmniboxText("query");
+  WaitFor(chrome::NOTIFICATION_INSTANT_CONTROLLER_SHOWN);
+  TabContents* preview_tab = instant()->GetPreviewContents();
+
+  // Refresh the loader, the preview contents should remain the same.
+  instant()->OnStaleLoader();
+  EXPECT_EQ(preview_tab, instant()->GetPreviewContents());

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

Since we know the pointers can be reused, this isn't a very strong check.
Instead, do this (because, if the loader had been deleted and recreated, it
wouldn't yet support instant, let alone be showing).

// Refresh the loader, the preview contents should remain the same.
instant()->OnStaleLoader();
EXPECT_TRUE(instant()->is_showing());

Also, add these lines after that (and document it at the top of the test as the
third bullet point):

// The refresh should happen once the omnibox loses focus.
EXPECT_TRUE(instant()->loader()->supports_instant());
instant()->OnAutocompleteLostFocus();
EXPECT_FALSE(instant()->loader()->supports_instant());

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

Done. Will have to stop timer here though.

+}