diff options
12 files changed, 223 insertions, 383 deletions
diff --git a/chrome/browser/sessions/session_restore_browsertest.cc b/chrome/browser/sessions/session_restore_browsertest.cc index b3a9864f..4030231 100644 --- a/chrome/browser/sessions/session_restore_browsertest.cc +++ b/chrome/browser/sessions/session_restore_browsertest.cc @@ -4,6 +4,7 @@ #include "base/command_line.h" #include "base/file_path.h" +#include "base/time.h" #include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/defaults.h" @@ -38,6 +39,7 @@ #include "content/public/browser/web_contents.h" #include "content/public/common/page_transition_types.h" #include "content/public/test/test_navigation_observer.h" +#include "sync/protocol/session_specifics.pb.h" #if defined(OS_MACOSX) #include "base/mac/scoped_nsautorelease_pool.h" @@ -90,16 +92,6 @@ class SessionRestoreTest : public InProcessBrowserTest { return InProcessBrowserTest::SetUpUserDataDirectory(); } - // Verifies that the given NavigationController has exactly two entries that - // correspond to the given URLs. - void VerifyNavigationEntries( - content::NavigationController& controller, GURL url1, GURL url2) { - ASSERT_EQ(2, controller.GetEntryCount()); - EXPECT_EQ(1, controller.GetCurrentEntryIndex()); - EXPECT_EQ(url1, controller.GetEntryAtIndex(0)->GetURL()); - EXPECT_EQ(url2, controller.GetEntryAtIndex(1)->GetURL()); - } - void CloseBrowserSynchronously(Browser* browser) { content::WindowedNotificationObserver observer( chrome::NOTIFICATION_BROWSER_CLOSED, @@ -299,7 +291,7 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreIndividualTabFromWindow) { browser()->window()->Close(); // Expect a window with three tabs. - EXPECT_EQ(1U, service->entries().size()); + ASSERT_EQ(1U, service->entries().size()); ASSERT_EQ(TabRestoreService::WINDOW, service->entries().front()->type); const TabRestoreService::Window* window = static_cast<TabRestoreService::Window*>(service->entries().front()); @@ -307,21 +299,34 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreIndividualTabFromWindow) { // Find the SessionID for entry2. Since the session service was destroyed, // there is no guarantee that the SessionID for the tab has remained the same. - std::vector<TabRestoreService::Tab>::const_iterator it = window->tabs.begin(); - for ( ; it != window->tabs.end(); ++it) { + base::Time timestamp; + for (std::vector<TabRestoreService::Tab>::const_iterator it = + window->tabs.begin(); it != window->tabs.end(); ++it) { const TabRestoreService::Tab& tab = *it; // If this tab held url2, then restore this single tab. if (tab.navigations[0].virtual_url() == url2) { + timestamp = tab.navigations[0].timestamp(); service->RestoreEntryById(NULL, tab.id, UNKNOWN); break; } } + EXPECT_FALSE(timestamp.is_null()); - // Make sure that the Window got updated. - EXPECT_EQ(1U, service->entries().size()); + // Make sure that the restored tab is removed from the service. + ASSERT_EQ(1U, service->entries().size()); ASSERT_EQ(TabRestoreService::WINDOW, service->entries().front()->type); window = static_cast<TabRestoreService::Window*>(service->entries().front()); EXPECT_EQ(2U, window->tabs.size()); + + // Make sure that the restored tab was restored with the correct + // timestamp. + const content::WebContents* contents = + chrome::GetActiveWebContents(browser()); + ASSERT_TRUE(contents); + const content::NavigationEntry* entry = + contents->GetController().GetActiveEntry(); + ASSERT_TRUE(entry); + EXPECT_EQ(timestamp, entry->GetTimestamp()); } IN_PROC_BROWSER_TEST_F(SessionRestoreTest, WindowWithOneTab) { @@ -387,6 +392,24 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, IncognitotoNonIncognito) { } #endif // !OS_CHROMEOS +namespace { + +// Verifies that the given NavigationController has exactly two +// entries that correspond to the given URLs and that all but the last +// entry have null timestamps. +void VerifyNavigationEntries( + const content::NavigationController& controller, + GURL url1, GURL url2) { + ASSERT_EQ(2, controller.GetEntryCount()); + EXPECT_EQ(1, controller.GetCurrentEntryIndex()); + EXPECT_EQ(url1, controller.GetEntryAtIndex(0)->GetURL()); + EXPECT_EQ(url2, controller.GetEntryAtIndex(1)->GetURL()); + EXPECT_TRUE(controller.GetEntryAtIndex(0)->GetTimestamp().is_null()); + EXPECT_FALSE(controller.GetEntryAtIndex(1)->GetTimestamp().is_null()); +} + +} // namespace + IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreForeignTab) { GURL url1("http://google.com"); GURL url2("http://google2.com"); @@ -396,14 +419,18 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreForeignTab) { SessionTypesTestHelper::CreateNavigation(url2.spec(), "two"); // Set up the restore data. + sync_pb::SessionTab sync_data; + sync_data.set_tab_visual_index(0); + sync_data.set_current_navigation_index(1); + sync_data.set_pinned(false); + sync_data.add_navigation()->CopyFrom(nav1.ToSyncData()); + sync_data.add_navigation()->CopyFrom(nav2.ToSyncData()); + SessionTab tab; - tab.tab_visual_index = 0; - tab.current_navigation_index = 1; - tab.pinned = false; - tab.navigations.push_back(nav1); - tab.navigations.push_back(nav2); - tab.user_agent_override = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.19" - " (KHTML, like Gecko) Chrome/18.0.1025.45 Safari/535.19"; + tab.SetFromSyncData(sync_data, base::Time::Now()); + ASSERT_EQ(2U, tab.navigations.size()); + EXPECT_TRUE(tab.navigations[0].timestamp().is_null()); + EXPECT_TRUE(tab.navigations[1].timestamp().is_null()); ASSERT_EQ(1, browser()->tab_count()); @@ -419,7 +446,7 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreForeignTab) { ASSERT_EQ(1, browser()->tab_count()); content::WebContents* web_contents = chrome::GetWebContentsAt(browser(), 0); VerifyNavigationEntries(web_contents->GetController(), url1, url2); - ASSERT_EQ(tab.user_agent_override, web_contents->GetUserAgentOverride()); + ASSERT_TRUE(web_contents->GetUserAgentOverride().empty()); // Restore in a new tab. { @@ -434,18 +461,25 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreForeignTab) { ASSERT_EQ(0, browser()->active_index()); web_contents = chrome::GetWebContentsAt(browser(), 1); VerifyNavigationEntries(web_contents->GetController(), url1, url2); - ASSERT_EQ(tab.user_agent_override, web_contents->GetUserAgentOverride()); + ASSERT_TRUE(web_contents->GetUserAgentOverride().empty()); // Restore in a new window. - ui_test_utils::BrowserAddedObserver browser_observer; - SessionRestore::RestoreForeignSessionTab( - chrome::GetActiveWebContents(browser()), tab, NEW_WINDOW); - Browser* new_browser = browser_observer.WaitForSingleNewBrowser(); + Browser* new_browser = NULL; + { + ui_test_utils::BrowserAddedObserver browser_observer; + content::WindowedNotificationObserver observer( + content::NOTIFICATION_LOAD_STOP, + content::NotificationService::AllSources()); + SessionRestore::RestoreForeignSessionTab( + chrome::GetActiveWebContents(browser()), tab, NEW_WINDOW); + new_browser = browser_observer.WaitForSingleNewBrowser(); + observer.Wait(); + } ASSERT_EQ(1, new_browser->tab_count()); web_contents = chrome::GetWebContentsAt(new_browser, 0); VerifyNavigationEntries(web_contents->GetController(), url1, url2); - ASSERT_EQ(tab.user_agent_override, web_contents->GetUserAgentOverride()); + ASSERT_TRUE(web_contents->GetUserAgentOverride().empty()); } IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreForeignSession) { @@ -463,19 +497,25 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreForeignSession) { std::vector<const SessionWindow*> session; SessionWindow window; SessionTab tab1; - tab1.tab_visual_index = 0; - tab1.current_navigation_index = 0; - tab1.pinned = true; - tab1.navigations.push_back(nav1); - tab1.user_agent_override = "user_agent_override"; + { + sync_pb::SessionTab sync_data; + sync_data.set_tab_visual_index(0); + sync_data.set_current_navigation_index(0); + sync_data.set_pinned(true); + sync_data.add_navigation()->CopyFrom(nav1.ToSyncData()); + tab1.SetFromSyncData(sync_data, base::Time::Now()); + } window.tabs.push_back(&tab1); SessionTab tab2; - tab2.tab_visual_index = 1; - tab2.current_navigation_index = 0; - tab2.pinned = false; - tab2.navigations.push_back(nav2); - tab2.user_agent_override = "user_agent_override_2"; + { + sync_pb::SessionTab sync_data; + sync_data.set_tab_visual_index(1); + sync_data.set_current_navigation_index(0); + sync_data.set_pinned(false); + sync_data.add_navigation()->CopyFrom(nav2.ToSyncData()); + tab2.SetFromSyncData(sync_data, base::Time::Now()); + } window.tabs.push_back(&tab2); session.push_back(static_cast<const SessionWindow*>(&window)); @@ -495,19 +535,17 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreForeignSession) { ASSERT_EQ(url2, web_contents_2->GetURL()); // Check user agent override state. - ASSERT_EQ(tab1.user_agent_override, web_contents_1->GetUserAgentOverride()); - ASSERT_EQ(tab2.user_agent_override, web_contents_2->GetUserAgentOverride()); + ASSERT_TRUE(web_contents_1->GetUserAgentOverride().empty()); + ASSERT_TRUE(web_contents_2->GetUserAgentOverride().empty()); content::NavigationEntry* entry = web_contents_1->GetController().GetActiveEntry(); ASSERT_TRUE(entry); - ASSERT_EQ(SessionTypesTestHelper::GetIsOverridingUserAgent(nav1), - entry->GetIsOverridingUserAgent()); + ASSERT_FALSE(entry->GetIsOverridingUserAgent()); entry = web_contents_2->GetController().GetActiveEntry(); ASSERT_TRUE(entry); - ASSERT_EQ(SessionTypesTestHelper::GetIsOverridingUserAgent(nav2), - entry->GetIsOverridingUserAgent()); + ASSERT_FALSE(entry->GetIsOverridingUserAgent()); // The SessionWindow destructor deletes the tabs, so we have to clear them // here to avoid a crash. diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc index 4d63bbf..a2f37dc 100644 --- a/chrome/browser/sessions/session_service.cc +++ b/chrome/browser/sessions/session_service.cc @@ -634,8 +634,7 @@ void SessionService::Observe(int type, content::Details<content::EntryChangedDetails> changed(details); const TabNavigation navigation = TabNavigation::FromNavigationEntry( - changed->index, *changed->changed_entry, - base::Time::Now()); + changed->index, *changed->changed_entry); UpdateTabNavigation(session_tab_helper->window_id(), session_tab_helper->session_id(), navigation); @@ -660,8 +659,7 @@ void SessionService::Observe(int type, TabNavigation::FromNavigationEntry( current_entry_index, *web_contents->GetController().GetEntryAtIndex( - current_entry_index), - base::Time::Now()); + current_entry_index)); UpdateTabNavigation( session_tab_helper->window_id(), session_tab_helper->session_id(), @@ -1305,7 +1303,7 @@ void SessionService::BuildCommandsForTab( DCHECK(entry); if (ShouldTrackEntry(entry->GetVirtualURL())) { const TabNavigation navigation = - TabNavigation::FromNavigationEntry(i, *entry, base::Time::Now()); + TabNavigation::FromNavigationEntry(i, *entry); commands->push_back( CreateUpdateTabNavigationCommand( kCommandUpdateTabNavigation, session_id.id(), navigation)); diff --git a/chrome/browser/sessions/session_types.cc b/chrome/browser/sessions/session_types.cc index ef2db32..183b3a9 100644 --- a/chrome/browser/sessions/session_types.cc +++ b/chrome/browser/sessions/session_types.cc @@ -34,8 +34,7 @@ TabNavigation::~TabNavigation() {} // static TabNavigation TabNavigation::FromNavigationEntry( int index, - const NavigationEntry& entry, - base::Time timestamp) { + const NavigationEntry& entry) { TabNavigation navigation; navigation.index_ = index; navigation.unique_id_ = entry.GetUniqueID(); @@ -48,7 +47,7 @@ TabNavigation TabNavigation::FromNavigationEntry( navigation.post_id_ = entry.GetPostID(); navigation.original_request_url_ = entry.GetOriginalRequestURL(); navigation.is_overriding_user_agent_ = entry.GetIsOverridingUserAgent(); - navigation.timestamp_ = timestamp; + navigation.timestamp_ = entry.GetTimestamp(); return navigation; } @@ -57,19 +56,13 @@ TabNavigation TabNavigation::FromSyncData( const sync_pb::TabNavigation& sync_data) { TabNavigation navigation; navigation.index_ = index; - if (sync_data.has_unique_id()) - navigation.unique_id_ = sync_data.unique_id(); - if (sync_data.has_referrer()) { - navigation.referrer_ = - content::Referrer(GURL(sync_data.referrer()), - WebKit::WebReferrerPolicyDefault); - } - if (sync_data.has_virtual_url()) - navigation.virtual_url_ = GURL(sync_data.virtual_url()); - if (sync_data.has_title()) - navigation.title_ = UTF8ToUTF16(sync_data.title()); - if (sync_data.has_state()) - navigation.content_state_ = sync_data.state(); + navigation.unique_id_ = sync_data.unique_id(); + navigation.referrer_ = + content::Referrer(GURL(sync_data.referrer()), + WebKit::WebReferrerPolicyDefault); + navigation.virtual_url_ = GURL(sync_data.virtual_url()); + navigation.title_ = UTF8ToUTF16(sync_data.title()); + navigation.content_state_ = sync_data.state(); uint32 transition = 0; if (sync_data.has_page_transition()) { @@ -138,8 +131,7 @@ TabNavigation TabNavigation::FromSyncData( navigation.transition_type_ = static_cast<content::PageTransition>(transition); - if (sync_data.has_timestamp()) - navigation.timestamp_ = syncer::ProtoTimeToTime(sync_data.timestamp()); + navigation.timestamp_ = base::Time(); return navigation; } @@ -208,6 +200,7 @@ enum TypeMask { // referrer_ // original_request_url_ // is_overriding_user_agent_ +// timestamp_ void TabNavigation::WriteToPickle(Pickle* pickle) const { pickle->WriteInt(index_); @@ -251,8 +244,7 @@ void TabNavigation::WriteToPickle(Pickle* pickle) const { original_request_url_.is_valid() ? original_request_url_.spec() : std::string()); pickle->WriteBool(is_overriding_user_agent_); - - // TODO(akalin): Persist timestamp. + pickle->WriteInt64(timestamp_.ToInternalValue()); } bool TabNavigation::ReadFromPickle(PickleIterator* iterator) { @@ -299,9 +291,15 @@ bool TabNavigation::ReadFromPickle(PickleIterator* iterator) { // Default to not overriding the user agent if we don't have info. if (!iterator->ReadBool(&is_overriding_user_agent_)) is_overriding_user_agent_ = false; + + int64 timestamp_internal_value = 0; + if (iterator->ReadInt64(×tamp_internal_value)) { + timestamp_ = base::Time::FromInternalValue(timestamp_internal_value); + } else { + timestamp_ = base::Time(); + } } - // TODO(akalin): Restore timestamp when it is persisted. return true; } @@ -327,9 +325,7 @@ scoped_ptr<NavigationEntry> TabNavigation::ToNavigationEntry( entry->SetPostID(post_id_); entry->SetOriginalRequestURL(original_request_url_); entry->SetIsOverridingUserAgent(is_overriding_user_agent_); - - // TODO(akalin): Set |entry|'s timestamp once NavigationEntry has a - // field for it. + entry->SetTimestamp(timestamp_); return entry.Pass(); } @@ -418,6 +414,8 @@ sync_pb::TabNavigation TabNavigation::ToSyncData() const { (transition_type_ & content::PAGE_TRANSITION_CHAIN_END) != 0); sync_data.set_unique_id(unique_id_); + // TODO(akalin): Don't lose resolution, i.e. define a new timestamp + // field with microsecond resolution and use that. sync_data.set_timestamp(syncer::TimeToProtoTime(timestamp_)); return sync_data; diff --git a/chrome/browser/sessions/session_types.h b/chrome/browser/sessions/session_types.h index bb0ee1b..407748c 100644 --- a/chrome/browser/sessions/session_types.h +++ b/chrome/browser/sessions/session_types.h @@ -43,20 +43,17 @@ class TabNavigation { TabNavigation(); ~TabNavigation(); - // Construct a TabNavigation for a particular index from a - // NavigationEntry with the given timestamp. - // - // TODO(akalin): Add a timestamp field to - // navigation::NavigationEntry and use that instead of passing a - // separate timestamp. + // Construct a TabNavigation for a particular index from the given + // NavigationEntry. static TabNavigation FromNavigationEntry( int index, - const content::NavigationEntry& entry, - base::Time timestamp); + const content::NavigationEntry& entry); // Construct a TabNavigation for a particular index from a sync // protocol buffer. Note that the sync protocol buffer doesn't - // contain all TabNavigation fields. + // contain all TabNavigation fields. Also, the timestamp of the + // returned TabNavigation is nulled out, as we assume that the + // protocol buffer is from a foreign session. static TabNavigation FromSyncData( int index, const sync_pb::TabNavigation& sync_data); @@ -119,10 +116,6 @@ class TabNavigation { int64 post_id_; GURL original_request_url_; bool is_overriding_user_agent_; - - // Timestamp when the navigation occurred. - // - // TODO(akalin): Add a timestamp field to NavigationEntry. base::Time timestamp_; }; diff --git a/chrome/browser/sessions/session_types_unittest.cc b/chrome/browser/sessions/session_types_unittest.cc index 92d630c..168dc76 100644 --- a/chrome/browser/sessions/session_types_unittest.cc +++ b/chrome/browser/sessions/session_types_unittest.cc @@ -59,6 +59,7 @@ scoped_ptr<content::NavigationEntry> MakeNavigationEntryForTest() { navigation_entry->SetPostID(kPostID); navigation_entry->SetOriginalRequestURL(kOriginalRequestURL); navigation_entry->SetIsOverridingUserAgent(kIsOverridingUserAgent); + navigation_entry->SetTimestamp(kTimestamp); return navigation_entry.Pass(); } @@ -96,7 +97,7 @@ TEST(TabNavigationTest, DefaultInitializer) { EXPECT_EQ(-1, SessionTypesTestHelper::GetPostID(navigation)); EXPECT_EQ(GURL(), SessionTypesTestHelper::GetOriginalRequestURL(navigation)); EXPECT_FALSE(SessionTypesTestHelper::GetIsOverridingUserAgent(navigation)); - EXPECT_EQ(base::Time(), navigation.timestamp()); + EXPECT_TRUE(navigation.timestamp().is_null()); } // Create a TabNavigation from a NavigationEntry. All its fields @@ -106,8 +107,7 @@ TEST(TabNavigationTest, FromNavigationEntry) { MakeNavigationEntryForTest()); const TabNavigation& navigation = - TabNavigation::FromNavigationEntry( - kIndex, *navigation_entry, kTimestamp); + TabNavigation::FromNavigationEntry(kIndex, *navigation_entry); EXPECT_EQ(kIndex, navigation.index()); @@ -154,7 +154,7 @@ TEST(TabNavigationTest, FromSyncData) { EXPECT_EQ(-1, SessionTypesTestHelper::GetPostID(navigation)); EXPECT_EQ(GURL(), SessionTypesTestHelper::GetOriginalRequestURL(navigation)); EXPECT_FALSE(SessionTypesTestHelper::GetIsOverridingUserAgent(navigation)); - EXPECT_EQ(kTimestamp, navigation.timestamp()); + EXPECT_TRUE(navigation.timestamp().is_null()); } // Create a TabNavigation, pickle it, then create another one by @@ -162,8 +162,7 @@ TEST(TabNavigationTest, FromSyncData) { // that aren't pickled, which should be set to default values. TEST(TabNavigationTest, Pickle) { const TabNavigation& old_navigation = - TabNavigation::FromNavigationEntry( - kIndex, *MakeNavigationEntryForTest(), kTimestamp); + TabNavigation::FromNavigationEntry(kIndex, *MakeNavigationEntryForTest()); Pickle pickle; old_navigation.WriteToPickle(&pickle); @@ -191,7 +190,7 @@ TEST(TabNavigationTest, Pickle) { SessionTypesTestHelper::GetOriginalRequestURL(new_navigation)); EXPECT_EQ(kIsOverridingUserAgent, SessionTypesTestHelper::GetIsOverridingUserAgent(new_navigation)); - EXPECT_EQ(base::Time(), new_navigation.timestamp()); + EXPECT_EQ(kTimestamp, new_navigation.timestamp()); } // Create a NavigationEntry, then create another one by converting to @@ -203,8 +202,7 @@ TEST(TabNavigationTest, ToNavigationEntry) { MakeNavigationEntryForTest()); const TabNavigation& navigation = - TabNavigation::FromNavigationEntry( - kIndex, *old_navigation_entry, kTimestamp); + TabNavigation::FromNavigationEntry(kIndex, *old_navigation_entry); const scoped_ptr<content::NavigationEntry> new_navigation_entry( navigation.ToNavigationEntry(kPageID, NULL)); @@ -233,8 +231,7 @@ TEST(TabNavigationTest, ToSyncData) { MakeNavigationEntryForTest()); const TabNavigation& navigation = - TabNavigation::FromNavigationEntry( - kIndex, *navigation_entry, kTimestamp); + TabNavigation::FromNavigationEntry(kIndex, *navigation_entry); const sync_pb::TabNavigation sync_data = navigation.ToSyncData(); @@ -269,8 +266,7 @@ TEST(TabNavigationTest, TransitionTypes) { navigation_entry->SetTransitionType(transition); const TabNavigation& navigation = - TabNavigation::FromNavigationEntry( - kIndex, *navigation_entry, kTimestamp); + TabNavigation::FromNavigationEntry(kIndex, *navigation_entry); const sync_pb::TabNavigation& sync_data = navigation.ToSyncData(); const TabNavigation& constructed_nav = TabNavigation::FromSyncData(kIndex, sync_data); diff --git a/chrome/browser/sessions/tab_restore_service.cc b/chrome/browser/sessions/tab_restore_service.cc index cb7b68f..cba2432 100644 --- a/chrome/browser/sessions/tab_restore_service.cc +++ b/chrome/browser/sessions/tab_restore_service.cc @@ -530,7 +530,7 @@ void TabRestoreService::PopulateTab(Tab* tab, NavigationEntry* entry = (i == pending_index) ? controller->GetPendingEntry() : controller->GetEntryAtIndex(i); tab->navigations[i] = - TabNavigation::FromNavigationEntry(i, *entry, base::Time::Now()); + TabNavigation::FromNavigationEntry(i, *entry); } tab->timestamp = TimeNow(); tab->current_navigation_index = controller->GetCurrentEntryIndex(); diff --git a/chrome/browser/sessions/tab_restore_service_browsertest.cc b/chrome/browser/sessions/tab_restore_service_browsertest.cc index 17ee22e..6a4bb88 100644 --- a/chrome/browser/sessions/tab_restore_service_browsertest.cc +++ b/chrome/browser/sessions/tab_restore_service_browsertest.cc @@ -517,7 +517,7 @@ TEST_F(TabRestoreServiceTest, ManyWindowsInSessionService) { EXPECT_TRUE(url1_ == window->tabs[0].navigations[0].virtual_url()); } -// Makes sure we restore the time stamp correctly. +// Makes sure we restore timestamps correctly. TEST_F(TabRestoreServiceTest, TimestampSurvivesRestore) { base::Time tab_timestamp(base::Time::FromInternalValue(123456789)); @@ -530,10 +530,20 @@ TEST_F(TabRestoreServiceTest, TimestampSurvivesRestore) { ASSERT_EQ(1U, service_->entries().size()); // Make sure the entry matches. - TabRestoreService::Entry* entry = service_->entries().front(); - ASSERT_EQ(TabRestoreService::TAB, entry->type); - Tab* tab = static_cast<Tab*>(entry); - tab->timestamp = tab_timestamp; + std::vector<TabNavigation> old_navigations; + { + // |entry|/|tab| doesn't survive after RecreateService(). + TabRestoreService::Entry* entry = service_->entries().front(); + ASSERT_EQ(TabRestoreService::TAB, entry->type); + Tab* tab = static_cast<Tab*>(entry); + tab->timestamp = tab_timestamp; + old_navigations = tab->navigations; + } + + ASSERT_EQ(3U, old_navigations.size()); + EXPECT_FALSE(old_navigations[0].timestamp().is_null()); + EXPECT_FALSE(old_navigations[1].timestamp().is_null()); + EXPECT_FALSE(old_navigations[2].timestamp().is_null()); // Set this, otherwise previous session won't be loaded. profile()->set_last_session_exited_cleanly(false); @@ -550,6 +560,13 @@ TEST_F(TabRestoreServiceTest, TimestampSurvivesRestore) { static_cast<Tab*>(restored_entry); EXPECT_EQ(tab_timestamp.ToInternalValue(), restored_tab->timestamp.ToInternalValue()); + ASSERT_EQ(3U, restored_tab->navigations.size()); + EXPECT_EQ(old_navigations[0].timestamp(), + restored_tab->navigations[0].timestamp()); + EXPECT_EQ(old_navigations[1].timestamp(), + restored_tab->navigations[1].timestamp()); + EXPECT_EQ(old_navigations[2].timestamp(), + restored_tab->navigations[2].timestamp()); } TEST_F(TabRestoreServiceTest, PruneEntries) { diff --git a/chrome/browser/sync/glue/session_model_associator.cc b/chrome/browser/sync/glue/session_model_associator.cc index c9d4494..cf37a0d 100644 --- a/chrome/browser/sync/glue/session_model_associator.cc +++ b/chrome/browser/sync/glue/session_model_associator.cc @@ -399,8 +399,7 @@ bool SessionModelAssociator::WriteTabContentsToSyncModel( synced_session_tracker_.GetTab(GetCurrentMachineTag(), tab_delegate.GetSessionId()); - base::Time now = base::Time::Now(); - UpdateSessionTabFromDelegate(tab_delegate, now, now, session_tab); + SetSessionTabFromDelegate(tab_delegate, base::Time::Now(), session_tab); const GURL new_url = GetCurrentVirtualURL(tab_delegate); DVLOG(1) << "Local tab " << tab_delegate.GetSessionId() @@ -450,26 +449,10 @@ bool SessionModelAssociator::WriteTabContentsToSyncModel( return true; } -// Updates |session_tab| by combining existing data and new data from -// |tab_delegate|. -// -// Timestamps are chosen from either |session_tab| or -// |default_navigation_timestamp| based on the following rules: -// 1. If a navigation exists in both |tab_delegate| and |session_tab|, -// as determined by the unique id, and the navigation didn't just -// become the current navigation, we preserve the old timestamp. -// 2. If the navigation exists in both but just become the current navigation -// (e.g. the user went back in history to this navigation), we update the -// timestamp to |default_navigation_timestamp|. -// 3. All new navigations not present in |session_tab| have their -// timestamps set to |default_navigation_timestamp| (if they're the -// current one) or nulled out (if not). -// // static -void SessionModelAssociator::UpdateSessionTabFromDelegate( +void SessionModelAssociator::SetSessionTabFromDelegate( const SyncedTabDelegate& tab_delegate, base::Time mtime, - base::Time default_navigation_timestamp, SessionTab* session_tab) { DCHECK(session_tab); session_tab->window_id.set_id(tab_delegate.GetWindowId()); @@ -486,59 +469,14 @@ void SessionModelAssociator::UpdateSessionTabFromDelegate( current_index - kMaxSyncNavigationCount); const int max_index = std::min(current_index + kMaxSyncNavigationCount, tab_delegate.GetEntryCount()); - std::vector<TabNavigation> previous_navigations; - previous_navigations.swap(session_tab->navigations); - std::vector<TabNavigation>::const_iterator prev_nav_iter = - previous_navigations.begin(); + session_tab->navigations.clear(); for (int i = min_index; i < max_index; ++i) { const NavigationEntry* entry = (i == pending_index) ? tab_delegate.GetPendingEntry() : tab_delegate.GetEntryAtIndex(i); DCHECK(entry); - if (i == min_index) { - // Find the location of the first navigation within the previous list of - // navigations. We only need to do this once, as all subsequent - // navigations are either contiguous or completely new. - for (;prev_nav_iter != previous_navigations.end(); - ++prev_nav_iter) { - if (prev_nav_iter->unique_id() == entry->GetUniqueID()) - break; - } - } if (entry->GetVirtualURL().is_valid()) { - // TODO(akalin): Remove this logic once we have a timestamp in - // NavigationEntry (since we can just use those directly). - base::Time timestamp; - // If this navigation is an old one, reuse the old timestamp. Otherwise we - // leave the timestamp as the current time. - if (prev_nav_iter != previous_navigations.end() && - prev_nav_iter->unique_id() == entry->GetUniqueID()) { - // Check that we haven't gone back/forward in the nav stack to this page - // (if so, we want to refresh the timestamp). - if (!(current_index != session_tab->current_navigation_index && - current_index == i)) { - timestamp = prev_nav_iter->timestamp(); - DVLOG(2) << "Nav to " << entry->GetVirtualURL() - << " already known, reusing old timestamp " - << timestamp.ToInternalValue(); - } - // Even if the user went back in their history, they may have skipped - // over navigations, so the subsequent navigation entries may need their - // old timestamps preserved. - ++prev_nav_iter; - } else if (current_index != i && previous_navigations.empty()) { - // If this is a new tab, and has more than one navigation, we don't - // actually want to assign the current timestamp to other navigations. - // Override the timestamp to 0 in that case. - // Note: this is primarily to handle restoring sessions at restart, - // opening recently closed tabs, or opening tabs from other devices. - // Only the current navigation should have a timestamp in those cases. - timestamp = base::Time(); - } else { - timestamp = default_navigation_timestamp; - } - session_tab->navigations.push_back( - TabNavigation::FromNavigationEntry(i, *entry, timestamp)); + TabNavigation::FromNavigationEntry(i, *entry)); } } session_tab->session_storage_persistent_id.clear(); diff --git a/chrome/browser/sync/glue/session_model_associator.h b/chrome/browser/sync/glue/session_model_associator.h index 00408fa..0a61fa1 100644 --- a/chrome/browser/sync/glue/session_model_associator.h +++ b/chrome/browser/sync/glue/session_model_associator.h @@ -419,15 +419,10 @@ class SessionModelAssociator // if no page is found to be referring to the favicon anymore. void DecrementAndCleanFaviconForURL(const std::string& page_url); - // Helper method to update the given session tab from the given - // delegate (preserving old timestamps as necessary). - // - // TODO(akalin): Remove |default_navigation_timestamp| once we have - // a timestamp in NavigationEntry. - static void UpdateSessionTabFromDelegate( + // Set |session_tab| from |tab_delegate| and |mtime|. + static void SetSessionTabFromDelegate( const SyncedTabDelegate& tab_delegate, base::Time mtime, - base::Time default_navigation_timestamp, SessionTab* session_tab); // Load the favicon for the tab specified by |tab_link|. Will cancel any diff --git a/chrome/browser/sync/glue/session_model_associator_unittest.cc b/chrome/browser/sync/glue/session_model_associator_unittest.cc index 650ffab..af9bae7 100644 --- a/chrome/browser/sync/glue/session_model_associator_unittest.cc +++ b/chrome/browser/sync/glue/session_model_associator_unittest.cc @@ -62,15 +62,13 @@ class SyncSessionModelAssociatorTest : public testing::Test { return SessionModelAssociator::GetCurrentVirtualURL(tab_delegate); } - static void UpdateSessionTabFromDelegate( + static void SetSessionTabFromDelegate( const SyncedTabDelegate& tab_delegate, base::Time mtime, - base::Time default_navigation_timestamp, SessionTab* session_tab) { - SessionModelAssociator::UpdateSessionTabFromDelegate( + SessionModelAssociator::SetSessionTabFromDelegate( tab_delegate, mtime, - default_navigation_timestamp, session_tab); } @@ -549,26 +547,27 @@ const base::Time kTime1 = base::Time::FromInternalValue(100); const base::Time kTime2 = base::Time::FromInternalValue(105); const base::Time kTime3 = base::Time::FromInternalValue(110); const base::Time kTime4 = base::Time::FromInternalValue(120); -const base::Time kTime5 = base::Time::FromInternalValue(150); -const base::Time kTime6 = base::Time::FromInternalValue(200); -const base::Time kTime7 = base::Time::FromInternalValue(500); -const base::Time kTime8 = base::Time::FromInternalValue(1000); - -// Ensure new tabs have the current timestamp set for the current -// navigation, while other navigations have null timestamps. -TEST_F(SyncSessionModelAssociatorTest, UpdateNewTab) { +const base::Time kTime5 = base::Time::FromInternalValue(130); + +// Populate the mock tab delegate with some data and navigation +// entries and make sure that setting a SessionTab from it preserves +// those entries (and clobbers any existing data). +TEST_F(SyncSessionModelAssociatorTest, SetSessionTabFromDelegate) { // Create a tab with three valid entries. NiceMock<SyncedTabDelegateMock> tab_mock; EXPECT_CALL(tab_mock, GetSessionId()).WillRepeatedly(Return(0)); scoped_ptr<content::NavigationEntry> entry1( content::NavigationEntry::Create()); entry1->SetVirtualURL(GURL("http://www.google.com")); + entry1->SetTimestamp(kTime1); scoped_ptr<content::NavigationEntry> entry2( content::NavigationEntry::Create()); entry2->SetVirtualURL(GURL("http://www.noodle.com")); + entry2->SetTimestamp(kTime2); scoped_ptr<content::NavigationEntry> entry3( content::NavigationEntry::Create()); entry3->SetVirtualURL(GURL("http://www.doodle.com")); + entry3->SetTimestamp(kTime3); EXPECT_CALL(tab_mock, GetCurrentEntryIndex()).WillRepeatedly(Return(2)); EXPECT_CALL(tab_mock, GetEntryAtIndex(0)).WillRepeatedly( Return(entry1.get())); @@ -580,7 +579,19 @@ TEST_F(SyncSessionModelAssociatorTest, UpdateNewTab) { EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1)); SessionTab session_tab; - UpdateSessionTabFromDelegate(tab_mock, kTime1, kTime2, &session_tab); + session_tab.window_id.set_id(1); + session_tab.tab_id.set_id(1); + session_tab.tab_visual_index = 1; + session_tab.current_navigation_index = 1; + session_tab.pinned = true; + session_tab.extension_app_id = "app id"; + session_tab.user_agent_override = "override"; + session_tab.timestamp = kTime5; + session_tab.navigations.push_back( + SessionTypesTestHelper::CreateNavigation( + "http://www.example.com", "Example")); + session_tab.session_storage_persistent_id = "persistent id"; + SetSessionTabFromDelegate(tab_mock, kTime4, &session_tab); EXPECT_EQ(0, session_tab.window_id.id()); EXPECT_EQ(0, session_tab.tab_id.id()); @@ -589,7 +600,7 @@ TEST_F(SyncSessionModelAssociatorTest, UpdateNewTab) { EXPECT_FALSE(session_tab.pinned); EXPECT_TRUE(session_tab.extension_app_id.empty()); EXPECT_TRUE(session_tab.user_agent_override.empty()); - EXPECT_EQ(kTime1, session_tab.timestamp); + EXPECT_EQ(kTime4, session_tab.timestamp); ASSERT_EQ(3u, session_tab.navigations.size()); EXPECT_EQ(entry1->GetVirtualURL(), session_tab.navigations[0].virtual_url()); @@ -597,197 +608,12 @@ TEST_F(SyncSessionModelAssociatorTest, UpdateNewTab) { session_tab.navigations[1].virtual_url()); EXPECT_EQ(entry3->GetVirtualURL(), session_tab.navigations[2].virtual_url()); - EXPECT_EQ(2, session_tab.current_navigation_index); - EXPECT_TRUE(session_tab.navigations[0].timestamp().is_null()); - EXPECT_TRUE(session_tab.navigations[1].timestamp().is_null()); - EXPECT_EQ(kTime2, session_tab.navigations[2].timestamp()); + EXPECT_EQ(kTime1, session_tab.navigations[0].timestamp()); + EXPECT_EQ(kTime2, session_tab.navigations[1].timestamp()); + EXPECT_EQ(kTime3, session_tab.navigations[2].timestamp()); EXPECT_TRUE(session_tab.session_storage_persistent_id.empty()); } -// Ensure we preserve old timestamps when the entries don't change. -TEST_F(SyncSessionModelAssociatorTest, UpdateExistingTab) { - // Create a tab with three valid entries. - NiceMock<SyncedTabDelegateMock> tab_mock; - EXPECT_CALL(tab_mock, GetSessionId()).WillRepeatedly(Return(0)); - scoped_ptr<content::NavigationEntry> entry1( - content::NavigationEntry::Create()); - entry1->SetVirtualURL(GURL("http://www.google.com")); - scoped_ptr<content::NavigationEntry> entry2( - content::NavigationEntry::Create()); - entry2->SetVirtualURL(GURL("http://www.noodle.com")); - scoped_ptr<content::NavigationEntry> entry3( - content::NavigationEntry::Create()); - entry3->SetVirtualURL(GURL("http://www.doodle.com")); - EXPECT_CALL(tab_mock, GetCurrentEntryIndex()).WillRepeatedly(Return(2)); - EXPECT_CALL(tab_mock, GetEntryAtIndex(0)).WillRepeatedly( - Return(entry1.get())); - EXPECT_CALL(tab_mock, GetEntryAtIndex(1)).WillRepeatedly( - Return(entry2.get())); - EXPECT_CALL(tab_mock, GetEntryAtIndex(2)).WillRepeatedly( - Return(entry3.get())); - EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(3)); - EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1)); - EXPECT_CALL(tab_mock, IsPinned()).WillRepeatedly(Return(true)); - - // The initial UpdateSessionTabFromDelegate call builds the session_tab. - SessionTab session_tab; - UpdateSessionTabFromDelegate(tab_mock, kTime1, kTime2, &session_tab); - - // Tweak the timestamps a bit. - ASSERT_EQ(3u, session_tab.navigations.size()); - SessionTypesTestHelper::SetTimestamp(&session_tab.navigations[0], kTime3); - SessionTypesTestHelper::SetTimestamp(&session_tab.navigations[1], kTime4); - SessionTypesTestHelper::SetTimestamp(&session_tab.navigations[2], kTime5); - - // Now re-associate with the same data. - UpdateSessionTabFromDelegate(tab_mock, kTime3, kTime4, &session_tab); - - EXPECT_TRUE(session_tab.pinned); - EXPECT_EQ(kTime3, session_tab.timestamp); - EXPECT_EQ(entry1->GetVirtualURL(), - session_tab.navigations[0].virtual_url()); - EXPECT_EQ(entry2->GetVirtualURL(), - session_tab.navigations[1].virtual_url()); - EXPECT_EQ(entry3->GetVirtualURL(), - session_tab.navigations[2].virtual_url()); - EXPECT_EQ(2, session_tab.current_navigation_index); - EXPECT_EQ(kTime3, session_tab.navigations[0].timestamp()); - EXPECT_EQ(kTime4, session_tab.navigations[1].timestamp()); - EXPECT_EQ(kTime5, session_tab.navigations[2].timestamp()); -} - -// Ensure we add a fresh timestamp for new entries appended to the end. -TEST_F(SyncSessionModelAssociatorTest, UpdateAppendedTab) { - // Create a tab with three valid entries. - NiceMock<SyncedTabDelegateMock> tab_mock; - EXPECT_CALL(tab_mock, GetSessionId()).WillRepeatedly(Return(0)); - scoped_ptr<content::NavigationEntry> entry1( - content::NavigationEntry::Create()); - entry1->SetVirtualURL(GURL("http://www.google.com")); - scoped_ptr<content::NavigationEntry> entry2( - content::NavigationEntry::Create()); - entry2->SetVirtualURL(GURL("http://www.noodle.com")); - scoped_ptr<content::NavigationEntry> entry3( - content::NavigationEntry::Create()); - entry3->SetVirtualURL(GURL("http://www.doodle.com")); - EXPECT_CALL(tab_mock, GetCurrentEntryIndex()).WillRepeatedly(Return(2)); - EXPECT_CALL(tab_mock, GetEntryAtIndex(0)).WillRepeatedly( - Return(entry1.get())); - EXPECT_CALL(tab_mock, GetEntryAtIndex(1)).WillRepeatedly( - Return(entry2.get())); - EXPECT_CALL(tab_mock, GetEntryAtIndex(2)).WillRepeatedly( - Return(entry3.get())); - EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(3)); - EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1)); - - // The initial UpdateSessionTabFromDelegate call builds the session_tab. - SessionTab session_tab; - UpdateSessionTabFromDelegate(tab_mock, kTime1, kTime2, &session_tab); - - // Add a new entry and change the current navigation index. - scoped_ptr<content::NavigationEntry> entry4( - content::NavigationEntry::Create()); - entry4->SetVirtualURL(GURL("http://www.poodle.com")); - EXPECT_CALL(tab_mock, GetEntryAtIndex(3)).WillRepeatedly( - Return(entry4.get())); - EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(4)); - EXPECT_CALL(tab_mock, GetCurrentEntryIndex()).WillRepeatedly(Return(3)); - - // Now re-associate with the new version. - UpdateSessionTabFromDelegate(tab_mock, kTime3, kTime4, &session_tab); - - ASSERT_EQ(4u, session_tab.navigations.size()); - EXPECT_EQ(entry1->GetVirtualURL(), - session_tab.navigations[0].virtual_url()); - EXPECT_EQ(entry2->GetVirtualURL(), - session_tab.navigations[1].virtual_url()); - EXPECT_EQ(entry3->GetVirtualURL(), - session_tab.navigations[2].virtual_url()); - EXPECT_EQ(entry4->GetVirtualURL(), - session_tab.navigations[3].virtual_url()); - EXPECT_EQ(3, session_tab.current_navigation_index); - EXPECT_TRUE(session_tab.navigations[0].timestamp().is_null()); - EXPECT_TRUE(session_tab.navigations[1].timestamp().is_null()); - EXPECT_EQ(kTime2, session_tab.navigations[2].timestamp()); - EXPECT_EQ(kTime4, session_tab.navigations[3].timestamp()); -} - -// We shouldn't get confused when old/new entries from the previous tab have -// been pruned in the new tab. Timestamps for old entries we move back to in the -// navigation stack should be refreshed. -TEST_F(SyncSessionModelAssociatorTest, UpdatePrunedTab) { - // Create a tab with four valid entries. - NiceMock<SyncedTabDelegateMock> tab_mock; - EXPECT_CALL(tab_mock, GetSessionId()).WillRepeatedly(Return(0)); - scoped_ptr<content::NavigationEntry> entry1( - content::NavigationEntry::Create()); - entry1->SetVirtualURL(GURL("http://www.google.com")); - scoped_ptr<content::NavigationEntry> entry2( - content::NavigationEntry::Create()); - entry2->SetVirtualURL(GURL("http://www.noodle.com")); - scoped_ptr<content::NavigationEntry> entry3( - content::NavigationEntry::Create()); - entry3->SetVirtualURL(GURL("http://www.doodle.com")); - scoped_ptr<content::NavigationEntry> entry4( - content::NavigationEntry::Create()); - entry4->SetVirtualURL(GURL("http://www.poodle.com")); - EXPECT_CALL(tab_mock, GetCurrentEntryIndex()).WillRepeatedly(Return(3)); - EXPECT_CALL(tab_mock, GetEntryAtIndex(0)).WillRepeatedly( - Return(entry1.get())); - EXPECT_CALL(tab_mock, GetEntryAtIndex(1)).WillRepeatedly( - Return(entry2.get())); - EXPECT_CALL(tab_mock, GetEntryAtIndex(2)).WillRepeatedly( - Return(entry3.get())); - EXPECT_CALL(tab_mock, GetEntryAtIndex(3)).WillRepeatedly( - Return(entry4.get())); - EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(4)); - EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1)); - - // The initial UpdateSessionTabFromDelegate call builds the session_tab. - SessionTab session_tab; - UpdateSessionTabFromDelegate(tab_mock, kTime1, kTime2, &session_tab); - - // Reset new tab to have the oldest entry pruned, the current navigation - // set to entry3, and a new entry added in place of entry4. - testing::Mock::VerifyAndClearExpectations(&tab_mock); - EXPECT_CALL(tab_mock, GetCurrentEntryIndex()).WillRepeatedly(Return(1)); - EXPECT_CALL(tab_mock, GetEntryAtIndex(0)).WillRepeatedly( - Return(entry2.get())); - EXPECT_CALL(tab_mock, GetEntryAtIndex(1)).WillRepeatedly( - Return(entry3.get())); - scoped_ptr<content::NavigationEntry> entry5( - content::NavigationEntry::Create()); - entry5->SetVirtualURL(GURL("http://www.noogle.com")); - EXPECT_CALL(tab_mock, GetEntryAtIndex(2)).WillRepeatedly( - Return(entry5.get())); - EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(3)); - EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1)); - - // Tweak the timestamps a bit. - ASSERT_EQ(4u, session_tab.navigations.size()); - SessionTypesTestHelper::SetTimestamp(&session_tab.navigations[0], kTime3); - SessionTypesTestHelper::SetTimestamp(&session_tab.navigations[1], kTime4); - SessionTypesTestHelper::SetTimestamp(&session_tab.navigations[2], kTime5); - SessionTypesTestHelper::SetTimestamp(&session_tab.navigations[3], kTime6); - - // Now re-associate with the new version. - UpdateSessionTabFromDelegate(tab_mock, kTime7, kTime8, &session_tab); - - // Only entry2's and entry3's timestamps should be preserved. The new - // entry should have a new timestamp. - ASSERT_EQ(3u, session_tab.navigations.size()); - EXPECT_EQ(entry2->GetVirtualURL(), - session_tab.navigations[0].virtual_url()); - EXPECT_EQ(entry3->GetVirtualURL(), - session_tab.navigations[1].virtual_url()); - EXPECT_EQ(entry5->GetVirtualURL(), - session_tab.navigations[2].virtual_url()); - EXPECT_EQ(1, session_tab.current_navigation_index); - EXPECT_EQ(kTime4, session_tab.navigations[0].timestamp()); - EXPECT_EQ(kTime5, session_tab.navigations[1].timestamp()); - EXPECT_EQ(kTime8, session_tab.navigations[2].timestamp()); -} - } // namespace } // namespace browser_sync diff --git a/chrome/browser/sync/test/integration/sessions_helper.cc b/chrome/browser/sync/test/integration/sessions_helper.cc index 7a0b916..706a3b2 100644 --- a/chrome/browser/sync/test/integration/sessions_helper.cc +++ b/chrome/browser/sync/test/integration/sessions_helper.cc @@ -90,8 +90,9 @@ bool ModelAssociatorHasTabWithUrl(int index, const GURL& url) { nav = (*tab_it)->navigations[nav_index]; if (nav.virtual_url() == url) { DVLOG(1) << "Found tab with url " << url.spec(); + DVLOG(1) << "Timestamp is " << nav.timestamp().ToInternalValue(); if (nav.title().empty()) { - DVLOG(1) << "No title!"; + DVLOG(1) << "Title empty -- tab hasn't finished loading yet"; continue; } return true; diff --git a/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc b/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc index f2bddd0..f54876b 100644 --- a/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc +++ b/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc @@ -3,18 +3,24 @@ // found in the LICENSE file. #include "base/memory/scoped_vector.h" +#include "chrome/browser/history/history_types.h" #include "chrome/browser/sessions/session_service.h" +#include "chrome/browser/sessions/session_types.h" #include "chrome/browser/sync/profile_sync_service_harness.h" -#include "chrome/browser/sync/test/integration/sync_test.h" #include "chrome/browser/sync/test/integration/sessions_helper.h" +#include "chrome/browser/sync/test/integration/sync_test.h" +#include "chrome/browser/sync/test/integration/typed_urls_helper.h" +#include "sync/util/time.h" using sessions_helper::CheckInitialState; using sessions_helper::GetLocalWindows; using sessions_helper::GetSessionData; using sessions_helper::OpenTabAndGetLocalWindows; using sessions_helper::ScopedWindowMap; +using sessions_helper::SessionWindowMap; using sessions_helper::SyncedSessionVector; using sessions_helper::WindowsMatch; +using typed_urls_helper::GetUrlFromClient; class SingleClientSessionsSyncTest : public SyncTest { public: @@ -55,3 +61,37 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, MAYBE_Sanity) { ASSERT_TRUE(GetLocalWindows(0, new_windows.GetMutable())); ASSERT_TRUE(WindowsMatch(*old_windows.Get(), *new_windows.Get())); } + +IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, TimestampMatchesHistory) { + ASSERT_TRUE(SetupSync()) << "SetupSync() failed."; + + ASSERT_TRUE(CheckInitialState(0)); + + // We want a URL that doesn't 404 and has a non-empty title. + // about:version is simple to render, too. + const GURL url("about:version"); + + ScopedWindowMap windows; + ASSERT_TRUE(OpenTabAndGetLocalWindows(0, url, windows.GetMutable())); + + int found_navigations = 0; + for (SessionWindowMap::const_iterator it = windows.Get()->begin(); + it != windows.Get()->end(); ++it) { + for (std::vector<SessionTab*>::const_iterator it2 = + it->second->tabs.begin(); it2 != it->second->tabs.end(); ++it2) { + for (std::vector<TabNavigation>::const_iterator it3 = + (*it2)->navigations.begin(); + it3 != (*it2)->navigations.end(); ++it3) { + const base::Time timestamp = it3->timestamp(); + + history::URLRow virtual_row; + ASSERT_TRUE(GetUrlFromClient(0, it3->virtual_url(), &virtual_row)); + const base::Time history_timestamp = virtual_row.last_visit(); + + ASSERT_EQ(timestamp, history_timestamp); + ++found_navigations; + } + } + } + ASSERT_EQ(1, found_navigations); +} |