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

24 files changed, 1304 insertions, 990 deletions

diff --git a/chrome/browser/back_forward_menu_model_unittest.cc b/chrome/browser/back_forward_menu_model_unittest.cc
index e2e04835a..f91081d 100644
--- a/chrome/browser/back_forward_menu_model_unittest.cc
+++ b/chrome/browser/back_forward_menu_model_unittest.cc
@@ -29,30 +29,20 @@ class BackFwdMenuModelTestTabContents : public TabContents {
   BackFwdMenuModelTestTabContents() : TabContents(kHTTPTabContentsType) {
   }
 
-  bool Navigate(const NavigationEntry& entry, bool reload) {
-    NavigationEntry* pending_entry = new NavigationEntry(entry);
-    if (pending_entry->page_id() == -1) {
-      pending_entry->set_page_id(g_page_id_++);
-    }
-    NavigationController::LoadCommittedDetails details;
-    DidNavigateToEntry(pending_entry, &details);
+  // We do the same thing as the TabContents one (just commit the navigation)
+  // but we *don't* want to reset the title since the test looks for this.
+  virtual bool NavigateToPendingEntry(bool reload) {
+    controller()->CommitPendingEntry();
     return true;
   }
 
   void UpdateState(const std::wstring& title) {
     NavigationEntry* entry =
-      controller()->GetEntryWithPageID(type(), NULL, g_page_id_ - 1);
+      controller()->GetEntryWithPageID(type(), NULL, GetMaxPageID());
     entry->set_title(title);
   }
-
- private:
-  // We need to use valid, incrementing page ids otherwise the TabContents
-  // and NavController will not play nice when we try to go back and forward.
-  static int g_page_id_;
 };
 
-int BackFwdMenuModelTestTabContents::g_page_id_ = 0;
-
 // This constructs our fake TabContents.
 class BackFwdMenuModelTestTabContentsFactory : public TabContentsFactory {
  public:
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index b01c2ce..235edbf 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -540,7 +540,7 @@ void Browser::OpenURLFromTab(TabContents* source,
       if (web_contents) {
         const GURL& current_url = web_contents->GetURL();
         if (SiteInstance::IsSameWebSite(current_url, url))
-          instance = web_contents->site_instance();
+          instance = web_contents->GetSiteInstance();
       }
     }
   }
diff --git a/chrome/browser/dom_ui/new_tab_ui.cc b/chrome/browser/dom_ui/new_tab_ui.cc
index b5cdf52..3d0cdf1 100644
--- a/chrome/browser/dom_ui/new_tab_ui.cc
+++ b/chrome/browser/dom_ui/new_tab_ui.cc
@@ -797,15 +797,9 @@ void NewTabUIContents::AttachMessageHandlers() {
   }
 }
 
