Do not submit: high level overview patch.

chrome/browser/sync/glue/session_model_associator.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/glue/session_model_associator.h"
 
 #include <algorithm>
 #include <set>
 #include <utility>
 
@@ skipping 125 matching lines @@
   DCHECK(CalledOnValidThread());
 }
 
 bool SessionModelAssociator::InitSyncNodeFromChromeId(
     const std::string& id,
     syncer::BaseNode* sync_node) {
   NOTREACHED();
   return false;
 }
 
+void SessionModelAssociator::NotifySyncIdGenerated(
+    const SyncedTabDelegate& tab) {
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_SESSION_SYNC_ID_GENERATED,
+      content::Source<SessionModelAssociator>(this),
+      content::Details<const SyncedTabDelegate>(&tab));
+}
+
 bool SessionModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) {
   DCHECK(CalledOnValidThread());
   CHECK(has_nodes);
   *has_nodes = false;
   syncer::ReadTransaction trans(FROM_HERE, sync_service_->GetUserShare());
   syncer::ReadNode root(&trans);
   if (root.InitByTagLookup(syncer::ModelTypeToRootTag(syncer::SESSIONS)) !=
                            syncer::BaseNode::INIT_OK) {
     LOG(ERROR) << kNoSessionsFolderError;
     return false;
@@ skipping 39 matching lines @@
   sync_pb::SessionSpecifics specifics;
   specifics.set_session_tag(local_tag);
   sync_pb::SessionHeader* header_s = specifics.mutable_header();
   SyncedSession* current_session =
       synced_session_tracker_.GetSession(local_tag);
   current_session->modified_time = base::Time::Now();
   header_s->set_client_name(current_session_name_);
   header_s->set_device_type(DeviceInfo::GetLocalDeviceType());
 
   synced_session_tracker_.ResetSessionTracking(local_tag);
+  std::vector<int64> used_sync_ids;
   std::set<SyncedWindowDelegate*> windows =
       SyncedWindowDelegate::GetSyncedWindowDelegates();
   for (std::set<SyncedWindowDelegate*>::const_iterator i =
            windows.begin(); i != windows.end(); ++i) {
     // Make sure the window has tabs and a viewable window. The viewable window
     // check is necessary because, for example, when a browser is closed the
     // destructor is not necessarily run immediately. This means its possible
     // for us to get a handle to a browser that is about to be removed. If
     // the tab count is 0 or the window is NULL, the browser is about to be
     // deleted, so we ignore it.
     if (ShouldSyncWindow(*i) && (*i)->GetTabCount() && (*i)->HasWindow()) {
       sync_pb::SessionWindow window_s;
       SessionID::id_type window_id = (*i)->GetSessionId();
       DVLOG(1) << "Associating window " << window_id << " with "
                << (*i)->GetTabCount() << " tabs.";
       window_s.set_window_id(window_id);
       // Note: We don't bother to set selected tab index anymore. We still
-      // consume it when receiving foreign sessions, as reading it is free, but
+      // consume it when   foreign sessions, as reading it is free, but
       // it triggers too many sync cycles with too little value to make setting
       // it worthwhile.
       if ((*i)->IsTypeTabbed()) {
         window_s.set_browser_type(
             sync_pb::SessionWindow_BrowserType_TYPE_TABBED);
       } else {
         window_s.set_browser_type(
             sync_pb::SessionWindow_BrowserType_TYPE_POPUP);
       }
 
       // Store the order of tabs.
       bool found_tabs = false;
+      std::vector<SessionID::id_type> used_tabs;
       for (int j = 0; j < (*i)->GetTabCount(); ++j) {
         SessionID::id_type tab_id = (*i)->GetTabIdAt(j);
+        SyncedTabDelegate* tab_delegate = (*i)->GetTabAt(j);
 
         if (reload_tabs) {
-          SyncedTabDelegate* tab = (*i)->GetTabAt(j);
+          if (tab_delegate && !tab_delegate->IsTabInMemory()) {
+            found_tabs = true;
+            // Update tab id since it might have changed.
+            UpdateTabIdForOldTab(tab_delegate->GetSyncSessionId(), tab_id);

[Nicolas Zea, 2013/05/16 22:59:03]

I don't think you should ever update the tab id. That's just sync's id for the
current tab node count at the time it was created. Multiple tabs, over the life
of a sync session, can use the tab sync node, but the tab id for that sync node
should never change. Perhaps I'm misunderstanding something?

[shashi, 2013/05/17 00:29:16]

When restore happens, the tab ids (different from tab_node_id) are
reinitialized.
I discovered this during prototyping, easiest way to explain this is by an
example:
Lets say before restore I had 3 tabs (A,B,C) with tab ids (1, 2, 3) after
restore the tab ids may be (20, 30, 10).
Suppose only tab A is in memory.
After tab association, the window_s will have tabs:
(2, 30, 10). We will rewrite the session node for A. 
So A will have tab_id=20 but since B and C are not in memory the session nodes
for them will be unchanged and will have 2 and 3 tab ids.

Remote client will see a window with tab ids: (20, 30, 10) but will not able to
make out the tab nodes corresponding to these tab ids.


On 2013/05/16 22:59:03, Nicolas Zea wrote:

+            window_s.add_tab(tab_id);
+            used_sync_ids.push_back(tab_delegate->GetSyncSessionId());
+            continue;
+          }
+
           // It's possible for GetTabAt to return a null tab if it's not in
           // memory. We can assume this means the tab already existed but hasn't
           // changed, so no need to reassociate.
-          if (tab && !AssociateTab(*tab, error)) {
+          if (tab_delegate && tab_delegate->IsTabInMemory() &&
+              !AssociateTab(*tab_delegate, error)) {
             // Association failed. Either we need to re-associate, or this is an
             // unrecoverable error.
             return false;
           }
         }
 
         // If the tab is valid, it would have been added to the tracker either
         // by the above AssociateTab call (at association time), or by the
         // change processor calling AssociateTab for all modified tabs.
         // Therefore, we can key whether this window has valid tabs based on
         // the tab's presence in the tracker.
         const SessionTab* tab = NULL;
         if (synced_session_tracker_.LookupSessionTab(local_tag, tab_id, &tab)) {
+          used_tabs.push_back(tab_id);
           found_tabs = true;
+          used_sync_ids.push_back(tab_delegate->GetSyncSessionId());
           window_s.add_tab(tab_id);
         }
       }
       // Only add a window if it contains valid tabs.
       if (found_tabs) {
         sync_pb::SessionWindow* header_window = header_s->add_window();
         *header_window = window_s;
 
         // Update this window's representation in the synced session tracker.
         synced_session_tracker_.PutWindowInSession(local_tag, window_id);
         PopulateSessionWindowFromSpecifics(
             local_tag,
             window_s,
             base::Time::Now(),
             current_session->windows[window_id],
             &synced_session_tracker_);
       }
     }
   }
   // Free memory for closed windows and tabs.
   synced_session_tracker_.CleanupSession(local_tag);
+  tab_pool_.PurgeOldSyncNodes(used_sync_ids);
 
   syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
   syncer::WriteNode header_node(&trans);
   if (header_node.InitByIdLookup(local_session_syncid_) !=
           syncer::BaseNode::INIT_OK) {
     if (error) {
       *error = error_handler_->CreateAndUploadError(
            FROM_HERE,
            "Failed to load local session header node.",
            model_type());
     }
     return false;
   }
   header_node.SetSessionSpecifics(specifics);
   if (waiting_for_change_) QuitLoopForSubtleTesting();
   return true;
 }
 
+void SessionModelAssociator::UpdateTabIdForOldTab(
+    int64 sync_id,
+    SessionID::id_type new_tab_id) {
+  syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
+  syncer::WriteNode tab_node(&trans);
+  // Rewrite tab id if required.
+  if (tab_node.InitByIdLookup(sync_id) != syncer::BaseNode::INIT_OK) {
+    return;
+  }
+  sync_pb::SessionSpecifics session_specifics = tab_node.GetSessionSpecifics();
+  DCHECK(session_specifics.has_tab());
+  sync_pb::SessionTab* tab_s = session_specifics.mutable_tab();
+  tab_s->set_tab_id(new_tab_id);
+  tab_node.SetSessionSpecifics(session_specifics);
+}
+
 // Static.
 bool SessionModelAssociator::ShouldSyncWindow(
     const SyncedWindowDelegate* window) {
   if (window->IsApp())
     return false;
   return window->IsTypeTabbed() || window->IsTypePopup();
 }
 
 bool SessionModelAssociator::AssociateTabs(
     const std::vector<SyncedTabDelegate*>& tabs,
     syncer::SyncError* error) {
   DCHECK(CalledOnValidThread());
   for (std::vector<SyncedTabDelegate*>::const_iterator i = tabs.begin();
        i != tabs.end();
        ++i) {
     if (!AssociateTab(**i, error))
       return false;
   }
   if (waiting_for_change_) QuitLoopForSubtleTesting();
   return true;
 }
 
-bool SessionModelAssociator::AssociateTab(const SyncedTabDelegate& tab,
+bool SessionModelAssociator::AssociateTab(SyncedTabDelegate& tab,
                                           syncer::SyncError* error) {
   DCHECK(CalledOnValidThread());
   int64 sync_id;
-  SessionID::id_type tab_id = tab.GetSessionId();
+  SessionID::id_type tab_id = tab.GetSessionId().id();
   if (tab.IsBeingDestroyed()) {
     // This tab is closing.
     TabLinksMap::iterator tab_iter = tab_map_.find(tab_id);
     if (tab_iter == tab_map_.end()) {
       // We aren't tracking this tab (for example, sync setting page).
       return true;
     }
     tab_pool_.FreeTabNode(tab_iter->second->sync_id());
     tab_map_.erase(tab_iter);
     return true;
   }
 
   if (!ShouldSyncTab(tab))
     return true;
 
   TabLinksMap::iterator tab_map_iter = tab_map_.find(tab_id);
   TabLink* tab_link = NULL;
   if (tab_map_iter == tab_map_.end()) {
-    // This is a new tab, get a sync node for it.
-    sync_id = tab_pool_.GetFreeTabNode();
+    // This is a new tab, get a sync node for it. Check if there is an old sync
+    // node for it.
+    sync_id = tab_pool_.GetOldSyncNode(tab.GetSyncSessionId());
+    if (sync_id < 0) {
+      sync_id = tab_pool_.GetFreeTabNode();
+    }
+    if (tab.GetSyncSessionId() != sync_id) {
+      tab.SetSyncSessionId(sync_id);
+      NotifySyncIdGenerated(tab);

[Nicolas Zea, 2013/05/16 22:59:03]

would it be better to have the equivalency check and the notify call part of the
SetSyncSessionId method?

[shashi, 2013/05/17 00:29:16]

Yes, I can replace it with an equivalency check, any maybe generate notification
directly from SetSyncSessionId.

On 2013/05/16 22:59:03, Nicolas Zea wrote:

+    }
+
     if (sync_id == syncer::kInvalidId) {
       if (error) {
         *error = error_handler_->CreateAndUploadError(
             FROM_HERE,
             "Received invalid tab node from tab pool.",
             model_type());
       }
       return false;
     }
     tab_link = new TabLink(sync_id, &tab);
     tab_map_[tab_id] = make_linked_ptr<TabLink>(tab_link);
   } else {
     // This tab is already associated with a sync node, reuse it.
     // Note: on some platforms the tab object may have changed, so we ensure
     // the tab link is up to date.
     tab_link = tab_map_iter->second.get();
     tab_map_iter->second->set_tab(&tab);
   }
   DCHECK(tab_link);
   DCHECK_NE(tab_link->sync_id(), syncer::kInvalidId);
 
   DVLOG(1) << "Reloading tab " << tab_id << " from window "
-           << tab.GetWindowId();
+           << tab.GetWindowId().id();
   return WriteTabContentsToSyncModel(tab_link, error);
 }
 
 // static
 GURL SessionModelAssociator::GetCurrentVirtualURL(
     const SyncedTabDelegate& tab_delegate) {
   const int current_index = tab_delegate.GetCurrentEntryIndex();
   const int pending_index = tab_delegate.GetPendingEntryIndex();
   const NavigationEntry* current_entry =
       (current_index == pending_index) ?
@@ skipping 13 matching lines @@
       tab_delegate.GetEntryAtIndex(current_index);
   return (current_entry->GetFavicon().valid ?
           current_entry->GetFavicon().url :
           GURL());
 }
 
 bool SessionModelAssociator::WriteTabContentsToSyncModel(
     TabLink* tab_link,
     syncer::SyncError* error) {
   DCHECK(CalledOnValidThread());
+  DCHECK(tab_link->tab());
   const SyncedTabDelegate& tab_delegate = *(tab_link->tab());
   int64 sync_id = tab_link->sync_id();
   GURL old_tab_url = tab_link->url();
 
   // Load the last stored version of this tab so we can compare changes. If this
   // is a new tab, session_tab will be a blank/newly created SessionTab object.
-  SessionTab* session_tab =
-      synced_session_tracker_.GetTab(GetCurrentMachineTag(),
-                                     tab_delegate.GetSessionId());
+  SessionTab* session_tab = synced_session_tracker_.GetTab(
+      GetCurrentMachineTag(), tab_delegate.GetSessionId().id());
 
   SetSessionTabFromDelegate(tab_delegate, base::Time::Now(), session_tab);
 
   const GURL new_url = GetCurrentVirtualURL(tab_delegate);
-  DVLOG(1) << "Local tab " << tab_delegate.GetSessionId()
-           << " now has URL " << new_url.spec();
+  VLOG(1) << "Local tab " << tab_delegate.GetSessionId().id() << " now has URL "
+          << new_url.spec();
 
   // Trigger the favicon load if needed. We do this before opening the write
   // transaction to avoid jank.
   tab_link->set_url(new_url);
   if (new_url != old_tab_url) {
     favicon_cache_.OnFaviconVisited(new_url,
                                     GetCurrentFaviconURL(tab_delegate));
   }
 
   // Update our last modified time.
   synced_session_tracker_.GetSession(GetCurrentMachineTag())->modified_time =
       base::Time::Now();
-
   syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
   syncer::WriteNode tab_node(&trans);
   if (tab_node.InitByIdLookup(sync_id) != syncer::BaseNode::INIT_OK) {
     if (error) {
       *error = error_handler_->CreateAndUploadError(
           FROM_HERE,
           "Failed to look up local tab node",
           model_type());
     }
     return false;
   }
-
   sync_pb::SessionTab tab_s = session_tab->ToSyncData();
   sync_pb::SessionSpecifics specifics = tab_node.GetSessionSpecifics();
   if (new_url == old_tab_url) {
     // Load the old specifics and copy over the favicon data if needed.
     // TODO(zea): remove this once favicon sync is enabled as a separate type.
     tab_s.set_favicon(specifics.tab().favicon());
     tab_s.set_favicon_source(specifics.tab().favicon_source());
     tab_s.set_favicon_type(specifics.tab().favicon_type());
   }
   // Retain the base SessionSpecifics data (tag, tab_node_id, etc.), and just
   // write the new SessionTabSpecifics.
   specifics.mutable_tab()->CopyFrom(tab_s);
-
   // Write into the actual sync model.
   tab_node.SetSessionSpecifics(specifics);
 
   return true;
 }
 
 // static
 void SessionModelAssociator::SetSessionTabFromDelegate(
     const SyncedTabDelegate& tab_delegate,
     base::Time mtime,
     SessionTab* session_tab) {
   DCHECK(session_tab);
-  session_tab->window_id.set_id(tab_delegate.GetWindowId());
-  session_tab->tab_id.set_id(tab_delegate.GetSessionId());
+  session_tab->window_id.set_id(tab_delegate.GetWindowId().id());
+  session_tab->tab_id.set_id(tab_delegate.GetSessionId().id());
   session_tab->tab_visual_index = 0;
   session_tab->current_navigation_index = tab_delegate.GetCurrentEntryIndex();
   session_tab->pinned = tab_delegate.IsPinned();
   session_tab->extension_app_id = tab_delegate.GetExtensionAppId();
+  session_tab->sync_session_id = tab_delegate.GetSyncSessionId();
   session_tab->user_agent_override.clear();
   session_tab->timestamp = mtime;
   const int current_index = tab_delegate.GetCurrentEntryIndex();
   const int pending_index = tab_delegate.GetPendingEntryIndex();
   const int min_index = std::max(0,
                                  current_index - kMaxSyncNavigationCount);
   const int max_index = std::min(current_index + kMaxSyncNavigationCount,
                                  tab_delegate.GetEntryCount());
   session_tab->navigations.clear();
   for (int i = min_index; i < max_index; ++i) {
@@ skipping 269 matching lines @@
         if (specifics.has_header()) {
           LOG(WARNING) << "Found more than one session header node with local "
                        << " tag.";
         } else if (!specifics.has_tab()) {
           LOG(WARNING) << "Found local node with no header or tag field.";
         }
 
         // TODO(zea): fix this once we add support for reassociating
         // pre-existing tabs with pre-existing tab nodes. We'll need to load
         // the tab_node_id and ensure the tab_pool_ keeps track of them.
-        sync_node.Tombstone();
+        if (specifics.has_tab())
+          tab_pool_.AddTabNode(id, specifics.tab_node_id());

[Nicolas Zea, 2013/05/16 22:59:03]

else tombstone? (it would be the above error case where more than one session
header was found or the node was just corrupted somehow)

[shashi, 2013/05/17 00:29:16]

Good point, I should  handle the case for multiple header nodes/corrupted nodes.
Will fix this.
On 2013/05/16 22:59:03, Nicolas Zea wrote:

       }
     }
     id = next_id;
   }
 
   // After updating from sync model all tabid's should be free.
   DCHECK(tab_pool_.full());
   return true;
 }
 
@@ skipping 320 matching lines @@
   }
 }
 
 bool SessionModelAssociator::IsValidTab(const SyncedTabDelegate& tab) const {
   if ((!sync_service_ || tab.profile() != sync_service_->profile()) &&
       !setup_for_test_) {
     return false;
   }
   const SyncedWindowDelegate* window =
       SyncedWindowDelegate::FindSyncedWindowDelegateWithId(
-          tab.GetWindowId());
+          tab.GetWindowId().id());
   if (!window && !setup_for_test_)
     return false;
   return true;
 }
 
 bool SessionModelAssociator::TabHasValidEntry(
     const SyncedTabDelegate& tab) const {
   int pending_index = tab.GetPendingEntryIndex();
   int entry_count = tab.GetEntryCount();
   bool found_valid_url = false;
@@ skipping 49 matching lines @@
 
 bool SessionModelAssociator::CryptoReadyIfNecessary() {
   // We only access the cryptographer while holding a transaction.
   syncer::ReadTransaction trans(FROM_HERE, sync_service_->GetUserShare());
   const syncer::ModelTypeSet encrypted_types = trans.GetEncryptedTypes();
   return !encrypted_types.Has(SESSIONS) ||
          sync_service_->IsCryptographerReady(&trans);
 }
 
 }  // namespace browser_sync