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

3 files changed, 26 insertions, 20 deletions

diff --git a/chrome/common/notification_details.h b/chrome/common/notification_details.h
index 9c7fdff..31378c4 100644
--- a/chrome/common/notification_details.h
+++ b/chrome/common/notification_details.h
@@ -41,6 +41,7 @@
 class NotificationDetails {
  public:
   NotificationDetails() : ptr_(NULL) {}
+  NotificationDetails(const NotificationDetails& other) : ptr_(other.ptr_) {}
   ~NotificationDetails() {}
 
   // NotificationDetails can be used as the index for a map; this method
@@ -58,12 +59,8 @@ class NotificationDetails {
 
  protected:
   NotificationDetails(void* ptr) : ptr_(ptr) {}
-  NotificationDetails(const NotificationDetails& other) : ptr_(other.ptr_) {}
 
   void* ptr_;
-
-private:
-  void operator=(const NotificationDetails&);
 };
 
 template <class T>
diff --git a/chrome/common/notification_source.h b/chrome/common/notification_source.h
index 31077e9..049a12d 100644
--- a/chrome/common/notification_source.h
+++ b/chrome/common/notification_source.h
@@ -40,6 +40,7 @@
 // NotificationService::AllSources().
 class NotificationSource {
  public:
+  NotificationSource(const NotificationSource& other) : ptr_(other.ptr_) { }
   ~NotificationSource() {}
 
   // NotificationSource can be used as the index for a map; this method
@@ -56,12 +57,8 @@ class NotificationSource {
 
  protected:
   NotificationSource(void* ptr) : ptr_(ptr) {}
-  NotificationSource(const NotificationSource& other) : ptr_(other.ptr_) { }
 
   void* ptr_;
-
- private:
-  void operator=(const NotificationSource&);
 };
 
 template <class T>
diff --git a/chrome/common/notification_types.h b/chrome/common/notification_types.h
index 1080ffa..d2392b7 100644
--- a/chrome/common/notification_types.h
+++ b/chrome/common/notification_types.h
@@ -55,18 +55,35 @@ enum NotificationType {
 
   // NavigationController ------------------------------------------------------
 
-  // A NavigationController's state has changed (a new pending load has been
-  // added, a load has committed, etc.).
+  // A new pending navigation has been created. Pending entries are created when
+  // the user requests the navigation. We don't know if it will actually happen
+  // until it does (at this point, it will be "committed." Note that renderer-
+  // initiated navigations such as link clicks will never be pending.
   //
-  // TODO(brettw) this is vague and I'm not sure that it covers every case. We
-  // need to figure out the exact cases where this notificaion should be sent
-  // and make sure it is sent in those cases.
-  NOTIFY_NAV_STATE_CHANGED,
+  // This notification is called after the pending entry is created, but before
+  // we actually try to navigate. The source will be the NavigationController
+  // that owns the pending entry, and there are no details.
+  NOTIFY_NAV_ENTRY_PENDING,
+
+  // A new non-pending navigation entry has been created. This will correspond
+  // to one NavigationController entry being created (in the case of new
+  // navigations) or renavigated to (for back/forward navigations).
+  NOTIFY_NAV_ENTRY_COMMITTED,
 
   // Indicates that the NavigationController given in the Source has decreased
   // its back/forward list count. This is usually the result of going back and
   // then doing a new navigation, meaning all the "forward" items are deleted.
-  // There are no details.
+  //
+  // This normally happens as a result of a new navigation. It will be followed
+  // by a NOTIFY_NAV_ENTRY_COMMITTED message for the new page that caused the
+  // pruning. It could also be a result of removing an item from the list to fix
+  // up after interstitials.
+  //
+  // The details are an integer indicating the number of items pruned.
+  // Watch out: the NavigationController may start throwing entries away once
+  // the list is a certain size, so you can't use the current size of the
+  // navigation list to tell how many items were pruned, you have to use the
+  // details provided here.
   NOTIFY_NAV_LIST_PRUNED,
 
   // Indicates that a NavigationEntry has changed. The source will be the
@@ -74,11 +91,6 @@ enum NotificationType {
   // a NavigationController::EntryChangedDetails struct.
   NOTIFY_NAV_ENTRY_CHANGED,
 
-  // Indicates that the index of the currently active NavigationEntry has
-  // changed. The source will be the NavigationController whose current item
-  // changed, and there are no details.
-  NOTIFY_NAV_INDEX_CHANGED,
-
   // Other load-related (not from NavigationController) ------------------------
 
   // A content load is starting.  The source will be a