Remove DidNavigate from the tab contents delegate and all the related plumbing.

I added additional information to the regular load commit notification so all
interested parties can listen for that instead.

I removed the old navigation type enum, and replaced it with the enum from
the NavigationController, so it's now public.
Review URL: http://codereview.chromium.org/3112

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

1 files changed, 33 insertions, 44 deletions

diff --git a/chrome/browser/navigation_controller.cc b/chrome/browser/navigation_controller.cc
index 91c1fb4..38fcf65 100644
--- a/chrome/browser/navigation_controller.cc
+++ b/chrome/browser/navigation_controller.cc
@@ -492,9 +492,14 @@ bool NavigationController::RendererDidNavigate(
     const ViewHostMsg_FrameNavigate_Params& params,
     bool is_interstitial,
     LoadCommittedDetails* details) {
-  // Save the previous URL before we clobber it.
-  if (GetLastCommittedEntry())
+  // Save the previous state before we clobber it.
+  if (GetLastCommittedEntry()) {
     details->previous_url = GetLastCommittedEntry()->url();
+    details->previous_entry_index = GetLastCommittedEntryIndex();
+  } else {
+    details->previous_url = GURL();
+    details->previous_entry_index = -1;
+  }
 
   // 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
@@ -508,27 +513,28 @@ bool NavigationController::RendererDidNavigate(
     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:
+  details->type = ClassifyNavigation(params);
+  switch (details->type) {
+    case NavigationType::NEW_PAGE:
       RendererDidNavigateToNewPage(params);
       break;
-    case NAV_EXISTING_PAGE:
+    case NavigationType::EXISTING_PAGE:
       RendererDidNavigateToExistingPage(params);
       break;
-    case NAV_SAME_PAGE:
+    case NavigationType::SAME_PAGE:
       RendererDidNavigateToSamePage(params);
       break;
-    case NAV_IN_PAGE:
+    case NavigationType::IN_PAGE:
       RendererDidNavigateInPage(params);
       break;
-    case NAV_NEW_SUBFRAME:
+    case NavigationType::NEW_SUBFRAME:
       RendererDidNavigateNewSubframe(params);
       break;
-    case NAV_AUTO_SUBFRAME:
+    case NavigationType::AUTO_SUBFRAME:
       if (!RendererDidNavigateAutoSubframe(params))
         return false;
       break;
-    case NAV_IGNORE:
+    case NavigationType::NAV_IGNORE:
       // There is nothing we can do with this navigation, so we just return to
       // the caller that nothing has happened.
       return false;
@@ -570,7 +576,7 @@ bool NavigationController::RendererDidNavigate(
   return true;
 }
 
-NavigationController::NavClass NavigationController::ClassifyNavigation(
+NavigationType::Type 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
@@ -578,7 +584,7 @@ NavigationController::NavClass NavigationController::ClassifyNavigation(
   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;
+    return NavigationType::NAV_IGNORE;
   }
 
   if (params.page_id > active_contents_->GetMaxPageID()) {
@@ -586,7 +592,7 @@ NavigationController::NavClass NavigationController::ClassifyNavigation(
     // 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 NavigationType::NEW_PAGE;
 
     // When this is a new subframe navigation, we should have a committed page
     // for which it's a suframe in. This may not be the case when an iframe is
@@ -594,10 +600,10 @@ NavigationController::NavClass NavigationController::ClassifyNavigation(
     // written into the popup by script on the main page). For these cases,
     // there isn't any navigation stuff we can do, so just ignore it.
     if (!GetLastCommittedEntry())
-      return NAV_IGNORE;
+      return NavigationType::NAV_IGNORE;
 
     // Valid subframe navigation.
-    return NAV_NEW_SUBFRAME;
+    return NavigationType::NEW_SUBFRAME;
   }
 
   // Now we know that the notification is for an existing page. Find that entry.
@@ -610,7 +616,7 @@ NavigationController::NavClass NavigationController::ClassifyNavigation(
     // 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;
+    return NavigationType::NAV_IGNORE;
   }
   NavigationEntry* existing_entry = entries_[existing_entry_index].get();
 
@@ -626,23 +632,23 @@ NavigationController::NavClass NavigationController::ClassifyNavigation(
     // (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 (the "existing entry").
-    return NAV_SAME_PAGE;
+    return NavigationType::SAME_PAGE;
   }
 
   if (AreURLsInPageNavigation(existing_entry->url(), params.url))
-    return NAV_IN_PAGE;
+    return NavigationType::IN_PAGE;
 
   if (!PageTransition::IsMainFrame(params.transition)) {
     // All manual subframes would get new IDs and were handled above, so we
     // know this is auto. Since the current page was found in the navigation
     // entry list, we're guaranteed to have a last committed entry.
     DCHECK(GetLastCommittedEntry());
-    return NAV_AUTO_SUBFRAME;
+    return NavigationType::AUTO_SUBFRAME;
   }
 
   // Since we weeded out "new" navigations above, we know this is an existing
   // (back/forward) navigation.
-  return NAV_EXISTING_PAGE;
+  return NavigationType::EXISTING_PAGE;
 }
 
 void NavigationController::RendererDidNavigateToNewPage(
@@ -706,9 +712,7 @@ void NavigationController::RendererDidNavigateToExistingPage(
   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(
@@ -782,9 +786,7 @@ bool NavigationController::RendererDidNavigateAutoSubframe(
 
   // 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;
@@ -796,19 +798,22 @@ void NavigationController::CommitPendingEntry() {
 
   // Need to save the previous URL for the notification.
   LoadCommittedDetails details;
-  if (GetLastCommittedEntry())
+  if (GetLastCommittedEntry()) {
     details.previous_url = GetLastCommittedEntry()->url();
+    details.previous_entry_index = GetLastCommittedEntryIndex();
+  } else {
+    details.previous_entry_index = -1;
+  }
 
   if (pending_entry_index_ >= 0) {
     // This is a previous navigation (back/forward) that we're just now
     // committing. Just mark it as committed.
+    details.type = NavigationType::EXISTING_PAGE;
     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
@@ -816,6 +821,7 @@ void NavigationController::CommitPendingEntry() {
     // 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.
+    details.type = NavigationType::NEW_PAGE;
     pending_entry_->set_page_id(active_contents_->GetMaxPageID() + 1);
     active_contents_->UpdateMaxPageID(pending_entry_->page_id());
     InsertEntry(new NavigationEntry(*pending_entry_));
@@ -963,10 +969,6 @@ void NavigationController::InsertEntry(NavigationEntry* entry) {
 
   // 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);
 }
 
 void NavigationController::SetWindowID(const SessionID& id) {
@@ -1026,19 +1028,6 @@ void NavigationController::NotifyNavigationEntryCommitted(
       Details<LoadCommittedDetails>(details));
 }
 
-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)
-    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);
-}
-
 TabContents* NavigationController::GetTabContentsCreateIfNecessary(
     HWND parent,
     const NavigationEntry& entry) {