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.

As part of this, I got very frustrated recompiling the world whenever
navigation_types.h changed. So I removed this dependency from the notification
service which everybody includes. Most of the changed files are adding
notification_types.h in the .cc file where it's needed.

BUG=1325636,1321376,1325779


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

1 files changed, 18 insertions, 7 deletions

diff --git a/chrome/browser/navigation_controller_base.cc b/chrome/browser/navigation_controller_base.cc
index f34876d..76371a5 100644
--- a/chrome/browser/navigation_controller_base.cc
+++ b/chrome/browser/navigation_controller_base.cc
@@ -33,6 +33,8 @@
 
 #include "base/logging.h"
 #include "chrome/browser/navigation_entry.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_types.h"
 #include "webkit/glue/webkit_glue.h"
 
 // The maximum number of entries that a navigation controller can store.
@@ -209,6 +211,12 @@ void NavigationControllerBase::LoadEntry(NavigationEntry* entry) {
   // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()?
   DiscardPendingEntryInternal();
   pending_entry_ = entry;
+  // TODO(brettw) the reinterpret cast can be removed once we combine the
+  // NavigationController and the NavigationControllerBase.
+  NotificationService::current()->Notify(
+      NOTIFY_NAV_ENTRY_PENDING,
+      Source<NavigationController>(reinterpret_cast<NavigationController*>(this)),
+      NotificationService::NoDetails());
   NavigateToPendingEntry(false);
 }
 
@@ -241,6 +249,7 @@ void NavigationControllerBase::DidNavigateToEntry(NavigationEntry* entry) {
   // active WebContents, because we have just navigated to it.
   if (entry->GetPageID() > GetMaxPageID()) {
     InsertEntry(entry);
+    NotifyNavigationEntryCommitted();
     return;
   }
 
@@ -296,7 +305,7 @@ void NavigationControllerBase::DidNavigateToEntry(NavigationEntry* entry) {
 
   delete entry;
 
-  NotifyNavigationStateChanged();
+  NotifyNavigationEntryCommitted();
 }
 
 void NavigationControllerBase::DiscardPendingEntry() {
@@ -335,12 +344,15 @@ void NavigationControllerBase::InsertEntry(NavigationEntry* entry) {
 
   // Prune any entries which are in front of the current entry.
   if (current_size > 0) {
+    bool pruned = false;
     while (last_committed_entry_index_ < (current_size - 1)) {
+      pruned = true;
       delete entries_[current_size - 1];
       entries_.pop_back();
       current_size--;
     }
-    NotifyPrunedEntries();
+    if (pruned)  // Only notify if we did prune something.
+      NotifyPrunedEntries();
   }
 
   if (entries_.size() >= max_entry_count_)
@@ -348,8 +360,6 @@ void NavigationControllerBase::InsertEntry(NavigationEntry* entry) {
 
   entries_.push_back(entry);
   last_committed_entry_index_ = static_cast<int>(entries_.size()) - 1;
-
-  NotifyNavigationStateChanged();
 }
 
 void NavigationControllerBase::RemoveLastEntry() {
@@ -368,10 +378,11 @@ void NavigationControllerBase::RemoveLastEntry() {
 
     NotifyPrunedEntries();
   }
-  NotifyNavigationStateChanged();
 }
 
 void NavigationControllerBase::RemoveEntryAtIndex(int index) {
+  // TODO(brettw) this is only called to remove the first one when we've got
+  // too many entries. It should probably be more specific for this case.
   if (index >= static_cast<int>(entries_.size()) ||
       index == pending_entry_index_ || index == last_committed_entry_index_) {
     NOTREACHED();
@@ -388,8 +399,8 @@ void NavigationControllerBase::RemoveEntryAtIndex(int index) {
       last_committed_entry_index_ = -1;
   }
 
-  NotifyPrunedEntries();
-  NotifyNavigationStateChanged();
+  // TODO(brettw) bug 1324021: we probably need some notification here so the
+  // session service can stay in sync.
 }
 
 void NavigationControllerBase::ResetInternal() {