Write a test for my previous patch to fix a crash in back/forward navigations

creating new tabs.
Review URL: http://codereview.chromium.org/100031

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

3 files changed, 88 insertions, 10 deletions

diff --git a/chrome/browser/browser_commands_unittest.cc b/chrome/browser/browser_commands_unittest.cc
index 5cecf0a..4a6735c 100644
--- a/chrome/browser/browser_commands_unittest.cc
+++ b/chrome/browser/browser_commands_unittest.cc
@@ -99,3 +99,69 @@ TEST_F(BrowserCommandsTest, BookmarkCurrentPage) {
   EXPECT_EQ(profile(), browser()->profile());
   EXPECT_TRUE(browser()->profile()->GetBookmarkModel()->IsBookmarked(url1));
 }
+
+// Tests back/forward in new tab (Control + Back/Forward button in the UI).
+TEST_F(BrowserCommandsTest, BackForwardInNewTab) {
+  GURL url1("http://foo/1");
+  GURL url2("http://foo/2");
+
+  // Make a tab with the two pages navigated in it.
+  AddTab(browser(), url1);
+  NavigateAndCommitActiveTab(url2);
+
+  // Go back in a new background tab.
+  browser()->GoBack(NEW_BACKGROUND_TAB);
+  EXPECT_EQ(0, browser()->selected_index());
+  ASSERT_EQ(2, browser()->tab_count());
+
+  // The original tab should be unchanged.
+  TabContents* zeroth = browser()->GetTabContentsAt(0);
+  EXPECT_EQ(url2, zeroth->GetURL());
+  EXPECT_TRUE(zeroth->controller().CanGoBack());
+  EXPECT_FALSE(zeroth->controller().CanGoForward());
+
+  // The new tab should be like the first one but navigated back.
+  TabContents* first = browser()->GetTabContentsAt(1);
+  EXPECT_EQ(url1, browser()->GetTabContentsAt(1)->GetURL());
+  EXPECT_FALSE(first->controller().CanGoBack());
+  EXPECT_TRUE(first->controller().CanGoForward());
+
+  // Select the second tab and make it go forward in a new background tab.
+  browser()->SelectTabContentsAt(1, true);
+  // TODO(brettw) bug 11055: It should not be necessary to commit the load here,
+  // but because of this bug, it will assert later if we don't. When the bug is
+  // fixed, one of the three commits here related to this bug should be removed
+  // (to test both codepaths).
+  CommitPendingLoad(&first->controller());
+  EXPECT_EQ(1, browser()->selected_index());
+  browser()->GoForward(NEW_BACKGROUND_TAB);
+
+  // The previous tab should be unchanged and still in the foreground.
+  EXPECT_EQ(url1, first->GetURL());
+  EXPECT_FALSE(first->controller().CanGoBack());
+  EXPECT_TRUE(first->controller().CanGoForward());
+  EXPECT_EQ(1, browser()->selected_index());
+
+  // There should be a new tab navigated forward.
+  ASSERT_EQ(3, browser()->tab_count());
+  TabContents* second = browser()->GetTabContentsAt(2);
+  EXPECT_EQ(url2, second->GetURL());
+  EXPECT_TRUE(second->controller().CanGoBack());
+  EXPECT_FALSE(second->controller().CanGoForward());
+
+  // Now do back in a new foreground tab. Don't bother re-checking every sngle
+  // thing above, just validate that it's opening properly.
+  browser()->SelectTabContentsAt(2, true);
+  // TODO(brettw) bug 11055: see the comment above about why we need this.
+  CommitPendingLoad(&second->controller());
+  browser()->GoBack(NEW_FOREGROUND_TAB);
+  ASSERT_EQ(3, browser()->selected_index());
+  ASSERT_EQ(url1, browser()->GetSelectedTabContents()->GetURL());
+
+  // Same thing again for forward.
+  // TODO(brettw) bug 11055: see the comment above about why we need this.
+  CommitPendingLoad(&browser()->GetSelectedTabContents()->controller());
+  browser()->GoForward(NEW_FOREGROUND_TAB);
+  ASSERT_EQ(4, browser()->selected_index());
+  ASSERT_EQ(url2, browser()->GetSelectedTabContents()->GetURL());
+}
diff --git a/chrome/test/browser_with_test_window_test.cc b/chrome/test/browser_with_test_window_test.cc
index b0d6a84..0746376 100644
--- a/chrome/test/browser_with_test_window_test.cc
+++ b/chrome/test/browser_with_test_window_test.cc
@@ -5,6 +5,7 @@
 #include "chrome/test/browser_with_test_window_test.h"
 
 #include "chrome/browser/browser.h"
