[Sync] Moved tab_node_id tracking to session object and improved foreign session garbage collection.

components/sync_sessions/synced_session_tracker.cc

 // Copyright (c) 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 "components/sync_sessions/synced_session_tracker.h"
 
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/sync_sessions/sync_sessions_client.h"
@@ skipping 10 matching lines @@
     for (const sessions::SerializedNavigationEntry& navigation :
          tab->navigations) {
       if (sessions_client->ShouldSyncURL(navigation.virtual_url())) {
         return true;
       }
     }
   }
   return false;
 }
 
+// To be presentable it means you must have windows with tabs with syncable

[Nicolas Zea, 2016/04/12 20:38:03]

"To be presentable it means you must have windows with tabs with syncable
content" -> "Presentable means |foreign_session| must have syncable content"

[skym, 2016/04/12 22:12:14]

Done.

+// content.
+bool IsPresentable(sync_sessions::SyncSessionsClient* sessions_client,
+                   sync_driver::SyncedSession* foreign_session) {
+  for (sync_driver::SyncedSession::SyncedWindowMap::const_iterator iter =
+           foreign_session->windows.begin();
+       iter != foreign_session->windows.end(); ++iter) {
+    if (ShouldSyncSessionWindow(sessions_client, *(iter->second))) {
+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
+SyncedSessionTracker::SessionTabWrapper::SessionTabWrapper()
+    : tab_ptr(NULL), owned(false) {}
+
+SyncedSessionTracker::SessionTabWrapper::SessionTabWrapper(
+    sessions::SessionTab* tab_ptr,
+    OwnedState owned,
+    int tab_node_id)
+    : tab_ptr(tab_ptr), owned(owned == IS_OWNED) {
+  tab_node_ids.insert(tab_node_id);
+}
+
+SyncedSessionTracker::SessionTabWrapper::~SessionTabWrapper() {}
+
 SyncedSessionTracker::SyncedSessionTracker(
     sync_sessions::SyncSessionsClient* sessions_client)
     : sessions_client_(sessions_client) {}
 
 SyncedSessionTracker::~SyncedSessionTracker() {
   Clear();
 }
 
 void SyncedSessionTracker::SetLocalSessionTag(
     const std::string& local_session_tag) {
   local_session_tag_ = local_session_tag;
 }
 
 bool SyncedSessionTracker::LookupAllForeignSessions(
-    std::vector<const sync_driver::SyncedSession*>* sessions) const {
+    std::vector<const sync_driver::SyncedSession*>* sessions,
+    bool presentable) const {
   DCHECK(sessions);
   sessions->clear();
-  // Fill vector of sessions from our synced session map.
   for (SyncedSessionMap::const_iterator i = synced_session_map_.begin();
        i != synced_session_map_.end(); ++i) {
-    // Only include foreign sessions with open tabs.
     sync_driver::SyncedSession* foreign_session = i->second;
-    if (i->first != local_session_tag_ && !foreign_session->windows.empty()) {
-      bool found_tabs = false;
-      for (sync_driver::SyncedSession::SyncedWindowMap::const_iterator iter =
-               foreign_session->windows.begin();
-           iter != foreign_session->windows.end(); ++iter) {
-        if (ShouldSyncSessionWindow(sessions_client_, *(iter->second))) {
-          found_tabs = true;
-          break;
-        }
-      }
-      if (found_tabs)
-        sessions->push_back(foreign_session);
+    if (i->first != local_session_tag_ &&
+        (!presentable || IsPresentable(sessions_client_, foreign_session))) {
+      sessions->push_back(foreign_session);
     }
   }
-
   return !sessions->empty();
 }
 
 bool SyncedSessionTracker::LookupSessionWindows(
     const std::string& session_tag,
     std::vector<const sessions::SessionWindow*>* windows) const {
   DCHECK(windows);
   windows->clear();
   SyncedSessionMap::const_iterator iter = synced_session_map_.find(session_tag);
   if (iter == synced_session_map_.end())
     return false;
   windows->clear();
   for (sync_driver::SyncedSession::SyncedWindowMap::const_iterator window_iter =
            iter->second->windows.begin();
        window_iter != iter->second->windows.end(); ++window_iter) {
     windows->push_back(window_iter->second);
   }
   return true;
 }
 
 bool SyncedSessionTracker::LookupSessionTab(
     const std::string& tag,
     SessionID::id_type tab_id,
     const sessions::SessionTab** tab) const {
   DCHECK(tab);
   SyncedTabMap::const_iterator tab_map_iter = synced_tab_map_.find(tag);
   if (tab_map_iter == synced_tab_map_.end()) {
     // We have no record of this session.
-    *tab = NULL;
+    *tab = nullptr;
     return false;
   }
   IDToSessionTabMap::const_iterator tab_iter =
       tab_map_iter->second.find(tab_id);
   if (tab_iter == tab_map_iter->second.end()) {
     // We have no record of this tab.
-    *tab = NULL;
+    *tab = nullptr;
     return false;
   }
   *tab = tab_iter->second.tab_ptr;
   return true;
 }
 
 bool SyncedSessionTracker::LookupTabNodeIds(const std::string& session_tag,
                                             std::set<int>* tab_node_ids) {
   tab_node_ids->clear();
   SyncedTabMap::const_iterator tab_map_iter = synced_tab_map_.find(session_tag);
-  if (tab_map_iter == synced_tab_map_.end())
+  if (tab_map_iter == synced_tab_map_.end()) {

[Nicolas Zea, 2016/04/12 20:38:03]

nit: single line if body

[skym, 2016/04/12 22:12:14]

Method has changed significantly.

     return false;
+  }
 
   IDToSessionTabMap::const_iterator tab_iter = tab_map_iter->second.begin();
   while (tab_iter != tab_map_iter->second.end()) {
-    if (tab_iter->second.tab_node_id != TabNodePool::kInvalidTabNodeID)
-      tab_node_ids->insert(tab_iter->second.tab_node_id);
+    tab_node_ids->insert(tab_iter->second.tab_node_ids.begin(),
+                         tab_iter->second.tab_node_ids.end());
     ++tab_iter;
   }
+
+  // Incase an invalid node id was included, remove it.
+  tab_node_ids->erase(TabNodePool::kInvalidTabNodeID);
+
   return true;
 }
 
 bool SyncedSessionTracker::LookupLocalSession(
     const sync_driver::SyncedSession** output) const {
   SyncedSessionMap::const_iterator it =
       synced_session_map_.find(local_session_tag_);
   if (it != synced_session_map_.end()) {
     *output = it->second;
     return true;
@@ skipping 11 matching lines @@
     DVLOG(1) << "Creating new session with tag " << session_tag << " at "
              << synced_session;
     synced_session->session_tag = session_tag;
     synced_session_map_[session_tag] = synced_session;
   }
   DCHECK(synced_session);
   return synced_session;
 }
 
 bool SyncedSessionTracker::DeleteSession(const std::string& session_tag) {
-  bool found_session = false;
+  // Cleanup first, we need to check every wrapper and remove model objects
+  // if the parent session isn't going to delete in it's destructor.
+  CleanupSession(session_tag);
+
+  bool header_existed = false;
   SyncedSessionMap::iterator iter = synced_session_map_.find(session_tag);
   if (iter != synced_session_map_.end()) {
     sync_driver::SyncedSession* session = iter->second;
+    // An implicitly created session that has children tabs but no header node
+    // will have never had the device_type changed from unset.
+    header_existed =
+        session->device_type != sync_driver::SyncedSession::TYPE_UNSET;
     synced_session_map_.erase(iter);
     // SyncedSession's destructor will trigger deletion of windows which will in
-    // turn trigger the deletion of tabs. This doens't affect wrappers.
+    // turn trigger the deletion of tabs. This doesn't affect wrappers.
     delete session;
-    found_session = true;
   }
+
   // These two erase(...) calls only affect the wrappers.
   synced_window_map_.erase(session_tag);
-  // It's possible there was no header node but there were tab nodes.
-  if (synced_tab_map_.erase(session_tag) > 0) {
-    found_session = true;
-  }
-  return found_session;
+  synced_tab_map_.erase(session_tag);
+
+  return header_existed;
 }
 
 void SyncedSessionTracker::ResetSessionTracking(
     const std::string& session_tag) {
   // Reset window tracking.
   GetSession(session_tag)->windows.clear();
   SyncedWindowMap::iterator window_iter = synced_window_map_.find(session_tag);
   if (window_iter != synced_window_map_.end()) {
     for (IDToSessionWindowMap::iterator window_map_iter =
              window_iter->second.begin();
          window_map_iter != window_iter->second.end(); ++window_map_iter) {
       window_map_iter->second.owned = false;
       // We clear out the tabs to prevent double referencing of the same tab.
       // All tabs that are in use will be added back as needed.
       window_map_iter->second.window_ptr->tabs.clear();
     }
   }
 
   // Reset tab tracking.
   SyncedTabMap::iterator tab_iter = synced_tab_map_.find(session_tag);
   if (tab_iter != synced_tab_map_.end()) {
     for (IDToSessionTabMap::iterator tab_map_iter = tab_iter->second.begin();
          tab_map_iter != tab_iter->second.end(); ++tab_map_iter) {
       tab_map_iter->second.owned = false;
     }
   }
 }
 
+void SyncedSessionTracker::TryDeleteTab(const std::string& session_tag,
+                                        SessionID::id_type tab_id,
+                                        int tab_node_id) {
+  SyncedTabMap::iterator tab_map_iter = synced_tab_map_.find(session_tag);
+  if (tab_map_iter != synced_tab_map_.end()) {
+    IDToSessionTabMap::iterator tab_iter = tab_map_iter->second.find(tab_id);
+    if (tab_iter != tab_map_iter->second.end()) {
+      // Scrub any occurrence of |tab_node_id| first to make verifying if we
+      // should actually delete easier.
+      tab_iter->second.tab_node_ids.erase(tab_node_id);
+
+      // Try to delete the tab if it no longer holds valid tab node ids and it's
+      // an orphan.
+      if (tab_iter->second.tab_node_ids.empty() &&
+          DeleteOldSessionTabIfNecessary(tab_iter->second)) {
+        tab_map_iter->second.erase(tab_iter);
+      }
+    }
+  }
+}
+
 bool SyncedSessionTracker::DeleteOldSessionWindowIfNecessary(
-    SessionWindowWrapper window_wrapper) {
+    const SessionWindowWrapper& window_wrapper) {
   if (!window_wrapper.owned) {
     DVLOG(1) << "Deleting closed window "
              << window_wrapper.window_ptr->window_id.id();
     // Clear the tabs first, since we don't want the destructor to destroy
     // them. Their deletion will be handled by DeleteOldSessionTabIfNecessary.
     window_wrapper.window_ptr->tabs.clear();
     delete window_wrapper.window_ptr;
     return true;
   }
   return false;
 }
 
 bool SyncedSessionTracker::DeleteOldSessionTabIfNecessary(
-    SessionTabWrapper tab_wrapper) {
+    const SessionTabWrapper& tab_wrapper) {
   if (!tab_wrapper.owned) {
     if (VLOG_IS_ON(1)) {
       sessions::SessionTab* tab_ptr = tab_wrapper.tab_ptr;
       std::string title;
       if (tab_ptr->navigations.size() > 0) {
         title =
             " (" +
             base::UTF16ToUTF8(
                 tab_ptr->navigations[tab_ptr->navigations.size() - 1].title()) +
             ")";
@@ skipping 31 matching lines @@
         tab_iter->second.erase(iter++);
       } else {
         ++iter;
       }
     }
   }
 }
 
 void SyncedSessionTracker::PutWindowInSession(const std::string& session_tag,
                                               SessionID::id_type window_id) {
-  sessions::SessionWindow* window_ptr = NULL;
+  sessions::SessionWindow* window_ptr = nullptr;
   IDToSessionWindowMap::iterator iter =
       synced_window_map_[session_tag].find(window_id);
   if (iter != synced_window_map_[session_tag].end()) {
     iter->second.owned = true;
     window_ptr = iter->second.window_ptr;
     DVLOG(1) << "Putting seen window " << window_id << " at " << window_ptr
              << "in " << (session_tag == local_session_tag_ ? "local session"
                                                             : session_tag);
   } else {
     // Create the window.
@@ skipping 20 matching lines @@
   // SessionWindow information of a SessionHeader node for a foreign session,
   // and 2) The SessionHeader node for our local session changed. In both cases
   // we need to update our tracking state to reflect the change.
   //
   // Because the SessionHeader nodes are separate from the individual tab nodes
   // and we don't store tab_node_ids in the header / SessionWindow specifics,
   // the tab_node_ids are not always available when processing headers.
   // We know that we will eventually process (via GetTab) every single tab node
   // in the system, so we permit ourselves to use kInvalidTabNodeID here and
   // rely on the later update to build the mapping (or a restart).
-  // TODO(tim): Bug 98892. Update comment when Sync API conversion finishes to
-  // mention that in the meantime, the only ill effect is that we may not be
-  // able to fully clean up a stale foreign session, but it will get garbage
-  // collected eventually.
   sessions::SessionTab* tab_ptr =
       GetTabImpl(session_tag, tab_id, TabNodePool::kInvalidTabNodeID);
 
   // It's up to the caller to ensure this never happens.  Tabs should not
   // belong to more than one window or appear twice within the same window.
   //
   // If this condition were violated, we would double-free during shutdown.
   // That could cause all sorts of hard to diagnose crashes, possibly in code
   // far away from here.  We crash early to avoid this.
   //
   // See http://crbug.com/360822.
   CHECK(!synced_tab_map_[session_tag][tab_id].owned);
 
   // Only tabs that were just created in GetTabImpl(...) are still present in
   // unmapped_tabs_. Most of the time when PutTabInWindow(...) is invoked, the
   // specified tab was already a child of the specified window, and hasn't been
   // in unmapped_tabs_ for quite some time. However, this is not a problem, as
   // std::set::erase(...) will simply have no effect in these cases.
   unmapped_tabs_.erase(tab_ptr);
   synced_tab_map_[session_tag][tab_id].owned = true;
 
   tab_ptr->window_id.set_id(window_id);
   DVLOG(1) << "  - tab " << tab_id << " added to window " << window_id;
   DCHECK(GetSession(session_tag)->windows.find(window_id) !=
          GetSession(session_tag)->windows.end());
   std::vector<sessions::SessionTab*>& window_tabs =
       GetSession(session_tag)->windows[window_id]->tabs;
   if (window_tabs.size() <= tab_index) {
-    window_tabs.resize(tab_index + 1, NULL);
+    window_tabs.resize(tab_index + 1, nullptr);
   }
   DCHECK(!window_tabs[tab_index]);
   window_tabs[tab_index] = tab_ptr;
 }
 
 sessions::SessionTab* SyncedSessionTracker::GetTab(
     const std::string& session_tag,
     SessionID::id_type tab_id,
     int tab_node_id) {
   DCHECK_NE(TabNodePool::kInvalidTabNodeID, tab_node_id);
   return GetTabImpl(session_tag, tab_id, tab_node_id);
 }
 
 sessions::SessionTab* SyncedSessionTracker::GetTabImpl(
     const std::string& session_tag,
     SessionID::id_type tab_id,
     int tab_node_id) {
-  sessions::SessionTab* tab_ptr = NULL;
+  sessions::SessionTab* tab_ptr = nullptr;
   IDToSessionTabMap::iterator iter = synced_tab_map_[session_tag].find(tab_id);
   if (iter != synced_tab_map_[session_tag].end()) {
     tab_ptr = iter->second.tab_ptr;
     if (tab_node_id != TabNodePool::kInvalidTabNodeID &&
         tab_id != TabNodePool::kInvalidTabID) {
       // TabIDs are not stable across restarts of a client. Consider this
       // example with two tabs:
       //
       // http://a.com  TabID1 --> NodeIDA
       // http://b.com  TabID2 --> NodeIDB
       //
       // After restart, tab ids are reallocated. e.g, one possibility:
       // http://a.com TabID2 --> NodeIDA
       // http://b.com TabID1 --> NodeIDB
       //
       // If that happend on a remote client, here we will see an update to
       // TabID1 with tab_node_id changing from NodeIDA to NodeIDB, and TabID2
       // with tab_node_id changing from NodeIDB to NodeIDA.
       //
       // We can also wind up here if we created this tab as an out-of-order
       // update to the header node for this session before actually associating
       // the tab itself, so the tab node id wasn't available at the time and
       // is currenlty kInvalidTabNodeID.
       //
-      // In both cases, we update the tab_node_id.
-      iter->second.tab_node_id = tab_node_id;
+      // In both cases, we can safely throw it into the set of node ids.
+      iter->second.tab_node_ids.insert(tab_node_id);
     }
 
     if (VLOG_IS_ON(1)) {
       std::string title;
       if (tab_ptr->navigations.size() > 0) {
         title =
             " (" +
             base::UTF16ToUTF8(
                 tab_ptr->navigations[tab_ptr->navigations.size() - 1].title()) +
             ")";
@@ skipping 13 matching lines @@
              << (session_tag == local_session_tag_ ? "local session"
                                                    : session_tag)
              << "'s new tab " << tab_id << " at " << tab_ptr;
   }
   DCHECK(tab_ptr);
   DCHECK_EQ(tab_ptr->tab_id.id(), tab_id);
   return tab_ptr;
 }
 
 void SyncedSessionTracker::Clear() {
+  // Cleanup unowned things first, before we let SyncedSession's destructor take
+  // care of everything else.
+  for (const auto& kv : synced_session_map_) {
+    CleanupSession(kv.first);
+  }
+
   // Delete SyncedSession objects (which also deletes all their windows/tabs).
   STLDeleteValues(&synced_session_map_);
 
   // Go through and delete any tabs we had allocated but had not yet placed into
   // a SyncedSession object.
   STLDeleteElements(&unmapped_tabs_);
 
   // Get rid of our Window/Tab maps (does not delete the actual Window/Tabs
   // themselves; they should have all been deleted above).
   synced_window_map_.clear();
   synced_tab_map_.clear();
   local_session_tag_.clear();
 }
 
 }  // namespace browser_sync