-bool NewTabUIContents::Navigate(const NavigationEntry& entry, bool reload) {
-  const bool result = WebContents::Navigate(entry, reload);
-
-  // Force the title to say 'New tab', even when loading. The supplied entry is
-  // also the pending entry.
-  NavigationEntry* pending_entry = controller()->GetPendingEntry();
-  DCHECK(pending_entry && pending_entry == &entry);
-  pending_entry->set_title(forced_title_);
-
+bool NewTabUIContents::NavigateToPendingEntry(bool reload) {
+  const bool result = WebContents::NavigateToPendingEntry(reload);
+  controller()->GetPendingEntry()->set_title(forced_title_);
   return result;
 }
 
diff --git a/chrome/browser/dom_ui/new_tab_ui.h b/chrome/browser/dom_ui/new_tab_ui.h
index 3584954..092cdb4 100644
--- a/chrome/browser/dom_ui/new_tab_ui.h
+++ b/chrome/browser/dom_ui/new_tab_ui.h
@@ -290,7 +290,7 @@ class NewTabUIContents : public DOMUIHost {
 
   // WebContents overrides.
   // Overriden to force the title of the page to forced_title_.
-  virtual bool Navigate(const NavigationEntry& entry, bool reload);
+  virtual bool NavigateToPendingEntry(bool reload);
   // We don't want a favicon on the new tab page.
   virtual bool ShouldDisplayFavIcon() { return false; }
   // The bookmark bar is always visible on the new tab.
diff --git a/chrome/browser/native_ui_contents.cc b/chrome/browser/native_ui_contents.cc
index 4c967a2..e4dce38 100644
--- a/chrome/browser/native_ui_contents.cc
+++ b/chrome/browser/native_ui_contents.cc
@@ -210,20 +210,20 @@ void NativeUIContents::SetPageState(PageState* page_state) {
   state_.reset(page_state);
   NavigationController* ctrl = controller();
   if (ctrl) {
-    NavigationEntry* ne = ctrl->GetLastCommittedEntry();
+    int ne_index = ctrl->GetLastCommittedEntryIndex();
+    NavigationEntry* ne = ctrl->GetEntryAtIndex(ne_index);
     if (ne) {
       // NavigationEntry is null if we're being restored.
       DCHECK(ne);
       std::string rep;
       state_->GetByteRepresentation(&rep);
       ne->set_content_state(rep);
-      // This is not a WebContents, so we use a NULL SiteInstance.
-      ctrl->NotifyEntryChangedByPageID(type(), NULL, ne->page_id());
+      ctrl->NotifyEntryChanged(ne, ne_index);
     }
   }
 }
 
-bool NativeUIContents::Navigate(const NavigationEntry& entry, bool reload) {
+bool NativeUIContents::NavigateToPendingEntry(bool reload) {
   ChromeViews::RootView* root_view = GetRootView();
   DCHECK(root_view);
 
@@ -234,7 +234,8 @@ bool NativeUIContents::Navigate(const NavigationEntry& entry, bool reload) {
     current_view_ = NULL;
   }
 
-  NativeUI* new_ui = GetNativeUIForURL(entry.url());
+  NavigationEntry* pending_entry = controller()->GetPendingEntry();
+  NativeUI* new_ui = GetNativeUIForURL(pending_entry->url());
   if (new_ui) {
     current_ui_ = new_ui;
     is_visible_ = true;
@@ -242,9 +243,9 @@ bool NativeUIContents::Navigate(const NavigationEntry& entry, bool reload) {
     current_view_ = new_ui->GetView();
     root_view->AddChildView(current_view_);
 
-    std::string s = entry.content_state();
+    std::string s = pending_entry->content_state();
     if (s.empty())
-      state_->InitWithURL(entry.url());
+      state_->InitWithURL(pending_entry->url());
     else
       state_->InitWithBytes(s);
 
@@ -252,29 +253,32 @@ bool NativeUIContents::Navigate(const NavigationEntry& entry, bool reload) {
     Layout();
   }
 
-  NavigationEntry* new_entry = new NavigationEntry(entry);
-  if (new_entry->page_id() == -1)
-    new_entry->set_page_id(++g_next_page_id);
-  new_entry->set_title(GetDefaultTitle());
-  new_entry->favicon().set_bitmap(GetFavIcon());
-  new_entry->favicon().set_is_valid(true);
+  // Commit the new load in the navigation controller. If the ID of the
+  // NavigationEntry we were given was -1, that means this is a new load, so
+  // we have to generate a new ID.
+  controller()->CommitPendingEntry();
+
+  // Populate the committed entry.
+  NavigationEntry* committed_entry = controller()->GetLastCommittedEntry();
+  committed_entry->set_title(GetDefaultTitle());
+  committed_entry->favicon().set_bitmap(GetFavIcon());
+  committed_entry->favicon().set_is_valid(true);
   if (new_ui) {
     // Strip out the query params, they should have moved to state.
     // TODO(sky): use GURL methods for replacements once bug is fixed.
     size_t scheme_end, host_end;
-    GetSchemeAndHostEnd(entry.url(), &scheme_end, &host_end);
-    new_entry->set_url(GURL(entry.url().spec().substr(0, host_end)));
+    GetSchemeAndHostEnd(committed_entry->url(), &scheme_end, &host_end);
+    committed_entry->set_url(
+        GURL(committed_entry->url().spec().substr(0, host_end)));
   }
   std::string content_state;
   state_->GetByteRepresentation(&content_state);
-  new_entry->set_content_state(content_state);
-  const int32 page_id = new_entry->page_id();
-
-  // The default details is "new navigation", and that's OK with us.
-  NavigationController::LoadCommittedDetails details;
-  DidNavigateToEntry(new_entry, &details);
-  // This is not a WebContents, so we use a NULL SiteInstance.
-  controller()->NotifyEntryChangedByPageID(type(), NULL, page_id);
+  committed_entry->set_content_state(content_state);
+
+  // Broadcast the fact that we just updated all that crap.
+  controller()->NotifyEntryChanged(
+      committed_entry,
+      controller()->GetIndexOfEntry(committed_entry));
   return true;
 }
 
diff --git a/chrome/browser/native_ui_contents.h b/chrome/browser/native_ui_contents.h
index 50407b2..443e3be 100644
--- a/chrome/browser/native_ui_contents.h
+++ b/chrome/browser/native_ui_contents.h
@@ -52,7 +52,7 @@ class NativeUIContents : public TabContents,
   //
   // TabContents implementation
   //
-  virtual bool Navigate(const NavigationEntry& entry, bool reload);
+  virtual bool NavigateToPendingEntry(bool reload);
   virtual const std::wstring GetDefaultTitle() const;
   virtual SkBitmap GetFavIcon() const;
   virtual bool ShouldDisplayURL() { return false; }
diff --git a/chrome/browser/navigation_controller.cc b/chrome/browser/navigation_controller.cc
index df55f9b..d294ace 100644
--- a/chrome/browser/navigation_controller.cc
+++ b/chrome/browser/navigation_controller.cc
@@ -23,6 +23,63 @@
 #include "net/base/net_util.h"
 #include "webkit/glue/webkit_glue.h"
 
+namespace {
+
+// Invoked when entries have been pruned, or removed. For example, if the
+// current entries are [google, digg, yahoo], with the current entry google,
+// and the user types in cnet, then digg and yahoo are pruned.
+void NotifyPrunedEntries(NavigationController* nav_controller) {
+  NotificationService::current()->Notify(
+      NOTIFY_NAV_LIST_PRUNED,
+      Source<NavigationController>(nav_controller),
+      NotificationService::NoDetails());
+}
+
+// Ensure the given NavigationEntry has a valid state, so that WebKit does not
+// get confused if we navigate back to it.
+// 
+// An empty state is treated as a new navigation by WebKit, which would mean
+// losing the navigation entries and generating a new navigation entry after
+// this one. We don't want that. To avoid this we create a valid state which
+// WebKit will not treat as a new navigation.
+void SetContentStateIfEmpty(NavigationEntry* entry) {
+  if (entry->content_state().empty() &&
+      (entry->tab_type() == TAB_CONTENTS_WEB ||
+       entry->tab_type() == TAB_CONTENTS_NEW_TAB_UI ||
+       entry->tab_type() == TAB_CONTENTS_ABOUT_UI ||
+       entry->tab_type() == TAB_CONTENTS_HTML_DIALOG)) {
+    entry->set_content_state(
+        webkit_glue::CreateHistoryStateForURL(entry->url()));
+  }
+}
+
+// Configure all the NavigationEntries in entries for restore. This resets
+// the transition type to reload and makes sure the content state isn't empty.
+void ConfigureEntriesForRestore(
+    std::vector<linked_ptr<NavigationEntry> >* entries) {
+  for (size_t i = 0; i < entries->size(); ++i) {
+    // Use a transition type of reload so that we don't incorrectly increase
+    // the typed count.
+    (*entries)[i]->set_transition_type(PageTransition::RELOAD);
+    (*entries)[i]->set_restored(true);
+    // NOTE(darin): This code is only needed for backwards compat.
+    SetContentStateIfEmpty((*entries)[i].get());
+  }
+}
+
+// See NavigationController::IsURLInPageNavigation for how this works and why.
+bool AreURLsInPageNavigation(const GURL& existing_url, const GURL& new_url) {
+  if (existing_url == new_url || !new_url.has_ref())
+    return false;
+
+  url_canon::Replacements<char> replacements;
+  replacements.ClearRef();
+  return existing_url.ReplaceComponents(replacements) ==
+      new_url.ReplaceComponents(replacements);
+}
+
+}  // namespace
+
 // TabContentsCollector ---------------------------------------------------
 
 // We never destroy a TabContents synchronously because there are some
@@ -101,20 +158,6 @@ static void CreateNavigationEntriesFromTabNavigations(
   }
 }
 
-// Configure all the NavigationEntries in entries for restore. This resets
-// the transition type to reload and makes sure the content state isn't empty.
-static void ConfigureEntriesForRestore(
-    std::vector<linked_ptr<NavigationEntry> >* entries) {
-  for (size_t i = 0, count = entries->size(); i < count; ++i) {
-    // Use a transition type of reload so that we don't incorrectly increase
-    // the typed count.
-    (*entries)[i]->set_transition_type(PageTransition::RELOAD);
-    (*entries)[i]->set_restored(true);
-    // NOTE(darin): This code is only needed for backwards compat.
-    NavigationController::SetContentStateIfEmpty((*entries)[i].get());
-  }
-}
-
 NavigationController::NavigationController(TabContents* contents,
                                            Profile* profile)
     : profile_(profile),
@@ -182,7 +225,6 @@ TabContents* NavigationController::GetTabContents(TabContentsType t) {
 }
 
 void NavigationController::Reload() {
-  // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()?
   DiscardPendingEntryInternal();
   int current_index = GetCurrentEntryIndex();
   if (check_for_repost_ && current_index != -1 &&
@@ -204,7 +246,6 @@ void NavigationController::Reload() {
     if (current_index == -1)
       return;
 
-    // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()?
     DiscardPendingEntryInternal();
 
     pending_entry_index_ = current_index;
@@ -223,8 +264,6 @@ void NavigationController::LoadEntry(NavigationEntry* entry) {
   // When navigating to a new page, we don't know for sure if we will actually
   // end up leaving the current page.  The new page load could for example
   // result in a download or a 'no content' response (e.g., a mailto: URL).
-
-  // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()?
   DiscardPendingEntryInternal();
   pending_entry_ = entry;
   NotificationService::current()->Notify(
@@ -234,24 +273,6 @@ void NavigationController::LoadEntry(NavigationEntry* entry) {
   NavigateToPendingEntry(false);
 }
 
-/* static */
-void NavigationController::SetContentStateIfEmpty(
-    NavigationEntry* entry) {
-  if (entry->content_state().empty() &&
-      (entry->tab_type() == TAB_CONTENTS_WEB ||
-       entry->tab_type() == TAB_CONTENTS_NEW_TAB_UI ||
-       entry->tab_type() == TAB_CONTENTS_ABOUT_UI ||
-       entry->tab_type() == TAB_CONTENTS_HTML_DIALOG)) {
-    // The state is empty and the url will be rendered by WebKit. An empty
-    // state is treated as a new navigation by WebKit, which would mean
-    // losing the navigation entries and generating a new navigation
-    // entry after this one. We don't want that. To avoid this we create
-    // a valid state which WebKit will not treat as a new navigation.
-    entry->set_content_state(
-        webkit_glue::CreateHistoryStateForURL(entry->url()));
-  }
-}
-
 NavigationEntry* NavigationController::GetActiveEntry() const {
   NavigationEntry* entry = pending_entry_;
   if (!entry)
@@ -279,11 +300,6 @@ NavigationEntry* NavigationController::GetEntryAtOffset(int offset) const {
   return entries_[index].get();
 }
 
-bool NavigationController::CanStop() const {
-  // TODO(darin): do we have something pending that we can stop?
-  return false;
-}
-
 bool NavigationController::CanGoBack() const {
   return entries_.size() > 1 && GetCurrentEntryIndex() > 0;
 }
@@ -343,14 +359,6 @@ void NavigationController::GoToOffset(int offset) {
   GoToIndex(index);
 }
 
-void NavigationController::Stop() {
-  DCHECK(CanStop());
-
-  // TODO(darin): we probably want to just call Stop on the active tab
-  // contents, but should we also call DiscardPendingEntry?
-  NOTREACHED() << "implement me";
-}
-
 void NavigationController::ReloadDontCheckForRepost() {
   Reload();
 }
@@ -459,7 +467,6 @@ void NavigationController::LoadURLLazily(const GURL& url,
   if (icon)
     entry->favicon().set_bitmap(*icon);
 
-  // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()?
   DiscardPendingEntryInternal();
   pending_entry_ = entry;
   load_pending_entry_when_active_ = true;
@@ -493,105 +500,351 @@ void NavigationController::SetAlternateNavURLFetcher(
   alternate_nav_url_fetcher_entry_unique_id_ = pending_entry_->unique_id();
 }
 
-void NavigationController::DidNavigateToEntry(NavigationEntry* entry,
-                                              LoadCommittedDetails* details) {
-  DCHECK(active_contents_);
-  DCHECK(entry->tab_type() == active_contents_->type());
+bool NavigationController::RendererDidNavigate(
+    const ViewHostMsg_FrameNavigate_Params& params,
+    bool is_interstitial,
+    LoadCommittedDetails* details) {
+  // Save the previous URL before we clobber it.
+  if (GetLastCommittedEntry())
+    details->previous_url = GetLastCommittedEntry()->url();
 
-  SetContentStateIfEmpty(entry);
+  // Assign the current site instance to any pending entry, so we can find it
+  // later by calling GetEntryIndexWithPageID. We only care about this if the
+  // pending entry is an existing navigation and not a new one (or else we
+  // wouldn't care about finding it with GetEntryIndexWithPageID).
+  //
+  // TODO(brettw) this seems slightly bogus as we don't really know if the
+  // pending entry is what this navigation is for. There is a similar TODO
+  // w.r.t. the pending entry in RendererDidNavigateToNewPage.
+  if (pending_entry_index_ >= 0)
+    pending_entry_->set_site_instance(active_contents_->GetSiteInstance());
+
+  // Do navigation-type specific actions. These will make and commit an entry.
+  switch (ClassifyNavigation(params)) {
+    case NAV_NEW_PAGE:
+      RendererDidNavigateToNewPage(params);
+      break;
+    case NAV_EXISTING_PAGE:
+      RendererDidNavigateToExistingPage(params);
+      break;
+    case NAV_SAME_PAGE:
+      RendererDidNavigateToSamePage(params);
+      break;
+    case NAV_IN_PAGE:
+      RendererDidNavigateInPage(params);
+      break;
+    case NAV_NEW_SUBFRAME:
+      RendererDidNavigateNewSubframe(params);
+      break;
+    case NAV_AUTO_SUBFRAME:
+      if (!RendererDidNavigateAutoSubframe(params))
+        return false;
+      break;
+    case NAV_IGNORE:
+      // There is nothing we can do with this navigation, so we just return to
+      // the caller that nothing has happened.
+      return false;
+    default:
+      NOTREACHED();
+  }
 
-  entry->set_restored(false);
+  // All committed entries should have nonempty content state so WebKit doesn't
+  // get confused when we go back to them (see the function for details).
+  SetContentStateIfEmpty(GetActiveEntry());
 
-  // Update the details to list the last URL. Later, we'll update the current
-  // entry (after it's committed) and the details will be complete.
-  if (GetLastCommittedEntry())
-    details->previous_url = GetLastCommittedEntry()->url();
+  // WebKit doesn't set the "auto" transition on meta refreshes properly (bug
+  // 1051891) so we manually set it for redirects which we normally treat as
+  // "non-user-gestures" where we want to update stuff after navigations.
+  //
+  // Note that the redirect check also checks for a pending entry to
+  // differentiate real redirects from browser initiated navigations to a
+  // redirected entry. This happens when you hit back to go to a page that was
+  // the destination of a redirect, we don't want to treat it as a redirect
+  // even though that's what its transition will be. See bug 1117048.
+  //
+  // TODO(brettw) write a test for this complicated logic.
+  details->is_auto = (PageTransition::IsRedirect(params.transition) &&
+                      !GetPendingEntry()) ||
+      params.gesture == NavigationGestureAuto;
 
-  // If the entry is that of a page with PageID larger than any this Tab has
-  // seen before, then consider it a new navigation.  Note that if the entry
-  // has a SiteInstance, it should be the same as the SiteInstance of the
-  // active WebContents, because we have just navigated to it.
-  DCHECK(entry->page_id() >= 0) << "Page ID must be set before calling us.";
-  if (entry->page_id() > GetMaxPageID()) {
-    InsertEntry(entry);
-    NotifyNavigationEntryCommitted(details);
-    // It is now a safe time to schedule collection for any tab contents of a
-    // different type, because a navigation is necessary to get back to them.
-    ScheduleTabContentsCollectionForInactiveTabs();
-    return;
+  // Now prep the rest of the details for the notification and broadcast.
+  details->entry = GetActiveEntry();
+  details->is_in_page = IsURLInPageNavigation(params.url);
+  details->is_main_frame = PageTransition::IsMainFrame(params.transition);
+  NotifyNavigationEntryCommitted(details);
+
+  // Because this call may synchronously show an infobar, we do it last, to
+  // make sure all other state is stable and the infobar won't get blown away
+  // by some transition.
+  //
+  // TODO(brettw) bug 1324500: This logic should be moved out of here, it should
+  // listen for the notification instead.
+  if (alternate_nav_url_fetcher_.get())
+    alternate_nav_url_fetcher_->OnNavigatedToEntry();
+
+  // Broadcast the NOTIFY_FRAME_PROVISIONAL_LOAD_COMMITTED notification for use
+  // by the SSL manager.
+  //
+  // TODO(brettw) bug 1352803: this information should be combined with
+  // NOTIFY_NAV_ENTRY_COMMITTED so this one can be deleted.
+  ProvisionalLoadDetails provisional_details(details->is_main_frame,
+                                             is_interstitial,
+                                             details->is_in_page,
+                                             params.url,
+                                             params.security_info);
+  NotificationService::current()->
+      Notify(NOTIFY_FRAME_PROVISIONAL_LOAD_COMMITTED,
+             Source<NavigationController>(this),
+             Details<ProvisionalLoadDetails>(&provisional_details));
+
+  // It is now a safe time to schedule collection for any tab contents of a
+  // different type, because a navigation is necessary to get back to them.
+  ScheduleTabContentsCollectionForInactiveTabs();
+  return true;
+}
+
+NavigationController::NavClass NavigationController::ClassifyNavigation(
+    const ViewHostMsg_FrameNavigate_Params& params) const {
+  // If a page makes a popup navigated to about blank, and then writes stuff
+  // like a subframe navigated to a real site, we'll get a notification with an
+  // invalid page ID. There's nothing we can do with these, so just ignore them.
+  if (params.page_id == -1) {
+    DCHECK(!GetActiveEntry()) << "Got an invalid page ID but we seem to be "
+      " navigated to a valid page. This should be impossible.";
+    return NAV_IGNORE;
+  }
+
+  if (params.page_id > active_contents_->GetMaxPageID()) {
+    // Greater page IDs than we've ever seen before are new pages. We may or may
+    // not have a pending entry for the page, and this may or may not be the
+    // main frame.
+    if (PageTransition::IsMainFrame(params.transition))
+      return NAV_NEW_PAGE;
+    return NAV_NEW_SUBFRAME;
   }
 
-  // Otherwise, we just need to update an existing entry with matching PageID.
-  // If the existing entry corresponds to the entry which is pending, then we
-  // must update the current entry index accordingly.  When navigating to the
-  // same URL, a new PageID is not created.
-
-  int existing_entry_index = GetEntryIndexWithPageID(entry->tab_type(),
-                                                     entry->site_instance(),
-                                                     entry->page_id());
-  NavigationEntry* existing_entry = (existing_entry_index != -1) ?
-      entries_[existing_entry_index].get() : NULL;
-  if (!existing_entry) {
-    // No existing entry, then simply ignore this navigation!
-    DLOG(WARNING) << "ignoring navigation for page: " << entry->page_id();
-  } else if ((existing_entry != pending_entry_) && pending_entry_ &&
-             (pending_entry_->page_id() == -1) &&
-             (pending_entry_->url() == existing_entry->url())) {
+  // Now we know that the notification is for an existing page. Find that entry.
+  int existing_entry_index = GetEntryIndexWithPageID(
+      active_contents_->type(),
+      active_contents_->GetSiteInstance(),
+      params.page_id);
+  if (existing_entry_index == -1) {
+    // The page was not found. It could have been pruned because of the limit on
+    // back/forward entries (not likely since we'll usually tell it to navigate
+    // to such entries). It could also mean that the renderer is smoking crack.
+    NOTREACHED();
+    return NAV_IGNORE;
+  }
+  NavigationEntry* existing_entry = entries_[existing_entry_index].get();
+
+  if (pending_entry_ &&
+      pending_entry_->url() == params.url &&
+      existing_entry != pending_entry_ &&
+      pending_entry_->page_id() == -1 &&
+      pending_entry_->url() == existing_entry->url()) {
     // In this case, we have a pending entry for a URL but WebCore didn't do a
     // new navigation. This happens when you press enter in the URL bar to
-    // reload. We will create a pending entry, but WebCore will convert it to
+    // reload. We will create a pending entry, but WebKit will convert it to
     // a reload since it's the same page and not create a new entry for it
     // (the user doesn't want to have a new back/forward entry when they do
-    // this). In this case, we want to just ignore the pending entry and go back
-    // to where we were.
-    existing_entry->set_unique_id(pending_entry_->unique_id());
-    DiscardPendingEntry();
-  } else {
-    DCHECK(existing_entry != entry);
-    // The given entry might provide a new URL... e.g., navigating back to a
-    // page in session history could have resulted in a new client redirect.
-    // The given entry might also provide a new title (typically an empty title
-    // to overwrite the existing title).
-    existing_entry->set_url(entry->url());
-    existing_entry->set_title(entry->title());
-    existing_entry->favicon() = entry->favicon();
-    existing_entry->set_content_state(entry->content_state());
-
-    // TODO(brettw) why only copy the security style and no other SSL stuff?
-    existing_entry->ssl().set_security_style(entry->ssl().security_style());
-
-    const int prev_entry_index = last_committed_entry_index_;
-    if (existing_entry == pending_entry_) {
-      DCHECK(pending_entry_index_ != -1);
-      last_committed_entry_index_ = pending_entry_index_;
-      // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()?
-      DiscardPendingEntryInternal();
-    } else {
-      // NOTE: Do not update the unique ID here, as we don't want infobars etc.
-      // to dismiss.
+    // this). In this case, we want to just ignore the pending entry and go
+    // back to where we were (the "existing entry").
+    return NAV_SAME_PAGE;
+  }
 
-      // The navigation could have been issued by the renderer, so be sure that
-      // we update our current index.
-      last_committed_entry_index_ = existing_entry_index;
-    }
-    IndexOfActiveEntryChanged(prev_entry_index);
+  if (AreURLsInPageNavigation(existing_entry->url(), params.url))
+    return NAV_IN_PAGE;
+
+  if (!PageTransition::IsMainFrame(params.transition))
+    return NAV_AUTO_SUBFRAME;  // All manual subframes would get new IDs and
+                               // were handled above.
+  // Since we weeded out "new" navigations above, we know this is an existing
+  // navigation.
+  return NAV_EXISTING_PAGE;
+}
+
+void NavigationController::RendererDidNavigateToNewPage(
+    const ViewHostMsg_FrameNavigate_Params& params) {
+  NavigationEntry* new_entry;
+  if (pending_entry_) {
+    // TODO(brettw) this assumes that the pending entry is appropriate for the
+    // new page that was just loaded. I don't think this is necessarily the
+    // case! We should have some more tracking to know for sure. This goes along
+    // with a similar TODO at the top of RendererDidNavigate where we blindly
+    // set the site instance on the pending entry.
+    new_entry = new NavigationEntry(*pending_entry_);
+
+    // Don't use the page type from the pending entry. Some interstitial page
+    // may have set the type to interstitial. Once we commit, however, the page
+    // type must always be normal.
+    new_entry->set_page_type(NavigationEntry::NORMAL_PAGE);
+  } else {
+    new_entry = new NavigationEntry(active_contents_->type());
   }
 
-  delete entry;
-  NotifyNavigationEntryCommitted(details);
+  new_entry->set_url(params.url);
+  new_entry->set_page_id(params.page_id);
+  new_entry->set_transition_type(params.transition);
+  new_entry->set_site_instance(active_contents_->GetSiteInstance());
+  new_entry->set_has_post_data(params.is_post);
 
-  if (alternate_nav_url_fetcher_.get()) {
-    // Because this call may synchronously show an infobar, we do it last, to
-    // make sure all other state is stable and the infobar won't get blown away
-    // by some transition.
-    alternate_nav_url_fetcher_->OnNavigatedToEntry();
+  InsertEntry(new_entry);
+}
+
+void NavigationController::RendererDidNavigateToExistingPage(
+    const ViewHostMsg_FrameNavigate_Params& params) {
+  // We should only get here for main frame navigations.
+  DCHECK(PageTransition::IsMainFrame(params.transition));
+
+  // This is a back/forward navigation. The existing page for the ID is
+  // guaranteed to exist, and we just need to update it with new information
+  // from the renderer.
+  int entry_index = GetEntryIndexWithPageID(
+      active_contents_->type(),
+      active_contents_->GetSiteInstance(),
+      params.page_id);
+  DCHECK(entry_index >= 0 &&
+         entry_index < static_cast<int>(entries_.size()));
+  NavigationEntry* entry = entries_[entry_index].get();
+
+  // The URL may have changed due to redirects. The site instance will normally
+  // be the same except during session restore, when no site instance will be
+  // assigned.
+  entry->set_url(params.url);
+  DCHECK(entry->site_instance() == NULL ||
+         entry->site_instance() == active_contents_->GetSiteInstance());
+  entry->set_site_instance(active_contents_->GetSiteInstance());
+
+  // The entry we found in the list might be pending if the user hit
+  // back/forward/reload. This load should commit it (since it's already in the
+  // list, we can just discard the pending pointer).
+  //
+  // Note that we need to use the "internal" version since we don't want to
+  // actually change any other state, just kill the pointer.
+  if (entry == pending_entry_)
+    DiscardPendingEntryInternal();
+  
+  int old_committed_entry_index = last_committed_entry_index_;
+  last_committed_entry_index_ = entry_index;
+  IndexOfActiveEntryChanged(old_committed_entry_index);
+}
+
+void NavigationController::RendererDidNavigateToSamePage(
+    const ViewHostMsg_FrameNavigate_Params& params) {
+  // This mode implies we have a pending entry that's the same as an existing
+  // entry for this page ID. All we need to do is update the existing entry.
+  NavigationEntry* existing_entry = GetEntryWithPageID(
+      active_contents_->type(),
+      active_contents_->GetSiteInstance(),
+      params.page_id);
+
+  // We assign the entry's unique ID to be that of the new one. Since this is
+  // always the result of a user action, we want to dismiss infobars, etc. like
+  // a regular user-initiated navigation.
+  existing_entry->set_unique_id(pending_entry_->unique_id());
+
+  DiscardPendingEntry();
+}
+
+void NavigationController::RendererDidNavigateInPage(
+    const ViewHostMsg_FrameNavigate_Params& params) {
+  DCHECK(PageTransition::IsMainFrame(params.transition)) <<
+      "WebKit should only tell us about in-page navs for the main frame.";
+  // We're guaranteed to have an entry for this one.
+  NavigationEntry* existing_entry = GetEntryWithPageID(
+      active_contents_->type(),
+      active_contents_->GetSiteInstance(),
+      params.page_id);
+
+  // Reference fragment navigation. We're guaranteed to have the last_committed
+  // entry and it will be the same page as the new navigation (minus the
+  // reference fragments, of course).
+  NavigationEntry* new_entry = new NavigationEntry(*existing_entry);
+  new_entry->set_page_id(params.page_id);
+  new_entry->set_url(params.url);
+  InsertEntry(new_entry);
+}
+
+void NavigationController::RendererDidNavigateNewSubframe(
+    const ViewHostMsg_FrameNavigate_Params& params) {
+  // Manual subframe navigations just get the current entry cloned so the user
+  // can go back or forward to it. The actual subframe information will be
+  // stored in the page state for each of those entries. This happens out of
+  // band with the actual navigations.
+  DCHECK(GetLastCommittedEntry());
+  NavigationEntry* new_entry = new NavigationEntry(*GetLastCommittedEntry());
+  new_entry->set_page_id(params.page_id);
+  InsertEntry(new_entry);
+}
+
+bool NavigationController::RendererDidNavigateAutoSubframe(
+    const ViewHostMsg_FrameNavigate_Params& params) {
+  // We're guaranteed to have a previously committed entry, and we now need to
+  // handle navigation inside of a subframe in it without creating a new entry.
+  DCHECK(GetLastCommittedEntry());
+
+  // Handle the case where we're navigating back/forward to a previous subframe
+  // navigation entry. This is case "2." in NAV_AUTO_SUBFRAME comment in the
+  // header file. In case "1." this will be a NOP.
+  int entry_index = GetEntryIndexWithPageID(
+      active_contents_->type(),
+      active_contents_->GetSiteInstance(),
+      params.page_id);
+  if (entry_index < 0 ||
+      entry_index >= static_cast<int>(entries_.size())) {
+    NOTREACHED();
+    return false;
   }
 
-  // It is now a safe time to schedule collection for any tab contents of a
-  // different type, because a navigation is necessary to get back to them.
-  ScheduleTabContentsCollectionForInactiveTabs();
+  // Update the current navigation entry in case we're going back/forward.
+  if (entry_index != last_committed_entry_index_) {
+    int old_committed_entry_index = last_committed_entry_index_;
+    last_committed_entry_index_ = entry_index;
+    IndexOfActiveEntryChanged(old_committed_entry_index);
+    return true;
+  }
+  return false;
 }
 
+void NavigationController::CommitPendingEntry() {
+  if (!GetPendingEntry())
+    return;  // Nothing to do.
+
+  // Need to save the previous URL for the notification.
+  LoadCommittedDetails details;
+  if (GetLastCommittedEntry())
+    details.previous_url = GetLastCommittedEntry()->url();
+
+  if (pending_entry_index_ >= 0) {
+    // This is a previous navigation (back/forward) that we're just now
+    // committing. Just mark it as committed.
+    int new_entry_index = pending_entry_index_;
+    DiscardPendingEntryInternal();
+
+    // Mark that entry as committed.
+    int old_committed_entry_index = last_committed_entry_index_;
+    last_committed_entry_index_ = new_entry_index;
+    IndexOfActiveEntryChanged(old_committed_entry_index);
+  } else {
+    // This is a new navigation. It's easiest to just copy the entry and insert
+    // it new again, since InsertEntry expects to take ownership and also
+    // discard the pending entry. We also need to synthesize a page ID. We can
+    // only do this because this function will only be called by our custom
+    // TabContents types. For WebContents, the IDs are generated by the
+    // renderer, so we can't do this.
+    pending_entry_->set_page_id(active_contents_->GetMaxPageID() + 1);
+    active_contents_->UpdateMaxPageID(pending_entry_->page_id());
+    InsertEntry(new NavigationEntry(*pending_entry_));
+  }
+
+  // Broadcast the notification of the navigation.
+  details.entry = GetActiveEntry();
+  details.is_auto = false;
+  details.is_in_page = AreURLsInPageNavigation(details.previous_url,
+                                               details.entry->url());
+  details.is_main_frame = true;
+  NotifyNavigationEntryCommitted(&details);
+}
 
 int NavigationController::GetIndexOfEntry(
     const NavigationEntry* entry) const {
@@ -602,7 +855,7 @@ int NavigationController::GetIndexOfEntry(
   return (i == entries_.end()) ? -1 : static_cast<int>(i - entries_.begin());
 }
 
-void NavigationController::RemoveLastEntry() {
+void NavigationController::RemoveLastEntryForInterstitial() {
   int current_size = static_cast<int>(entries_.size());
 
   if (current_size > 0) {
@@ -612,13 +865,51 @@ void NavigationController::RemoveLastEntry() {
 
     entries_.pop_back();
 
-    if (last_committed_entry_index_ >= current_size - 1)
+    if (last_committed_entry_index_ >= current_size - 1) {
       last_committed_entry_index_ = current_size - 2;
 
-    NotifyPrunedEntries();
+      // Broadcast the notification of the navigation. This is kind of a hack,
+      // since the navigation wasn't actually committed. But this function is
+      // used for interstital pages, and the UI needs to get updated when the
+      // interstitial page
+      LoadCommittedDetails details;
+      details.entry = GetActiveEntry();
+      details.is_auto = false;
+      details.is_in_page = false;
+      details.is_main_frame = true;
+      NotifyNavigationEntryCommitted(&details);
+    }
+
+    NotifyPrunedEntries(this);
   }
 }
 
+void NavigationController::AddDummyEntryForInterstitial(
+    const NavigationEntry& clone_me) {
+  // We need to send a commit notification for this transition.
+  LoadCommittedDetails details;
+  if (GetLastCommittedEntry())
+    details.previous_url = GetLastCommittedEntry()->url();  
+
+  NavigationEntry* new_entry = new NavigationEntry(clone_me);
+  InsertEntry(new_entry);
+  // Watch out, don't use clone_me after this. The caller may have passed in a
+  // reference to our pending entry, which means it would have been destroyed.
+
+  details.is_auto = false;
+  details.entry = GetActiveEntry();
+  details.is_in_page = false;
+  details.is_main_frame = true;
+  NotifyNavigationEntryCommitted(&details);
+}
+
+bool NavigationController::IsURLInPageNavigation(const GURL& url) const {
+  NavigationEntry* last_committed = GetLastCommittedEntry();
+  if (!last_committed)
+    return false;
+  return AreURLsInPageNavigation(last_committed->url(), url);
+}
+
 void NavigationController::DiscardPendingEntry() {
   DiscardPendingEntryInternal();
 
@@ -675,7 +966,7 @@ void NavigationController::InsertEntry(NavigationEntry* entry) {
       current_size--;
     }
     if (pruned)  // Only notify if we did prune something.
-      NotifyPrunedEntries();
+      NotifyPrunedEntries(this);
   }
 
   if (entries_.size() >= max_entry_count_)
@@ -683,7 +974,12 @@ void NavigationController::InsertEntry(NavigationEntry* entry) {
 
   entries_.push_back(linked_ptr<NavigationEntry>(entry));
   last_committed_entry_index_ = static_cast<int>(entries_.size()) - 1;
+
+  // This is a new page ID, so we need everybody to know about it.
+  active_contents_->UpdateMaxPageID(entry->page_id());
   
+  // TODO(brettw) this seems bogus. The tab contents can listen for the
+  // notification or use the details that we pass back to it.
   active_contents_->NotifyDidNavigate(NAVIGATION_NEW, 0);
 }
 
@@ -723,7 +1019,8 @@ void NavigationController::NavigateToPendingEntry(bool reload) {
       from_contents->delegate()->ReplaceContents(from_contents, contents);
   }
 
-  if (!contents->Navigate(*pending_entry_, reload))
+  NavigationEntry temp_entry(*pending_entry_);
+  if (!contents->NavigateToPendingEntry(reload))
     DiscardPendingEntry();
 }
 
@@ -755,20 +1052,16 @@ void NavigationController::NotifyNavigationEntryCommitted(
       Details<LoadCommittedDetails>(details));
 }
 
-void NavigationController::NotifyPrunedEntries() {
-  NotificationService::current()->Notify(NOTIFY_NAV_LIST_PRUNED,
-                                         Source<NavigationController>(this),
-                                         NotificationService::NoDetails());
-}
-
-void NavigationController::IndexOfActiveEntryChanged(
-    int prev_committed_index) {
+void NavigationController::IndexOfActiveEntryChanged(int prev_committed_index) {
   NavigationType nav_type = NAVIGATION_BACK_FORWARD;
   int relative_navigation_offset =
       GetLastCommittedEntryIndex() - prev_committed_index;
-  if (relative_navigation_offset == 0) {
+  if (relative_navigation_offset == 0)
     nav_type = NAVIGATION_REPLACE;
-  }
+
+  // TODO(brettw) I don't think this call should be necessary. There is already
+  // a notification of this event that could be used, or maybe all the tab
+  // contents' know when we navigate (WebContents does).
   active_contents_->NotifyDidNavigate(nav_type, relative_navigation_offset);
 }
 
@@ -819,15 +1112,6 @@ void NavigationController::RegisterTabContents(TabContents* some_contents) {
     some_contents->AsDOMUIHost()->AttachMessageHandlers();
 }
 
-void NavigationController::NotifyEntryChangedByPageID(
-    TabContentsType type,
-    SiteInstance *instance,
-    int32 page_id) {
-  int index = GetEntryIndexWithPageID(type, instance, page_id);
-  if (index != -1)
-    NotifyEntryChanged(entries_[index].get(), index);
-}
-
 // static
 void NavigationController::DisablePromptOnRepost() {
   check_for_repost_ = false;
@@ -888,10 +1172,6 @@ void NavigationController::RemoveEntryAtIndex(int index) {
   // session service can stay in sync.
 }
 
-int NavigationController::GetMaxPageID() const {
-  return active_contents_->GetMaxPageID();
-}
-
 NavigationController* NavigationController::Clone(HWND parent_hwnd) {
   NavigationController* nc = new NavigationController(NULL, profile_);
 
@@ -974,18 +1254,6 @@ void NavigationController::DiscardPendingEntryInternal() {
 
 int NavigationController::GetEntryIndexWithPageID(
     TabContentsType type, SiteInstance* instance, int32 page_id) const {
-  // The instance should only be specified for contents displaying web pages.
-  // TODO(evanm): checking against NEW_TAB_UI and HTML_DLG here is lame.
-  // It'd be nice for DomUIHost to just use SiteInstances for keeping content
-  // separated properly.
-  if (type != TAB_CONTENTS_WEB &&
-      type != TAB_CONTENTS_NEW_TAB_UI &&
-      type != TAB_CONTENTS_ABOUT_UI &&
-      type != TAB_CONTENTS_HTML_DIALOG &&
-      type != TAB_CONTENTS_VIEW_SOURCE &&
-      type != TAB_CONTENTS_DEBUGGER)
-    DCHECK(instance == NULL);
-
   for (int i = static_cast<int>(entries_.size()) - 1; i >= 0; --i) {
     if ((entries_[i]->tab_type() == type) &&
         (entries_[i]->site_instance() == instance) &&
diff --git a/chrome/browser/navigation_controller.h b/chrome/browser/navigation_controller.h
index 976313e..f099704 100644
--- a/chrome/browser/navigation_controller.h
+++ b/chrome/browser/navigation_controller.h
@@ -5,7 +5,8 @@
 #ifndef CHROME_BROWSER_NAVIGATION_CONTROLLER_H_
 #define CHROME_BROWSER_NAVIGATION_CONTROLLER_H_
 
-#include "base/hash_tables.h"
+#include <map>
+
 #include "base/linked_ptr.h"
 #include "base/ref_counted.h"
 #include "chrome/browser/alternate_nav_url_fetcher.h"
@@ -20,24 +21,17 @@ class TabContents;
 class WebContents;
 class TabContentsCollector;
 struct TabNavigation;
+struct ViewHostMsg_FrameNavigate_Params;
 
-namespace printing {
-class PrintViewManager;
-}
-
-////////////////////////////////////////////////////////////////////////////////
-//
-// NavigationController class
-//
-// A NavigationController maintains navigation data. We have one
-// NavigationController instance per tab.
+// A NavigationController maintains the back-forward list for a single tab and
+// manages all navigation within that list.
 //
 // The NavigationController also owns all TabContents for the tab. This is to
 // make sure that we have at most one TabContents instance per type.
-//
-////////////////////////////////////////////////////////////////////////////////
 class NavigationController {
  public:
+  // Notification details ------------------------------------------------------
+
   // Provides the details for a NOTIFY_NAV_ENTRY_CHANGED notification.
   struct EntryChangedDetails {
     // The changed navigation entry after it has been updated.
@@ -47,6 +41,7 @@ class NavigationController {
     int index;
   };
 
+  // Provides the details for a NOTIFY_NAV_ENTRY_COMMITTED notification.
   struct LoadCommittedDetails {
     // By default, the entry will be filled according to a new main frame
     // navigation.
@@ -87,7 +82,10 @@ class NavigationController {
     }
   };
 
+  // ---------------------------------------------------------------------------
+
   NavigationController(TabContents* initial_contents, Profile* profile);
+
   // Creates a NavigationController from the specified history. Processing
   // for this is asynchronous and handled via the RestoreHelper (in
   // navigation_controller.cc).
@@ -98,8 +96,24 @@ class NavigationController {
       HWND parent);
   ~NavigationController();
 
-  // Same as Reload, but doesn't check if current entry has POST data.
-  void ReloadDontCheckForRepost();
+  // Begin the destruction sequence for this NavigationController and all its
+  // registered tabs.  The sequence is as follows:
+  // 1. All tabs are asked to Destroy themselves.
+  // 2. When each tab is finished Destroying, it will notify the controller.
+  // 3. Once all tabs are Destroyed, the NavigationController deletes itself.
+  // This ensures that all the TabContents outlive the NavigationController.
+  void Destroy();
+
+  // Clone the receiving navigation controller. Only the active tab contents is
+  // duplicated. It is created as a child of the provided HWND.
+  NavigationController* Clone(HWND hwnd);
+
+  // Returns the profile for this controller. It can never be NULL.
+  Profile* profile() const {
+    return profile_;
+  }
+
+  // Active entry --------------------------------------------------------------
 
   // Returns the active entry, which is the pending entry if a navigation is in
   // progress or the last committed entry otherwise.  NOTE: This can be NULL!!
@@ -114,19 +128,7 @@ class NavigationController {
   // it is the pending_entry_index_.
   int GetCurrentEntryIndex() const;
 
-  // Returns the pending entry corresponding to the navigation that is
-  // currently in progress, or null if there is none.
-  NavigationEntry* GetPendingEntry() const {
-    return pending_entry_;
-  }
-
-  // Returns the index of the pending entry or -1 if the pending entry
-  // corresponds to a new navigation (created via LoadURL).
-  int GetPendingEntryIndex() const {
-    return pending_entry_index_;
-  }
-
-    // Returns the last committed entry, which may be null if there are no
+  // Returns the last committed entry, which may be null if there are no
   // committed entries.
   NavigationEntry* GetLastCommittedEntry() const;
 
@@ -135,6 +137,8 @@ class NavigationController {
     return last_committed_entry_index_;
   }
 
+  // Navigation list -----------------------------------------------------------
+
   // Returns the number of entries in the NavigationControllerBase, excluding
   // the pending entry if there is one.
   int GetEntryCount() const {
@@ -149,85 +153,155 @@ class NavigationController {
   // if out of bounds.
   NavigationEntry* GetEntryAtOffset(int offset) const;
 
-  bool CanStop() const;
+  // Returns the index of the specified entry, or -1 if entry is not contained
+  // in this NavigationControllerBase.
+  int GetIndexOfEntry(const NavigationEntry* entry) const;
 
-  // Return whether this controller can go back.
-  bool CanGoBack() const;
+  // Return the index of the entry with the corresponding type, instance, and
+  // page_id, or -1 if not found.  Use a NULL instance if the type is not
+  // TAB_CONTENTS_WEB.
+  int GetEntryIndexWithPageID(TabContentsType type,
+                              SiteInstance* instance,
+                              int32 page_id) const;
 
-  // Return whether this controller can go forward.
-  bool CanGoForward() const;
+  // Return the entry with the corresponding type, instance, and page_id, or
+  // NULL if not found.  Use a NULL instance if the type is not
+  // TAB_CONTENTS_WEB.
+  NavigationEntry* GetEntryWithPageID(TabContentsType type,
+                                      SiteInstance* instance,
+                                      int32 page_id) const;
 
-  // Causes the controller to go back.
-  void GoBack();
+  // Pending entry -------------------------------------------------------------
+
+  // Commits the current pending entry and issues the NOTIFY_NAV_ENTRY_COMMIT
+  // notification. No changes are made to the entry during this process, it is
+  // just moved from pending to committed. This is an alternative to
+  // RendererDidNavigate for simple TabContents types.
+  //
+  // When the pending entry is a new navigation, it will have a page ID of -1.
+  // The caller should leave this as-is. CommitPendingEntry will generate a
+  // new page ID for you and update the TabContents with that ID.
+  void CommitPendingEntry();
+
+  // Calling this may cause the active tab contents to switch if the current
+  // entry corresponds to a different tab contents type.
+  void DiscardPendingEntry();
+
+  // Returns the pending entry corresponding to the navigation that is
+  // currently in progress, or null if there is none.
+  NavigationEntry* GetPendingEntry() const {
+    return pending_entry_;
+  }
+
+  // Returns the index of the pending entry or -1 if the pending entry
+  // corresponds to a new navigation (created via LoadURL).
+  int GetPendingEntryIndex() const {
+    return pending_entry_index_;
+  }
+
+  // New navigations -----------------------------------------------------------
+
+  // Loads the specified URL.
+  void LoadURL(const GURL& url, PageTransition::Type type);
 
-  // Causes the controller to go forward.
+  // Load the specified URL the next time it becomes active.
+  void LoadURLLazily(const GURL& url, PageTransition::Type type,
+                     const std::wstring& title, SkBitmap* icon);
+
+  // Loads the current page if this NavigationController was restored from
+  // history and the current page has not loaded yet.
+  void LoadIfNecessary();
+
+  // Renavigation --------------------------------------------------------------
+
+  // Navigation relative to the "current entry"
+  bool CanGoBack() const;
+  bool CanGoForward() const;
+  void GoBack();
   void GoForward();
 
-  // Causes the controller to go to the specified index.
+  // Navigates to the specified absolute index.
   void GoToIndex(int index);
 
-  // Causes the controller to go to the specified offset from current.  Does
-  // nothing if out of bounds.
+  // Navigates to the specified offset from the "current entry". Does nothing if
+  // the offset is out of bounds.
   void GoToOffset(int offset);
 
-  // Causes the controller to stop a pending navigation if any.
-  void Stop();
-
-  // Causes the controller to reload the current entry. Will prompt the user if
-  // reloading a URL with POST data and the active WebContents isn't showing the
-  // POST interstitial page.
+  // Reloads the current entry. The user will be prompted if the URL has POST
+  // data and the active WebContents isn't showing the POST interstitial page.
   void Reload();
 
-  // Return the entry with the corresponding type, instance, and page_id, or
-  // NULL if not found.  Use a NULL instance if the type is not
-  // TAB_CONTENTS_WEB.
-  NavigationEntry* GetEntryWithPageID(TabContentsType type,
-                                      SiteInstance* instance,
-                                      int32 page_id) const;
-
-  // Causes the controller to load the specified entry.  The controller
-  // assumes ownership of the entry.
-  // NOTE: Do not pass an entry that the controller already owns!
-  void LoadEntry(NavigationEntry* entry);
-
-  // Ensure the given NavigationEntry has a valid state, so that WebKit does
-  // not get confused.
-  static void SetContentStateIfEmpty(NavigationEntry* entry);
+  // Same as Reload, but doesn't check if current entry has POST data.
+  void ReloadDontCheckForRepost();
 
-  // Begin the destruction sequence for this NavigationController and all its
-  // registered tabs.  The sequence is as follows:
-  // 1. All tabs are asked to Destroy themselves.
-  // 2. When each tab is finished Destroying, it will notify the controller.
-  // 3. Once all tabs are Destroyed, the NavigationController deletes itself.
-  // This ensures that all the TabContents outlive the NavigationController.
-  void Destroy();
+  // TabContents ---------------------------------------------------------------
 
   // Notifies the controller that a TabContents that it owns has been destroyed.
   // This is part of the NavigationController's Destroy sequence.
   void TabContentsWasDestroyed(TabContentsType type);
 
+  // Returns the TabContents cached on this controller for the given type or
+  // NULL if there is none.
+  TabContents* GetTabContents(TabContentsType type);
+
   // Returns the currently-active TabContents associated with this controller.
   // You should use GetActiveEntry instead of this in most cases.
   TabContents* active_contents() const {
     return active_contents_;
   }
 
-  // This can never be null.
-  Profile* profile() const {
-    return profile_;
-  }
+  // For use by TabContents ----------------------------------------------------
 
-  // Returns the TabContents cached on this controller for the given type or
-  // NULL if there is none.
-  TabContents* GetTabContents(TabContentsType type);
+  // Handles updating the navigation state after the renderer has navigated.
+  // This is used by the WebContents. Simpler tab contents types can use
+  // CommitPendingEntry below.
+  //
+  // If a new entry is created, it will return true and will have filled the
+  // given details structure and broadcast the NOTIFY_NAV_ENTRY_COMMITTED
+  // notification. The caller can then use the details without worrying about
+  // listening for the notification.
+  //
+  // In the case that nothing has changed, the details structure is undefined
+  // and it will return false.
+  bool RendererDidNavigate(const ViewHostMsg_FrameNavigate_Params& params,
+                           bool is_interstitial,
+                           LoadCommittedDetails* details);
+
+  // Inserts a new entry by making a copy of the given navigation entry. This is
+  // used by interstitials to create dummy entries that they will be in charge
+  // of removing later.
+  void AddDummyEntryForInterstitial(const NavigationEntry& clone_me);
+
+  // Removes the last entry in the list. This is used by the interstitial code
+  // to delete the dummy entry created by AddDummyEntryForInterstitial. If the
+  // last entry is the currently committed one, a ENTRY_COMMITTED notification
+  // will be broadcast.
+  void RemoveLastEntryForInterstitial();
+
+  // Notifies us that we just became active. This is used by the TabContents
+  // so that we know to load URLs that were pending as "lazy" loads.
+  void SetActive(bool is_active);
 
-  // Causes the controller to load the specified URL.
-  void LoadURL(const GURL& url, PageTransition::Type type);
+  // Broadcasts the NOTIFY_NAV_ENTRY_CHANGED notification for the given entry
+  // (which must be at the given index). This will keep things in sync like
+  // the saved session.
+  void NotifyEntryChanged(const NavigationEntry* entry, int index);
 
-  // Causes the controller to load the specified URL the next time it becomes
-  // active.
-  void LoadURLLazily(const GURL& url, PageTransition::Type type,
-                     const std::wstring& title, SkBitmap* icon);
+  // Returns true if the given URL would be an in-page navigation (i.e. only
+  // the reference fragment is different) from the "last committed entry". We do
+  // not compare it against the "active entry" since the active entry can be
+  // pending and in page navigations only happen on committed pages. If there
+  // is no last committed entry, then nothing will be in-page.
+  //
+  // Special note: if the URLs are the same, it does NOT count as an in-page
+  // navigation. Neither does an input URL that has no ref, even if the rest is
+  // the same. This may seem weird, but when we're considering whether a
+  // navigation happened without loading anything, the same URL would be a
+  // reload, while only a different ref would be in-page (pages can't clear
+  // refs without reload, only change to "#" which we don't count as empty).
+  bool IsURLInPageNavigation(const GURL& url) const;
+
+  // Random data ---------------------------------------------------------------
 
   // Returns true if this NavigationController is is configured to load a URL
   // lazily. If true, use GetLazyTitle() and GetLazyFavIcon() to discover the
@@ -237,34 +311,10 @@ class NavigationController {
   const std::wstring& GetLazyTitle() const;
   const SkBitmap& GetLazyFavIcon() const;
 
+  // TODO(brettw) bug 1324500: move this out of here.
   void SetAlternateNavURLFetcher(
       AlternateNavURLFetcher* alternate_nav_url_fetcher);
 
-  // --------------------------------------------------------------------------
-  // For use by TabContents implementors:
-
-  // Used to inform the NavigationControllerBase of a navigation being committed
-  // for a tab.  The controller takes ownership of the entry.  Any entry located
-  // forward to the current entry will be deleted.  The new entry becomes the
-  // current entry.
-  //
-  // The details are populated by the caller except for the new NavigationEntry
-  // pointer and the previous URL. We will fill these in before using it to
-  // broadcast notifications, so it can also be used by the caller.
-  //
-  // TODO(brettw) bug 1343146: The NavigationController should internally make
-  // the entry and the notification details.
-  void DidNavigateToEntry(NavigationEntry* entry,
-                          LoadCommittedDetails* details);
-
-  // Calling this may cause the active tab contents to switch if the current
-  // entry corresponds to a different tab contents type.
-  void DiscardPendingEntry();
-
-  // Inserts an entry after the current position, removing all entries after it.
-  // The new entry will become the active one.
-  void InsertEntry(NavigationEntry* entry);
-
   // Returns the identifier used by session restore.
   const SessionID& session_id() const { return session_id_; }
 
@@ -274,58 +324,97 @@ class NavigationController {
 
   SSLManager* ssl_manager() { return &ssl_manager_; }
 
-  // Broadcasts the NOTIFY_NAV_ENTRY_CHANGED notification for the
-  // navigation corresponding to the given page. This will keep things in sync
-  // like saved session.
-  void NotifyEntryChangedByPageID(TabContentsType type,
-                                  SiteInstance* instance,
-                                  int32 page_id);
-
-  void SetActive(bool is_active);
-
-  // If this NavigationController was restored from history and the selected
-  // page has not loaded, it is loaded.
-  void LoadIfNecessary();
-
-  // Clone the receiving navigation controller. Only the active tab contents is
-  // duplicated. It is created as a child of the provided HWND.
-  NavigationController* Clone(HWND hwnd);
-
   // Returns true if a reload happens when activated (SetActive(true) is
   // invoked). This is true for session/tab restore and cloned tabs.
   bool needs_reload() const { return needs_reload_; }
 
-  // Disables checking for a repost and prompting the user. This is used during
-  // testing.
-  static void DisablePromptOnRepost();
-
   // Returns the largest restored page ID seen in this navigation controller,
   // if it was restored from a previous session.  (-1 otherwise)
   int max_restored_page_id() const { return max_restored_page_id_; }
 
-  // Returns the index of the specified entry, or -1 if entry is not contained
-  // in this NavigationControllerBase.
-  int GetIndexOfEntry(const NavigationEntry* entry) const;
-
-  // Removes the last committed entry.
-  void RemoveLastEntry();
+  // Disables checking for a repost and prompting the user. This is used during
+  // testing.
+  static void DisablePromptOnRepost();
 
  private:
   FRIEND_TEST(NavigationControllerTest, EnforceMaxNavigationCount);
-
   class RestoreHelper;
   friend class RestoreHelper;
+  friend class TabContents;  // For invoking OnReservedPageIDRange.
+
+  // Indicates different types of navigations that can occur that we will handle
+  // separately. This is computed by ClassifyNavigation.
+  enum NavClass {
+    // A new page was navigated in the main frame.
+    NAV_NEW_PAGE,
+
+    // Renavigating to an existing navigation entry. The entry is guaranteed to
+    // exist in the list, or else it would be a new page or IGNORE navigation.
+    NAV_EXISTING_PAGE,
+
+    // The same page has been reloaded as a result of the user requesting
+    // navigation to that same page (like pressing Enter in the URL bar). This
+    // is not the same as an in-page navigation because we'll actually have a
+    // pending entry for the load, which is then meaningless.
+    NAV_SAME_PAGE,
+
+    // In page navigations are when the reference fragment changes. This will
+    // be in the main frame only (we won't even get notified of in-page
+    // subframe navigations). It may be for any page, not necessarily the last
+    // committed one (for example, whey going back to a page with a ref).
+    NAV_IN_PAGE,
+
+    // A new subframe was manually navigated by the user. We will create a new
+    // NavigationEntry so they can go back to the previous subframe content
+    // using the back button.
+    NAV_NEW_SUBFRAME,
+
+    // A subframe in the page was automatically loaded or navigated to such that
+    // a new navigation entry should not be created. There are two cases:
+    //  1. Stuff like iframes containing ads that the page loads automatically.
+    //     The user doesn't want to see these, so we just update the existing
+    //     navigation entry.
+    //  2. Going back/forward to previous subframe navigations. We don't create
+    //     a new entry here either, just update the last committed entry.
+    // These two cases are actually pretty different, they just happen to
+    // require almost the same code to handle.
+    NAV_AUTO_SUBFRAME,
+
+    // Nothing happened. This happens when we get information about a page we
+    // don't know anything about. It can also happen when an iframe in a popup
+    // navigated to about:blank is navigated. Nothing needs to be done.
+    NAV_IGNORE,
+  };
 
-  // For invoking OnReservedPageIDRange.
-  friend class TabContents;
-  // For invoking GetMaxPageID.
-  friend class WebContents;
-  // For invoking GetMaxPageID.
-  friend class printing::PrintViewManager;
+  // Classifies the given renderer navigation (see the NavigationType enum).
+  NavClass ClassifyNavigation(
+      const ViewHostMsg_FrameNavigate_Params& params) const;
 
-  // Returns the largest page ID seen.  When PageIDs come in larger than
-  // this (via DidNavigateToEntry), we know that we've navigated to a new page.
-  int GetMaxPageID() const;
+  // Causes the controller to load the specified entry. The function assumes
+  // ownership of the pointer since it is put in the navigation list.
+  // NOTE: Do not pass an entry that the controller already owns!
+  void LoadEntry(NavigationEntry* entry);
+
+  // Handlers for the different types of navigation types. They will actually
+  // handle the navigations corresponding to the different NavClasses above.
+  // They will NOT broadcast the commit notification, that should be handled by
+  // the caller.
+  //
+  // RendererDidNavigateAutoSubframe is special, it may not actually change
+  // anything if some random subframe is loaded. It will return true if anything
+  // changed, or false if not.
+  void RendererDidNavigateToNewPage(
+      const ViewHostMsg_FrameNavigate_Params& params);
+  void RendererDidNavigateToExistingPage(
+      const ViewHostMsg_FrameNavigate_Params& params);
+  void RendererDidNavigateToSamePage(
+      const ViewHostMsg_FrameNavigate_Params& params);
+  void RendererDidNavigateInPage(
+      const ViewHostMsg_FrameNavigate_Params& params);
+  void RendererDidNavigateNewSubframe(
+      const ViewHostMsg_FrameNavigate_Params& params);
+  bool RendererDidNavigateAutoSubframe(
+      const ViewHostMsg_FrameNavigate_Params& params);
 
   // Actually issues the navigation held in pending_entry.
   void NavigateToPendingEntry(bool reload);
@@ -334,11 +423,6 @@ class NavigationController {
   // committed. This will fill in the active entry to the details structure.
   void NotifyNavigationEntryCommitted(LoadCommittedDetails* details);
 
-  // Invoked when entries have been pruned, or removed. For example, if the
-  // current entries are [google, digg, yahoo], with the current entry google,
-  // and the user types in cnet, then digg and yahoo are pruned.
-  void NotifyPrunedEntries();
-
   // Invoked when the index of the active entry may have changed.
   // The prev_commited_index parameter specifies the previous value
   // of the last commited index before this navigation event happened
@@ -354,9 +438,6 @@ class NavigationController {
   // and deleted by this navigation controller
   void RegisterTabContents(TabContents* some_contents);
 
-  // Broadcasts a notification that the given entry changed.
-  void NotifyEntryChanged(const NavigationEntry* entry, int index);
-
   // Removes the entry at the specified index.  Note that you should not remove
   // the pending entry or the last committed entry.
   void RemoveEntryAtIndex(int index);
@@ -385,15 +466,14 @@ class NavigationController {
   // contents.
   void FinishRestore(HWND parent_hwnd, int selected_index);
 
+  // Inserts an entry after the current position, removing all entries after it.
+  // The new entry will become the active one.
+  void InsertEntry(NavigationEntry* entry);
+
   // Discards the pending entry without updating active_contents_
   void DiscardPendingEntryInternal();
 
-  // Return the index of the entry with the corresponding type, instance, and
-  // page_id, or -1 if not found.  Use a NULL instance if the type is not
-  // TAB_CONTENTS_WEB.
-  int GetEntryIndexWithPageID(TabContentsType type,
-                              SiteInstance* instance,
-                              int32 page_id) const;
+  // ---------------------------------------------------------------------------
 
   // The user profile associated with this controller
   Profile* profile_;
@@ -420,7 +500,7 @@ class NavigationController {
 
   // Tab contents. One entry per type used. The tab controller owns
   // every tab contents used.
-  typedef base::hash_map<TabContentsType, TabContents*> TabContentsMap;
+  typedef std::map<TabContentsType, TabContents*> TabContentsMap;
   TabContentsMap tab_contents_map_;
 
   // A map of TabContentsType -> TabContentsCollector containing all the
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]);
 }
-
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index c5d0fe1..0a9d6cf 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -169,20 +169,10 @@ void SafeBrowsingBlockingPage::DisplayBlockingPage() {
   // (typically the navigation was initiated by the page), we create a fake
   // navigation entry (so the location bar shows the page's URL).
   if (is_main_frame_ && tab_->controller()->GetPendingEntryIndex() == -1) {
-    // New navigation.
-    NavigationEntry* nav_entry = new NavigationEntry(TAB_CONTENTS_WEB);
-
-    // We set the page ID to max page id so to ensure the controller considers
-    // this dummy entry a new one.  Because we'll remove the entry when the
-    // interstitial is going away, it will not conflict with any future
-    // navigations.
-    nav_entry->set_page_id(tab_->GetMaxPageID() + 1);
-    nav_entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE);
-    nav_entry->set_url(url_);
-
-    // The default details is "new navigation", and that's OK with us.
-    NavigationController::LoadCommittedDetails details;
-    tab_->controller()->DidNavigateToEntry(nav_entry, &details);
+    NavigationEntry new_entry(TAB_CONTENTS_WEB);
+    new_entry.set_url(url_);
+    new_entry.set_page_type(NavigationEntry::INTERSTITIAL_PAGE);
+    tab_->controller()->AddDummyEntryForInterstitial(new_entry);
     created_temporary_entry_ = true;
   }
 
@@ -244,7 +234,7 @@ bool SafeBrowsingBlockingPage::GoBack() {
   // Remove the navigation entry for the malware page.  Note that we always
   // remove the entry even if we did not create it as it has been flagged as
   // malware and we don't want the user navigating back to it.
-  web_contents->controller()->RemoveLastEntry();
+  web_contents->controller()->RemoveLastEntryForInterstitial();
 
   return true;
 }
@@ -296,7 +286,7 @@ void SafeBrowsingBlockingPage::Continue(const std::string& user_action) {
     // We are continuing, if we have created a temporary navigation entry,
     // delete it as a new will be created on navigation.
     if (created_temporary_entry_)
-      web->controller()->RemoveLastEntry();
+      web->controller()->RemoveLastEntryForInterstitial();
     if (is_main_frame_)
       web->HideInterstitialPage(true, true);
     else
diff --git a/chrome/browser/session_service.cc b/chrome/browser/session_service.cc
index 9784f8c..2694357 100644
--- a/chrome/browser/session_service.cc
+++ b/chrome/browser/session_service.cc
@@ -424,11 +424,16 @@ void SessionService::Observe(NotificationType type,
                           changed->index, *changed->changed_entry);
       break;
     }
-    case NOTIFY_NAV_ENTRY_COMMITTED:
+    case NOTIFY_NAV_ENTRY_COMMITTED: {
+      int current_entry_index = controller->GetCurrentEntryIndex();
       SetSelectedNavigationIndex(controller->window_id(),
                                  controller->session_id(),
-                                 controller->GetCurrentEntryIndex());
+                                 current_entry_index);
+      UpdateTabNavigation(controller->window_id(), controller->session_id(),
+                          current_entry_index,
+                          *controller->GetEntryAtIndex(current_entry_index));
       break;
+    }
     default:
       NOTREACHED();
   }
diff --git a/chrome/browser/session_service_test_helper.cc b/chrome/browser/session_service_test_helper.cc
index a793ed0..49cedd8 100644
--- a/chrome/browser/session_service_test_helper.cc
+++ b/chrome/browser/session_service_test_helper.cc
@@ -68,7 +68,7 @@ void SessionServiceTestHelper::AssertNavigationEquals(
 void SessionServiceTestHelper::AssertSingleWindowWithSingleTab(
     const std::vector<SessionWindow*>& windows,
     int nav_count) {
-  EXPECT_EQ(1, windows.size());
+  ASSERT_EQ(1, windows.size());
   EXPECT_EQ(1, windows[0]->tabs.size());
   EXPECT_EQ(nav_count, windows[0]->tabs[0]->navigations.size());
 }
diff --git a/chrome/browser/ssl_blocking_page.cc b/chrome/browser/ssl_blocking_page.cc
index 699c9e2..ba90efe 100644
--- a/chrome/browser/ssl_blocking_page.cc
+++ b/chrome/browser/ssl_blocking_page.cc
@@ -51,7 +51,7 @@ SSLBlockingPage::SSLBlockingPage(SSLManager::CertError* error,
 
     // Since WebContents::InterstitialPageGone won't be called, we need
     // to clear the last NavigationEntry manually.
-    tab_->controller()->RemoveLastEntry();
+    tab_->controller()->RemoveLastEntryForInterstitial();
   }
   (*tab_to_blocking_page_)[tab_] = this;
 
@@ -132,31 +132,31 @@ void SSLBlockingPage::Show() {
   int cert_id = CertStore::GetSharedInstance()->StoreCert(
       ssl_info.cert, tab->render_view_host()->process()->host_id());
 
-  NavigationEntry* nav_entry = new NavigationEntry(TAB_CONTENTS_WEB);
   if (tab_->controller()->GetPendingEntryIndex() == -1) {
-    // New navigation.
-    // We set the page ID to max page id so to ensure the controller considers
-    // this dummy entry a new one.  Because we'll remove the entry when the
-    // interstitial is going away, it will not conflict with any future
-    // navigations.
+    // For new navigations, we just create a new navigation entry.
+    NavigationEntry new_entry(TAB_CONTENTS_WEB);
+    new_entry.set_url(error_->request_url());
+    tab_->controller()->AddDummyEntryForInterstitial(new_entry);
     created_nav_entry_ = true;
-    nav_entry->set_page_id(tab_->GetMaxPageID() + 1);
-    nav_entry->set_url(error_->request_url());
   } else {
-    // Make sure to update the current entry ssl state to reflect the error.
-    *nav_entry = *(tab_->controller()->GetPendingEntry());
+    // When there is a pending entry index, that means we're doing a
+    // back/forward navigation. Clone that entry instead.
+    tab_->controller()->AddDummyEntryForInterstitial(
+        *tab_->controller()->GetPendingEntry());
   }
-  nav_entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE);
 
-  nav_entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN);
-  nav_entry->ssl().set_cert_id(cert_id);
-  nav_entry->ssl().set_cert_status(ssl_info.cert_status);
-  nav_entry->ssl().set_security_bits(ssl_info.security_bits);
-  // The controller will own the entry.
-
-  // The default details is "new navigation", and that's OK with us.
-  NavigationController::LoadCommittedDetails details;
-  tab_->controller()->DidNavigateToEntry(nav_entry, &details);
+  NavigationEntry* entry = tab_->controller()->GetActiveEntry();
+  entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE);
+
+  entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN);
+  entry->ssl().set_cert_id(cert_id);
+  entry->ssl().set_cert_status(ssl_info.cert_status);
+  entry->ssl().set_security_bits(ssl_info.security_bits);
+  NotificationService::current()->Notify(
+      NOTIFY_SSL_STATE_CHANGED,
+      Source<NavigationController>(tab_->controller()),
+      NotificationService::NoDetails());
+ 
   tab->ShowInterstitialPage(html_text, NULL);
 }
 
@@ -176,7 +176,7 @@ void SSLBlockingPage::Observe(NotificationType type,
       // as their navigation history is not saved.
       if (remove_last_entry_ && browser &&
           !browser->tabstrip_model()->closing_all()) {
-        tab_->controller()->RemoveLastEntry();
+        tab_->controller()->RemoveLastEntryForInterstitial();
       }
       delete this;
       break;
@@ -220,7 +220,7 @@ void SSLBlockingPage::DontProceed() {
 
   // We are navigating, remove the current entry before we mess with it.
   remove_last_entry_ = false;
-  tab_->controller()->RemoveLastEntry();
+  tab_->controller()->RemoveLastEntryForInterstitial();
 
   NavigationEntry* entry = tab_->controller()->GetActiveEntry();
   if (!entry) {
diff --git a/chrome/browser/ssl_manager.cc b/chrome/browser/ssl_manager.cc
index bc4e957..ffb0645 100644
--- a/chrome/browser/ssl_manager.cc
+++ b/chrome/browser/ssl_manager.cc
@@ -658,7 +658,7 @@ void SSLManager::DidCommitProvisionalLoad(ProvisionalLoadDetails* details) {
     NotificationService::current()->Notify(
         NOTIFY_SSL_STATE_CHANGED,
         Source<NavigationController>(controller_),
-        Details<NavigationEntry>(controller_->GetActiveEntry()));
+        NotificationService::NoDetails());
   }
 }
 
diff --git a/chrome/browser/ssl_policy.cc b/chrome/browser/ssl_policy.cc
index 4ae02a2..beb2978 100644
--- a/chrome/browser/ssl_policy.cc
+++ b/chrome/browser/ssl_policy.cc
@@ -448,7 +448,7 @@ void SSLPolicy::OnRequestStarted(SSLManager* manager, const GURL& url,
     NotificationService::current()->Notify(
         NOTIFY_SSL_STATE_CHANGED,
         Source<NavigationController>(manager->controller()),
-        Details<NavigationEntry>(entry));
+        NotificationService::NoDetails());
   }
 }
 
diff --git a/chrome/browser/ssl_uitest.cc b/chrome/browser/ssl_uitest.cc
index f712fc5..f2bcdef 100644
--- a/chrome/browser/ssl_uitest.cc
+++ b/chrome/browser/ssl_uitest.cc
@@ -224,6 +224,9 @@ TEST_F(SSLUITest, TestMixedContents) {
   EXPECT_EQ(NavigationEntry::SSLStatus::MIXED_CONTENT, mixed_content_state);
 }
 
+
+/* TODO(jcampan) bug 2004: fix this test.
+
 // Visits a page with unsafe content and make sure that:
 // - frames content is replaced with warning
 // - images and scripts are filtered out entirely
@@ -279,6 +282,7 @@ TEST_F(SSLUITest, TestUnsafeContents) {
       &js_result));
   EXPECT_FALSE(js_result);
 }
+*/
 
 // Visits a page with mixed content loaded by JS (after the initial page load).
 TEST_F(SSLUITest, TestMixedContentsLoadedFromJS) {
diff --git a/chrome/browser/tab_contents.cc b/chrome/browser/tab_contents.cc
index d0606d6..e80431c 100644
--- a/chrome/browser/tab_contents.cc
+++ b/chrome/browser/tab_contents.cc
@@ -60,21 +60,23 @@ void TabContents::HideContents() {
 }
 
 int32 TabContents::GetMaxPageID() {
-  if (AsWebContents())
-    return AsWebContents()->site_instance()->max_page_id();
+  if (GetSiteInstance())
+    return GetSiteInstance()->max_page_id();
   else
     return max_page_id_;
 }
 
 void TabContents::UpdateMaxPageID(int32 page_id) {
-  if (AsWebContents()) {
-    // Ensure both the SiteInstance and RenderProcessHost update their max page
-    // IDs in sync.
-    AsWebContents()->site_instance()->UpdateMaxPageID(page_id);
+  // Ensure both the SiteInstance and RenderProcessHost update their max page
+  // IDs in sync. Only WebContents will also have site instances, except during
+  // testing.
+  if (GetSiteInstance())
+    GetSiteInstance()->UpdateMaxPageID(page_id);
+
+  if (AsWebContents())
     AsWebContents()->process()->UpdateMaxPageID(page_id);
-  } else {
+  else
     max_page_id_ = std::max(max_page_id_, page_id);
-  }
 }
 
 const std::wstring TabContents::GetDefaultTitle() const {
@@ -256,35 +258,10 @@ void TabContents::SetIsLoading(bool is_loading,
                         NotificationService::NoDetails());
 }
 
-void TabContents::DidNavigateToEntry(
-    NavigationEntry* entry,
-    NavigationController::LoadCommittedDetails* details) {
-  // The entry may be deleted by DidNavigateToEntry...
-  int new_page_id = entry->page_id();
-
-  controller_->DidNavigateToEntry(entry, details);
-
-  // update after informing the navigation controller so it can check the
-  // previous value of the max page id.
-  UpdateMaxPageID(new_page_id);
-}
-
-bool TabContents::Navigate(const NavigationEntry& entry, bool reload) {
-  NavigationEntry* new_entry = new NavigationEntry(entry);
-  if (new_entry->page_id() == -1) {
-    // This is a new navigation.  Our behavior is to always navigate to the
-    // same page (page 0) in response to a navigation.
-    new_entry->set_page_id(0);
-    new_entry->set_title(GetDefaultTitle());
-  }
-
-  // When we're commanded to navigate like this, it's always a new main frame
-  // navigation (which is the default for the details).
-  NavigationController::LoadCommittedDetails details;
-  if (controller()->GetLastCommittedEntry())
-    details.previous_url = controller()->GetLastCommittedEntry()->url();
-
-  DidNavigateToEntry(new_entry, &details);
+bool TabContents::NavigateToPendingEntry(bool reload) {
+  // Our benavior is just to report that the entry was committed.
+  controller()->GetPendingEntry()->set_title(GetDefaultTitle());
+  controller()->CommitPendingEntry();
   return true;
 }
 
diff --git a/chrome/browser/tab_contents.h b/chrome/browser/tab_contents.h
index a8eaec3..e8990d6 100644
--- a/chrome/browser/tab_contents.h
+++ b/chrome/browser/tab_contents.h
@@ -134,6 +134,11 @@ class TabContents : public PageNavigator,
   // Updates the max PageID to be at least the given PageID.
   void UpdateMaxPageID(int32 page_id);
 
+  // Returns the site instance associated with the current page. By default,
+  // there is no site instance. WebContents overrides this to provide proper
+  // access to its site instance.
+  virtual SiteInstance* GetSiteInstance() const { return NULL; }
+
   // Initial title assigned to NavigationEntries from Navigate.
   virtual const std::wstring GetDefaultTitle() const;
 
@@ -274,17 +279,17 @@ class TabContents : public PageNavigator,
   bool is_active() const { return is_active_; }
 
   // Called by the NavigationController to cause the TabContents to navigate to
-  // the specified entry.  Either TabContents::DidNavigateToEntry or the
-  // navigation controller's DiscardPendingEntry method should be called in
-  // response (possibly sometime later).
+  // the current pending entry. The NavigationController should be called back
+  // with CommitPendingEntry/RendererDidNavigate on success or
+  // DiscardPendingEntry. The callbacks can be inside of this function, or at
+  // some future time.
   //
   // The entry has a PageID of -1 if newly created (corresponding to navigation
   // to a new URL).
   //
   // If this method returns false, then the navigation is discarded (equivalent
   // to calling DiscardPendingEntry on the NavigationController).
-  //
-  virtual bool Navigate(const NavigationEntry& entry, bool reload);
+  virtual bool NavigateToPendingEntry(bool reload);
 
   // Stop any pending navigation.
   virtual void Stop() {}
@@ -481,13 +486,6 @@ class TabContents : public PageNavigator,
   // (but can be null if not applicable)
   void SetIsLoading(bool is_loading, LoadNotificationDetails* details);
 
-  // Called by subclasses when a navigation occurs.  Ownership of the entry
-  // object is passed to this method. The details object should be filled in
-  // *except* for the entry (which the NavigationController will set). This
-  // behaves the same as NavigationController::DidNavigate, see that for more.
-  void DidNavigateToEntry(NavigationEntry* entry,
-                          NavigationController::LoadCommittedDetails* details);
-
   // Called by a derived class when the TabContents is resized, causing
   // suppressed constrained web popups to be repositioned to the new bounds
   // if necessary.
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 492d9c8..98552cf 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -31,25 +31,8 @@ class TabStripModelTestTabContents : public TabContents {
   TabStripModelTestTabContents(const TabContentsType type)
       : TabContents(type) {
   }
-
-  bool Navigate(const NavigationEntry& entry, bool reload) {
-    NavigationEntry* pending_entry = new NavigationEntry(entry);
-    if (pending_entry->page_id() == -1) {
-      pending_entry->set_page_id(g_page_id_++);
-    }
-    NavigationController::LoadCommittedDetails details;
-    DidNavigateToEntry(pending_entry, &details);
-
-    return true;
-  }
-
- private:
-  // We need to use valid, incrementing page ids otherwise the TabContents
-  // and NavController will not play nice when we try to go back and forward.
-  static int g_page_id_;
 };
 
-int TabStripModelTestTabContents::g_page_id_ = 0;
 
 // This constructs our fake TabContents.
 class TabStripModelTestTabContentsFactory : public TabContentsFactory {
diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc
index a5226b6..7084bd8 100644
--- a/chrome/browser/web_contents.cc
+++ b/chrome/browser/web_contents.cc
@@ -104,12 +104,6 @@ void InitWebContentsClass() {
   }
 }
 
-GURL GURLWithoutRef(const GURL& url) {
-  url_canon::Replacements<char> replacements;
-  replacements.ClearRef();
-  return url.ReplaceComponents(replacements);
-}
-
 }  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -570,22 +564,23 @@ void WebContents::SavePage(const std::wstring& main_file,
 //   DidNavigate method.  This replaces the current RVH with the
 //   pending RVH and goes back to the NORMAL RendererState.
 
-bool WebContents::Navigate(const NavigationEntry& entry, bool reload) {
-  RenderViewHost* dest_render_view_host = render_manager_.Navigate(entry);
+bool WebContents::NavigateToPendingEntry(bool reload) {
+  NavigationEntry* entry = controller()->GetPendingEntry();
+  RenderViewHost* dest_render_view_host = render_manager_.Navigate(*entry);
 
   // Used for page load time metrics.
   current_load_start_ = TimeTicks::Now();
 
-  // Navigate in the desired RenderViewHost
-  dest_render_view_host->NavigateToEntry(entry, reload);
+  // Navigate in the desired RenderViewHost.
+  dest_render_view_host->NavigateToEntry(*entry, reload);
 
-  if (entry.page_id() == -1) {
+  if (entry->page_id() == -1) {
     // HACK!!  This code suppresses javascript: URLs from being added to
     // session history, which is what we want to do for javascript: URLs that
     // do not generate content.  What we really need is a message from the
     // renderer telling us that a new page was not created.  The same message
     // could be used for mailto: URLs and the like.
-    if (entry.url().SchemeIs("javascript"))
+    if (entry->url().SchemeIs("javascript"))
       return false;
   }
 
@@ -593,7 +588,7 @@ bool WebContents::Navigate(const NavigationEntry& entry, bool reload) {
     HistoryService* history =
         profile()->GetHistoryService(Profile::IMPLICIT_ACCESS);
     if (history)
-      history->SetFavIconOutOfDateForPage(entry.url());
+      history->SetFavIconOutOfDateForPage(entry->url());
   }
 
   return true;
@@ -1020,7 +1015,7 @@ PluginInstaller* WebContents::GetPluginInstaller() {
 bool WebContents::IsActiveEntry(int32 page_id) {
   NavigationEntry* active_entry = controller()->GetActiveEntry();
   return (active_entry != NULL &&
-          active_entry->site_instance() == site_instance() &&
+          active_entry->site_instance() == GetSiteInstance() &&
           active_entry->page_id() == page_id);
 }
 
@@ -1039,7 +1034,7 @@ Profile* WebContents::GetProfile() const {
 
 void WebContents::CreateView(int route_id, HANDLE modal_dialog_event) {
   WebContents* new_view = new WebContents(profile(),
-                                          site_instance(),
+                                          GetSiteInstance(),
                                           render_view_factory_,
                                           route_id,
                                           modal_dialog_event);
@@ -1179,83 +1174,16 @@ void WebContents::DidNavigate(RenderViewHost* rvh,
     render_manager_.DidNavigateMainFrame(rvh);
 
   // In the case of interstitial, we don't mess with the navigation entries.
+  // TODO(brettw) this seems like a bug. What happens if the page goes and
+  // does something on its own (or something that just got delayed), then
+  // we won't have a navigation entry for that stuff when the interstitial
+  // is hidden.
   if (render_manager_.showing_interstitial_page())
     return;
 
-  // Check for navigations we don't expect.
-  if (!controller() || !is_active_ || params.page_id == -1) {
-    if (params.page_id == -1) {
-      DCHECK(controller()->GetActiveEntry() == NULL)
-          << "The renderer is permitted to send a FrameNavigate event for an "
-             "invalid |page_id| if, and only if, this is the initial blank "
-             "page for a main frame.";
-    }
-    BroadcastProvisionalLoadCommit(rvh, params);
-    return;
-  }
-
-  // Generate the details for the notification.
-  // TODO(brettw) bug 1343146: Move into the NavigationController.
-  NavigationController::LoadCommittedDetails details;
-  // WebKit doesn't set the "auto" transition on meta refreshes properly (bug
-  // 1051891) so we manually set it for redirects which we normally treat as
-  // "non-user-gestures" where we want to update stuff after navigations.
-  //
-  // Note that the redirect check also checks for a pending entry to
-  // differentiate real redirects from browser initiated navigations to a
-  // redirected entry. This happens when you hit back to go to a page that was
-  // the destination of a redirect, we don't want to treat it as a redirect
-  // even though that's what its transition will be. See bug  1117048.
-  //
-  // TODO(brettw) write a test for this complicated logic.
-  details.is_auto = (PageTransition::IsRedirect(params.transition) &&
-                     !controller()->GetPendingEntry()) ||
-      params.gesture == NavigationGestureAuto;
-  details.is_in_page = IsInPageNavigation(params.url);
-  details.is_main_frame = PageTransition::IsMainFrame(params.transition);
-
-  // DO NOT ADD MORE STUFF TO THIS FUNCTION!  Don't make me come over there!
-  // =======================================================================
-  // Add your code to DidNavigateAnyFramePostCommit if you have a helper object
-  // that needs to know about all navigations. If it needs to do it only for
-  // main frame or sub-frame navigations, add your code to
-  // DidNavigate*PostCommit.
-
-  // Create the new navigation entry for this navigation and do work specific to
-  // this frame type. The main frame / sub frame functions will do additional
-  // updates to the NavigationEntry appropriate for the navigation type (in
-  // addition to a lot of other stuff).
-  scoped_ptr<NavigationEntry> entry(CreateNavigationEntryForCommit(params));
-
-  // Commit the entry to the navigation controller.
-  DidNavigateToEntry(entry.release(), &details);
-  // WARNING: NavigationController will have taken ownership of entry at this
-  // point, and may have deleted it. As such, do NOT use entry after this.
-
-  // Run post-commit tasks.
-  if (details.is_main_frame)
-    DidNavigateMainFramePostCommit(details, params);
-  DidNavigateAnyFramePostCommit(rvh, details, params);
-}
-
-// TODO(brettw) bug 1343146: This logic should be moved to NavigationController.
-NavigationEntry* WebContents::CreateNavigationEntryForCommit(
-    const ViewHostMsg_FrameNavigate_Params& params) {
-  // This new navigation entry will represent the navigation. Note that we
-  // don't set the URL. This will happen in the DidNavigateMainFrame/SubFrame
-  // because the entry's URL should represent the toplevel frame only.
-  NavigationEntry* entry = new NavigationEntry(type());
-  if (PageTransition::IsMainFrame(params.transition))
-    entry->set_url(params.url);
-  entry->set_page_id(params.page_id);
-  entry->set_transition_type(params.transition);
-  entry->set_site_instance(site_instance());
-
-  // Now that we've assigned a SiteInstance to this entry, we need to
-  // assign it to the NavigationController's pending entry as well.  This
-  // allows us to find it via GetEntryWithPageID, etc.
-  if (controller()->GetPendingEntry())
-    controller()->GetPendingEntry()->set_site_instance(entry->site_instance());
+  // We can't do anything about navigations when we're inactive.
+  if (!controller() || !is_active_)
+    return;  
 
   // Update the site of the SiteInstance if it doesn't have one yet, unless we
   // are showing an interstitial page.  If we are, we should wait until the
@@ -1264,67 +1192,26 @@ NavigationEntry* WebContents::CreateNavigationEntryForCommit(
   // TODO(brettw) the old code only checked for INTERSTIAL, this new code also
   // checks for LEAVING_INTERSTITIAL mode in the manager. Is this difference
   // important?
-  if (!site_instance()->has_site() &&
+  if (!GetSiteInstance()->has_site() &&
       !render_manager_.showing_interstitial_page())
-    site_instance()->SetSite(params.url);
-
-  // When the navigation is just a change in ref or a sub-frame navigation, the
-  // new page should inherit the existing entry's title and favicon, since it
-  // will be the same.  A change in ref also inherits the security style and SSL
-  // associated info.
-  bool in_page_nav;
-  if ((in_page_nav = IsInPageNavigation(params.url)) ||
-      !PageTransition::IsMainFrame(params.transition)) {
-    // In the case of a sub-frame navigation within a window that was created
-    // without an URL (via window.open), we may not have a committed entry yet!
-    NavigationEntry* old_entry = controller()->GetLastCommittedEntry();
-    if (old_entry) {
-      entry->set_title(old_entry->title());
-      entry->favicon() = old_entry->favicon();
-      if (in_page_nav)
-        entry->ssl() = old_entry->ssl();
-    }
-  }
+    GetSiteInstance()->SetSite(params.url);
 
-  if (PageTransition::IsMainFrame(params.transition)) {
-    NavigationEntry* pending = controller()->GetPendingEntry();
-    if (pending) {
-      // Copy fields from the pending NavigationEntry into the actual
-      // NavigationEntry that we're committing to.
-      entry->set_user_typed_url(pending->user_typed_url());
-      if (pending->has_display_url())
-        entry->set_display_url(pending->display_url());
-      if (pending->url().SchemeIsFile())
-        entry->set_title(pending->title());
-      entry->set_content_state(pending->content_state());
-    }
-    entry->set_has_post_data(params.is_post);
-  } else {
-    NavigationEntry* last_committed = controller()->GetLastCommittedEntry();
-    if (last_committed) {
-      // In the case of a sub-frame navigation within a window that was created
-      // without an URL (window.open), we may not have a committed entry yet!
-
-      // Reset entry state to match that of the pending entry.
-      entry->set_unique_id(last_committed->unique_id());
-      entry->set_url(last_committed->url());
-      entry->set_transition_type(last_committed->transition_type());
-      entry->set_user_typed_url(last_committed->user_typed_url());
-      entry->set_content_state(last_committed->content_state());
-
-      // TODO(jcampan): when navigating to an insecure/unsafe inner frame, the
-      // main entry is the one that gets notified of the mixed/unsafe contents
-      // (see SSLPolicy::OnRequestStarted()). Here we are just transferring
-      // that state. We should find a better way to do this. Note that it is OK
-      // that the mixed/unsafe contents is set on the wrong navigation entry, as
-      // that state is reset when navigating back to it.
-      DCHECK(entry->ssl().content_status() == 0) << "We should never "
-          "be setting the status bits from 1 to 0 on navigate.";
-      entry->ssl() = last_committed->ssl();
-    }
-  }
+  NavigationController::LoadCommittedDetails details;
+  if (!controller()->RendererDidNavigate(
+      params,
+      render_manager_.IsRenderViewInterstitial(rvh),
+      &details))
+    return;  // No navigation happened.
 
-  return entry;
+  // DO NOT ADD MORE STUFF TO THIS FUNCTION! Your component should either listen
+  // for the appropriate notification (best) or you can add it to
+  // DidNavigateMainFramePostCommit / DidNavigateAnyFramePostCommit (only if
+  // necessary, please).
+
+  // Run post-commit tasks.
+  if (details.is_main_frame)
+    DidNavigateMainFramePostCommit(details, params);
+  DidNavigateAnyFramePostCommit(rvh, details, params);
 }
 
 void WebContents::DidNavigateMainFramePostCommit(
@@ -1413,17 +1300,11 @@ void WebContents::DidNavigateAnyFramePostCommit(
     UpdateHistoryForNavigation(GetURL(), params);
   }
 
-  // Have the controller save the current session.
-  controller()->NotifyEntryChangedByPageID(type(), site_instance(),
-                                           details.entry->page_id());
-
   // Notify the password manager of the navigation or form submit.
   // TODO(brettw) bug 1343111: Password manager stuff in here needs to be
   // cleaned up and covered by tests.
   if (params.password_form.origin.is_valid())
     GetPasswordManager()->ProvisionallySavePassword(params.password_form);
-
-  BroadcastProvisionalLoadCommit(render_view_host, params);
 }
 
 bool WebContents::IsWebApplicationActive() const {
@@ -1445,20 +1326,6 @@ void WebContents::WebAppImagesChanged(WebApp* web_app) {
     delegate()->NavigationStateChanged(this, TabContents::INVALIDATE_FAVICON);
 }
 
-void WebContents::BroadcastProvisionalLoadCommit(
-    RenderViewHost* render_view_host,
-    const ViewHostMsg_FrameNavigate_Params& params) {
-  ProvisionalLoadDetails details(
-      PageTransition::IsMainFrame(params.transition),
-      render_manager_.IsRenderViewInterstitial(render_view_host),
-      IsInPageNavigation(params.url),
-      params.url, params.security_info);
-  NotificationService::current()->
-      Notify(NOTIFY_FRAME_PROVISIONAL_LOAD_COMMITTED,
-             Source<NavigationController>(controller()),
-             Details<ProvisionalLoadDetails>(&details));
-}
-
 void WebContents::UpdateStarredStateForCurrentURL() {
   BookmarkModel* model = profile()->GetBookmarkModel();
   const bool old_state = is_starred_;
@@ -1511,11 +1378,11 @@ void WebContents::UpdateState(RenderViewHost* rvh,
   // the next page. The navigation controller will look up the appropriate
   // NavigationEntry and update it when it is notified via the delegate.
 
-  NavigationEntry* entry = controller()->GetEntryWithPageID(
-      type(), site_instance(), page_id);
-  if (!entry)
+  int entry_index = controller()->GetEntryIndexWithPageID(
+      type(), GetSiteInstance(), page_id);
+  if (entry_index < 0)
     return;
-
+  NavigationEntry* entry = controller()->GetEntryAtIndex(entry_index);
   unsigned changed_flags = 0;
 
   // Update the URL.
@@ -1558,7 +1425,7 @@ void WebContents::UpdateState(RenderViewHost* rvh,
   // Notify everybody of the changes (only when the current page changed).
   if (changed_flags && entry == controller()->GetActiveEntry())
     NotifyNavigationStateChanged(changed_flags);
-  controller()->NotifyEntryChangedByPageID(type(), site_instance(), page_id);
+  controller()->NotifyEntryChanged(entry, entry_index);
 }
 
 void WebContents::UpdateTitle(RenderViewHost* rvh,
@@ -1579,7 +1446,8 @@ void WebContents::UpdateTitle(RenderViewHost* rvh,
     // so just use that.
     entry = controller()->GetLastCommittedEntry();
   } else {
-    entry = controller()->GetEntryWithPageID(type(), site_instance(), page_id);
+    entry = controller()->GetEntryWithPageID(type(), GetSiteInstance(),
+                                             page_id);
   }
 
   if (!entry)
@@ -1685,7 +1553,7 @@ void WebContents::DidStartProvisionalLoadForFrame(
   ProvisionalLoadDetails details(
       is_main_frame,
       render_manager_.IsRenderViewInterstitial(render_view_host),
-      IsInPageNavigation(url),
+      controller()->IsURLInPageNavigation(url),
       url, std::string());
   NotificationService::current()->
       Notify(NOTIFY_FRAME_PROVISIONAL_LOAD_START,
@@ -1697,12 +1565,14 @@ void WebContents::DidRedirectProvisionalLoad(int32 page_id,
                                              const GURL& source_url,
                                              const GURL& target_url) {
   NavigationEntry* entry;
-  if (page_id == -1)
+  if (page_id == -1) {
     entry = controller()->GetPendingEntry();
-  else
-    entry = controller()->GetEntryWithPageID(type(), site_instance(), page_id);
+  } else {
+    entry = controller()->GetEntryWithPageID(type(), GetSiteInstance(),
+                                             page_id);
+  }
   if (!entry || entry->tab_type() != type() || entry->url() != source_url)
-      return;
+    return;
   entry->set_url(target_url);
 }
 
@@ -1750,7 +1620,7 @@ void WebContents::DidFailProvisionalLoadWithError(
   ProvisionalLoadDetails details(
       is_main_frame,
       render_manager_.IsRenderViewInterstitial(render_view_host),
-      IsInPageNavigation(url),
+      controller()->IsURLInPageNavigation(url),
       url, std::string());
   details.set_error_code(error_code);
 
@@ -2465,18 +2335,6 @@ void WebContents::FileSelectionCanceled(void* params) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-bool WebContents::IsInPageNavigation(const GURL& url) const {
-  // We compare to the last committed entry and not the active entry as the
-  // active entry is the current pending entry (if any).
-  // When this method is called when a navigation initiated from the browser
-  // (ex: when typing the URL in the location bar) is committed, the pending
-  // entry URL is the same as |url|.
-  NavigationEntry* entry = controller()->GetLastCommittedEntry();
-  return (entry && url.has_ref() &&
-         (url != entry->url()) &&  // Test for reload of a URL with a ref.
-          GURLWithoutRef(entry->url()) == GURLWithoutRef(url));
-}
-
 SkBitmap WebContents::GetFavIcon() {
   if (web_app_.get() && IsWebApplicationActive()) {
     SkBitmap app_icon = web_app_->GetFavIcon();
diff --git a/chrome/browser/web_contents.h b/chrome/browser/web_contents.h
index cc6c11c..866862a 100644
--- a/chrome/browser/web_contents.h
+++ b/chrome/browser/web_contents.h
@@ -65,10 +65,12 @@ class WebContents : public TabContents,
   // cancel the close of the page.
   virtual void FirePageUnload();
 
-
   // TabContents
   virtual WebContents* AsWebContents() { return this; }
-  virtual bool Navigate(const NavigationEntry& entry, bool reload);
+  virtual SiteInstance* GetSiteInstance() const {
+    return render_manager_.current_host()->site_instance();
+  }
+  virtual bool NavigateToPendingEntry(bool reload);
   virtual void Stop();
   virtual void DidBecomeSelected();
   virtual void WasHidden();
@@ -242,9 +244,6 @@ class WebContents : public TabContents,
   RenderViewHost* render_view_host() const {
     return render_manager_.current_host();
   }
-  SiteInstance* site_instance() const {
-    return render_manager_.current_host()->site_instance();
-  }
   RenderWidgetHostView* view() const {
     return render_manager_.current_view();
   }
@@ -324,7 +323,7 @@ class WebContents : public TabContents,
   bool notify_disconnection() const { return notify_disconnection_; }
 
  protected:
-  FRIEND_TEST(WebContentsTest, OnMessageReceived);
+  FRIEND_TEST(WebContentsTest, UpdateTitle);
 
   // Should be deleted via CloseContents.
   virtual ~WebContents();
@@ -545,11 +544,6 @@ class WebContents : public TabContents,
   //
   // These functions are helpers for Navigate() and DidNavigate().
 
-  // Creates a new navigation entry (malloced, the caller will have to free)
-  // for the given committed load. Used by DidNavigate. Will not return NULL.
-  NavigationEntry* CreateNavigationEntryForCommit(
-      const ViewHostMsg_FrameNavigate_Params& params);
-
   // Handles post-navigation tasks in DidNavigate AFTER the entry has been
   // committed to the navigation controller. Note that the navigation entry is
   // not provided since it may be invalid/changed after being committed. The
@@ -566,17 +560,6 @@ class WebContents : public TabContents,
   // domain is changing.
   void MaybeCloseChildWindows(const ViewHostMsg_FrameNavigate_Params& params);
 
-  // Broadcasts a notification for the provisional load committing, used by
-  // DidNavigate.
-  void BroadcastProvisionalLoadCommit(
-      RenderViewHost* render_view_host,
-      const ViewHostMsg_FrameNavigate_Params& params);
-
-  // Convenience method that returns true if navigating to the specified URL
-  // from the current one is an in-page navigation (jumping to a ref in the
-  // page).
-  bool IsInPageNavigation(const GURL& url) const;
-
   // Updates the starred state from the bookmark bar model. If the state has
   // changed, the delegate is notified.
   void UpdateStarredStateForCurrentURL();
diff --git a/chrome/browser/web_contents_unittest.cc b/chrome/browser/web_contents_unittest.cc
index 515a963..5d386c5 100644
--- a/chrome/browser/web_contents_unittest.cc
+++ b/chrome/browser/web_contents_unittest.cc
@@ -229,11 +229,6 @@ class TestWebContents : public WebContents {
     render_view_host->is_loading = false;
   }
 
-  // Promote IsInPageNavigation to public.
-  bool TestIsInPageNavigation(const GURL& url) {
-    return IsInPageNavigation(url);
-  }
-
   // Promote GetWebkitPrefs to public.
   WebPreferences TestGetWebkitPrefs() {
     return GetWebkitPrefs();
@@ -301,12 +296,13 @@ class WebContentsTest : public testing::Test {
   MessageLoopForUI message_loop_;
 };
 
-// Test to make sure that title updates get stripped of whitespace
-TEST_F(WebContentsTest, OnMessageReceived) {
+// Test to make sure that title updates get stripped of whitespace.
+TEST_F(WebContentsTest, UpdateTitle) {
+  ViewHostMsg_FrameNavigate_Params params;
+  InitNavigateParams(&params, 0, GURL("about:blank"));
+
   NavigationController::LoadCommittedDetails details;
-  contents->controller()->DidNavigateToEntry(new NavigationEntry(
-      contents->type(), contents->site_instance(), 0, GURL("about:blank"),
-      std::wstring(), PageTransition::TYPED), &details);
+  contents->controller()->RendererDidNavigate(params, false, &details);
 
   contents->UpdateTitle(NULL, 0, L"    Lots O' Whitespace\n");
   EXPECT_EQ(std::wstring(L"Lots O' Whitespace"), contents->GetTitle());
@@ -315,7 +311,7 @@ TEST_F(WebContentsTest, OnMessageReceived) {
 // Test simple same-SiteInstance navigation.
 TEST_F(WebContentsTest, SimpleNavigation) {
   TestRenderViewHost* orig_rvh = contents->rvh();
-  SiteInstance* instance1 = contents->site_instance();
+  SiteInstance* instance1 = contents->GetSiteInstance();
   EXPECT_TRUE(contents->pending_rvh() == NULL);
   EXPECT_TRUE(contents->original_rvh() == NULL);
   EXPECT_TRUE(contents->interstitial_rvh() == NULL);
@@ -585,7 +581,7 @@ TEST_F(WebContentsTest, CrossSiteBoundaries) {
   TestRenderViewHost* orig_rvh = contents->rvh();
   int orig_rvh_delete_count = 0;
   orig_rvh->set_delete_counter(&orig_rvh_delete_count);
-  SiteInstance* instance1 = contents->site_instance();
+  SiteInstance* instance1 = contents->GetSiteInstance();
 
   // Navigate to URL.  First URL should use first RenderViewHost.
   const GURL url("http://www.google.com");
@@ -611,7 +607,7 @@ TEST_F(WebContentsTest, CrossSiteBoundaries) {
   ViewHostMsg_FrameNavigate_Params params2;
   InitNavigateParams(&params2, 1, url2);
   contents->TestDidNavigate(pending_rvh, params2);
-  SiteInstance* instance2 = contents->site_instance();
+  SiteInstance* instance2 = contents->GetSiteInstance();
 
   EXPECT_TRUE(contents->state_is_normal());
   EXPECT_EQ(pending_rvh, contents->render_view_host());
@@ -632,7 +628,7 @@ TEST_F(WebContentsTest, CrossSiteBoundaries) {
   EXPECT_TRUE(contents->state_is_normal());
   EXPECT_EQ(goback_rvh, contents->render_view_host());
   EXPECT_EQ(pending_rvh_delete_count, 1);
-  EXPECT_EQ(instance1, contents->site_instance());
+  EXPECT_EQ(instance1, contents->GetSiteInstance());
 }
 
 // Test that navigating across a site boundary after a crash creates a new
@@ -642,7 +638,7 @@ TEST_F(WebContentsTest, CrossSiteBoundariesAfterCrash) {
   TestRenderViewHost* orig_rvh = contents->rvh();
   int orig_rvh_delete_count = 0;
   orig_rvh->set_delete_counter(&orig_rvh_delete_count);
-  SiteInstance* instance1 = contents->site_instance();
+  SiteInstance* instance1 = contents->GetSiteInstance();
 
   // Navigate to URL.  First URL should use first RenderViewHost.
   const GURL url("http://www.google.com");
@@ -674,7 +670,7 @@ TEST_F(WebContentsTest, CrossSiteBoundariesAfterCrash) {
   ViewHostMsg_FrameNavigate_Params params2;
   InitNavigateParams(&params2, 1, url2);
   contents->TestDidNavigate(new_rvh, params2);
-  SiteInstance* instance2 = contents->site_instance();
+  SiteInstance* instance2 = contents->GetSiteInstance();
 
   EXPECT_TRUE(contents->state_is_normal());
   EXPECT_EQ(new_rvh, contents->render_view_host());
@@ -689,7 +685,7 @@ TEST_F(WebContentsTest, CrossSiteBoundariesAfterCrash) {
 TEST_F(WebContentsTest, CrossSiteInterstitialDontProceed) {
   contents->transition_cross_site = true;
   TestRenderViewHost* orig_rvh = contents->rvh();
-  SiteInstance* instance1 = contents->site_instance();
+  SiteInstance* instance1 = contents->GetSiteInstance();
 
   // Navigate to URL.  First URL should use first RenderViewHost.
   const GURL url("http://www.google.com");
@@ -740,7 +736,7 @@ TEST_F(WebContentsTest, CrossSiteInterstitialProceed) {
   int orig_rvh_delete_count = 0;
   TestRenderViewHost* orig_rvh = contents->rvh();
   orig_rvh->set_delete_counter(&orig_rvh_delete_count);
-  SiteInstance* instance1 = contents->site_instance();
+  SiteInstance* instance1 = contents->GetSiteInstance();
 
   // Navigate to URL.  First URL should use first RenderViewHost.
   const GURL url("http://www.google.com");
@@ -782,7 +778,7 @@ TEST_F(WebContentsTest, CrossSiteInterstitialProceed) {
   ViewHostMsg_FrameNavigate_Params params3;
   InitNavigateParams(&params3, 2, url2);
   contents->TestDidNavigate(pending_rvh, params3);
-  SiteInstance* instance2 = contents->site_instance();
+  SiteInstance* instance2 = contents->GetSiteInstance();
   EXPECT_TRUE(contents->state_is_normal());
   EXPECT_EQ(pending_rvh, contents->render_view_host());
   EXPECT_TRUE(contents->original_rvh() == NULL);
@@ -804,7 +800,7 @@ TEST_F(WebContentsTest, CrossSiteInterstitialProceed) {
   contents->TestDidNavigate(goback_rvh, params1);
   EXPECT_TRUE(contents->state_is_normal());
   EXPECT_EQ(goback_rvh, contents->render_view_host());
-  EXPECT_EQ(instance1, contents->site_instance());
+  EXPECT_EQ(instance1, contents->GetSiteInstance());
   EXPECT_EQ(pending_rvh_delete_count, 1);  // The second page's rvh should die.
 }
 
@@ -987,7 +983,7 @@ TEST_F(WebContentsTest, CrossSiteInterstitialCrashesThenNavigate) {
 TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) {
   contents->transition_cross_site = true;
   TestRenderViewHost* orig_rvh = contents->rvh();
-  SiteInstance* instance1 = contents->site_instance();
+  SiteInstance* instance1 = contents->GetSiteInstance();
 
   // Navigate to URL.  First URL should use first RenderViewHost.
   const GURL url("http://www.google.com");
@@ -1010,7 +1006,7 @@ TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) {
   ViewHostMsg_FrameNavigate_Params params2a;
   InitNavigateParams(&params2a, 1, url2a);
   contents->TestDidNavigate(pending_rvh_a, params2a);
-  SiteInstance* instance2a = contents->site_instance();
+  SiteInstance* instance2a = contents->GetSiteInstance();
   EXPECT_NE(instance1, instance2a);
 
   // Navigate second tab to the same site as the first tab
@@ -1027,7 +1023,7 @@ TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) {
   ViewHostMsg_FrameNavigate_Params params2b;
   InitNavigateParams(&params2b, 2, url2b);
   contents2->TestDidNavigate(pending_rvh_b, params2b);
-  SiteInstance* instance2b = contents2->site_instance();
+  SiteInstance* instance2b = contents2->GetSiteInstance();
   EXPECT_NE(instance1, instance2b);
 
   // Both tabs should now be in the same SiteInstance.
@@ -1041,7 +1037,7 @@ TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) {
 TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) {
   contents->transition_cross_site = true;
   TestRenderViewHost* orig_rvh = contents->rvh();
-  SiteInstance* instance1 = contents->site_instance();
+  SiteInstance* instance1 = contents->GetSiteInstance();
 
   // Navigate to URL.
   const GURL url("http://www.google.com");
@@ -1063,7 +1059,7 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) {
   ViewHostMsg_FrameNavigate_Params params2;
   InitNavigateParams(&params2, 2, url2);
   contents2->TestDidNavigate(rvh2, params2);
-  SiteInstance* instance2 = contents2->site_instance();
+  SiteInstance* instance2 = contents2->GetSiteInstance();
   EXPECT_NE(instance1, instance2);
   EXPECT_TRUE(contents2->state_is_normal());
 
@@ -1072,7 +1068,7 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) {
   ViewHostMsg_FrameNavigate_Params params3;
   InitNavigateParams(&params3, 2, url2);
   contents->TestDidNavigate(orig_rvh, params3);
-  SiteInstance* instance3 = contents->site_instance();
+  SiteInstance* instance3 = contents->GetSiteInstance();
   EXPECT_EQ(instance1, instance3);
   EXPECT_TRUE(contents->state_is_normal());
 
@@ -1084,7 +1080,7 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) {
   ViewHostMsg_FrameNavigate_Params params4;
   InitNavigateParams(&params4, 3, url3);
   contents->TestDidNavigate(orig_rvh, params4);
-  SiteInstance* instance4 = contents->site_instance();
+  SiteInstance* instance4 = contents->GetSiteInstance();
   EXPECT_EQ(instance1, instance4);
 
   contents2->CloseContents();
@@ -1095,7 +1091,7 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) {
 TEST_F(WebContentsTest, CrossSiteUnloadHandlers) {
   contents->transition_cross_site = true;
   TestRenderViewHost* orig_rvh = contents->rvh();
-  SiteInstance* instance1 = contents->site_instance();
+  SiteInstance* instance1 = contents->GetSiteInstance();
 
   // Navigate to URL.  First URL should use first RenderViewHost.
   const GURL url("http://www.google.com");
@@ -1128,7 +1124,7 @@ TEST_F(WebContentsTest, CrossSiteUnloadHandlers) {
   ViewHostMsg_FrameNavigate_Params params2;
   InitNavigateParams(&params2, 1, url2);
   contents->TestDidNavigate(pending_rvh, params2);
-  SiteInstance* instance2 = contents->site_instance();
+  SiteInstance* instance2 = contents->GetSiteInstance();
   EXPECT_TRUE(contents->state_is_normal());
   EXPECT_EQ(pending_rvh, contents->render_view_host());
   EXPECT_NE(instance1, instance2);
@@ -1193,38 +1189,6 @@ TEST_F(WebContentsTest, NavigationEntryContentStateNewWindow) {
   EXPECT_FALSE(entry->content_state().empty());
 }
 
-// Tests that IsInPageNavigation returns appropriate results.  Prevents
-// regression for bug 1126349.
-TEST_F(WebContentsTest, IsInPageNavigation) {
-  TestRenderViewHost* rvh = contents->rvh();
-
-  // Navigate to URL with no refs.
-  const GURL url("http://www.google.com/home.html");
-  contents->controller()->LoadURL(url, PageTransition::TYPED);
-  ViewHostMsg_FrameNavigate_Params params;
-  InitNavigateParams(&params, 1, url);
-  contents->TestDidNavigate(rvh, params);
-
-  // Reloading the page is not an in-page navigation.
-  EXPECT_FALSE(contents->TestIsInPageNavigation(url));
-  const GURL other_url("http://www.google.com/add.html");
-  EXPECT_FALSE(contents->TestIsInPageNavigation(other_url));
-  const GURL url_with_ref("http://www.google.com/home.html#my_ref");
-  EXPECT_TRUE(contents->TestIsInPageNavigation(url_with_ref));
-
-  // Navigate to URL with refs.
-  contents->controller()->LoadURL(url_with_ref, PageTransition::TYPED);
-  InitNavigateParams(&params, 2, url_with_ref);
-  contents->TestDidNavigate(rvh, params);
-
-  // Reloading the page is not an in-page navigation.
-  EXPECT_FALSE(contents->TestIsInPageNavigation(url_with_ref));
-  EXPECT_FALSE(contents->TestIsInPageNavigation(url));
-  EXPECT_FALSE(contents->TestIsInPageNavigation(other_url));
-  const GURL other_url_with_ref("http://www.google.com/home.html#my_other_ref");
-  EXPECT_TRUE(contents->TestIsInPageNavigation(other_url_with_ref));
-}
-
 // Tests to see that webkit preferences are properly loaded and copied over
 // to a WebPreferences object.
 TEST_F(WebContentsTest, WebKitPrefs) {
diff --git a/chrome/common/notification_types.h b/chrome/common/notification_types.h
index 9bf7170..d9e3b44 100644
--- a/chrome/common/notification_types.h
+++ b/chrome/common/notification_types.h
@@ -67,6 +67,10 @@ enum NotificationType {
   // Indicates that a NavigationEntry has changed. The source will be the
   // NavigationController that owns the NavigationEntry. The details will be
   // a NavigationController::EntryChangedDetails struct.
+  //
+  // This will NOT be sent on navigation, interested parties should also listen
+  // for NOTIFY_NAV_ENTRY_COMMITTED to handle that case. This will be sent when
+  // the entry is updated outside of navigation (like when a new title comes).
   NOTIFY_NAV_ENTRY_CHANGED,
 
   // Other load-related (not from NavigationController) ------------------------
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index e39b392e..6f82feb 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -61,11 +61,11 @@ struct ViewHostMsg_FrameNavigate_Params {
   // iframes are loaded automatically.
   int32 page_id;
 
-  // URL of the page being loaded. NON-CANONICAL.
+  // URL of the page being loaded.
   GURL url;
 
   // URL of the referrer of this load. WebKit generates this based on the
-  // source of the event that caused the load. NON-CANONICAL.
+  // source of the event that caused the load.
   GURL referrer;
 
   // The type of transition.