This is almost a complete rewrite of DidNavigate and the associated NavigationController logic. The approach is that the NavigationController should be responsible for the logic and memory management of navigation. Previously, half the logic and memory management lived in WebContents which made it very hard to figure out what was going on.

I split out the various navigation types into separate functions, which then copy and update any existing NavigationEntry as necessary. Previously, WebContents would make a new one which would be manually populated with random fields (I think some were forgotten, too), and then the NavigationController may or may not commit it.

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

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

1 files changed, 438 insertions, 226 deletions

diff --git a/chrome/browser/navigation_controller_unittest.cc b/chrome/browser/navigation_controller_unittest.cc
index 035c2ad..cfd372b 100644
--- a/chrome/browser/navigation_controller_unittest.cc
+++ b/chrome/browser/navigation_controller_unittest.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/navigation_entry.h"
 #include "chrome/browser/profile_manager.h"
 #include "chrome/browser/history/history.h"
+#include "chrome/browser/session_service.h"
 #include "chrome/browser/session_service_test_helper.h"
 #include "chrome/browser/tab_contents.h"
 #include "chrome/browser/tab_contents_delegate.h"
@@ -33,6 +34,10 @@ const TabContentsType kTestContentsType1 =
 const TabContentsType kTestContentsType2 =
     static_cast<TabContentsType>(TAB_CONTENTS_NUM_TYPES + 2);
 
