Cleans up notifications for the NavigationController. There were several

notifications before and some of them were very unclear and misused (STATE_CHANGED). This one, and PRUNED were called unnecessarily in some cases as well.

I replaced STATE_CHANGED and INDEX_CHANGED with ENTRY_COMMITTED which is more clear and covers (I think!) all the cases that the callers care about.

I added a simple notification testing helper class, and used in the navigation controller unit tests to make sure we get the proper notifications. I had to change NotificationSource/Details to have a = and copy constructor so I can track them easily in my helper. I don't see why this would be bad.

BUG=1325636,1321376,1325779

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

1 files changed, 5 insertions, 28 deletions

diff --git a/chrome/browser/navigation_controller.cc b/chrome/browser/navigation_controller.cc
index 91b404a4..5100922 100644
--- a/chrome/browser/navigation_controller.cc
+++ b/chrome/browser/navigation_controller.cc
@@ -349,13 +349,6 @@ const SkBitmap& NavigationController::GetLazyFavIcon() const {
   }
 }
 
-void NavigationController::EntryUpdated(NavigationEntry* entry) {
-  if (entry == GetActiveEntry()) {
-    // Someone has modified our active navigation entry.
-    NotifyNavigationStateChanged();
-  }
-}
-
 void NavigationController::SetAlternateNavURLFetcher(
     AlternateNavURLFetcher* alternate_nav_url_fetcher) {
   DCHECK(!alternate_nav_url_fetcher_.get());
@@ -423,17 +416,10 @@ void NavigationController::DiscardPendingEntry() {
     DCHECK(from_contents != active_contents_);
     ScheduleTabContentsCollection(from_contents->type());
   }
-
-  // Note: this may be redundant in some cases.  we may want to optimize away
-  // the redundant notifications.
-  NotifyNavigationStateChanged();
 }
 
 void NavigationController::InsertEntry(NavigationEntry* entry) {
   NavigationControllerBase::InsertEntry(entry);
-  NotificationService::current()->Notify(NOTIFY_NAV_INDEX_CHANGED,
-                                         Source<NavigationController>(this),
-                                         NotificationService::NoDetails());
   active_contents_->NotifyDidNavigate(NAVIGATION_NEW, 0);
 }
 
@@ -473,19 +459,16 @@ void NavigationController::NavigateToPendingEntry(bool reload) {
       from_contents->delegate()->ReplaceContents(from_contents, contents);
   }
 
-  if (contents->Navigate(*pending_entry_, reload)) {
-    // Note: this is redundant if navigation completed synchronously because
-    // DidNavigateToEntry call this as well.
-    NotifyNavigationStateChanged();
-  } else {
+  if (!contents->Navigate(*pending_entry_, reload))
     DiscardPendingEntry();
-  }
 }
 
-void NavigationController::NotifyNavigationStateChanged() {
+void NavigationController::NotifyNavigationEntryCommitted() {
   // Reset the Alternate Nav URL Fetcher if we're loading some page it doesn't
   // care about.  We must do this before calling Notify() below as that may
   // result in the creation of a new fetcher.
+  //
+  // TODO(brettw) bug 1324500: this logic should be moved out of the controller!
   const NavigationEntry* const entry = GetActiveEntry();
   if (!entry ||
       (entry->unique_id() != alternate_nav_url_fetcher_entry_unique_id_)) {
@@ -500,7 +483,7 @@ void NavigationController::NotifyNavigationStateChanged() {
   active_contents_->NotifyNavigationStateChanged(
       TabContents::INVALIDATE_EVERYTHING);
 
-  NotificationService::current()->Notify(NOTIFY_NAV_STATE_CHANGED,
+  NotificationService::current()->Notify(NOTIFY_NAV_ENTRY_COMMITTED,
                                          Source<NavigationController>(this),
                                          NotificationService::NoDetails());
 }
@@ -520,12 +503,6 @@ void NavigationController::IndexOfActiveEntryChanged(
     nav_type = NAVIGATION_REPLACE;
   }
   active_contents_->NotifyDidNavigate(nav_type, relative_navigation_offset);
-  if (GetCurrentEntryIndex() != -1) {
-    NotificationService::current()->Notify(
-        NOTIFY_NAV_INDEX_CHANGED,
-        Source<NavigationController>(this),
-        NotificationService::NoDetails());
-  }
 }
 
 TabContents* NavigationController::GetTabContentsCreateIfNecessary(