[Sync] Fix Session's handling of windows.

chrome/browser/sync/test/integration/sessions_helper.cc

Index: chrome/browser/sync/test/integration/sessions_helper.cc
diff --git a/chrome/browser/sync/test/integration/sessions_helper.cc b/chrome/browser/sync/test/integration/sessions_helper.cc
index 104629ac9c3cc944c2160e3df471aa944cc2c9b7..d9a669c938800af7b0bbc4864dab53a03b4c9826 100644
--- a/chrome/browser/sync/test/integration/sessions_helper.cc
+++ b/chrome/browser/sync/test/integration/sessions_helper.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/sync/test/integration/sessions_helper.h"
 
+#include <algorithm>
+
 #include "base/stl_util.h"
 #include "base/test/test_timeouts.h"
 #include "base/time.h"
@@ -23,6 +25,29 @@ using sync_datatype_helper::test;
 
 namespace sessions_helper {
 
+ScopedWindowMap::ScopedWindowMap() {
+}
+
+ScopedWindowMap::ScopedWindowMap(SessionWindowMap* windows) {
+  set(windows);
+}
+
+ScopedWindowMap::~ScopedWindowMap() {
+  STLDeleteContainerPairSecondPointers(windows_.begin(), windows_.end());
+}
+
+SessionWindowMap* ScopedWindowMap::get() {
+  return &windows_;
+}
+
+const SessionWindowMap* ScopedWindowMap::const_get() const {
+  return &windows_;
+}
+
+void ScopedWindowMap::set(SessionWindowMap* windows) {
+  std::swap(*windows, windows_);

[akalin, 2011/10/03 18:50:21]

set should probably delete the pointers in windows_ before swapping, right?

[Nicolas Zea, 2011/10/03 19:36:15]

Done.

+}
+
 bool GetLocalSession(int index, const browser_sync::SyncedSession** session) {
   return test()->GetProfile(index)->GetProfileSyncService()->
       GetSessionModelAssociator()->GetLocalSession(session);
@@ -42,15 +67,16 @@ bool ModelAssociatorHasTabWithUrl(int index, const GURL& url) {
 
   int nav_index;
   TabNavigation nav;
-  for (SessionWindowVector::const_iterator it =
-           local_session->windows.begin(); it != local_session->windows.end();
-           ++it) {
-    if ((*it)->tabs.size() == 0) {
+  for (SessionWindowMap::const_iterator it =
+           local_session->windows.begin();
+       it != local_session->windows.end(); ++it) {
+    if (it->second->tabs.size() == 0) {
       VLOG(1) << "Empty tabs vector";
       continue;
     }
     for (std::vector<SessionTab*>::const_iterator tab_it =
-             (*it)->tabs.begin(); tab_it != (*it)->tabs.end(); ++tab_it) {
+             it->second->tabs.begin();
+         tab_it != it->second->tabs.end(); ++tab_it) {
       if ((*tab_it)->navigations.size() == 0) {
         VLOG(1) << "Empty navigations vector";
         continue;
@@ -116,16 +142,18 @@ bool WaitForTabsToLoad(int index, const std::vector<GURL>& urls) {
   return true;
 }
 
-bool GetLocalWindows(int index, SessionWindowVector& local_windows) {
+bool GetLocalWindows(int index, SessionWindowMap* local_windows) {
   // The local session provided by GetLocalSession is owned, and has lifetime
   // controlled, by the model associator, so we must make our own copy.
   const browser_sync::SyncedSession* local_session;
   if (!GetLocalSession(index, &local_session)) {
     return false;
   }
-  for (size_t w = 0; w < local_session->windows.size(); ++w) {
-    const SessionWindow& window = *local_session->windows.at(w);
+  for (SessionWindowMap::const_iterator w = local_session->windows.begin();
+       w != local_session->windows.end(); ++w) {
+    const SessionWindow& window = *(w->second);
     SessionWindow* new_window = new SessionWindow();
+    new_window->window_id.set_id(window.window_id.id());
     for (size_t t = 0; t < window.tabs.size(); ++t) {
       const SessionTab& tab = *window.tabs.at(t);
       SessionTab* new_tab = new SessionTab();
@@ -134,18 +162,15 @@ bool GetLocalWindows(int index, SessionWindowVector& local_windows) {
                 new_tab->navigations.begin());
       new_window->tabs.push_back(new_tab);
     }
-    local_windows.push_back(new_window);
+    (*local_windows)[new_window->window_id.id()] = new_window;
   }
 
-  // Sort the windows so their order is deterministic.
-  SortSessionWindows(local_windows);
-
   return true;
 }
 
 bool OpenTabAndGetLocalWindows(int index,
                                const GURL& url,
-                               SessionWindowVector& local_windows) {
+                               SessionWindowMap* local_windows) {
   if (!OpenTab(index, url)) {
     return false;
   }
@@ -184,26 +209,6 @@ bool GetSessionData(int index, SyncedSessionVector* sessions) {
   return true;
 }
 
-bool CompareSessionWindows(SessionWindow* lhs, SessionWindow* rhs) {
-  if (!lhs ||
-      !rhs ||
-      lhs->tabs.size() < 1 ||
-      rhs->tabs.size() < 1 ||
-      lhs->tabs[0]->navigations.size() < 1 ||
-      rhs->tabs[0]->navigations.size() < 1) {
-    // Catchall for uncomparable data.
-    return false;
-  }
-
-  return lhs->tabs[0]->navigations[0].virtual_url() <
-      rhs->tabs[0]->navigations[0].virtual_url();
-}
-
-void SortSessionWindows(SessionWindowVector& windows) {
-  std::sort(windows.begin(), windows.end(),
-            CompareSessionWindows);
-}
-
 bool CompareSyncedSessions(const browser_sync::SyncedSession* lhs,
                            const browser_sync::SyncedSession* rhs) {
   if (!lhs ||
@@ -214,7 +219,7 @@ bool CompareSyncedSessions(const browser_sync::SyncedSession* lhs,
     return false;
   }
 
-  return CompareSessionWindows(lhs->windows[0], rhs->windows[0]);
+  return lhs->windows < rhs->windows;
 }
 
 void SortSyncedSessions(SyncedSessionVector* sessions) {
@@ -247,21 +252,25 @@ bool NavigationEquals(const TabNavigation& expected,
   return true;
 }
 
-bool WindowsMatch(const SessionWindowVector& win1,
-                  const SessionWindowVector& win2) {
+bool WindowsMatch(const SessionWindowMap& win1,
+                  const SessionWindowMap& win2) {
   SessionTab* client0_tab;
   SessionTab* client1_tab;
   if (win1.size() != win2.size())
     return false;
-  for (size_t i = 0; i < win1.size(); ++i) {
-    if (win1[i]->tabs.size() != win2[i]->tabs.size())
+  for (SessionWindowMap::const_iterator i = win1.begin();
+       i != win1.end(); ++i) {
+    SessionWindowMap::const_iterator j = win2.find(i->first);
+    if (j == win2.end())
+      return false;
+    if (i->second->tabs.size() != j->second->tabs.size())
       return false;
-    for (size_t j = 0; j < win1[i]->tabs.size(); ++j) {
-      client0_tab = win1[i]->tabs[j];
-      client1_tab = win2[i]->tabs[j];
-      for (size_t k = 0; k < client0_tab->navigations.size(); ++k) {
-        if (!NavigationEquals(client0_tab->navigations[k],
-                              client1_tab->navigations[k])) {
+    for (size_t t = 0; t < i->second->tabs.size(); ++t) {
+      client0_tab = i->second->tabs[t];
+      client1_tab = j->second->tabs[t];
+      for (size_t n = 0; n < client0_tab->navigations.size(); ++n) {
+        if (!NavigationEquals(client0_tab->navigations[n],
+                              client1_tab->navigations[n])) {
           return false;
         }
       }
@@ -273,7 +282,7 @@ bool WindowsMatch(const SessionWindowVector& win1,
 
 bool CheckForeignSessionsAgainst(
     int index,
-    const std::vector<SessionWindowVector*>& windows) {
+    const std::vector<ScopedWindowMap>& windows) {
   SyncedSessionVector sessions;
   if (!GetSessionData(index, &sessions))
     return false;
@@ -284,7 +293,8 @@ bool CheckForeignSessionsAgainst(
   for (size_t j = 0; j < sessions.size(); ++j, ++window_index) {
     if (window_index == index)
       window_index++;  // Skip self.
-    if (!WindowsMatch(sessions[j]->windows, *windows[window_index]))
+    if (!WindowsMatch(sessions[j]->windows,
+                      *(windows[window_index].const_get())))
       return false;
   }