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

chrome/browser/instant/instant_browsertest.cc

Index: chrome/browser/instant/instant_browsertest.cc
diff --git a/chrome/browser/instant/instant_browsertest.cc b/chrome/browser/instant/instant_browsertest.cc
index 1c210afd3a9e469918418c15c1176d6cdcfb6b5b..3de603776ce0fe546a673a6ea11e9d0e5b4b0870 100644
--- a/chrome/browser/instant/instant_browsertest.cc
+++ b/chrome/browser/instant/instant_browsertest.cc
@@ -237,11 +237,14 @@ IN_PROC_BROWSER_TEST_F(InstantTest, OnSubmitEvent) {
   // 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()));
 
@@ -295,11 +298,14 @@ IN_PROC_BROWSER_TEST_F(InstantTest, OnCancelEvent) {
   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()));
 
@@ -825,3 +831,27 @@ IN_PROC_BROWSER_TEST_F(InstantTest, NewWindowDismissesInstant) {
   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.

+}