Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2184)

Unified Diff: chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc

Issue 2683263002: Reland v4 of Session refactor (Closed)
Patch Set: Better handle possible corruption in sync node and add some testing Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | components/sync_sessions/sessions_sync_manager.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc
diff --git a/chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc b/chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc
index 886c2a4bf72a1cf615ff1244ab5abf6a85fbddbd..f4c49e40b063816aedb97b9ce2c536a86abd5808 100644
--- a/chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc
+++ b/chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc
@@ -48,6 +48,17 @@ namespace sync_sessions {
namespace {
+const char kFoo1[] = "http://foo1/";
+const char kFoo2[] = "http://foo2/";
+const char kBar1[] = "http://bar1/";
+const char kBar2[] = "http://bar2/";
+const char kBaz1[] = "http://baz1/";
+const char kBaz2[] = "http://baz2/";
+
+std::string TabNodeIdToTag(const std::string& machine_tag, int tab_node_id) {
+ return base::StringPrintf("%s %d", machine_tag.c_str(), tab_node_id);
+}
+
class SessionNotificationObserver {
public:
SessionNotificationObserver()
@@ -128,7 +139,7 @@ class SyncedWindowDelegateOverride : public SyncedWindowDelegate {
std::map<int, SyncedTabDelegate*> tab_overrides_;
std::map<int, SessionID::id_type> tab_id_overrides_;
const SyncedWindowDelegate* const wrapped_;
- SessionID::id_type session_id_override_ = -1;
+ SessionID::id_type session_id_override_ = TabNodePool::kInvalidTabID;
};
class TestSyncedWindowDelegatesGetter : public SyncedWindowDelegatesGetter {
@@ -399,6 +410,10 @@ class SessionsSyncManagerTest
return sessions_client_shim_.get();
}
+ TabNodePool* GetTabPool() {
+ return &manager()->session_tracker_.local_tab_pool_;
+ }
+
syncer::SyncPrefs* sync_prefs() { return sync_prefs_.get(); }
SyncedWindowDelegatesGetter* get_synced_window_getter() {
@@ -502,40 +517,32 @@ TEST_F(SessionsSyncManagerTest, PopulateSessionWindow) {
namespace {
+// A SyncedTabDelegate fake for testing. It simulates a normal
+// SyncedTabDelegate with a proper WebContents. For a SyncedTabDelegate without
+// a WebContents, see PlaceholderTabDelegate below.
class SyncedTabDelegateFake : public SyncedTabDelegate {
public:
- SyncedTabDelegateFake()
- : current_entry_index_(0), is_supervised_(false), sync_id_(-1) {}
+ SyncedTabDelegateFake() {}
~SyncedTabDelegateFake() override {}
+ // SyncedTabDelegate overrides.
bool IsInitialBlankNavigation() const override {
// This differs from NavigationControllerImpl, which has an initial blank
// NavigationEntry.
return GetEntryCount() == 0;
}
int GetCurrentEntryIndex() const override { return current_entry_index_; }
- void set_current_entry_index(int i) {
- current_entry_index_ = i;
- }
-
- void AppendEntry(std::unique_ptr<content::NavigationEntry> entry) {
- entries_.push_back(std::move(entry));
- }
-
GURL GetVirtualURLAtIndex(int i) const override {
if (static_cast<size_t>(i) >= entries_.size())
return GURL();
return entries_[i]->GetVirtualURL();
}
-
GURL GetFaviconURLAtIndex(int i) const override { return GURL(); }
-
ui::PageTransition GetTransitionAtIndex(int i) const override {
if (static_cast<size_t>(i) >= entries_.size())
return ui::PAGE_TRANSITION_LINK;
return entries_[i]->GetTransitionType();
}
-
void GetSerializedNavigationAtIndex(
int i,
sessions::SerializedNavigationEntry* serialized_entry) const override {
@@ -545,17 +552,9 @@ class SyncedTabDelegateFake : public SyncedTabDelegate {
sessions::ContentSerializedNavigationBuilder::FromNavigationEntry(
i, *entries_[i]);
}
-
int GetEntryCount() const override { return entries_.size(); }
-
- SessionID::id_type GetWindowId() const override {
- return SessionID::id_type();
- }
-
- SessionID::id_type GetSessionId() const override {
- return SessionID::id_type();
- }
-
+ SessionID::id_type GetWindowId() const override { return window_id_; }
+ SessionID::id_type GetSessionId() const override { return tab_id_; }
bool IsBeingDestroyed() const override { return false; }
std::string GetExtensionAppId() const override { return std::string(); }
bool ProfileIsSupervised() const override { return is_supervised_; }
@@ -564,6 +563,23 @@ class SyncedTabDelegateFake : public SyncedTabDelegate {
GetBlockedNavigations() const override {
return &blocked_navigations_;
}
+ bool IsPlaceholderTab() const override { return false; }
+ int GetSyncId() const override { return sync_id_; }
+ void SetSyncId(int sync_id) override { sync_id_ = sync_id; }
+ bool ShouldSync(SyncSessionsClient* sessions_client) override {
+ return false;
+ }
+
+ void AppendEntry(std::unique_ptr<content::NavigationEntry> entry) {
+ entries_.push_back(std::move(entry));
+ }
+
+ void set_current_entry_index(int i) { current_entry_index_ = i; }
+
+ void SetWindowId(SessionID::id_type window_id) { window_id_ = window_id; }
+
+ void SetSessionId(SessionID::id_type id) { tab_id_ = id; }
+
void set_blocked_navigations(
std::vector<const content::NavigationEntry*>* navs) {
for (auto* entry : *navs) {
@@ -574,31 +590,101 @@ class SyncedTabDelegateFake : public SyncedTabDelegate {
blocked_navigations_.push_back(std::move(serialized_entry));
}
}
- bool IsPlaceholderTab() const override { return true; }
-
- // Session sync related methods.
- int GetSyncId() const override { return sync_id_; }
- void SetSyncId(int sync_id) override { sync_id_ = sync_id; }
-
- bool ShouldSync(SyncSessionsClient* sessions_client) override {
- return false;
- }
void reset() {
current_entry_index_ = 0;
- sync_id_ = -1;
+ sync_id_ = TabNodePool::kInvalidTabNodeID;
entries_.clear();
}
private:
- int current_entry_index_;
- bool is_supervised_;
- int sync_id_;
+ int current_entry_index_ = 0;
+ bool is_supervised_ = false;
+ int sync_id_ = -1;
+ SessionID::id_type tab_id_ = 0;
+ SessionID::id_type window_id_ = 0;
std::vector<std::unique_ptr<const sessions::SerializedNavigationEntry>>
blocked_navigations_;
std::vector<std::unique_ptr<content::NavigationEntry>> entries_;
};
+// A placeholder delegate. These delegates have no WebContents, simulating a tab
+// that has been restored without bringing its state fully into memory (for
+// example on Android), or where the tab's contents have been evicted from
+// memory. See SyncedTabDelegate::IsPlaceHolderTab for more info.
+class PlaceholderTabDelegate : public SyncedTabDelegate {
+ public:
+ PlaceholderTabDelegate(SessionID::id_type session_id, int sync_id)
+ : session_id_(session_id), sync_id_(sync_id) {}
+ ~PlaceholderTabDelegate() override {}
+
+ // SyncedTabDelegate overrides.
+ SessionID::id_type GetSessionId() const override { return session_id_; }
+ int GetSyncId() const override { return sync_id_; }
+ void SetSyncId(int sync_id) override { sync_id_ = sync_id; }
+ bool IsPlaceholderTab() const override { return true; }
+
+ // Everything else is invalid to invoke as it depends on a valid WebContents.
+ SessionID::id_type GetWindowId() const override {
+ NOTREACHED();
+ return 0;
+ }
+ bool IsBeingDestroyed() const override {
+ NOTREACHED();
+ return false;
+ }
+ std::string GetExtensionAppId() const override {
+ NOTREACHED();
+ return "";
+ }
+ bool IsInitialBlankNavigation() const override {
+ NOTREACHED();
+ return false;
+ }
+ int GetCurrentEntryIndex() const override {
+ NOTREACHED();
+ return 0;
+ }
+ int GetEntryCount() const override {
+ NOTREACHED();
+ return 0;
+ }
+ GURL GetVirtualURLAtIndex(int i) const override {
+ NOTREACHED();
+ return GURL();
+ }
+ GURL GetFaviconURLAtIndex(int i) const override {
+ NOTREACHED();
+ return GURL();
+ }
+ ui::PageTransition GetTransitionAtIndex(int i) const override {
+ NOTREACHED();
+ return ui::PageTransition();
+ }
+ void GetSerializedNavigationAtIndex(
+ int i,
+ sessions::SerializedNavigationEntry* serialized_entry) const override {
+ NOTREACHED();
+ }
+ bool ProfileIsSupervised() const override {
+ NOTREACHED();
+ return false;
+ }
+ const std::vector<std::unique_ptr<const sessions::SerializedNavigationEntry>>*
+ GetBlockedNavigations() const override {
+ NOTREACHED();
+ return nullptr;
+ }
+ bool ShouldSync(SyncSessionsClient* sessions_client) override {
+ NOTREACHED();
+ return false;
+ }
+
+ private:
+ SessionID::id_type session_id_;
+ int sync_id_;
+};
+
} // namespace
static const base::Time kTime0 = base::Time::FromInternalValue(100);
@@ -953,29 +1039,31 @@ TEST_F(SessionsSyncManagerTest, MergeLocalSessionNoTabs) {
// Ensure model association associates the pre-existing tabs.
TEST_F(SessionsSyncManagerTest, SwappedOutOnRestore) {
- AddTab(browser(), GURL("http://foo1"));
- NavigateAndCommitActiveTab(GURL("http://foo2"));
- AddTab(browser(), GURL("http://bar1"));
- NavigateAndCommitActiveTab(GURL("http://bar2"));
- AddTab(browser(), GURL("http://baz1"));
- NavigateAndCommitActiveTab(GURL("http://baz2"));
const int kRestoredTabId = 1337;
const int kNewTabId = 2468;
+ // AddTab inserts at index 0, so go in reverse order (tab 3 -> tab 1).
+ AddTab(browser(), GURL(kBaz1));
+ NavigateAndCommitActiveTab(GURL(kBaz2));
+ AddTab(browser(), GURL(kBar1));
+ NavigateAndCommitActiveTab(GURL(kBar2));
+ AddTab(browser(), GURL(kFoo1));
+ NavigateAndCommitActiveTab(GURL(kFoo2));
+
syncer::SyncDataList in;
syncer::SyncChangeList out;
InitWithSyncDataTakeOutput(in, &out);
- // Should be one header add, 3 tab add/update pairs, one header update.
- ASSERT_EQ(8U, out.size());
+ // Should be one header add, 3 tab adds, one header update.
+ ASSERT_EQ(5U, out.size());
// For input, we set up:
// * one "normal" fully loaded tab
- // * one "frozen" tab with no WebContents and a tab_id change
- // * one "frozen" tab with no WebContents and no tab_id change
- sync_pb::EntitySpecifics t0_entity = out[2].sync_data().GetSpecifics();
- sync_pb::EntitySpecifics t1_entity = out[4].sync_data().GetSpecifics();
- sync_pb::EntitySpecifics t2_entity = out[6].sync_data().GetSpecifics();
+ // * one placeholder tab with no WebContents and a tab_id change
+ // * one placeholder tab with no WebContents and no tab_id change
+ sync_pb::EntitySpecifics t0_entity = out[1].sync_data().GetSpecifics();
+ sync_pb::EntitySpecifics t1_entity = out[2].sync_data().GetSpecifics();
+ sync_pb::EntitySpecifics t2_entity = out[3].sync_data().GetSpecifics();
t1_entity.mutable_session()->mutable_tab()->set_tab_id(kRestoredTabId);
in.push_back(CreateRemoteData(t0_entity));
in.push_back(CreateRemoteData(t1_entity));
@@ -986,9 +1074,8 @@ TEST_F(SessionsSyncManagerTest, SwappedOutOnRestore) {
const std::set<const SyncedWindowDelegate*>& windows =
manager()->synced_window_delegates_getter()->GetSyncedWindowDelegates();
ASSERT_EQ(1U, windows.size());
- SyncedTabDelegateFake t1_override, t2_override;
- t1_override.SetSyncId(1); // No WebContents by default.
- t2_override.SetSyncId(2); // No WebContents by default.
+ PlaceholderTabDelegate t1_override(kNewTabId, 1);
+ PlaceholderTabDelegate t2_override(t2_entity.session().tab().tab_id(), 2);
SyncedWindowDelegateOverride window_override(*windows.begin());
window_override.OverrideTabAt(1, &t1_override, kNewTabId);
window_override.OverrideTabAt(2, &t2_override,
@@ -1005,26 +1092,39 @@ TEST_F(SessionsSyncManagerTest, SwappedOutOnRestore) {
std::unique_ptr<syncer::SyncErrorFactory>(
new syncer::SyncErrorFactoryMock()));
- // There should be two changes, one for the fully associated tab, and
- // one for the tab_id update to t1. t2 shouldn't need to be updated.
- ASSERT_EQ(2U, FilterOutLocalHeaderChanges(&out)->size());
+ // There should be three changes, one for the fully associated tab, and
+ // one each for the tab_id updates to t1 and t2.
+ ASSERT_EQ(3U, FilterOutLocalHeaderChanges(&out)->size());
EXPECT_EQ(SyncChange::ACTION_UPDATE, out[0].change_type());
+ EXPECT_EQ(kFoo2, out[0]
+ .sync_data()
+ .GetSpecifics()
+ .session()
+ .tab()
+ .navigation(1)
+ .virtual_url());
+
EXPECT_EQ(SyncChange::ACTION_UPDATE, out[1].change_type());
EXPECT_EQ(kNewTabId,
out[1].sync_data().GetSpecifics().session().tab().tab_id());
+ EXPECT_EQ(kBar2, out[1]
+ .sync_data()
+ .GetSpecifics()
+ .session()
+ .tab()
+ .navigation(1)
+ .virtual_url());
- // Verify TabLinks.
- SessionsSyncManager::TabLinksMap tab_map = manager()->local_tab_map_;
- ASSERT_EQ(3U, tab_map.size());
- int t2_tab_id = t2_entity.session().tab().tab_id();
- EXPECT_EQ(2, tab_map.find(t2_tab_id)->second->tab_node_id());
- EXPECT_EQ(1, tab_map.find(kNewTabId)->second->tab_node_id());
- int t0_tab_id = out[0].sync_data().GetSpecifics().session().tab().tab_id();
- EXPECT_EQ(0, tab_map.find(t0_tab_id)->second->tab_node_id());
- // TODO(tim): Once bug 337057 is fixed, we can issue an OnLocalTabModified
- // from here (using an override similar to above) to return a new tab id
- // and verify that we don't see any node creations in the SyncChangeProcessor
- // (similar to how SessionsSyncManagerTest.OnLocalTabModified works.)
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, out[2].change_type());
+ EXPECT_EQ(t2_entity.session().tab().tab_id(),
+ out[2].sync_data().GetSpecifics().session().tab().tab_id());
+ EXPECT_EQ(kBaz2, out[2]
+ .sync_data()
+ .GetSpecifics()
+ .session()
+ .tab()
+ .navigation(1)
+ .virtual_url());
}
// Ensure model association updates the window ID for tabs whose window's ID has
@@ -1035,21 +1135,22 @@ TEST_F(SessionsSyncManagerTest, WindowIdUpdatedOnRestore) {
syncer::SyncChangeList out;
// Set up one tab and start sync with it.
- AddTab(browser(), GURL("http://foo1"));
- NavigateAndCommitActiveTab(GURL("http://foo2"));
+ AddTab(browser(), GURL(kFoo1));
+ NavigateAndCommitActiveTab(GURL(kFoo2));
InitWithSyncDataTakeOutput(in, &out);
- // Should be one header add, 1 tab add/update pair, and one header update.
- ASSERT_EQ(4U, out.size());
- const sync_pb::EntitySpecifics t0_entity = out[2].sync_data().GetSpecifics();
+ // Should be one header add, 1 tab add, and one header update.
+ ASSERT_EQ(3U, out.size());
+ const sync_pb::EntitySpecifics t0_entity = out[1].sync_data().GetSpecifics();
+ ASSERT_TRUE(t0_entity.session().has_tab());
in.push_back(CreateRemoteData(t0_entity));
out.clear();
manager()->StopSyncing(syncer::SESSIONS);
- // SyncedTabDelegateFake is a placeholder (no WebContents) by default.
- SyncedTabDelegateFake t0_override;
- t0_override.SetSyncId(t0_entity.session().tab_node_id());
+ // Override the tab with a placeholder tab delegate.
+ PlaceholderTabDelegate t0_override(t0_entity.session().tab().tab_id(),
+ t0_entity.session().tab_node_id());
// Set up the window override with the new window ID and placeholder tab.
const std::set<const SyncedWindowDelegate*>& windows =
@@ -1078,6 +1179,64 @@ TEST_F(SessionsSyncManagerTest, WindowIdUpdatedOnRestore) {
EXPECT_EQ(SyncChange::ACTION_UPDATE, out[0].change_type());
EXPECT_EQ(kNewWindowId,
out[0].sync_data().GetSpecifics().session().tab().window_id());
+ EXPECT_EQ(kFoo2, out[0]
+ .sync_data()
+ .GetSpecifics()
+ .session()
+ .tab()
+ .navigation(1)
+ .virtual_url());
+}
+
+// Ensure that the manager properly ignores a restored placeholder that refers
+// to a tab node that doesn't exist
+TEST_F(SessionsSyncManagerTest, RestoredPlacholderTabNodeDeleted) {
+ const int kNewWindowId = 1337;
+ syncer::SyncDataList in;
+ syncer::SyncChangeList out;
+
+ // Set up one tab and start sync with it.
+ AddTab(browser(), GURL(kFoo1));
+ NavigateAndCommitActiveTab(GURL(kFoo2));
+ InitWithSyncDataTakeOutput(in, &out);
+
+ // Should be one header add, 1 tab add, and one header update.
+ ASSERT_EQ(3U, out.size());
+ const sync_pb::EntitySpecifics t0_entity = out[1].sync_data().GetSpecifics();
+ ASSERT_TRUE(t0_entity.session().has_tab());
+
+ out.clear();
+ manager()->StopSyncing(syncer::SESSIONS);
+
+ // Override the tab with a placeholder tab delegate.
+ PlaceholderTabDelegate t0_override(t0_entity.session().tab().tab_id(),
+ t0_entity.session().tab_node_id());
+
+ // Set up the window override with the new window ID and placeholder tab.
+ const std::set<const SyncedWindowDelegate*>& windows =
+ get_synced_window_getter()->GetSyncedWindowDelegates();
+ ASSERT_EQ(1U, windows.size());
+ SyncedWindowDelegateOverride window_override(*windows.begin());
+ window_override.OverrideSessionId(kNewWindowId);
+ window_override.OverrideTabAt(0, &t0_override,
+ t0_entity.session().tab().tab_id());
+
+ // Inject the window override.
+ std::set<const SyncedWindowDelegate*> delegates;
+ delegates.insert(&window_override);
+ std::unique_ptr<TestSyncedWindowDelegatesGetter> getter(
+ new TestSyncedWindowDelegatesGetter(delegates));
+ set_synced_window_getter(getter.get());
+
+ syncer::SyncMergeResult result = manager()->MergeDataAndStartSyncing(
+ syncer::SESSIONS, in, std::unique_ptr<syncer::SyncChangeProcessor>(
+ new TestSyncProcessorStub(&out)),
+ std::unique_ptr<syncer::SyncErrorFactory>(
+ new syncer::SyncErrorFactoryMock()));
+
+ // Because no entities were passed in at associate time, there should be no
+ // tab changes.
+ ASSERT_EQ(0U, FilterOutLocalHeaderChanges(&out)->size());
}
// Tests MergeDataAndStartSyncing with sync data but no local data.
@@ -1140,7 +1299,8 @@ TEST_F(SessionsSyncManagerTest, MergeWithLocalAndForeignTabs) {
syncer::SyncChangeList output;
InitWithSyncDataTakeOutput(foreign_data, &output);
- ASSERT_EQ(4U, output.size());
+ // Should be one header add, 1 tab add, and one header update.
+ ASSERT_EQ(3U, output.size());
// Verify the local header.
EXPECT_TRUE(output[0].IsValid());
@@ -1168,12 +1328,12 @@ TEST_F(SessionsSyncManagerTest, MergeWithLocalAndForeignTabs) {
}
EXPECT_EQ(SyncChange::ACTION_ADD, output[1].change_type());
EXPECT_EQ(SyncChange::ACTION_UPDATE, output[2].change_type());
- EXPECT_TRUE(output[2].sync_data().GetSpecifics().session().has_tab());
+ EXPECT_TRUE(output[1].sync_data().GetSpecifics().session().has_tab());
// Verify the header was updated to reflect window state.
- EXPECT_TRUE(output[3].IsValid());
- EXPECT_EQ(SyncChange::ACTION_UPDATE, output[3].change_type());
- const SyncData data_2(output[3].sync_data());
+ EXPECT_TRUE(output[2].IsValid());
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, output[2].change_type());
+ const SyncData data_2(output[2].sync_data());
EXPECT_EQ(manager()->current_machine_tag(),
syncer::SyncDataLocal(data_2).GetTag());
const sync_pb::SessionSpecifics& specifics2(data_2.GetSpecifics().session());
@@ -1216,7 +1376,9 @@ TEST_F(SessionsSyncManagerTest, UpdatesAfterMixedMerge) {
syncer::SyncChangeList output1;
InitWithSyncDataTakeOutput(foreign_data1, &output1);
- ASSERT_EQ(4U, output1.size());
+
+ // 1 header add, one tab add, one header update.
+ ASSERT_EQ(3U, output1.size());
// Add a second window to the foreign session.
// TODO(tim): Bug 98892. Add local window too when observers are hooked up.
@@ -1307,10 +1469,10 @@ TEST_F(SessionsSyncManagerTest, DeleteForeignSession) {
tag, tab_nums1, &tabs));
// Update associator with the session's meta node, window, and tabs.
- manager()->UpdateTrackerWithForeignSession(meta, base::Time());
+ manager()->UpdateTrackerWithSpecifics(meta, base::Time());
for (std::vector<sync_pb::SessionSpecifics>::iterator iter = tabs.begin();
iter != tabs.end(); ++iter) {
- manager()->UpdateTrackerWithForeignSession(*iter, base::Time());
+ manager()->UpdateTrackerWithSpecifics(*iter, base::Time());
}
ASSERT_TRUE(manager()->GetAllForeignSessions(&foreign_sessions));
ASSERT_EQ(1U, foreign_sessions.size());
@@ -1322,7 +1484,7 @@ TEST_F(SessionsSyncManagerTest, DeleteForeignSession) {
EXPECT_EQ(5U, changes.size());
std::set<std::string> expected_tags(&tag, &tag + 1);
for (int i = 0; i < 5; i++)
- expected_tags.insert(TabNodePool::TabIdToTag(tag, i));
+ expected_tags.insert(TabNodeIdToTag(tag, i));
for (int i = 0; i < 5; i++) {
SCOPED_TRACE(changes[i].ToString());
@@ -1445,40 +1607,31 @@ TEST_F(SessionsSyncManagerTest, ProcessRemoteDeleteOfLocalSession) {
// committing the NavigationEntry. The first notification results in a tab
// we don't associate although we do update the header node. The second
// notification triggers association of the tab, and the subsequent window
- // update. So we should see 4 changes at the SyncChangeProcessor.
- ASSERT_EQ(4U, out.size());
+ // update. So we should see 3 changes at the SyncChangeProcessor.
+ ASSERT_EQ(3U, out.size());
EXPECT_EQ(SyncChange::ACTION_UPDATE, out[0].change_type());
ASSERT_TRUE(out[0].sync_data().GetSpecifics().session().has_header());
EXPECT_EQ(SyncChange::ACTION_ADD, out[1].change_type());
int tab_node_id = out[1].sync_data().GetSpecifics().session().tab_node_id();
- EXPECT_EQ(TabNodePool::TabIdToTag(
- manager()->current_machine_tag(), tab_node_id),
+ EXPECT_EQ(TabNodeIdToTag(manager()->current_machine_tag(), tab_node_id),
syncer::SyncDataLocal(out[1].sync_data()).GetTag());
EXPECT_EQ(SyncChange::ACTION_UPDATE, out[2].change_type());
- ASSERT_TRUE(out[2].sync_data().GetSpecifics().session().has_tab());
- EXPECT_EQ(SyncChange::ACTION_UPDATE, out[3].change_type());
- ASSERT_TRUE(out[3].sync_data().GetSpecifics().session().has_header());
+ ASSERT_TRUE(out[2].sync_data().GetSpecifics().session().has_header());
// Verify the actual content.
const sync_pb::SessionHeader& session_header =
- out[3].sync_data().GetSpecifics().session().header();
+ out[2].sync_data().GetSpecifics().session().header();
ASSERT_EQ(1, session_header.window_size());
EXPECT_EQ(1, session_header.window(0).tab_size());
const sync_pb::SessionTab& tab1 =
- out[2].sync_data().GetSpecifics().session().tab();
+ out[1].sync_data().GetSpecifics().session().tab();
ASSERT_EQ(1, tab1.navigation_size());
EXPECT_EQ(foo1.spec(), tab1.navigation(0).virtual_url());
// Verify TabNodePool integrity.
- EXPECT_EQ(1U, manager()->local_tab_pool_.Capacity());
- EXPECT_TRUE(manager()->local_tab_pool_.Empty());
-
- // Verify TabLinks.
- SessionsSyncManager::TabLinksMap tab_map = manager()->local_tab_map_;
- ASSERT_EQ(1U, tab_map.size());
- int tab_id = out[2].sync_data().GetSpecifics().session().tab().tab_id();
- EXPECT_EQ(tab_node_id, tab_map.find(tab_id)->second->tab_node_id());
+ EXPECT_EQ(1U, GetTabPool()->Capacity());
+ EXPECT_TRUE(GetTabPool()->Empty());
}
// Test that receiving a session delete from sync removes the session
@@ -1622,7 +1775,8 @@ TEST_F(SessionsSyncManagerTest, ProcessForeignDeleteTabsWithShadowing) {
manager()->session_tracker_.LookupSessionTab(session_tag, 2, &tab));
std::set<int> tab_node_ids;
- manager()->session_tracker_.LookupTabNodeIds(session_tag, &tab_node_ids);
+ manager()->session_tracker_.LookupForeignTabNodeIds(session_tag,
+ &tab_node_ids);
EXPECT_EQ(6U, tab_node_ids.size());
EXPECT_TRUE(tab_node_ids.find(tab1A.tab_node_id()) != tab_node_ids.end());
EXPECT_TRUE(tab_node_ids.find(tab1B.tab_node_id()) != tab_node_ids.end());
@@ -1638,7 +1792,8 @@ TEST_F(SessionsSyncManagerTest, ProcessForeignDeleteTabsWithShadowing) {
manager()->ProcessSyncChanges(FROM_HERE, changes);
tab_node_ids.clear();
- manager()->session_tracker_.LookupTabNodeIds(session_tag, &tab_node_ids);
+ manager()->session_tracker_.LookupForeignTabNodeIds(session_tag,
+ &tab_node_ids);
EXPECT_EQ(3U, tab_node_ids.size());
EXPECT_TRUE(tab_node_ids.find(tab1C.tab_node_id()) != tab_node_ids.end());
EXPECT_TRUE(tab_node_ids.find(tab2A.tab_node_id()) != tab_node_ids.end());
@@ -1676,7 +1831,8 @@ TEST_F(SessionsSyncManagerTest, ProcessForeignDeleteTabsWithReusedNodeIds) {
output.clear();
std::set<int> tab_node_ids;
- manager()->session_tracker_.LookupTabNodeIds(session_tag, &tab_node_ids);
+ manager()->session_tracker_.LookupForeignTabNodeIds(session_tag,
+ &tab_node_ids);
EXPECT_EQ(2U, tab_node_ids.size());
EXPECT_TRUE(tab_node_ids.find(tab_node_id_shared) != tab_node_ids.end());
EXPECT_TRUE(tab_node_ids.find(tab_node_id_unique) != tab_node_ids.end());
@@ -1686,7 +1842,8 @@ TEST_F(SessionsSyncManagerTest, ProcessForeignDeleteTabsWithReusedNodeIds) {
manager()->ProcessSyncChanges(FROM_HERE, changes);
tab_node_ids.clear();
- manager()->session_tracker_.LookupTabNodeIds(session_tag, &tab_node_ids);
+ manager()->session_tracker_.LookupForeignTabNodeIds(session_tag,
+ &tab_node_ids);
EXPECT_EQ(1U, tab_node_ids.size());
EXPECT_TRUE(tab_node_ids.find(tab_node_id_unique) != tab_node_ids.end());
@@ -1694,51 +1851,87 @@ TEST_F(SessionsSyncManagerTest, ProcessForeignDeleteTabsWithReusedNodeIds) {
EXPECT_EQ(1U, output.size());
}
-// TODO(shashishekhar): "Move this to TabNodePool unittests."
-TEST_F(SessionsSyncManagerTest, SaveUnassociatedNodesForReassociation) {
+TEST_F(SessionsSyncManagerTest, AssociationReusesNodes) {
syncer::SyncChangeList changes;
- InitWithNoSyncData();
-
- std::string local_tag = manager()->current_machine_tag();
- // Create a free node and then dissassociate sessions so that it ends up
- // unassociated.
- manager()->local_tab_pool_.GetFreeTabNode(&changes);
-
- // Update the tab_id of the node, so that it is considered a valid
- // unassociated node otherwise it will be mistaken for a corrupted node and
- // will be deleted before being added to the tab node pool.
- sync_pb::EntitySpecifics entity(changes[0].sync_data().GetSpecifics());
- entity.mutable_session()->mutable_tab()->set_tab_id(1);
- SyncData d = CreateRemoteData(entity);
- syncer::SyncDataList in(&d, &d + 1);
+ AddTab(browser(), GURL("http://foo1"));
+ InitWithSyncDataTakeOutput(syncer::SyncDataList(), &changes);
+ ASSERT_EQ(3U, changes.size()); // Header add, tab add, header update.
+ ASSERT_TRUE(changes[1].sync_data().GetSpecifics().session().has_tab());
+ int tab_node_id =
+ changes[1].sync_data().GetSpecifics().session().tab_node_id();
+
+ // Pass back the previous tab and header nodes at association, along with a
+ // second tab node (with a rewritten tab node id).
+ syncer::SyncDataList in;
+ in.push_back(
+ CreateRemoteData(changes[2].sync_data().GetSpecifics())); // Header node.
+ sync_pb::SessionSpecifics new_tab(
+ changes[1].sync_data().GetSpecifics().session());
+ new_tab.set_tab_node_id(tab_node_id + 1);
+ in.push_back(CreateRemoteData(new_tab)); // New tab node.
+ in.push_back(CreateRemoteData(
+ changes[1].sync_data().GetSpecifics())); // Old tab node.
changes.clear();
- SessionsSyncManager manager2(GetSyncSessionsClient(), sync_prefs(),
- local_device(), NewDummyRouter(),
- base::Closure(), base::Closure());
- syncer::SyncMergeResult result = manager2.MergeDataAndStartSyncing(
+
+ // Reassociate (with the same single tab/window open).
+ manager()->StopSyncing(syncer::SESSIONS);
+ syncer::SyncMergeResult result = manager()->MergeDataAndStartSyncing(
syncer::SESSIONS, in, std::unique_ptr<syncer::SyncChangeProcessor>(
new TestSyncProcessorStub(&changes)),
std::unique_ptr<syncer::SyncErrorFactory>(
new syncer::SyncErrorFactoryMock()));
ASSERT_FALSE(result.error().IsSet());
- EXPECT_TRUE(FilterOutLocalHeaderChanges(&changes)->empty());
+
+ // No tab entities should be deleted. The original (lower) tab node id should
+ // be reused for association.
+ FilterOutLocalHeaderChanges(&changes);
+ ASSERT_EQ(1U, changes.size());
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, changes[0].change_type());
+ EXPECT_TRUE(changes[0].sync_data().GetSpecifics().session().has_tab());
+ EXPECT_EQ(tab_node_id,
+ changes[0].sync_data().GetSpecifics().session().tab_node_id());
}
-TEST_F(SessionsSyncManagerTest, MergeDeletesCorruptNode) {
+TEST_F(SessionsSyncManagerTest, MergeDeletesCorruptTabNodeId) {
syncer::SyncChangeList changes;
InitWithNoSyncData();
std::string local_tag = manager()->current_machine_tag();
- int tab_node_id = manager()->local_tab_pool_.GetFreeTabNode(&changes);
- SyncData d = CreateRemoteData(changes[0].sync_data().GetSpecifics());
+ int tab_node_id = TabNodePool::kInvalidTabNodeID;
+ GetTabPool()->GetTabNodeForTab(0, &tab_node_id);
+ sync_pb::SessionSpecifics specifics;
+ specifics.set_session_tag(local_tag);
+ specifics.set_tab_node_id(tab_node_id);
+ SyncData d = CreateRemoteData(specifics);
+ syncer::SyncDataList in(&d, &d + 1);
+ TearDown();
+ SetUp();
+ InitWithSyncDataTakeOutput(in, &changes);
+ EXPECT_EQ(1U, FilterOutLocalHeaderChanges(&changes)->size());
+ EXPECT_EQ(SyncChange::ACTION_DELETE, changes[0].change_type());
+ EXPECT_EQ(TabNodeIdToTag(local_tag, tab_node_id),
+ syncer::SyncDataLocal(changes[0].sync_data()).GetTag());
+}
+
+TEST_F(SessionsSyncManagerTest, MergeDeletesCorruptTabId) {
+ syncer::SyncChangeList changes;
+ InitWithNoSyncData();
+
+ std::string local_tag = manager()->current_machine_tag();
+ int tab_node_id = 0;
+ GetTabPool()->GetTabNodeForTab(0, &tab_node_id);
+ sync_pb::SessionSpecifics specifics;
+ specifics.set_session_tag(local_tag);
+ specifics.set_tab_node_id(tab_node_id);
+ specifics.mutable_tab()->set_tab_id(TabNodePool::kInvalidTabID);
+ SyncData d = CreateRemoteData(specifics);
syncer::SyncDataList in(&d, &d + 1);
- changes.clear();
TearDown();
SetUp();
InitWithSyncDataTakeOutput(in, &changes);
EXPECT_EQ(1U, FilterOutLocalHeaderChanges(&changes)->size());
EXPECT_EQ(SyncChange::ACTION_DELETE, changes[0].change_type());
- EXPECT_EQ(TabNodePool::TabIdToTag(local_tag, tab_node_id),
+ EXPECT_EQ(TabNodeIdToTag(local_tag, tab_node_id),
syncer::SyncDataLocal(changes[0].sync_data()).GetTag());
}
@@ -1807,7 +2000,7 @@ TEST_F(SessionsSyncManagerTest, AssociateWindowsDontReloadTabs) {
// Go to a sync-interesting URL.
NavigateAndCommitActiveTab(GURL("http://foo2"));
- EXPECT_EQ(3U, out.size()); // Tab add, update, and header update.
+ EXPECT_EQ(2U, out.size()); // Tab add and header update.
EXPECT_TRUE(
base::StartsWith(syncer::SyncDataLocal(out[0].sync_data()).GetTag(),
@@ -1817,18 +2010,9 @@ TEST_F(SessionsSyncManagerTest, AssociateWindowsDontReloadTabs) {
out[0].sync_data().GetSpecifics().session().session_tag());
EXPECT_EQ(SyncChange::ACTION_ADD, out[0].change_type());
- EXPECT_TRUE(
- base::StartsWith(syncer::SyncDataLocal(out[1].sync_data()).GetTag(),
- manager()->current_machine_tag(),
- base::CompareCase::SENSITIVE));
- EXPECT_EQ(manager()->current_machine_tag(),
- out[1].sync_data().GetSpecifics().session().session_tag());
- EXPECT_TRUE(out[1].sync_data().GetSpecifics().session().has_tab());
+ EXPECT_TRUE(out[1].IsValid());
EXPECT_EQ(SyncChange::ACTION_UPDATE, out[1].change_type());
-
- EXPECT_TRUE(out[2].IsValid());
- EXPECT_EQ(SyncChange::ACTION_UPDATE, out[2].change_type());
- const SyncData data(out[2].sync_data());
+ const SyncData data(out[1].sync_data());
EXPECT_EQ(manager()->current_machine_tag(),
syncer::SyncDataLocal(data).GetTag());
const sync_pb::SessionSpecifics& specifics(data.GetSpecifics().session());
@@ -1860,17 +2044,17 @@ TEST_F(SessionsSyncManagerTest, OnLocalTabModified) {
AddTab(browser(), bar1);
NavigateAndCommitActiveTab(bar2);
- // One add, one update for each AddTab.
- // One update for each NavigateAndCommit.
- // = 6 total tab updates.
- // One header update corresponding to each of those.
- // = 6 total header updates.
- // 12 total updates.
- ASSERT_EQ(12U, out.size());
+ // Change type breakdown:
+ const size_t kChangesPerTabCreation = 3; // 1 tab add + 2 header updates.
+ const size_t kChangesPerTabNav = 2; // 1 tab update + 1 header update.
+ const size_t kChangesPerTab = kChangesPerTabNav + kChangesPerTabCreation;
+ const size_t kNumTabs = 2;
+ const size_t kTotalUpdates = kChangesPerTab * kNumTabs;
+ ASSERT_EQ(kTotalUpdates, out.size());
// Verify the tab node creations and updates to ensure the SyncProcessor
// sees the right operations.
- for (int i = 0; i < 12; i++) {
+ for (size_t i = 0; i < kTotalUpdates; i++) {
SCOPED_TRACE(i);
EXPECT_TRUE(out[i].IsValid());
const SyncData data(out[i].sync_data());
@@ -1879,40 +2063,30 @@ TEST_F(SessionsSyncManagerTest, OnLocalTabModified) {
base::CompareCase::SENSITIVE));
const sync_pb::SessionSpecifics& specifics(data.GetSpecifics().session());
EXPECT_EQ(manager()->current_machine_tag(), specifics.session_tag());
- if (i % 6 == 0) {
+ if (i % kChangesPerTab == 0) {
// First thing on an AddTab is a no-op header update for parented tab.
EXPECT_EQ(header.SerializeAsString(),
data.GetSpecifics().SerializeAsString());
EXPECT_EQ(manager()->current_machine_tag(),
syncer::SyncDataLocal(data).GetTag());
- } else if (i % 6 == 1) {
- // Next, the TabNodePool should create the tab node.
+ } else if (i % kChangesPerTab == 1) {
+ // Next, the tab should be added.
EXPECT_EQ(SyncChange::ACTION_ADD, out[i].change_type());
- EXPECT_EQ(TabNodePool::TabIdToTag(
- manager()->current_machine_tag(),
- data.GetSpecifics().session().tab_node_id()),
- syncer::SyncDataLocal(data).GetTag());
- } else if (i % 6 == 2) {
- // Then we see the tab update to the URL.
- EXPECT_EQ(SyncChange::ACTION_UPDATE, out[i].change_type());
- EXPECT_EQ(TabNodePool::TabIdToTag(
- manager()->current_machine_tag(),
- data.GetSpecifics().session().tab_node_id()),
+ EXPECT_EQ(TabNodeIdToTag(manager()->current_machine_tag(),
+ data.GetSpecifics().session().tab_node_id()),
syncer::SyncDataLocal(data).GetTag());
- ASSERT_TRUE(specifics.has_tab());
- } else if (i % 6 == 3) {
+ } else if (i % kChangesPerTab == 2) {
// The header needs to be updated to reflect the new window state.
EXPECT_EQ(SyncChange::ACTION_UPDATE, out[i].change_type());
EXPECT_TRUE(specifics.has_header());
- } else if (i % 6 == 4) {
+ } else if (i % kChangesPerTab == 3) {
// Now we move on to NavigateAndCommit. Update the tab.
EXPECT_EQ(SyncChange::ACTION_UPDATE, out[i].change_type());
- EXPECT_EQ(TabNodePool::TabIdToTag(
- manager()->current_machine_tag(),
- data.GetSpecifics().session().tab_node_id()),
+ EXPECT_EQ(TabNodeIdToTag(manager()->current_machine_tag(),
+ data.GetSpecifics().session().tab_node_id()),
syncer::SyncDataLocal(data).GetTag());
ASSERT_TRUE(specifics.has_tab());
- } else if (i % 6 == 5) {
+ } else if (i % kChangesPerTab == 4) {
// The header needs to be updated to reflect the new window state.
EXPECT_EQ(SyncChange::ACTION_UPDATE, out[i].change_type());
ASSERT_TRUE(specifics.has_header());
@@ -1929,25 +2103,33 @@ TEST_F(SessionsSyncManagerTest, OnLocalTabModified) {
// ASSERT_TRUEs above allow us to dive in freely here.
// Verify first tab.
const sync_pb::SessionTab& tab1_1 =
- out[2].sync_data().GetSpecifics().session().tab();
+ out[1].sync_data().GetSpecifics().session().tab();
ASSERT_EQ(1, tab1_1.navigation_size());
EXPECT_EQ(foo1.spec(), tab1_1.navigation(0).virtual_url());
const sync_pb::SessionTab& tab1_2 =
- out[4].sync_data().GetSpecifics().session().tab();
+ out[3].sync_data().GetSpecifics().session().tab();
ASSERT_EQ(2, tab1_2.navigation_size());
EXPECT_EQ(foo1.spec(), tab1_2.navigation(0).virtual_url());
EXPECT_EQ(foo2.spec(), tab1_2.navigation(1).virtual_url());
// Verify second tab.
const sync_pb::SessionTab& tab2_1 =
- out[8].sync_data().GetSpecifics().session().tab();
+ out[6].sync_data().GetSpecifics().session().tab();
ASSERT_EQ(1, tab2_1.navigation_size());
EXPECT_EQ(bar1.spec(), tab2_1.navigation(0).virtual_url());
const sync_pb::SessionTab& tab2_2 =
- out[10].sync_data().GetSpecifics().session().tab();
+ out[8].sync_data().GetSpecifics().session().tab();
ASSERT_EQ(2, tab2_2.navigation_size());
EXPECT_EQ(bar1.spec(), tab2_2.navigation(0).virtual_url());
EXPECT_EQ(bar2.spec(), tab2_2.navigation(1).virtual_url());
+
+ // Verify tab delegates have Sync ids.
+ std::set<const SyncedWindowDelegate*> window_delegates =
+ get_synced_window_getter()->GetSyncedWindowDelegates();
+ // Sync ids are in reverse order because tabs are inserted at the beginning
+ // of the tab list.
+ EXPECT_EQ(1, (*window_delegates.begin())->GetTabAt(0)->GetSyncId());
+ EXPECT_EQ(0, (*window_delegates.begin())->GetTabAt(1)->GetSyncId());
}
// Check that if a tab becomes uninteresting (for example no syncable URLs),
@@ -1965,21 +2147,21 @@ TEST_F(SessionsSyncManagerTest, TabBecomesUninteresting) {
// Add an interesting tab.
AddTab(browser(), kValidUrl);
- // No-op header update, tab creation, tab update, header update.
- ASSERT_EQ(4U, out.size());
+ // No-op header update, tab creation, header update.
+ ASSERT_EQ(3U, out.size());
// The last two are the interesting updates.
- ASSERT_TRUE(out[2].sync_data().GetSpecifics().session().has_tab());
- EXPECT_EQ(kValidUrl.spec(), out[2]
+ ASSERT_TRUE(out[1].sync_data().GetSpecifics().session().has_tab());
+ EXPECT_EQ(kValidUrl.spec(), out[1]
.sync_data()
.GetSpecifics()
.session()
.tab()
.navigation(0)
.virtual_url());
- ASSERT_TRUE(out[3].sync_data().GetSpecifics().session().has_header());
+ ASSERT_TRUE(out[2].sync_data().GetSpecifics().session().has_header());
ASSERT_EQ(1,
- out[3].sync_data().GetSpecifics().session().header().window_size());
- ASSERT_EQ(1, out[3]
+ out[2].sync_data().GetSpecifics().session().header().window_size());
+ ASSERT_EQ(1, out[2]
.sync_data()
.GetSpecifics()
.session()
@@ -2006,6 +2188,68 @@ TEST_F(SessionsSyncManagerTest, TabBecomesUninteresting) {
out[1].sync_data().GetSpecifics().session().header().window_size());
}
+// Ensure that if there are tabs with duplicate ids, Sync ignores the
+// duplicates.
+TEST_F(SessionsSyncManagerTest, DuplicateTabIdFromNative) {
+ syncer::SyncDataList in;
+ syncer::SyncChangeList out;
+
+ // Set up two tabs. The second (kFoo2) will be the one at index 0.
+ AddTab(browser(), GURL(kFoo1));
+ AddTab(browser(), GURL(kFoo2));
+ InitWithSyncDataTakeOutput(in, &out);
+
+ // Should be one header add, 2 tab adds, and one header update.
+ ASSERT_EQ(4U, out.size());
+
+ // Grab the first tab.
+ const sync_pb::EntitySpecifics t0_entity = out[1].sync_data().GetSpecifics();
+ ASSERT_TRUE(t0_entity.session().has_tab());
+
+ in.clear();
+ out.clear();
+ manager()->StopSyncing(syncer::SESSIONS);
+
+ // Override the second tab with a placeholder tab delegate that matches
+ // the first tab.
+ PlaceholderTabDelegate t0_override(t0_entity.session().tab().tab_id(),
+ t0_entity.session().tab_node_id());
+
+ // Set up the window override with the new window ID and placeholder tab.
+ const std::set<const SyncedWindowDelegate*>& windows =
+ get_synced_window_getter()->GetSyncedWindowDelegates();
+ ASSERT_EQ(1U, windows.size());
+ SyncedWindowDelegateOverride window_override(*windows.begin());
+ window_override.OverrideTabAt(1, &t0_override,
+ t0_entity.session().tab().tab_id());
+
+ // Inject the window override.
+ std::set<const SyncedWindowDelegate*> delegates;
+ delegates.insert(&window_override);
+ std::unique_ptr<TestSyncedWindowDelegatesGetter> getter(
+ new TestSyncedWindowDelegatesGetter(delegates));
+ set_synced_window_getter(getter.get());
+
+ // Resync without any existing nodes (simulating a first time sync).
+ syncer::SyncMergeResult result = manager()->MergeDataAndStartSyncing(
+ syncer::SESSIONS, in, std::unique_ptr<syncer::SyncChangeProcessor>(
+ new TestSyncProcessorStub(&out)),
+ std::unique_ptr<syncer::SyncErrorFactory>(
+ new syncer::SyncErrorFactoryMock()));
+
+ // There should be only one tab change: an add.
+ ASSERT_EQ(1U, FilterOutLocalHeaderChanges(&out)->size());
+ EXPECT_EQ(SyncChange::ACTION_ADD, out[0].change_type());
+ EXPECT_EQ(t0_entity.session().tab().navigation(0).virtual_url(),
+ out[0]
+ .sync_data()
+ .GetSpecifics()
+ .session()
+ .tab()
+ .navigation(0)
+ .virtual_url());
+}
+
// Ensure model association associates the pre-existing tabs.
TEST_F(SessionsSyncManagerTest, MergeLocalSessionExistingTabs) {
AddTab(browser(), GURL("http://foo1"));
@@ -2015,7 +2259,7 @@ TEST_F(SessionsSyncManagerTest, MergeLocalSessionExistingTabs) {
syncer::SyncChangeList out;
InitWithSyncDataTakeOutput(syncer::SyncDataList(), &out);
- ASSERT_EQ(6U, out.size());
+ ASSERT_EQ(4U, out.size()); // Header creation, add two tabs, header update
// Check that this machine's data is not included in the foreign windows.
std::vector<const SyncedSession*> foreign_sessions;
@@ -2036,7 +2280,7 @@ TEST_F(SessionsSyncManagerTest, MergeLocalSessionExistingTabs) {
EXPECT_EQ(0, header_s.window_size());
// Verify the tab node creations and updates with content.
- for (int i = 1; i < 5; i++) {
+ for (int i = 1; i < 3; i++) {
EXPECT_TRUE(out[i].IsValid());
const SyncData data(out[i].sync_data());
EXPECT_TRUE(base::StartsWith(syncer::SyncDataLocal(data).GetTag(),
@@ -2044,18 +2288,13 @@ TEST_F(SessionsSyncManagerTest, MergeLocalSessionExistingTabs) {
base::CompareCase::SENSITIVE));
const sync_pb::SessionSpecifics& specifics(data.GetSpecifics().session());
EXPECT_EQ(manager()->current_machine_tag(), specifics.session_tag());
- if (i % 2 == 1) {
- EXPECT_EQ(SyncChange::ACTION_ADD, out[i].change_type());
- } else {
- EXPECT_EQ(SyncChange::ACTION_UPDATE, out[i].change_type());
- EXPECT_TRUE(specifics.has_tab());
- }
+ EXPECT_EQ(SyncChange::ACTION_ADD, out[i].change_type());
}
// Verify the header was updated to reflect new window state.
- EXPECT_TRUE(out[5].IsValid());
- EXPECT_EQ(SyncChange::ACTION_UPDATE, out[5].change_type());
- const SyncData data_2(out[5].sync_data());
+ EXPECT_TRUE(out[3].IsValid());
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, out[3].change_type());
+ const SyncData data_2(out[3].sync_data());
EXPECT_EQ(manager()->current_machine_tag(),
syncer::SyncDataLocal(data_2).GetTag());
const sync_pb::SessionSpecifics& specifics2(data_2.GetSpecifics().session());
@@ -2064,19 +2303,13 @@ TEST_F(SessionsSyncManagerTest, MergeLocalSessionExistingTabs) {
const sync_pb::SessionHeader& header_s2 = specifics2.header();
EXPECT_EQ(1, header_s2.window_size());
- // Verify TabLinks.
- SessionsSyncManager::TabLinksMap tab_map = manager()->local_tab_map_;
- ASSERT_EQ(2U, tab_map.size());
- // Tabs are ordered by sessionid in tab_map, so should be able to traverse
- // the tree based on order of tabs created
- SessionsSyncManager::TabLinksMap::iterator iter = tab_map.begin();
- ASSERT_EQ(2, iter->second->tab()->GetEntryCount());
- EXPECT_EQ(GURL("http://foo1"), iter->second->tab()->GetVirtualURLAtIndex(0));
- EXPECT_EQ(GURL("http://foo2"), iter->second->tab()->GetVirtualURLAtIndex(1));
- iter++;
- ASSERT_EQ(2, iter->second->tab()->GetEntryCount());
- EXPECT_EQ(GURL("http://bar1"), iter->second->tab()->GetVirtualURLAtIndex(0));
- EXPECT_EQ(GURL("http://bar2"), iter->second->tab()->GetVirtualURLAtIndex(1));
+ // Verify tab delegates have Sync ids.
+ std::set<const SyncedWindowDelegate*> window_delegates =
+ get_synced_window_getter()->GetSyncedWindowDelegates();
+ // Sync ids are in same order as tabs because the association happens after
+ // the tabs are opened (and therefore iterates through same order).
+ EXPECT_EQ(0, (*window_delegates.begin())->GetTabAt(0)->GetSyncId());
+ EXPECT_EQ(1, (*window_delegates.begin())->GetTabAt(1)->GetSyncId());
}
TEST_F(SessionsSyncManagerTest, ForeignSessionModifiedTime) {
@@ -2179,8 +2412,7 @@ TEST_F(SessionsSyncManagerTest, DoGarbageCollection) {
for (int i = 1; i < 5; i++) {
EXPECT_EQ(SyncChange::ACTION_DELETE, output[i].change_type());
const SyncData data(output[i].sync_data());
- EXPECT_EQ(TabNodePool::TabIdToTag(tag1, i),
- syncer::SyncDataLocal(data).GetTag());
+ EXPECT_EQ(TabNodeIdToTag(tag1, i), syncer::SyncDataLocal(data).GetTag());
}
ASSERT_TRUE(manager()->GetAllForeignSessions(&foreign_sessions));
@@ -2303,7 +2535,8 @@ TEST_F(SessionsSyncManagerTest, CheckPrerenderedWebContentsSwap) {
syncer::SyncChangeList out;
InitWithSyncDataTakeOutput(syncer::SyncDataList(), &out);
- ASSERT_EQ(4U, out.size()); // Header, tab ADD, tab UPDATE, header UPDATE.
+ ASSERT_EQ(3U, out.size()); // Header ADD, tab ADD, header UPDATE.
+ out.clear();
// To simulate WebContents swap during prerendering, create new WebContents
// and swap with old WebContents.
@@ -2324,24 +2557,28 @@ TEST_F(SessionsSyncManagerTest, CheckPrerenderedWebContentsSwap) {
old_web_contents.get());
browser()->tab_strip_model()->ReplaceWebContentsAt(index, new_web_contents);
- ASSERT_EQ(9U, out.size());
- EXPECT_EQ(SyncChange::ACTION_ADD, out[4].change_type());
- EXPECT_EQ(SyncChange::ACTION_UPDATE, out[5].change_type());
+ ASSERT_EQ(4U, out.size());
+ EXPECT_EQ(SyncChange::ACTION_ADD, out[0].change_type());
+ out.clear();
- // Navigate away.
+ // Navigate away. +1 tab updates, 1 header update.
NavigateAndCommitActiveTab(GURL("http://bar2"));
// Delete old WebContents. This should not crash.
+ // +1 no-op header update.
old_web_contents.reset();
// Try more navigations and verify output size. This can also reveal
// bugs (leaks) on memcheck bots if the SessionSyncManager
// didn't properly clean up the tab pool or session tracker.
+ // +1 tab updates, 1 header update.
NavigateAndCommitActiveTab(GURL("http://bar3"));
+ // +1 no-op header update, tab add, header update.
AddTab(browser(), GURL("http://bar4"));
+ // +1 tab update, header update.
NavigateAndCommitActiveTab(GURL("http://bar5"));
- ASSERT_EQ(19U, out.size());
+ ASSERT_EQ(10U, out.size());
}
// Test that NOTIFICATION_FOREIGN_SESSION_UPDATED is sent when processing
@@ -2586,8 +2823,7 @@ TEST_F(SessionsSyncManagerTest, GetForeignSessionTabs) {
sync_pb::EntitySpecifics entity;
helper()->BuildTabSpecifics(kTag, 0, tab_list2[i],
entity.mutable_session());
- // Order the tabs oldest to most ReceiveDuplicateUnassociatedTabs and
- // left to right visually.
+ // Order the tabs oldest to most recent and left to right visually.
initial_data.push_back(
CreateRemoteData(entity, base::Time::FromInternalValue(i + 1)));
}
« no previous file with comments | « no previous file | components/sync_sessions/sessions_sync_manager.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698