diff options
| -rw-r--r-- | chrome/browser/alternate_nav_url_fetcher.cc | 6 | ||||
| -rw-r--r-- | chrome/browser/jsmessage_box_handler.cc | 6 | ||||
| -rw-r--r-- | chrome/browser/navigation_controller.cc | 33 | ||||
| -rw-r--r-- | chrome/browser/navigation_controller.h | 5 | ||||
| -rw-r--r-- | chrome/browser/navigation_controller_base.cc | 25 | ||||
| -rw-r--r-- | chrome/browser/navigation_controller_base.h | 2 | ||||
| -rw-r--r-- | chrome/browser/navigation_controller_unittest.cc | 136 | ||||
| -rw-r--r-- | chrome/browser/session_service.cc | 6 | ||||
| -rw-r--r-- | chrome/browser/ssl_manager.cc | 5 | ||||
| -rw-r--r-- | chrome/browser/ssl_policy.cc | 1 | ||||
| -rw-r--r-- | chrome/common/notification_details.h | 5 | ||||
| -rw-r--r-- | chrome/common/notification_source.h | 5 | ||||
| -rw-r--r-- | chrome/common/notification_types.h | 36 | ||||
| -rw-r--r-- | chrome/test/test_notification_tracker.cc | 105 | ||||
| -rw-r--r-- | chrome/test/test_notification_tracker.h | 105 | ||||
| -rw-r--r-- | chrome/test/unit/unittests.vcproj | 16 |
16 files changed, 83 insertions, 414 deletions
diff --git a/chrome/browser/alternate_nav_url_fetcher.cc b/chrome/browser/alternate_nav_url_fetcher.cc index e86105b..a791106 100644 --- a/chrome/browser/alternate_nav_url_fetcher.cc +++ b/chrome/browser/alternate_nav_url_fetcher.cc @@ -41,14 +41,14 @@ AlternateNavURLFetcher::AlternateNavURLFetcher( state_(NOT_STARTED), navigated_to_entry_(false) { NotificationService::current()->AddObserver(this, - NOTIFY_NAV_ENTRY_PENDING, NotificationService::AllSources()); + NOTIFY_NAV_STATE_CHANGED, NotificationService::AllSources()); } AlternateNavURLFetcher::~AlternateNavURLFetcher() { if (state_ == NOT_STARTED) { // Never caught the NavigationController notification. NotificationService::current()->RemoveObserver(this, - NOTIFY_NAV_ENTRY_PENDING, NotificationService::AllSources()); + NOTIFY_NAV_STATE_CHANGED, NotificationService::AllSources()); } // Otherwise, Observe() removed the observer already. } @@ -69,7 +69,7 @@ void AlternateNavURLFetcher::Observe(NotificationType type, controller_->SetAlternateNavURLFetcher(this); NotificationService::current()->RemoveObserver(this, - NOTIFY_NAV_ENTRY_PENDING, NotificationService::AllSources()); + NOTIFY_NAV_STATE_CHANGED, NotificationService::AllSources()); DCHECK_EQ(NOT_STARTED, state_); state_ = IN_PROGRESS; diff --git a/chrome/browser/jsmessage_box_handler.cc b/chrome/browser/jsmessage_box_handler.cc index 549981a..09b9757 100644 --- a/chrome/browser/jsmessage_box_handler.cc +++ b/chrome/browser/jsmessage_box_handler.cc @@ -63,7 +63,7 @@ void JavascriptMessageBoxHandler::RunJavascriptMessageBox( JavascriptMessageBoxHandler::~JavascriptMessageBoxHandler() { NotificationService::current()-> - RemoveObserver(this, NOTIFY_NAV_ENTRY_COMMITTED, + RemoveObserver(this, NOTIFY_NAV_STATE_CHANGED, NotificationService::AllSources()); NotificationService::current()-> RemoveObserver(this, NOTIFY_TAB_CONTENTS_DESTROYED, @@ -188,7 +188,7 @@ void JavascriptMessageBoxHandler::Observe(NotificationType type, if (!web_contents_) return; - if (type == NOTIFY_NAV_ENTRY_COMMITTED && + if (type == NOTIFY_NAV_STATE_CHANGED && Source<NavigationController>(source).ptr() == web_contents_->controller()) web_contents_gone = true; @@ -230,7 +230,7 @@ JavascriptMessageBoxHandler::JavascriptMessageBoxHandler( // Make sure we get navigation notifications so we know when our parent // contents will disappear or navigate to a different page. NotificationService::current()-> - AddObserver(this, NOTIFY_NAV_ENTRY_COMMITTED, + AddObserver(this, NOTIFY_NAV_STATE_CHANGED, NotificationService::AllSources()); NotificationService::current()-> AddObserver(this, NOTIFY_TAB_CONTENTS_DESTROYED, diff --git a/chrome/browser/navigation_controller.cc b/chrome/browser/navigation_controller.cc index 5bb5fa5..ac4392f 100644 --- a/chrome/browser/navigation_controller.cc +++ b/chrome/browser/navigation_controller.cc @@ -349,6 +349,13 @@ 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()); @@ -416,10 +423,17 @@ 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); } @@ -459,16 +473,19 @@ void NavigationController::NavigateToPendingEntry(bool reload) { from_contents->delegate()->ReplaceContents(from_contents, contents); } - if (!contents->Navigate(*pending_entry_, reload)) + if (contents->Navigate(*pending_entry_, reload)) { + // Note: this is redundant if navigation completed synchronously because + // DidNavigateToEntry call this as well. + NotifyNavigationStateChanged(); + } else { DiscardPendingEntry(); + } } -void NavigationController::NotifyNavigationEntryCommitted() { +void NavigationController::NotifyNavigationStateChanged() { // 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_)) { @@ -483,7 +500,7 @@ void NavigationController::NotifyNavigationEntryCommitted() { active_contents_->NotifyNavigationStateChanged( TabContents::INVALIDATE_EVERYTHING); - NotificationService::current()->Notify(NOTIFY_NAV_ENTRY_COMMITTED, + NotificationService::current()->Notify(NOTIFY_NAV_STATE_CHANGED, Source<NavigationController>(this), NotificationService::NoDetails()); } @@ -503,6 +520,12 @@ 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( diff --git a/chrome/browser/navigation_controller.h b/chrome/browser/navigation_controller.h index 09bdadf..070abfa 100644 --- a/chrome/browser/navigation_controller.h +++ b/chrome/browser/navigation_controller.h @@ -135,6 +135,9 @@ class NavigationController : public NavigationControllerBase { const std::wstring& GetLazyTitle() const; const SkBitmap& GetLazyFavIcon() const; + // Called when |entry| has been updated outside its NavigationController. + void EntryUpdated(NavigationEntry* entry); + void SetAlternateNavURLFetcher( AlternateNavURLFetcher* alternate_nav_url_fetcher); @@ -199,7 +202,7 @@ class NavigationController : public NavigationControllerBase { virtual int GetMaxPageID() const; virtual void NavigateToPendingEntry(bool reload); - virtual void NotifyNavigationEntryCommitted(); + virtual void NotifyNavigationStateChanged(); // Lets the history database know navigation entries have been removed. virtual void NotifyPrunedEntries(); diff --git a/chrome/browser/navigation_controller_base.cc b/chrome/browser/navigation_controller_base.cc index 76371a5..f34876d 100644 --- a/chrome/browser/navigation_controller_base.cc +++ b/chrome/browser/navigation_controller_base.cc @@ -33,8 +33,6 @@ #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. @@ -211,12 +209,6 @@ 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); } @@ -249,7 +241,6 @@ void NavigationControllerBase::DidNavigateToEntry(NavigationEntry* entry) { // active WebContents, because we have just navigated to it. if (entry->GetPageID() > GetMaxPageID()) { InsertEntry(entry); - NotifyNavigationEntryCommitted(); return; } @@ -305,7 +296,7 @@ void NavigationControllerBase::DidNavigateToEntry(NavigationEntry* entry) { delete entry; - NotifyNavigationEntryCommitted(); + NotifyNavigationStateChanged(); } void NavigationControllerBase::DiscardPendingEntry() { @@ -344,15 +335,12 @@ 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--; } - if (pruned) // Only notify if we did prune something. - NotifyPrunedEntries(); + NotifyPrunedEntries(); } if (entries_.size() >= max_entry_count_) @@ -360,6 +348,8 @@ void NavigationControllerBase::InsertEntry(NavigationEntry* entry) { entries_.push_back(entry); last_committed_entry_index_ = static_cast<int>(entries_.size()) - 1; + + NotifyNavigationStateChanged(); } void NavigationControllerBase::RemoveLastEntry() { @@ -378,11 +368,10 @@ 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(); @@ -399,8 +388,8 @@ void NavigationControllerBase::RemoveEntryAtIndex(int index) { last_committed_entry_index_ = -1; } - // TODO(brettw) bug 1324021: we probably need some notification here so the - // session service can stay in sync. + NotifyPrunedEntries(); + NotifyNavigationStateChanged(); } void NavigationControllerBase::ResetInternal() { diff --git a/chrome/browser/navigation_controller_base.h b/chrome/browser/navigation_controller_base.h index bcc894b..6b33a58 100644 --- a/chrome/browser/navigation_controller_base.h +++ b/chrome/browser/navigation_controller_base.h @@ -178,7 +178,7 @@ class NavigationControllerBase { // Allows the derived class to issue notifications that a load has been // committed. - virtual void NotifyNavigationEntryCommitted() {} + virtual void NotifyNavigationStateChanged() {} // Invoked when entries have been pruned, or removed. For example, if the // current entries are [google, digg, yahoo], with the current entry google, diff --git a/chrome/browser/navigation_controller_unittest.cc b/chrome/browser/navigation_controller_unittest.cc index 5cc0402..cce2cf9 100644 --- a/chrome/browser/navigation_controller_unittest.cc +++ b/chrome/browser/navigation_controller_unittest.cc @@ -39,9 +39,7 @@ #include "chrome/browser/tab_contents.h" #include "chrome/browser/tab_contents_delegate.h" #include "chrome/browser/tab_contents_factory.h" -#include "chrome/common/notification_types.h" #include "chrome/common/stl_util-inl.h" -#include "chrome/test/test_notification_tracker.h" #include "chrome/test/testing_profile.h" #include "net/base/net_util.h" #include "testing/gtest/include/gtest/gtest.h" @@ -291,16 +289,6 @@ class NavigationControllerHistoryTest : public NavigationControllerTest { std::wstring profile_path_; }; -void RegisterForAllNavNotifications(TestNotificationTracker* tracker, - NavigationController* controller) { - tracker->ListenFor(NOTIFY_NAV_ENTRY_COMMITTED, - Source<NavigationController>(controller)); - tracker->ListenFor(NOTIFY_NAV_LIST_PRUNED, - Source<NavigationController>(controller)); - tracker->ListenFor(NOTIFY_NAV_ENTRY_CHANGED, - Source<NavigationController>(controller)); -} - } // namespace TEST_F(NavigationControllerTest, Defaults) { @@ -317,18 +305,12 @@ TEST_F(NavigationControllerTest, Defaults) { } TEST_F(NavigationControllerTest, LoadURL) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); - // Creating a pending notification should not have issued any of the - // notifications we're listening for. - EXPECT_EQ(0, notifications.size()); - // The load should now be pending. + // the load should now be pending EXPECT_TRUE(contents->pending_entry()); EXPECT_EQ(contents->controller()->GetEntryCount(), 0); EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), -1); @@ -339,14 +321,9 @@ TEST_F(NavigationControllerTest, LoadURL) { EXPECT_FALSE(contents->controller()->CanGoForward()); EXPECT_EQ(contents->GetMaxPageID(), -1); - // We should have gotten no notifications from the preceeding checks. - EXPECT_EQ(0, notifications.size()); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); - // The load should now be committed. + // the load should now be committed EXPECT_EQ(contents->controller()->GetEntryCount(), 1); EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 0); EXPECT_EQ(contents->controller()->GetPendingEntryIndex(), -1); @@ -356,10 +333,10 @@ TEST_F(NavigationControllerTest, LoadURL) { EXPECT_FALSE(contents->controller()->CanGoForward()); EXPECT_EQ(contents->GetMaxPageID(), 0); - // Load another... + // load another... contents->controller()->LoadURL(url2, PageTransition::TYPED); - // The load should now be pending. + // the load should now be pending EXPECT_TRUE(contents->pending_entry()); EXPECT_EQ(contents->controller()->GetEntryCount(), 1); EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 0); @@ -372,10 +349,8 @@ TEST_F(NavigationControllerTest, LoadURL) { EXPECT_EQ(contents->GetMaxPageID(), 0); contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); - // The load should now be committed. + // the load should now be committed EXPECT_EQ(contents->controller()->GetEntryCount(), 2); EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 1); EXPECT_EQ(contents->controller()->GetPendingEntryIndex(), -1); @@ -389,23 +364,13 @@ TEST_F(NavigationControllerTest, LoadURL) { // Tests what happens when the same page is loaded again. Should not create a // new session history entry. TEST_F(NavigationControllerTest, LoadURL_SamePage) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); contents->controller()->LoadURL(url1, PageTransition::TYPED); - EXPECT_EQ(0, notifications.size()); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); - contents->controller()->LoadURL(url1, PageTransition::TYPED); - EXPECT_EQ(0, notifications.size()); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); // should not have produced a new session history entry EXPECT_EQ(contents->controller()->GetEntryCount(), 1); @@ -418,21 +383,14 @@ TEST_F(NavigationControllerTest, LoadURL_SamePage) { } TEST_F(NavigationControllerTest, LoadURL_Discarded) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); - EXPECT_EQ(0, notifications.size()); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->LoadURL(url2, PageTransition::TYPED); contents->controller()->DiscardPendingEntry(); - EXPECT_EQ(0, notifications.size()); // should not have produced a new session history entry EXPECT_EQ(contents->controller()->GetEntryCount(), 1); @@ -445,19 +403,12 @@ TEST_F(NavigationControllerTest, LoadURL_Discarded) { } TEST_F(NavigationControllerTest, Reload) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); contents->controller()->LoadURL(url1, PageTransition::TYPED); - EXPECT_EQ(0, notifications.size()); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->Reload(); - EXPECT_EQ(0, notifications.size()); // the reload is pending... EXPECT_EQ(contents->controller()->GetEntryCount(), 1); @@ -469,8 +420,6 @@ TEST_F(NavigationControllerTest, Reload) { EXPECT_FALSE(contents->controller()->CanGoForward()); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); // now the reload is committed... EXPECT_EQ(contents->controller()->GetEntryCount(), 1); @@ -484,25 +433,17 @@ TEST_F(NavigationControllerTest, Reload) { // Tests what happens when a reload navigation produces a new page. TEST_F(NavigationControllerTest, Reload_GeneratesNewPage) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->Reload(); - EXPECT_EQ(0, notifications.size()); contents->pending_entry()->SetURL(url2); contents->pending_entry()->SetTransitionType(PageTransition::LINK); contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); // now the reload is committed... EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -516,24 +457,16 @@ TEST_F(NavigationControllerTest, Reload_GeneratesNewPage) { // Tests what happens when we navigate back successfully TEST_F(NavigationControllerTest, Back) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->LoadURL(url2, PageTransition::TYPED); contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->GoBack(); - EXPECT_EQ(0, notifications.size()); // should now have a pending navigation to go back... EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -545,8 +478,6 @@ TEST_F(NavigationControllerTest, Back) { EXPECT_TRUE(contents->controller()->CanGoForward()); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); // the back navigation completed successfully EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -560,25 +491,17 @@ TEST_F(NavigationControllerTest, Back) { // Tests what happens when a back navigation produces a new page. TEST_F(NavigationControllerTest, Back_GeneratesNewPage) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); const GURL url3("test1:foo3"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->LoadURL(url2, PageTransition::TYPED); contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->GoBack(); - EXPECT_EQ(0, notifications.size()); // should now have a pending navigation to go back... EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -592,8 +515,6 @@ TEST_F(NavigationControllerTest, Back_GeneratesNewPage) { contents->pending_entry()->SetURL(url3); contents->pending_entry()->SetTransitionType(PageTransition::LINK); contents->CompleteNavigation(2); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); // the back navigation resulted in a completely new navigation. // TODO(darin): perhaps this behavior will be confusing to users? @@ -608,26 +529,17 @@ TEST_F(NavigationControllerTest, Back_GeneratesNewPage) { // Tests what happens when we navigate forward successfully TEST_F(NavigationControllerTest, Forward) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->LoadURL(url2, PageTransition::TYPED); contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->GoBack(); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->GoForward(); @@ -641,8 +553,6 @@ TEST_F(NavigationControllerTest, Forward) { EXPECT_FALSE(contents->controller()->CanGoForward()); contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); // the forward navigation completed successfully EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -656,30 +566,20 @@ TEST_F(NavigationControllerTest, Forward) { // Tests what happens when a forward navigation produces a new page. TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); const GURL url3("test1:foo3"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->LoadURL(url2, PageTransition::TYPED); contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->GoBack(); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->controller()->GoForward(); - EXPECT_EQ(0, notifications.size()); // should now have a pending navigation to go forward... EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -693,9 +593,6 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) { contents->pending_entry()->SetURL(url3); contents->pending_entry()->SetTransitionType(PageTransition::LINK); contents->CompleteNavigation(2); - EXPECT_TRUE(notifications.Check3AndReset(NOTIFY_NAV_LIST_PRUNED, - NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); EXPECT_EQ(contents->controller()->GetEntryCount(), 2); EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 1); @@ -707,24 +604,17 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) { } TEST_F(NavigationControllerTest, LinkClick) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); contents->set_pending_entry(new NavigationEntry(kTestContentsType1, NULL, 0, url2, std::wstring(), PageTransition::LINK)); contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); // should not have produced a new session history entry EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -737,16 +627,11 @@ TEST_F(NavigationControllerTest, LinkClick) { } TEST_F(NavigationControllerTest, SwitchTypes) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo"); const GURL url2("test2:foo"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); TestContents* initial_contents = contents; @@ -756,8 +641,6 @@ TEST_F(NavigationControllerTest, SwitchTypes) { ASSERT_TRUE(initial_contents != contents); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); // A second navigation entry should have been committed even though the // PageIDs are the same. PageIDs are scoped to the tab contents type. @@ -773,8 +656,6 @@ TEST_F(NavigationControllerTest, SwitchTypes) { contents->controller()->GoBack(); ASSERT_TRUE(initial_contents == contents); // switched again! contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); EXPECT_EQ(contents->controller()->GetEntryCount(), 2); EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 0); @@ -791,27 +672,20 @@ TEST_F(NavigationControllerTest, SwitchTypes) { // Tests what happens when we begin to navigate to a new contents type, but // then that navigation gets discarded instead. TEST_F(NavigationControllerTest, SwitchTypes_Discard) { - TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, contents->controller()); - const GURL url1("test1:foo"); const GURL url2("test2:foo"); contents->controller()->LoadURL(url1, PageTransition::TYPED); contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); TestContents* initial_contents = contents; contents->controller()->LoadURL(url2, PageTransition::TYPED); - EXPECT_EQ(0, notifications.size()); // The tab contents should have been replaced ASSERT_TRUE(initial_contents != contents); contents->controller()->DiscardPendingEntry(); - EXPECT_EQ(0, notifications.size()); // The tab contents should have been replaced back ASSERT_TRUE(initial_contents == contents); diff --git a/chrome/browser/session_service.cc b/chrome/browser/session_service.cc index da72060..9ed3d9f 100644 --- a/chrome/browser/session_service.cc +++ b/chrome/browser/session_service.cc @@ -160,7 +160,7 @@ SessionService::~SessionService() { NotificationService::current()->RemoveObserver( this, NOTIFY_NAV_ENTRY_CHANGED, NotificationService::AllSources()); NotificationService::current()->RemoveObserver( - this, NOTIFY_NAV_ENTRY_COMMITTED, NotificationService::AllSources()); + this, NOTIFY_NAV_INDEX_CHANGED, NotificationService::AllSources()); } void SessionService::ResetFromCurrentBrowsers() { @@ -417,7 +417,7 @@ void SessionService::Init(const std::wstring& path) { NotificationService::current()->AddObserver( this, NOTIFY_NAV_ENTRY_CHANGED, NotificationService::AllSources()); NotificationService::current()->AddObserver( - this, NOTIFY_NAV_ENTRY_COMMITTED, NotificationService::AllSources()); + this, NOTIFY_NAV_INDEX_CHANGED, NotificationService::AllSources()); DCHECK(!path.empty()); commands_since_reset_ = 0; @@ -450,7 +450,7 @@ void SessionService::Observe(NotificationType type, changed->index, *changed->changed_entry); break; } - case NOTIFY_NAV_ENTRY_COMMITTED: + case NOTIFY_NAV_INDEX_CHANGED: SetSelectedNavigationIndex(controller->window_id(), controller->session_id(), controller->GetCurrentEntryIndex()); diff --git a/chrome/browser/ssl_manager.cc b/chrome/browser/ssl_manager.cc index c1ecf1d..0e9b16a 100644 --- a/chrome/browser/ssl_manager.cc +++ b/chrome/browser/ssl_manager.cc @@ -247,8 +247,10 @@ void SSLManager::SetMaxSecurityStyle(SecurityStyle style) { return; } - if (entry->GetSecurityStyle() > style) + if (entry->GetSecurityStyle() > style) { entry->SetSecurityStyle(style); + controller_->EntryUpdated(entry); + } } // Delegate API method. @@ -651,6 +653,7 @@ void SSLManager::DidCommitProvisionalLoad(ProvisionalLoadDetails* details) { entry->SetSSLCertID(details->ssl_cert_id()); entry->SetSSLCertStatus(details->ssl_cert_status()); entry->SetSSLSecurityBits(details->ssl_security_bits()); + controller_->EntryUpdated(entry); } if (details->interstitial_page()) { diff --git a/chrome/browser/ssl_policy.cc b/chrome/browser/ssl_policy.cc index 4f5f914..1ae9ae1 100644 --- a/chrome/browser/ssl_policy.cc +++ b/chrome/browser/ssl_policy.cc @@ -346,6 +346,7 @@ class DefaultPolicy : public SSLPolicy { NavigationEntry* entry = navigation_controller->GetActiveEntry(); entry->SetHasMixedContent(); + navigation_controller->EntryUpdated(entry); } void OnDenyCertificate(SSLManager::CertError* error) { diff --git a/chrome/common/notification_details.h b/chrome/common/notification_details.h index 31378c4..9c7fdff 100644 --- a/chrome/common/notification_details.h +++ b/chrome/common/notification_details.h @@ -41,7 +41,6 @@ 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 @@ -59,8 +58,12 @@ 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 049a12d..31077e9 100644 --- a/chrome/common/notification_source.h +++ b/chrome/common/notification_source.h @@ -40,7 +40,6 @@ // 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 @@ -57,8 +56,12 @@ 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 d2392b7..1080ffa 100644 --- a/chrome/common/notification_types.h +++ b/chrome/common/notification_types.h @@ -55,35 +55,18 @@ enum NotificationType { // NavigationController ------------------------------------------------------ - // 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. + // A NavigationController's state has changed (a new pending load has been + // added, a load has committed, etc.). // - // 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, + // 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, // 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. - // - // 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. + // There are no details. NOTIFY_NAV_LIST_PRUNED, // Indicates that a NavigationEntry has changed. The source will be the @@ -91,6 +74,11 @@ 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 diff --git a/chrome/test/test_notification_tracker.cc b/chrome/test/test_notification_tracker.cc deleted file mode 100644 index e17de73..0000000 --- a/chrome/test/test_notification_tracker.cc +++ /dev/null @@ -1,105 +0,0 @@ -// 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 deleted file mode 100644 index ca22f6b..0000000 --- a/chrome/test/test_notification_tracker.h +++ /dev/null @@ -1,105 +0,0 @@ -// 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 ee496de..9aa1c90 100644 --- a/chrome/test/unit/unittests.vcproj +++ b/chrome/test/unit/unittests.vcproj @@ -182,14 +182,6 @@ > </File> <File - RelativePath="..\test_notification_tracker.cc" - > - </File> - <File - RelativePath="..\test_notification_tracker.h" - > - </File> - <File RelativePath="..\testing_browser_process.h" > </File> @@ -202,19 +194,19 @@ > </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> <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> </Filter> |