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 30 matching lines @@
   // Type a query into the omnibox. This should cause an onresize() event, with
   // a valid (non-zero) height.
   SetOmniboxText("search");
   WaitFor(chrome::NOTIFICATION_INSTANT_CONTROLLER_SHOWN);
 
   EXPECT_TRUE(UpdateSearchState(instant()->GetPreviewContents()));
   EXPECT_EQ(2, onresizecalls_);
   EXPECT_LT(0, height_);
 }
 
+// Tests that the instant loader is refreshed when it gets stale but is not

[sreeram, 2012/08/22 17:39:35]

Also, why put this test here, in the middle of the file? Seems logical to add
new tests (that are not obviously part of an existing group) to the bottom of
the file.

[sreeram, 2012/08/22 17:39:35]

Tests -> Test
instant -> Instant

I'd reword this comment slightly, since you are not actually testing the "when
it gets stale" part:
// Test that the Instant loader can be refreshed, but not if it's showing.

[Shishir, 2012/08/23 18:26:33]

Done.

[Shishir, 2012/08/23 18:26:33]

Done.

+// refreshed if it is showing.
+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 the preview

[sreeram, 2012/08/22 17:39:35]

a new the preview -> a new preview

[Shishir, 2012/08/23 18:26:33]

Done.

+  // contents.
+  instant()->RefreshLoader();
+  WaitFor(chrome::NOTIFICATION_INSTANT_SUPPORT_DETERMINED);

[sreeram, 2012/08/22 17:39:35]

Put in a comment about why we are not checking EXPECT_NE(before, after). I.e.,
because after deleting and recreating, we could end up with the same address.

[Shishir, 2012/08/23 18:26:33]

Done.

+
+  // Show instant.

[sreeram, 2012/08/22 17:39:35]

instant -> Instant

[Shishir, 2012/08/23 18:26:33]

Done.

+  SetOmniboxText("query");
+  WaitFor(chrome::NOTIFICATION_INSTANT_CONTROLLER_SHOWN);
+  TabContents* preview_tab = instant()->GetPreviewContents();
+
+  // Refresh the loader, the preview contents should remain the same.
+  instant()->RefreshLoader();
+  EXPECT_EQ(preview_tab, instant()->GetPreviewContents());
+}
+
 // Test that the INSTANT_COMPLETE_NOW behavior works as expected.
 IN_PROC_BROWSER_TEST_F(InstantTest, SuggestionIsCompletedNow) {
   ASSERT_NO_FATAL_FAILURE(SetupInstant("instant.html"));
   instant()->OnAutocompleteGotFocus();
   WaitFor(chrome::NOTIFICATION_INSTANT_SUPPORT_DETERMINED);
 
   // Tell the JS to request for the given behavior.
   EXPECT_TRUE(ExecuteScript("behavior = 'now'"));
 
   SetOmniboxText("query");
@@ skipping 456 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