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, 222 insertions, 4 deletions

diff --git a/chrome/test/test_notification_tracker.cc b/chrome/test/test_notification_tracker.cc
new file mode 100644
index 0000000..e17de73
--- /dev/null
+++ b/chrome/test/test_notification_tracker.cc
@@ -0,0 +1,105 @@
+// Copyright 2008, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//    * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//    * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//    * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include "chrome/test/test_notification_tracker.h"
+#include "chrome/common/notification_types.h"
+
+TestNotificationTracker::Event::Event()
+    : type(NOTIFY_ALL),
+      source(NotificationService::AllSources()),
+      details(NotificationService::NoDetails()) {
+}
+TestNotificationTracker::Event::Event(NotificationType t,
+                                      NotificationSource s,
+                                      NotificationDetails d)
+    : type(t),
+      source(s),
+      details(d) {
+}
+
+TestNotificationTracker::TestNotificationTracker() {
+}
+
+TestNotificationTracker::~TestNotificationTracker() {
+  NotificationService* service = NotificationService::current();
+  for (size_t i = 0; i < listening_.size(); i++)
+    service->RemoveObserver(this, listening_[i].first, listening_[i].second);
+  listening_.clear();
+}
+
+void TestNotificationTracker::ListenFor(NotificationType type,
+                                        const NotificationSource& source) {
+  listening_.push_back(std::make_pair(type, source));
+  NotificationService::current()->AddObserver(this, type, source);
+}
+
+void TestNotificationTracker::Reset() {
+  events_.clear();
+}
+
+bool TestNotificationTracker::Check1AndReset(NotificationType type) {
+  if (size() != 1) {
+    Reset();
+    return false;
+  }
+  bool success = events_[0].type == type;
+  Reset();
+  return success;
+}
+
+bool TestNotificationTracker::Check2AndReset(NotificationType type1,
+                                             NotificationType type2) {
+  if (size() != 2) {
+    Reset();
+    return false;
+  }
+  bool success = events_[0].type == type1 && events_[1].type == type2;
+  Reset();
+  return success;
+}
+
+bool TestNotificationTracker::Check3AndReset(NotificationType type1,
+                                             NotificationType type2,
+                                             NotificationType type3) {
+  if (size() != 3) {
+    Reset();
+    return false;
+  }
+  bool success = events_[0].type == type1 &&
+                 events_[1].type == type2 &&
+                 events_[2].type == type3;
+  Reset();
+  return success;
+}
+
+void TestNotificationTracker::Observe(NotificationType type,
+                                      const NotificationSource& source,
+                                      const NotificationDetails& details) {
+  events_.push_back(Event(type, source, details));
+}
diff --git a/chrome/test/test_notification_tracker.h b/chrome/test/test_notification_tracker.h
new file mode 100644
index 0000000..ca22f6b
--- /dev/null
+++ b/chrome/test/test_notification_tracker.h
@@ -0,0 +1,105 @@
+// Copyright 2008, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//    * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//    * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//    * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#ifndef CHROME_TEST_NOTIFICATION_TRACKER_H_
+#define CHROME_TEST_NOTIFICATION_TRACKER_H_
+
+#include <vector>
+
+#include "chrome/common/notification_service.h"
+
+// Provides an easy way for tests to verify that a given set of notifications
+// was received during test execution.
+class TestNotificationTracker : public NotificationObserver {
+ public:
+  // Records one received notification.
+  struct Event {
+    Event();
+    Event(NotificationType t, NotificationSource s, NotificationDetails d);
+
+    NotificationType type;
+    NotificationSource source;
+    NotificationDetails details;
+  };
+
+  // By default, it won't listen for any notifications. You'll need to call
+  // ListenFor for the notifications you are interested in.
+  TestNotificationTracker();
+
+  ~TestNotificationTracker();
+
+  // Makes this object listen for the given notification with the given source.
+  void ListenFor(NotificationType type, const NotificationSource& source);
+
+  // Makes this object listen for notifications of the given type coming from
+  // any source.
+  void ListenForAll(NotificationType type);
+
+  // Clears the list of events.
+  void Reset();
+
+  // Given notifications type(sp, returns true if the list of notifications
+  // were exactly those listed in the given arg(s), and in the same order.
+  //
+  // This will also reset the list so that the next call will only check for
+  // new notifications. Example:
+  //   <do stuff>
+  //   Check1AndReset(NOTIFY_A);
+  //   <do stuff>
+  //   Check2AndReset(NOTIFY_B, NOTIFY_C)
+  bool Check1AndReset(NotificationType type);
+  bool Check2AndReset(NotificationType type1,
+                      NotificationType type2);
+  bool Check3AndReset(NotificationType type1,
+                      NotificationType type2,
+                      NotificationType type3);
+
+  // Returns the number of notifications received since the last reset.
+  size_t size() const { return events_.size(); }
+
+  // Returns the information about the event at the given index. The index must
+  // be in [0, size).
+  const Event& at(size_t i) const { return events_[i]; }
+
+ private:
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
+  // Lists all type/source combinations that we're listening for. These will
+  // need to be unregistered when we are destroyed.
+  std::vector< std::pair<NotificationType, NotificationSource> > listening_;
+
+  // Lists all received since last cleared, in the order they were received.
+  std::vector<Event> events_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestNotificationTracker);
+};
+
+#endif  // CHROME_TEST_NOTIFICATION_TRACKER_H_
diff --git a/chrome/test/unit/unittests.vcproj b/chrome/test/unit/unittests.vcproj
index 9aa1c90..ee496de 100644
--- a/chrome/test/unit/unittests.vcproj
+++ b/chrome/test/unit/unittests.vcproj
@@ -182,6 +182,14 @@
 				>
 			</File>
 			<File
+				RelativePath="..\test_notification_tracker.cc"
+				>
+			</File>
+			<File
+				RelativePath="..\test_notification_tracker.h"
+				>
+			</File>
+			<File
 				RelativePath="..\testing_browser_process.h"
 				>
 			</File>
@@ -194,19 +202,19 @@
 				>
 			</File>
 			<File
-				RelativePath="..\ui\view_event_test_base.cc"
+				RelativePath="..\..\..\net\url_request\url_request_test_job.cc"
 				>
 			</File>
 			<File
-				RelativePath="..\ui\view_event_test_base.h"
+				RelativePath="..\..\..\net\url_request\url_request_test_job.h"
 				>
 			</File>
 			<File
-				RelativePath="..\..\..\net\url_request\url_request_test_job.cc"
+				RelativePath="..\ui\view_event_test_base.cc"
 				>
 			</File>
 			<File
-				RelativePath="..\..\..\net\url_request\url_request_test_job.h"
+				RelativePath="..\ui\view_event_test_base.h"
 				>
 			</File>
 		</Filter>