+#include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/test/test_browser_window.h"
 #include "chrome/test/testing_profile.h"
@@ -51,25 +52,37 @@ void BrowserWithTestWindowTest::AddTab(Browser* browser, const GURL& url) {
   TabContents* new_tab = browser->AddTabWithURL(url, GURL(),
                                                 PageTransition::TYPED, true,
                                                 0, NULL);
-  CommitPendingLoadAsNewNavigation(&new_tab->controller(), url);
+  CommitPendingLoad(&new_tab->controller());
 }
 
-void BrowserWithTestWindowTest::CommitPendingLoadAsNewNavigation(
-    NavigationController* controller,
-    const GURL& url) {
+void BrowserWithTestWindowTest::CommitPendingLoad(
+    NavigationController* controller) {
+  if (!controller->pending_entry())
+    return;  // Nothing to commit.
+
   TestRenderViewHost* test_rvh =
       TestRenderViewHostForTab(controller->tab_contents());
   MockRenderProcessHost* mock_rph = static_cast<MockRenderProcessHost*>(
     test_rvh->process());
 
-  test_rvh->SendNavigate(mock_rph->max_page_id() + 1, url);
+  // For new navigations, we need to send a larger page ID. For renavigations,
+  // we need to send the preexisting page ID. We can tell these apart because
+  // renavigations will have a pending_entry_index while new ones won't (they'll
+  // just have a standalong pending_entry that isn't in the list already).
+  if (controller->pending_entry_index() >= 0) {
+    test_rvh->SendNavigate(controller->pending_entry()->page_id(),
+                           controller->pending_entry()->url());
+  } else {
+    test_rvh->SendNavigate(mock_rph->max_page_id() + 1,
+                           controller->pending_entry()->url());
+  }
 }
 
 void BrowserWithTestWindowTest::NavigateAndCommit(
     NavigationController* controller,
     const GURL& url) {
   controller->LoadURL(url, GURL(), 0);
-  CommitPendingLoadAsNewNavigation(controller, url);
+  CommitPendingLoad(controller);
 }
 
 void BrowserWithTestWindowTest::NavigateAndCommitActiveTab(const GURL& url) {
diff --git a/chrome/test/browser_with_test_window_test.h b/chrome/test/browser_with_test_window_test.h
index eb5d02a..7331872 100644
--- a/chrome/test/browser_with_test_window_test.h
+++ b/chrome/test/browser_with_test_window_test.h
@@ -58,10 +58,9 @@ class BrowserWithTestWindowTest : public testing::Test {
   // This is a convenience function. The new tab will be added at index 0.
   void AddTab(Browser* browser, const GURL& url);
 
-  // Commits the pending load as if we went to a new page (as opposed to back or
-  // forward).
-  void CommitPendingLoadAsNewNavigation(NavigationController* controller,
-                                        const GURL& url);
+  // Commits the pending load on the given controller. It will keep the
+  // URL of the pending load. If there is no pending load, this does nothing.
+  void CommitPendingLoad(NavigationController* controller);
 
   // Creates a pending navigation on the given navigation controller to the
   // given URL with the default parameters and the commits the load with a page