Fixes bug where needs_reload_ was not getting set to false in some

situations, leading to a stuck throbber.

BUG=31462
TEST=see bug

Review URL: http://codereview.chromium.org/554025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@36811 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 31 insertions, 1 deletions

diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc
index 129bca2..a038779 100644
--- a/chrome/browser/tab_contents/navigation_controller.cc
+++ b/chrome/browser/tab_contents/navigation_controller.cc
@@ -941,6 +941,8 @@ void NavigationController::SetWindowID(const SessionID& id) {
 }
 
 void NavigationController::NavigateToPendingEntry(bool reload) {
+  needs_reload_ = false;
+
   // For session history navigations only the pending_entry_index_ is set.
   if (!pending_entry_) {
     DCHECK(pending_entry_index_ != -1);
@@ -989,7 +991,6 @@ void NavigationController::LoadIfNecessary() {
   if (!needs_reload_)
     return;
 
-  needs_reload_ = false;
   // Calling Reload() results in ignoring state, and not loading.
   // Explicitly use NavigateToPendingEntry so that the renderer uses the
   // cached state.
diff --git a/chrome/browser/tab_contents/navigation_controller_unittest.cc b/chrome/browser/tab_contents/navigation_controller_unittest.cc
index a585829..86b6da1 100644
--- a/chrome/browser/tab_contents/navigation_controller_unittest.cc
+++ b/chrome/browser/tab_contents/navigation_controller_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "base/file_util.h"
 #include "base/path_service.h"
+#include "base/scoped_ptr.h"
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "chrome/browser/profile_manager.h"
@@ -1519,6 +1520,23 @@ TEST_F(NavigationControllerTest, ViewSourceRedirect) {
   EXPECT_EQ(true, contents()->ShouldDisplayURL());
 }
 
+// Make sure that on cloning a tabcontents and going back needs_reload is false.
+TEST_F(NavigationControllerTest, CloneAndGoBack) {
+  const GURL url1("http://foo1");
+  const GURL url2("http://foo2");
+
+  NavigateAndCommit(url1);
+  NavigateAndCommit(url2);
+
+  scoped_ptr<TabContents> clone(controller().tab_contents()->Clone());
+
+  ASSERT_EQ(2, clone->controller().entry_count());
+  EXPECT_TRUE(clone->controller().needs_reload());
+  clone->controller().GoBack();
+  // Navigating back should have triggered needs_reload_ to go false.
+  EXPECT_FALSE(clone->controller().needs_reload());
+}
+
 /* TODO(brettw) These test pass on my local machine but fail on the XP buildbot
    (but not Vista) cleaning up the directory after they run.
    This should be fixed.
diff --git a/chrome/browser/tab_contents/test_tab_contents.cc b/chrome/browser/tab_contents/test_tab_contents.cc
index 97643f3..3724b8c 100644
--- a/chrome/browser/tab_contents/test_tab_contents.cc
+++ b/chrome/browser/tab_contents/test_tab_contents.cc
@@ -15,3 +15,10 @@ TestRenderViewHost* TestTabContents::pending_rvh() {
   return static_cast<TestRenderViewHost*>(
       render_manager_.pending_render_view_host_);
 }
+
+TabContents* TestTabContents::Clone() {
+  TabContents* tc = new TestTabContents(
+      profile(), SiteInstance::CreateSiteInstance(profile()));
+  tc->controller().CopyStateFrom(controller_);
+  return tc;
+}
diff --git a/chrome/browser/tab_contents/test_tab_contents.h b/chrome/browser/tab_contents/test_tab_contents.h
index 7bab690..b626686 100644
--- a/chrome/browser/tab_contents/test_tab_contents.h
+++ b/chrome/browser/tab_contents/test_tab_contents.h
@@ -48,6 +48,10 @@ class TestTabContents : public TabContents {
   }
   void UpdateRenderViewSizeForRenderManager() {}
 
+  // Returns a clone of this TestTabContents. The returned object is also a
+  // TestTabContents. The caller owns the returned object.
+  virtual TabContents* Clone();
+
   // Set by individual tests.
   bool transition_cross_site;
 };