+// Tests can set this to set the site instance for all the test contents. This
+// refcounted pointer will be automatically derefed on cleanup.
+static SiteInstance* site_instance;
+
 // TestContents ----------------------------------------------------------------
 
 class TestContents : public TabContents {
@@ -43,24 +48,41 @@ class TestContents : public TabContents {
   TestContents(TabContentsType type) : TabContents(type) {
   }
 
-  // Just record the navigation so it can be checked by the test case
-  bool Navigate(const NavigationEntry& entry, bool reload) {
-    pending_entry_.reset(new NavigationEntry(entry));
+  // Overridden from TabContents so we can provide a non-NULL site instance in
+  // some cases. To use, the test will have to set the site_instance_ member
+  // variable to some site instance it creates.
+  virtual SiteInstance* GetSiteInstance() const {
+    return site_instance;
+  }
+
+  // Just record the navigation so it can be checked by the test case. We don't
+  // want the normal behavior of TabContents just saying it committed since we
+  // want to behave more like the renderer and call RendererDidNavigate.
+  virtual bool NavigateToPendingEntry(bool reload) {
+    pending_entry_.reset(new NavigationEntry(*controller()->GetPendingEntry()));
     return true;
   }
 
-  void CompleteNavigation(int page_id) {
-    DCHECK(pending_entry_.get());
-    pending_entry_->set_page_id(page_id);
+  // Sets up a call to RendererDidNavigate pretending to be a main frame
+  // navigation to the given URL.
+  void CompleteNavigationAsRenderer(int page_id, const GURL& url) {
+    ViewHostMsg_FrameNavigate_Params params;
+    params.page_id = page_id;
+    params.url = url;
+    params.transition = PageTransition::LINK;
+    params.should_update_history = false;
+    params.gesture = NavigationGestureUser;
+    params.is_post = false;
+
     NavigationController::LoadCommittedDetails details;
-    DidNavigateToEntry(pending_entry_.get(), &details);
-    controller()->NotifyEntryChangedByPageID(type(), NULL, page_id);
-    pending_entry_.release();
+    controller()->RendererDidNavigate(params, false, &details);
   }
 
   NavigationEntry* pending_entry() const { return pending_entry_.get(); }
   void set_pending_entry(NavigationEntry* e) { pending_entry_.reset(e); }
 
+ protected:
+
  private:
   scoped_ptr<NavigationEntry> pending_entry_;
 };
@@ -116,6 +138,11 @@ class NavigationControllerTest : public testing::Test,
   }
 
   virtual void TearDown() {
+    if (site_instance) {
+      site_instance->Release();
+      site_instance = NULL;
+    }
+
     // Make sure contents is valid. NavigationControllerHistoryTest ends up
     // resetting this before TearDown is invoked.
     if (contents)
@@ -300,7 +327,6 @@ TEST_F(NavigationControllerTest, Defaults) {
   EXPECT_EQ(contents->controller()->GetEntryCount(), 0);
   EXPECT_FALSE(contents->controller()->CanGoBack());
   EXPECT_FALSE(contents->controller()->CanGoForward());
-  EXPECT_FALSE(contents->controller()->CanStop());
 }
 
 TEST_F(NavigationControllerTest, LoadURL) {
@@ -329,9 +355,8 @@ TEST_F(NavigationControllerTest, LoadURL) {
   // We should have gotten no notifications from the preceeding checks.
   EXPECT_EQ(0, notifications.size());
 
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // The load should now be committed.
   EXPECT_EQ(contents->controller()->GetEntryCount(), 1);
@@ -358,9 +383,8 @@ TEST_F(NavigationControllerTest, LoadURL) {
   EXPECT_FALSE(contents->controller()->CanGoForward());
   EXPECT_EQ(contents->GetMaxPageID(), 0);
 
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(1, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // The load should now be committed.
   EXPECT_EQ(contents->controller()->GetEntryCount(), 2);
@@ -385,18 +409,15 @@ TEST_F(NavigationControllerTest, LoadURL_SamePage) {
 
   contents->controller()->LoadURL(url1, PageTransition::TYPED);
   EXPECT_EQ(0, notifications.size());
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
-
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->LoadURL(url1, PageTransition::TYPED);
   EXPECT_EQ(0, notifications.size());
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
-  // should not have produced a new session history entry
+  // We should not have produced a new session history entry.
   EXPECT_EQ(contents->controller()->GetEntryCount(), 1);
   EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 0);
   EXPECT_EQ(contents->controller()->GetPendingEntryIndex(), -1);
@@ -416,9 +437,8 @@ TEST_F(NavigationControllerTest, LoadURL_Discarded) {
 
   contents->controller()->LoadURL(url1, PageTransition::TYPED);
   EXPECT_EQ(0, notifications.size());
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->LoadURL(url2, PageTransition::TYPED);
   contents->controller()->DiscardPendingEntry();
@@ -443,18 +463,12 @@ TEST_F(NavigationControllerTest, LoadURL_NoPending) {
   // First make an existing committed entry.
   const GURL kExistingURL1("test1:eh");
   contents->controller()->LoadURL(kExistingURL1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
-
+  contents->CompleteNavigationAsRenderer(0, kExistingURL1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+	
   // Do a new navigation without making a pending one.
   const GURL kNewURL("test1:see");
-  NavigationEntry* entry = new NavigationEntry(kTestContentsType1);
-  entry->set_page_id(2);
-  entry->set_url(kNewURL);
-  entry->set_title(L"Hello, world");
-  NavigationController::LoadCommittedDetails details;
-  contents->controller()->DidNavigateToEntry(entry, &details);
+  contents->CompleteNavigationAsRenderer(99, kNewURL);
 
   // There should no longer be any pending entry, and the third navigation we
   // just made should be committed.
@@ -475,9 +489,8 @@ TEST_F(NavigationControllerTest, LoadURL_NewPending) {
   // First make an existing committed entry.
   const GURL kExistingURL1("test1:eh");
   contents->controller()->LoadURL(kExistingURL1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, kExistingURL1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // Make a pending entry to somewhere new.
   const GURL kExistingURL2("test1:bee");
@@ -486,18 +499,10 @@ TEST_F(NavigationControllerTest, LoadURL_NewPending) {
 
   // Before that commits, do a new navigation.
   const GURL kNewURL("test1:see");
-  NavigationEntry* entry = new NavigationEntry(kTestContentsType1);
-  entry->set_page_id(3);
-  entry->set_url(kNewURL);
-  entry->set_title(L"Hello, world");
-  NavigationController::LoadCommittedDetails details;
-  contents->controller()->DidNavigateToEntry(entry, &details);
+  contents->CompleteNavigationAsRenderer(3, kNewURL);
 
   // There should no longer be any pending entry, and the third navigation we
   // just made should be committed.
-  // Note that we don't expect a CHANGED notification. It turns out that this
-  // is sent by the TestContents and not any code we're interested in testing,
-  // and this function doesn't get called when we manually call the controller.
   EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
   EXPECT_EQ(-1, contents->controller()->GetPendingEntryIndex());
   EXPECT_EQ(1, contents->controller()->GetLastCommittedEntryIndex());
@@ -514,15 +519,13 @@ TEST_F(NavigationControllerTest, LoadURL_ExistingPending) {
   // First make some history.
   const GURL kExistingURL1("test1:eh");
   contents->controller()->LoadURL(kExistingURL1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, kExistingURL1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   const GURL kExistingURL2("test1:bee");
   contents->controller()->LoadURL(kExistingURL2, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(1, kExistingURL2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // Now make a pending back/forward navigation. The zeroth entry should be
   // pending.
@@ -533,12 +536,8 @@ TEST_F(NavigationControllerTest, LoadURL_ExistingPending) {
 
   // Before that commits, do a new navigation.
   const GURL kNewURL("test1:see");
-  NavigationEntry* entry = new NavigationEntry(kTestContentsType1);
-  entry->set_page_id(3);
-  entry->set_url(kNewURL);
-  entry->set_title(L"Hello, world");
   NavigationController::LoadCommittedDetails details;
-  contents->controller()->DidNavigateToEntry(entry, &details);
+  contents->CompleteNavigationAsRenderer(3, kNewURL);
 
   // There should no longer be any pending entry, and the third navigation we
   // just made should be committed.
@@ -556,9 +555,8 @@ TEST_F(NavigationControllerTest, Reload) {
 
   contents->controller()->LoadURL(url1, PageTransition::TYPED);
   EXPECT_EQ(0, notifications.size());
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->Reload();
   EXPECT_EQ(0, notifications.size());
@@ -572,9 +570,8 @@ TEST_F(NavigationControllerTest, Reload) {
   EXPECT_FALSE(contents->controller()->CanGoBack());
   EXPECT_FALSE(contents->controller()->CanGoForward());
 
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // Now the reload is committed.
   EXPECT_EQ(contents->controller()->GetEntryCount(), 1);
@@ -595,18 +592,16 @@ TEST_F(NavigationControllerTest, Reload_GeneratesNewPage) {
   const GURL url2("test1:foo2");
 
   contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->Reload();
   EXPECT_EQ(0, notifications.size());
 
   contents->pending_entry()->set_url(url2);
   contents->pending_entry()->set_transition_type(PageTransition::LINK);
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(1, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // Now the reload is committed.
   EXPECT_EQ(contents->controller()->GetEntryCount(), 2);
@@ -624,17 +619,12 @@ TEST_F(NavigationControllerTest, Back) {
   RegisterForAllNavNotifications(&notifications, contents->controller());
 
   const GURL url1("test1:foo1");
-  const GURL url2("test1:foo2");
-
-  contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
-  contents->controller()->LoadURL(url2, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  const GURL url2("test1:foo2");
+  contents->CompleteNavigationAsRenderer(1, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->GoBack();
   EXPECT_EQ(0, notifications.size());
@@ -648,9 +638,8 @@ TEST_F(NavigationControllerTest, Back) {
   EXPECT_FALSE(contents->controller()->CanGoBack());
   EXPECT_TRUE(contents->controller()->CanGoForward());
 
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // The back navigation completed successfully.
   EXPECT_EQ(contents->controller()->GetEntryCount(), 2);
@@ -672,14 +661,12 @@ TEST_F(NavigationControllerTest, Back_GeneratesNewPage) {
   const GURL url3("test1:foo3");
 
   contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->LoadURL(url2, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(1, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->GoBack();
   EXPECT_EQ(0, notifications.size());
@@ -693,11 +680,8 @@ TEST_F(NavigationControllerTest, Back_GeneratesNewPage) {
   EXPECT_FALSE(contents->controller()->CanGoBack());
   EXPECT_TRUE(contents->controller()->CanGoForward());
 
-  contents->pending_entry()->set_url(url3);
-  contents->pending_entry()->set_transition_type(PageTransition::LINK);
-  contents->CompleteNavigation(2);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(2, url3);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // The back navigation resulted in a completely new navigation.
   // TODO(darin): perhaps this behavior will be confusing to users?
@@ -720,15 +704,12 @@ TEST_F(NavigationControllerTest, Back_NewPending) {
   const GURL kUrl3("test1:foo3");
 
   // First navigate two places so we have some back history.
-  contents->controller()->LoadURL(kUrl1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, kUrl1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
-  contents->controller()->LoadURL(kUrl2, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  //contents->controller()->LoadURL(kUrl2, PageTransition::TYPED);
+  contents->CompleteNavigationAsRenderer(1, kUrl2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // Now start a new pending navigation and go back before it commits.
   contents->controller()->LoadURL(kUrl3, PageTransition::TYPED);
@@ -750,32 +731,23 @@ TEST_F(NavigationControllerTest, Back_OtherBackPending) {
   const GURL kUrl3("test1:foo3");
 
   // First navigate three places so we have some back history.
-  contents->controller()->LoadURL(kUrl1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  contents->controller()->LoadURL(kUrl2, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
-  contents->controller()->LoadURL(kUrl3, PageTransition::TYPED);
-  contents->CompleteNavigation(2);
+  contents->CompleteNavigationAsRenderer(0, kUrl1);
+  contents->CompleteNavigationAsRenderer(1, kUrl2);
+  contents->CompleteNavigationAsRenderer(2, kUrl3);
 
   // With nothing pending, say we get a navigation to the second entry.
-  const std::wstring kNewTitle1(L"Hello, world");
-  NavigationEntry* entry = new NavigationEntry(kTestContentsType1);
-  entry->set_page_id(1);
-  entry->set_url(kUrl2);
-  entry->set_title(kNewTitle1);
-  NavigationController::LoadCommittedDetails details;
-  contents->controller()->DidNavigateToEntry(entry, &details);
+  contents->CompleteNavigationAsRenderer(1, kUrl2);
 
   // That second URL should be the last committed and it should have gotten the
   // new title.
-  EXPECT_EQ(kNewTitle1, contents->controller()->GetEntryWithPageID(
-      kTestContentsType1, NULL, 1)->title());
+  EXPECT_EQ(kUrl2, contents->controller()->GetEntryWithPageID(
+      kTestContentsType1, NULL, 1)->url());
   EXPECT_EQ(1, contents->controller()->GetLastCommittedEntryIndex());
   EXPECT_EQ(-1, contents->controller()->GetPendingEntryIndex());
 
   // Now go forward to the last item again and say it was committed.
   contents->controller()->GoForward();
-  contents->CompleteNavigation(2);
+  contents->CompleteNavigationAsRenderer(2, kUrl3);
 
   // Now start going back one to the second page. It will be pending.
   contents->controller()->GoBack();
@@ -784,21 +756,14 @@ TEST_F(NavigationControllerTest, Back_OtherBackPending) {
 
   // Not synthesize a totally new back event to the first page. This will not
   // match the pending one.
-  const std::wstring kNewTitle2(L"Hello, world");
-  entry = new NavigationEntry(kTestContentsType1);
-  entry->set_page_id(0);
-  entry->set_url(kUrl1);
-  entry->set_title(kNewTitle2);
-  contents->controller()->DidNavigateToEntry(entry, &details);
+  contents->CompleteNavigationAsRenderer(0, kUrl1);
   
   // The navigation should not have affected the pending entry.
   EXPECT_EQ(1, contents->controller()->GetPendingEntryIndex());
 
-  // But the navigated entry should be updated to the new title, and should be
-  // the last committed.
-  EXPECT_EQ(kNewTitle2, contents->controller()->GetEntryWithPageID(
-      kTestContentsType1, NULL, 0)->title());
+  // But the navigated entry should be the last committed.
   EXPECT_EQ(0, contents->controller()->GetLastCommittedEntryIndex());
+  EXPECT_EQ(kUrl1, contents->controller()->GetLastCommittedEntry()->url());
 }
 
 // Tests what happens when we navigate forward successfully.
@@ -809,20 +774,15 @@ TEST_F(NavigationControllerTest, Forward) {
   const GURL url1("test1:foo1");
   const GURL url2("test1:foo2");
 
-  contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
-  contents->controller()->LoadURL(url2, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(1, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->GoBack();
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->GoForward();
 
@@ -835,9 +795,8 @@ TEST_F(NavigationControllerTest, Forward) {
   EXPECT_TRUE(contents->controller()->CanGoBack());
   EXPECT_FALSE(contents->controller()->CanGoForward());
 
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(1, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // The forward navigation completed successfully.
   EXPECT_EQ(contents->controller()->GetEntryCount(), 2);
@@ -858,20 +817,14 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) {
   const GURL url2("test1:foo2");
   const GURL url3("test1:foo3");
 
-  contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
-
-  contents->controller()->LoadURL(url2, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+  contents->CompleteNavigationAsRenderer(1, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->GoBack();
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->controller()->GoForward();
   EXPECT_EQ(0, notifications.size());
@@ -885,12 +838,9 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) {
   EXPECT_TRUE(contents->controller()->CanGoBack());
   EXPECT_FALSE(contents->controller()->CanGoForward());
 
-  contents->pending_entry()->set_url(url3);
-  contents->pending_entry()->set_transition_type(PageTransition::LINK);
-  contents->CompleteNavigation(2);
-  EXPECT_TRUE(notifications.Check3AndReset(NOTIFY_NAV_LIST_PRUNED,
-                                           NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(2, url3);
+  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_LIST_PRUNED,
+                                           NOTIFY_NAV_ENTRY_COMMITTED));
 
   EXPECT_EQ(contents->controller()->GetEntryCount(), 2);
   EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 1);
@@ -901,6 +851,130 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) {
   EXPECT_FALSE(contents->controller()->CanGoForward());
 }
 
+// Tests navigation via link click within a subframe. A new navigation entry
+// should be created.
+TEST_F(NavigationControllerTest, NewSubframe) {
+  TestNotificationTracker notifications;
+  RegisterForAllNavNotifications(&notifications, contents->controller());
+
+  const GURL url1("test1:foo1");
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+
+  const GURL url2("test1:foo2");
+  ViewHostMsg_FrameNavigate_Params params;
+  params.page_id = 1;
+  params.url = url2;
+  params.transition = PageTransition::MANUAL_SUBFRAME;
+  params.should_update_history = false;
+  params.gesture = NavigationGestureUser;
+  params.is_post = false;
+
+  NavigationController::LoadCommittedDetails details;
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false,
+                                                          &details));
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+  EXPECT_EQ(url1, details.previous_url);
+  EXPECT_FALSE(details.is_auto);
+  EXPECT_FALSE(details.is_in_page);
+  EXPECT_FALSE(details.is_main_frame);
+
+  // The new entry should be appended.
+  EXPECT_EQ(2, contents->controller()->GetEntryCount());
+
+  // New entry should refer to the new page, but the old URL (entries only
+  // reflect the toplevel URL).
+  EXPECT_EQ(url1, details.entry->url());
+  EXPECT_EQ(params.page_id, details.entry->page_id());
+}
+
+// Auto subframes are ones the page loads automatically like ads. They should
+// not create new navigation entries.
+TEST_F(NavigationControllerTest, AutoSubframe) {
+  TestNotificationTracker notifications;
+  RegisterForAllNavNotifications(&notifications, contents->controller());
+
+  const GURL url1("test1:foo1");
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+
+  const GURL url2("test1:foo2");
+  ViewHostMsg_FrameNavigate_Params params;
+  params.page_id = 0;
+  params.url = url2;
+  params.transition = PageTransition::AUTO_SUBFRAME;
+  params.should_update_history = false;
+  params.gesture = NavigationGestureUser;
+  params.is_post = false;
+
+  // Navigating should do nothing.
+  NavigationController::LoadCommittedDetails details;
+  EXPECT_FALSE(contents->controller()->RendererDidNavigate(params, false,
+                                                           &details));
+  EXPECT_EQ(0, notifications.size());
+
+  // There should still be only one entry.
+  EXPECT_EQ(1, contents->controller()->GetEntryCount());
+}
+
+// Tests navigation and then going back to a subframe navigation.
+TEST_F(NavigationControllerTest, BackSubframe) {
+  TestNotificationTracker notifications;
+  RegisterForAllNavNotifications(&notifications, contents->controller());
+
+  // Main page.
+  const GURL url1("test1:foo1");
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+
+  // First manual subframe navigation.
+  const GURL url2("test1:foo2");
+  ViewHostMsg_FrameNavigate_Params params;
+  params.page_id = 1;
+  params.url = url2;
+  params.transition = PageTransition::MANUAL_SUBFRAME;
+  params.should_update_history = false;
+  params.gesture = NavigationGestureUser;
+  params.is_post = false;
+
+  // This should generate a new entry.
+  NavigationController::LoadCommittedDetails details;
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false,
+                                                          &details));
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+  EXPECT_EQ(2, contents->controller()->GetEntryCount());
+
+  // Second manual subframe navigation should also make a new entry.
+  const GURL url3("test1:foo3");
+  params.page_id = 2;
+  params.url = url3;
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false,
+                                                          &details));
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+  EXPECT_EQ(3, contents->controller()->GetEntryCount());
+  EXPECT_EQ(2, contents->controller()->GetCurrentEntryIndex());
+
+  // Go back one.
+  contents->controller()->GoBack();
+  params.url = url2;
+  params.page_id = 1;
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false,
+                                                          &details));
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+  EXPECT_EQ(3, contents->controller()->GetEntryCount());
+  EXPECT_EQ(1, contents->controller()->GetCurrentEntryIndex());
+
+  // Go back one more.
+  contents->controller()->GoBack();
+  params.url = url1;
+  params.page_id = 0;
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false,
+                                                          &details));
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+  EXPECT_EQ(3, contents->controller()->GetEntryCount());
+  EXPECT_EQ(0, contents->controller()->GetCurrentEntryIndex());
+}
+
 TEST_F(NavigationControllerTest, LinkClick) {
   TestNotificationTracker notifications;
   RegisterForAllNavNotifications(&notifications, contents->controller());
@@ -908,18 +982,15 @@ TEST_F(NavigationControllerTest, LinkClick) {
   const GURL url1("test1:foo1");
   const GURL url2("test1:foo2");
 
-  contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   contents->set_pending_entry(new NavigationEntry(kTestContentsType1, NULL, 0,
                                                   url2,
                                                   std::wstring(),
                                                   PageTransition::LINK));
-  contents->CompleteNavigation(1);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(1, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // Should not have produced a new session history entry.
   EXPECT_EQ(contents->controller()->GetEntryCount(), 2);
@@ -931,6 +1002,72 @@ TEST_F(NavigationControllerTest, LinkClick) {
   EXPECT_FALSE(contents->controller()->CanGoForward());
 }
 
+TEST_F(NavigationControllerTest, InPage) {
+  TestNotificationTracker notifications;
+  RegisterForAllNavNotifications(&notifications, contents->controller());
+
+  // Main page. Note that we need "://" so this URL is treated as "standard"
+  // which are the only ones that can have a ref.
+  const GURL url1("test1://foo");
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+
+  // First navigation.
+  const GURL url2("test1://foo#a");
+  ViewHostMsg_FrameNavigate_Params params;
+  params.page_id = 1;
+  params.url = url2;
+  params.transition = PageTransition::LINK;
+  params.should_update_history = false;
+  params.gesture = NavigationGestureUser;
+  params.is_post = false;
+
+  // This should generate a new entry.
+  NavigationController::LoadCommittedDetails details;
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false,
+                                                          &details));
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+  EXPECT_EQ(2, contents->controller()->GetEntryCount());
+
+  // Go back one.
+  ViewHostMsg_FrameNavigate_Params back_params(params);
+  contents->controller()->GoBack();
+  back_params.url = url1;
+  back_params.page_id = 0;
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(back_params, false,
+                                                          &details));
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+  EXPECT_EQ(2, contents->controller()->GetEntryCount());
+  EXPECT_EQ(0, contents->controller()->GetCurrentEntryIndex());
+  EXPECT_EQ(back_params.url, contents->controller()->GetActiveEntry()->url());
+
+  // Go forward
+  ViewHostMsg_FrameNavigate_Params forward_params(params);
+  contents->controller()->GoForward();
+  forward_params.url = url2;
+  forward_params.page_id = 1;
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(forward_params, false,
+                                                          &details));
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
+  EXPECT_EQ(2, contents->controller()->GetEntryCount());
+  EXPECT_EQ(1, contents->controller()->GetCurrentEntryIndex());
+  EXPECT_EQ(forward_params.url,
+            contents->controller()->GetActiveEntry()->url());
+
+  // Now go back and forward again. This is to work around a bug where we would
+  // compare the incoming URL with the last committed entry rather than the
+  // one identified by an existing page ID. This would result in the second URL
+  // losing the reference fragment when you navigate away from it and then back.
+  contents->controller()->GoBack();
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(back_params, false,
+                                                          &details));
+  contents->controller()->GoForward();
+  EXPECT_TRUE(contents->controller()->RendererDidNavigate(forward_params, false,
+                                                          &details));
+  EXPECT_EQ(forward_params.url,
+            contents->controller()->GetActiveEntry()->url());
+}
+
 TEST_F(NavigationControllerTest, SwitchTypes) {
   TestNotificationTracker notifications;
   RegisterForAllNavNotifications(&notifications, contents->controller());
@@ -938,21 +1075,17 @@ TEST_F(NavigationControllerTest, SwitchTypes) {
   const GURL url1("test1:foo");
   const GURL url2("test2:foo");
 
-  contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   TestContents* initial_contents = contents;
-
   contents->controller()->LoadURL(url2, PageTransition::TYPED);
 
   // The tab contents should have been replaced
   ASSERT_TRUE(initial_contents != contents);
 
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(1, url2);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   // A second navigation entry should have been committed even though the
   // PageIDs are the same.  PageIDs are scoped to the tab contents type.
@@ -967,9 +1100,8 @@ TEST_F(NavigationControllerTest, SwitchTypes) {
   // Navigate back...
   contents->controller()->GoBack();
   ASSERT_TRUE(initial_contents == contents);  // switched again!
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   EXPECT_EQ(contents->controller()->GetEntryCount(), 2);
   EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 0);
@@ -992,10 +1124,8 @@ TEST_F(NavigationControllerTest, SwitchTypes_Discard) {
   const GURL url1("test1:foo");
   const GURL url2("test2:foo");
 
-  contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-  EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED,
-                                           NOTIFY_NAV_ENTRY_CHANGED));
+  contents->CompleteNavigationAsRenderer(0, url1);
+  EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED));
 
   TestContents* initial_contents = contents;
 
@@ -1030,22 +1160,23 @@ TEST_F(NavigationControllerTest, SwitchTypesCleanup) {
   const GURL url2("test2:foo");
   const GURL url3("test2:bar");
 
+  // Note that we need the LoadURL calls so that pending entries and the
+  // different tab contents types are created. "Renderer" navigations won't
+  // actually cross tab contents boundaries without these.
   contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-
+  contents->CompleteNavigationAsRenderer(0, url1);
   contents->controller()->LoadURL(url2, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-
+  contents->CompleteNavigationAsRenderer(1, url2);
   contents->controller()->LoadURL(url3, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
+  contents->CompleteNavigationAsRenderer(2, url3);
 
-  // Navigate back to the start
+  // Navigate back to the start.
   contents->controller()->GoToIndex(0);
-  contents->CompleteNavigation(0);
+  contents->CompleteNavigationAsRenderer(0, url1);
 
-  // Now jump to the end
+  // Now jump to the end.
   contents->controller()->GoToIndex(2);
-  contents->CompleteNavigation(1);
+  contents->CompleteNavigationAsRenderer(2, url3);
 
   // There may be TabContentsCollector tasks pending, so flush them from queue.
   MessageLoop::current()->RunAllPending();
@@ -1068,16 +1199,18 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) {
   // Load up to the max count, all entries should be there.
   for (url_index = 0; url_index < kMaxEntryCount; url_index++) {
     SNPrintF(buffer, 128, "test1://www.a.com/%d", url_index);
-    contents->controller()->LoadURL(GURL(buffer), PageTransition::TYPED);
-    contents->CompleteNavigation(url_index);
+    GURL url(buffer);
+    contents->controller()->LoadURL(url, PageTransition::TYPED);
+    contents->CompleteNavigationAsRenderer(url_index, url);
   }
 
   EXPECT_EQ(contents->controller()->GetEntryCount(), kMaxEntryCount);
 
   // Navigate some more.
   SNPrintF(buffer, 128, "test1://www.a.com/%d", url_index);
-  contents->controller()->LoadURL(GURL(buffer), PageTransition::TYPED);
-  contents->CompleteNavigation(url_index);
+  GURL url(buffer);
+  contents->controller()->LoadURL(url, PageTransition::TYPED);
+  contents->CompleteNavigationAsRenderer(url_index, url);
   url_index++;
 
   // We expect http://www.a.com/0 to be gone.
@@ -1088,8 +1221,9 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) {
   // More navigations.
   for (int i = 0; i < 3; i++) {
     SNPrintF(buffer, 128, "test1://www.a.com/%d", url_index);
-    contents->controller()->LoadURL(GURL(buffer), PageTransition::TYPED);
-    contents->CompleteNavigation(url_index);
+    url = GURL(buffer);
+    contents->controller()->LoadURL(url, PageTransition::TYPED);
+      contents->CompleteNavigationAsRenderer(url_index, url);
     url_index++;
   }
   EXPECT_EQ(contents->controller()->GetEntryCount(), kMaxEntryCount);
@@ -1097,35 +1231,120 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) {
             GURL("test1://www.a.com/4"));
 }
 
+// Tests that we can do a restore and navigate to the restored entries and
+// everything is updated properly. This can be tricky since there is no
+// SiteInstance for the entries created initially.
+TEST_F(NavigationControllerTest, RestoreNavigate) {
+  site_instance = SiteInstance::CreateSiteInstance(profile);
+
+  // Create a NavigationController with a restored set of tabs.
+  GURL url("test1:foo");
+  std::vector<TabNavigation> navigations;
+  navigations.push_back(TabNavigation(0, url, L"Title", "state",
+                                      PageTransition::LINK));
+  NavigationController* controller =
+      new NavigationController(profile, navigations, 0, NULL);
+  controller->GoToIndex(0);
+
+  // We should now have one entry, and it should be "pending".
+  EXPECT_EQ(1, controller->GetEntryCount());
+  EXPECT_EQ(controller->GetEntryAtIndex(0), controller->GetPendingEntry());
+  EXPECT_EQ(0, controller->GetEntryAtIndex(0)->page_id());
+
+  // Say we navigated to that entry.
+  ViewHostMsg_FrameNavigate_Params params;
+  params.page_id = 0;
+  params.url = url;
+  params.transition = PageTransition::LINK;
+  params.should_update_history = false;
+  params.gesture = NavigationGestureUser;
+  params.is_post = false;
+  NavigationController::LoadCommittedDetails details;
+  controller->RendererDidNavigate(params, false, &details);
+
+  // There should be no longer any pending entry and one committed one. This
+  // means that we were able to locate the entry, assign its site instance, and
+  // commit it properly.
+  EXPECT_EQ(1, controller->GetEntryCount());
+  EXPECT_EQ(0, controller->GetLastCommittedEntryIndex());
+  EXPECT_FALSE(controller->GetPendingEntry());
+  EXPECT_EQ(site_instance,
+            controller->GetLastCommittedEntry()->site_instance());
+}
+
+// Make sure that the page type and stuff is correct after an interstitial.
+TEST_F(NavigationControllerTest, Interstitial) {
+  // First navigate somewhere normal.
+  const GURL url1("test1:foo");
+  contents->controller()->LoadURL(url1, PageTransition::TYPED);
+  contents->CompleteNavigationAsRenderer(0, url1);
+
+  // Now navigate somewhere with an interstitial.
+  const GURL url2("test1:bar");
+  contents->controller()->LoadURL(url1, PageTransition::TYPED);
+  contents->controller()->GetPendingEntry()->set_page_type(
+      NavigationEntry::INTERSTITIAL_PAGE);
+
+  // At this point the interstitial will be displayed and the load will still
+  // be pending. If the user continues, the load will commit.
+  contents->CompleteNavigationAsRenderer(1, url2);
+
+  // The page should be a normal page again.
+  EXPECT_EQ(url2, contents->controller()->GetLastCommittedEntry()->url());
+  EXPECT_EQ(NavigationEntry::NORMAL_PAGE,
+            contents->controller()->GetLastCommittedEntry()->page_type());
+}
+
+// Tests that IsInPageNavigation returns appropriate results.  Prevents
+// regression for bug 1126349.
+TEST_F(NavigationControllerTest, IsInPageNavigation) {
+  // Navigate to URL with no refs.
+  const GURL url("http://www.google.com/home.html");
+  contents->CompleteNavigationAsRenderer(0, url);
+
+  // Reloading the page is not an in-page navigation.
+  EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(url));
+  const GURL other_url("http://www.google.com/add.html");
+  EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(other_url));
+  const GURL url_with_ref("http://www.google.com/home.html#my_ref");
+  EXPECT_TRUE(contents->controller()->IsURLInPageNavigation(url_with_ref));
+
+  // Navigate to URL with refs.
+  contents->CompleteNavigationAsRenderer(1, url_with_ref);
+
+  // Reloading the page is not an in-page navigation.
+  EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(url_with_ref));
+  EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(url));
+  EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(other_url));
+  const GURL other_url_with_ref("http://www.google.com/home.html#my_other_ref");
+  EXPECT_TRUE(contents->controller()->IsURLInPageNavigation(
+      other_url_with_ref));
+}
+
 // A basic test case. Navigates to a single url, and make sure the history
 // db matches.
 TEST_F(NavigationControllerHistoryTest, Basic) {
-  contents->controller()->LoadURL(url0, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
+  contents->controller()->LoadURL(url0, PageTransition::LINK);
+  contents->CompleteNavigationAsRenderer(0, url0);
 
   GetLastSession();
 
   helper_.AssertSingleWindowWithSingleTab(windows_, 1);
   helper_.AssertTabEquals(0, 0, 1, *(windows_[0]->tabs[0]));
   TabNavigation nav1(0, url0, std::wstring(), std::string(),
-                     PageTransition::TYPED);
+                     PageTransition::LINK);
   helper_.AssertNavigationEquals(nav1, windows_[0]->tabs[0]->navigations[0]);
 }
 
 // Navigates to three urls, then goes back and make sure the history database
 // is in sync.
 TEST_F(NavigationControllerHistoryTest, NavigationThenBack) {
-  contents->controller()->LoadURL(url0, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-
-  contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
-
-  contents->controller()->LoadURL(url2, PageTransition::TYPED);
-  contents->CompleteNavigation(2);
+  contents->CompleteNavigationAsRenderer(0, url0);
+  contents->CompleteNavigationAsRenderer(1, url1);
+  contents->CompleteNavigationAsRenderer(2, url2);
 
   contents->controller()->GoBack();
-  contents->CompleteNavigation(1);
+  contents->CompleteNavigationAsRenderer(1, url1);
 
   GetLastSession();
 
@@ -1133,7 +1352,7 @@ TEST_F(NavigationControllerHistoryTest, NavigationThenBack) {
   helper_.AssertTabEquals(0, 1, 3, *(windows_[0]->tabs[0]));
 
   TabNavigation nav(0, url0, std::wstring(), std::string(),
-                    PageTransition::TYPED);
+                    PageTransition::LINK);
   helper_.AssertNavigationEquals(nav, windows_[0]->tabs[0]->navigations[0]);
   nav.url = url1;
   helper_.AssertNavigationEquals(nav, windows_[0]->tabs[0]->navigations[1]);
@@ -1143,23 +1362,17 @@ TEST_F(NavigationControllerHistoryTest, NavigationThenBack) {
 
 // Navigates to three urls, then goes back twice, then loads a new url.
 TEST_F(NavigationControllerHistoryTest, NavigationPruning) {
-  contents->controller()->LoadURL(url0, PageTransition::TYPED);
-  contents->CompleteNavigation(0);
-
-  contents->controller()->LoadURL(url1, PageTransition::TYPED);
-  contents->CompleteNavigation(1);
-
-  contents->controller()->LoadURL(url2, PageTransition::TYPED);
-  contents->CompleteNavigation(2);
+  contents->CompleteNavigationAsRenderer(0, url0);
+  contents->CompleteNavigationAsRenderer(1, url1);
+  contents->CompleteNavigationAsRenderer(2, url2);
 
   contents->controller()->GoBack();
-  contents->CompleteNavigation(1);
+  contents->CompleteNavigationAsRenderer(1, url1);
 
   contents->controller()->GoBack();
-  contents->CompleteNavigation(0);
+  contents->CompleteNavigationAsRenderer(0, url0);
 
-  contents->controller()->LoadURL(url2, PageTransition::TYPED);
-  contents->CompleteNavigation(3);
+  contents->CompleteNavigationAsRenderer(3, url2);
 
   // Now have url0, and url2.
 
@@ -1169,9 +1382,8 @@ TEST_F(NavigationControllerHistoryTest, NavigationPruning) {
   helper_.AssertTabEquals(0, 1, 2, *(windows_[0]->tabs[0]));
 
   TabNavigation nav(0, url0, std::wstring(), std::string(),
-                    PageTransition::TYPED);
+                    PageTransition::LINK);
   helper_.AssertNavigationEquals(nav, windows_[0]->tabs[0]->navigations[0]);
   nav.url = url2;
   helper_.AssertNavigationEquals(nav, windows_[0]->tabs[0]->navigations[1]);
 }
-