diff options
Diffstat (limited to 'chrome/browser/sync/glue')
5 files changed, 117 insertions, 14 deletions
diff --git a/chrome/browser/sync/glue/session_change_processor.cc b/chrome/browser/sync/glue/session_change_processor.cc index f08b4d3..34f850b 100644 --- a/chrome/browser/sync/glue/session_change_processor.cc +++ b/chrome/browser/sync/glue/session_change_processor.cc @@ -20,6 +20,7 @@ #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/common/chrome_notification_types.h" #include "content/public/browser/navigation_controller.h" +#include "content/public/browser/navigation_entry.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_source.h" @@ -33,6 +34,10 @@ namespace browser_sync { namespace { +// The URL at which the set of synced tabs is displayed. We treat it differently +// from all other URL's as accessing it triggers a sync refresh of Sessions. +static const char kNTPOpenTabSyncURL[] = "chrome://newtab/#opentabs"; + // Extract the source SyncedTabDelegate from a NotificationSource originating // from a NavigationController, if it exists. Returns |NULL| otherwise. SyncedTabDelegate* ExtractSyncedTabDelegate( @@ -186,6 +191,25 @@ void SessionChangeProcessor::Observe( break; } + // Check if this tab should trigger a session sync refresh. By virtue of + // it being a modified tab, we know the tab is active (so we won't do + // refreshes just because the refresh page is open in a background tab). + if (!modified_tabs.empty()) { + SyncedTabDelegate* tab = modified_tabs.front(); + const content::NavigationEntry* entry = tab->GetActiveEntry(); + if (!tab->IsBeingDestroyed() && + entry && + entry->GetVirtualURL().is_valid() && + entry->GetVirtualURL().spec() == kNTPOpenTabSyncURL) { + DVLOG(1) << "Triggering sync refresh for sessions datatype."; + const syncable::ModelType type = syncable::SESSIONS; + content::NotificationService::current()->Notify( + chrome::NOTIFICATION_SYNC_REFRESH, + content::Source<Profile>(profile_), + content::Details<const syncable::ModelType>(&type)); + } + } + // Associate tabs first so the synced session tracker is aware of them. // Note that if we fail to associate, it means something has gone wrong, // such as our local session being deleted, so we disassociate and associate diff --git a/chrome/browser/sync/glue/session_change_processor.h b/chrome/browser/sync/glue/session_change_processor.h index d6cc011..392e839 100644 --- a/chrome/browser/sync/glue/session_change_processor.h +++ b/chrome/browser/sync/glue/session_change_processor.h @@ -31,6 +31,7 @@ class SessionChangeProcessor : public ChangeProcessor, SessionChangeProcessor( UnrecoverableErrorHandler* error_handler, SessionModelAssociator* session_model_associator); + // For testing only. SessionChangeProcessor( UnrecoverableErrorHandler* error_handler, SessionModelAssociator* session_model_associator, @@ -56,12 +57,14 @@ class SessionChangeProcessor : public ChangeProcessor, private: friend class ScopedStopObserving<SessionChangeProcessor>; + void StartObserving(); void StopObserving(); + SessionModelAssociator* session_model_associator_; content::NotificationRegistrar notification_registrar_; - // Owner of the SessionService. Non-NULL iff |running()| is true. + // Profile being synced. Non-null if |running()| is true. Profile* profile_; // To bypass some checks/codepaths not applicable in tests. diff --git a/chrome/browser/sync/glue/session_model_associator.cc b/chrome/browser/sync/glue/session_model_associator.cc index 1ce4f84..f8aa61d 100644 --- a/chrome/browser/sync/glue/session_model_associator.cc +++ b/chrome/browser/sync/glue/session_model_associator.cc @@ -65,9 +65,11 @@ SessionModelAssociator::SessionModelAssociator(ProfileSyncService* sync_service) stale_session_threshold_days_(kDefaultStaleSessionThresholdDays), setup_for_test_(false), waiting_for_change_(false), - ALLOW_THIS_IN_INITIALIZER_LIST(test_weak_factory_(this)) { + ALLOW_THIS_IN_INITIALIZER_LIST(test_weak_factory_(this)), + profile_(sync_service->profile()) { DCHECK(CalledOnValidThread()); DCHECK(sync_service_); + DCHECK(profile_); } SessionModelAssociator::SessionModelAssociator(ProfileSyncService* sync_service, @@ -78,8 +80,11 @@ SessionModelAssociator::SessionModelAssociator(ProfileSyncService* sync_service, stale_session_threshold_days_(kDefaultStaleSessionThresholdDays), setup_for_test_(setup_for_test), waiting_for_change_(false), - ALLOW_THIS_IN_INITIALIZER_LIST(test_weak_factory_(this)) { + ALLOW_THIS_IN_INITIALIZER_LIST(test_weak_factory_(this)), + profile_(sync_service->profile()) { DCHECK(CalledOnValidThread()); + DCHECK(sync_service_); + DCHECK(profile_); } SessionModelAssociator::~SessionModelAssociator() { @@ -953,6 +958,15 @@ void SessionModelAssociator::TabNodePool::FreeTabNode(int64 sync_id) { tab_syncid_pool_[static_cast<size_t>(++tab_pool_fp_)] = sync_id; } +void SessionModelAssociator::AttemptSessionsDataRefresh() const { + DVLOG(1) << "Triggering sync refresh for sessions datatype."; + const syncable::ModelType type = syncable::SESSIONS; + content::NotificationService::current()->Notify( + chrome::NOTIFICATION_SYNC_REFRESH, + content::Source<Profile>(profile_), + content::Details<const syncable::ModelType>(&type)); +} + bool SessionModelAssociator::GetLocalSession( const SyncedSession* * local_session) { DCHECK(CalledOnValidThread()); diff --git a/chrome/browser/sync/glue/session_model_associator.h b/chrome/browser/sync/glue/session_model_associator.h index 76b2345..9237626 100644 --- a/chrome/browser/sync/glue/session_model_associator.h +++ b/chrome/browser/sync/glue/session_model_associator.h @@ -153,6 +153,12 @@ class SessionModelAssociator // found for that session. bool DisassociateForeignSession(const std::string& foreign_session_tag); + // Attempts to asynchronously refresh the sessions sync data. If new data is + // received, the FOREIGN_SESSIONS_UPDATED notification is sent. No + // notification will be sent otherwise. This method is not guaranteed to + // trigger a sync cycle. + void AttemptSessionsDataRefresh() const; + // Sets |*local_session| to point to the associator's representation of the // local machine. Used primarily for testing. bool GetLocalSession(const SyncedSession* * local_session); @@ -211,8 +217,7 @@ class SessionModelAssociator WriteForeignSessionToNode); FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest, TabNodePoolEmpty); FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest, TabNodePoolNonEmpty); - FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest, - ValidTabs); + FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest, ValidTabs); FRIEND_TEST_ALL_PREFIXES(SessionModelAssociatorTest, PopulateSessionHeader); FRIEND_TEST_ALL_PREFIXES(SessionModelAssociatorTest, PopulateSessionWindow); FRIEND_TEST_ALL_PREFIXES(SessionModelAssociatorTest, PopulateSessionTab); @@ -449,6 +454,9 @@ class SessionModelAssociator bool waiting_for_change_; base::WeakPtrFactory<SessionModelAssociator> test_weak_factory_; + // Profile being synced. + const Profile* const profile_; + DISALLOW_COPY_AND_ASSIGN(SessionModelAssociator); }; diff --git a/chrome/browser/sync/glue/session_model_associator_unittest.cc b/chrome/browser/sync/glue/session_model_associator_unittest.cc index e22e3a5..fa951bd 100644 --- a/chrome/browser/sync/glue/session_model_associator_unittest.cc +++ b/chrome/browser/sync/glue/session_model_associator_unittest.cc @@ -11,7 +11,13 @@ #include "chrome/browser/sync/glue/session_model_associator.h" #include "chrome/browser/sync/glue/synced_tab_delegate.h" #include "chrome/browser/sync/protocol/session_specifics.pb.h" +#include "chrome/browser/sync/profile_sync_service_mock.h" +#include "chrome/common/chrome_notification_types.h" #include "chrome/common/url_constants.h" +#include "chrome/test/base/profile_mock.h" +#include "content/public/browser/navigation_entry.h" +#include "content/public/browser/notification_service.h" +#include "content/public/browser/notification_details.h" #include "content/public/browser/navigation_entry.h" #include "content/public/common/page_transition_types.h" #include "content/test/test_browser_thread.h" @@ -24,7 +30,25 @@ using testing::Return; namespace browser_sync { -typedef testing::Test SessionModelAssociatorTest; +class SessionModelAssociatorTest : public testing::Test { + public: + SessionModelAssociatorTest() + : ui_thread_(BrowserThread::UI, &message_loop_), + sync_service_(&profile_) {} + virtual void SetUp() OVERRIDE { + model_associator_.reset(new SessionModelAssociator(&sync_service_, true)); + } + virtual void TearDown() OVERRIDE { + model_associator_.reset(); + } + + protected: + MessageLoopForUI message_loop_; + content::TestBrowserThread ui_thread_; + NiceMock<ProfileMock> profile_; + NiceMock<ProfileSyncServiceMock> sync_service_; + scoped_ptr<SessionModelAssociator> model_associator_; +}; TEST_F(SessionModelAssociatorTest, SessionWindowHasNoTabsToSync) { SessionWindow win; @@ -208,13 +232,43 @@ class SyncedTabDelegateMock : public SyncedTabDelegate { MOCK_CONST_METHOD0(GetActiveEntry, content::NavigationEntry*()); }; +class SyncRefreshListener : public content::NotificationObserver { + public: + SyncRefreshListener() : notified_of_refresh_(false) { + registrar_.Add(this, chrome::NOTIFICATION_SYNC_REFRESH, + content::NotificationService::AllSources()); + } + + void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) { + if (type == chrome::NOTIFICATION_SYNC_REFRESH) { + notified_of_refresh_ = true; + } + } + + bool notified_of_refresh() const { return notified_of_refresh_; } + + private: + bool notified_of_refresh_; + content::NotificationRegistrar registrar_; +}; + } // namespace. -TEST_F(SessionModelAssociatorTest, ValidTabs) { - MessageLoopForUI message_loop; - content::TestBrowserThread ui_thread(BrowserThread::UI, &message_loop); - SessionModelAssociator model_associator(NULL, true); +// Test that AttemptSessionsDataRefresh() triggers the NOTIFICATION_SYNC_REFRESH +// notification. +TEST_F(SessionModelAssociatorTest, TriggerSessionRefresh) { + SyncRefreshListener refresh_listener; + EXPECT_FALSE(refresh_listener.notified_of_refresh()); + model_associator_->AttemptSessionsDataRefresh(); + EXPECT_TRUE(refresh_listener.notified_of_refresh()); +} + +// Test that we exclude tabs with only chrome:// and file:// schemed navigations +// from ShouldSyncTab(..). +TEST_F(SessionModelAssociatorTest, ValidTabs) { NiceMock<SyncedTabDelegateMock> tab_mock; // A null entry shouldn't crash. @@ -223,7 +277,7 @@ TEST_F(SessionModelAssociatorTest, ValidTabs) { Return((content::NavigationEntry *)NULL)); EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(1)); EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1)); - EXPECT_FALSE(model_associator.ShouldSyncTab(tab_mock)); + EXPECT_FALSE(model_associator_->ShouldSyncTab(tab_mock)); // A chrome:// entry isn't valid. scoped_ptr<content::NavigationEntry> entry( @@ -234,7 +288,7 @@ TEST_F(SessionModelAssociatorTest, ValidTabs) { EXPECT_CALL(tab_mock, GetEntryAtIndex(0)).WillRepeatedly(Return(entry.get())); EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(1)); EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1)); - EXPECT_FALSE(model_associator.ShouldSyncTab(tab_mock)); + EXPECT_FALSE(model_associator_->ShouldSyncTab(tab_mock)); // A file:// entry isn't valid, even in addition to another entry. scoped_ptr<content::NavigationEntry> entry2( @@ -247,7 +301,7 @@ TEST_F(SessionModelAssociatorTest, ValidTabs) { Return(entry2.get())); EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(2)); EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1)); - EXPECT_FALSE(model_associator.ShouldSyncTab(tab_mock)); + EXPECT_FALSE(model_associator_->ShouldSyncTab(tab_mock)); // Add a valid scheme entry to tab, making the tab valid. scoped_ptr<content::NavigationEntry> entry3( @@ -263,7 +317,7 @@ TEST_F(SessionModelAssociatorTest, ValidTabs) { Return(entry3.get())); EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(3)); EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1)); - EXPECT_TRUE(model_associator.ShouldSyncTab(tab_mock)); + EXPECT_TRUE(model_associator_->ShouldSyncTab(tab_mock)); } } // namespace browser_sync |