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

Unified Diff: components/sync_sessions/tab_node_pool.cc

Issue 2712743006: Reland v5 of Sessions Refactor (Closed)
Patch Set: Fix typo 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 | « components/sync_sessions/tab_node_pool.h ('k') | components/sync_sessions/tab_node_pool_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/sync_sessions/tab_node_pool.cc
diff --git a/components/sync_sessions/tab_node_pool.cc b/components/sync_sessions/tab_node_pool.cc
index 4067b92894da4b50887a56fcbd07566090e98ab1..f187d23dc7644236b3074b5f9026e55660d7cc24 100644
--- a/components/sync_sessions/tab_node_pool.cc
+++ b/components/sync_sessions/tab_node_pool.cc
@@ -4,12 +4,10 @@
#include "components/sync_sessions/tab_node_pool.h"
-#include "base/format_macros.h"
+#include <algorithm>
+
#include "base/logging.h"
-#include "base/strings/stringprintf.h"
#include "components/sync/base/model_type.h"
-#include "components/sync/model/sync_change.h"
-#include "components/sync/model/sync_data.h"
#include "components/sync/protocol/session_specifics.pb.h"
#include "components/sync/protocol/sync.pb.h"
@@ -26,114 +24,91 @@ const int TabNodePool::kInvalidTabNodeID = -1;
TabNodePool::~TabNodePool() {}
-// Static
-std::string TabNodePool::TabIdToTag(const std::string& machine_tag,
- int tab_node_id) {
- return base::StringPrintf("%s %d", machine_tag.c_str(), tab_node_id);
-}
-
void TabNodePool::AddTabNode(int tab_node_id) {
DCHECK_GT(tab_node_id, kInvalidTabNodeID);
DCHECK(nodeid_tabid_map_.find(tab_node_id) == nodeid_tabid_map_.end());
- unassociated_nodes_.insert(tab_node_id);
- if (max_used_tab_node_id_ < tab_node_id)
- max_used_tab_node_id_ = tab_node_id;
+ DVLOG(1) << "Adding tab node " << tab_node_id << " to pool.";
+ max_used_tab_node_id_ = std::max(max_used_tab_node_id_, tab_node_id);
+ free_nodes_pool_.insert(tab_node_id);
}
void TabNodePool::AssociateTabNode(int tab_node_id, SessionID::id_type tab_id) {
DCHECK_GT(tab_node_id, kInvalidTabNodeID);
- // Remove sync node if it is in unassociated nodes pool.
- std::set<int>::iterator u_it = unassociated_nodes_.find(tab_node_id);
- if (u_it != unassociated_nodes_.end()) {
- unassociated_nodes_.erase(u_it);
- } else {
- // This is a new node association, the sync node should be free.
- // Remove node from free node pool and then associate it with the tab.
- std::set<int>::iterator it = free_nodes_pool_.find(tab_node_id);
- DCHECK(it != free_nodes_pool_.end());
- free_nodes_pool_.erase(it);
- }
+ DCHECK_GT(tab_id, kInvalidTabID);
+
+ // This is a new node association, the sync node should be free.
+ // Remove node from free node pool and then associate it with the tab.
+ std::set<int>::iterator it = free_nodes_pool_.find(tab_node_id);
+ DCHECK(it != free_nodes_pool_.end());
+ free_nodes_pool_.erase(it);
+
DCHECK(nodeid_tabid_map_.find(tab_node_id) == nodeid_tabid_map_.end());
+ DVLOG(1) << "Associating tab node " << tab_node_id << " with tab " << tab_id;
nodeid_tabid_map_[tab_node_id] = tab_id;
+ tabid_nodeid_map_[tab_id] = tab_node_id;
}
-int TabNodePool::GetFreeTabNode(syncer::SyncChangeList* append_changes) {
- DCHECK_GT(machine_tag_.length(), 0U);
- DCHECK(append_changes);
+bool TabNodePool::GetTabNodeForTab(SessionID::id_type tab_id,
+ int* tab_node_id) {
+ if (tabid_nodeid_map_.find(tab_id) != tabid_nodeid_map_.end()) {
+ *tab_node_id = tabid_nodeid_map_[tab_id];
+ return true;
+ }
+
if (free_nodes_pool_.empty()) {
// Tab pool has no free nodes, allocate new one.
- int tab_node_id = ++max_used_tab_node_id_;
- std::string tab_node_tag = TabIdToTag(machine_tag_, tab_node_id);
-
- // We fill the new node with just enough data so that in case of a crash/bug
- // we can identify the node as our own on re-association and reuse it.
- sync_pb::EntitySpecifics entity;
- sync_pb::SessionSpecifics* specifics = entity.mutable_session();
- specifics->set_session_tag(machine_tag_);
- specifics->set_tab_node_id(tab_node_id);
- append_changes->push_back(syncer::SyncChange(
- FROM_HERE, syncer::SyncChange::ACTION_ADD,
- syncer::SyncData::CreateLocalData(tab_node_tag, tab_node_tag, entity)));
-
- // Grow the pool by 1 since we created a new node.
- DVLOG(1) << "Adding sync node " << tab_node_id << " to tab node id pool";
- free_nodes_pool_.insert(tab_node_id);
- return tab_node_id;
+ *tab_node_id = ++max_used_tab_node_id_;
+ AddTabNode(*tab_node_id);
+
+ AssociateTabNode(*tab_node_id, tab_id);
+ return false;
} else {
// Return the next free node.
- return *free_nodes_pool_.begin();
+ *tab_node_id = *free_nodes_pool_.begin();
+ AssociateTabNode(*tab_node_id, tab_id);
+ return true;
}
}
-void TabNodePool::FreeTabNode(int tab_node_id,
- syncer::SyncChangeList* append_changes) {
- DCHECK(append_changes);
- TabNodeIDToTabIDMap::iterator it = nodeid_tabid_map_.find(tab_node_id);
- DCHECK(it != nodeid_tabid_map_.end());
- nodeid_tabid_map_.erase(it);
- FreeTabNodeInternal(tab_node_id, append_changes);
-}
-
-void TabNodePool::FreeTabNodeInternal(int tab_node_id,
- syncer::SyncChangeList* append_changes) {
- DCHECK(free_nodes_pool_.find(tab_node_id) == free_nodes_pool_.end());
- DCHECK(append_changes);
- free_nodes_pool_.insert(tab_node_id);
-
- // If number of free nodes exceed kFreeNodesHighWatermark,
- // delete sync nodes till number reaches kFreeNodesLowWatermark.
- // Note: This logic is to mitigate temporary disassociation issues with old
- // clients: http://crbug.com/259918. Newer versions do not need this.
- if (free_nodes_pool_.size() > kFreeNodesHighWatermark) {
- for (std::set<int>::iterator free_it = free_nodes_pool_.begin();
- free_it != free_nodes_pool_.end();) {
- const std::string tab_node_tag = TabIdToTag(machine_tag_, *free_it);
- append_changes->push_back(syncer::SyncChange(
- FROM_HERE, syncer::SyncChange::ACTION_DELETE,
- syncer::SyncData::CreateLocalDelete(tab_node_tag, syncer::SESSIONS)));
- free_nodes_pool_.erase(free_it++);
- if (free_nodes_pool_.size() <= kFreeNodesLowWatermark) {
- return;
- }
- }
+void TabNodePool::FreeTab(int tab_id) {
+ DCHECK_GT(tab_id, kInvalidTabID);
+ TabIDToTabNodeIDMap::iterator it = tabid_nodeid_map_.find(tab_id);
+ if (it == tabid_nodeid_map_.end()) {
+ return; // Already freed.
}
-}
-bool TabNodePool::IsUnassociatedTabNode(int tab_node_id) {
- return unassociated_nodes_.find(tab_node_id) != unassociated_nodes_.end();
+ int tab_node_id = it->second;
+ DVLOG(1) << "Freeing tab " << tab_id << " at node " << tab_node_id;
+ nodeid_tabid_map_.erase(nodeid_tabid_map_.find(tab_node_id));
+ tabid_nodeid_map_.erase(it);
+ free_nodes_pool_.insert(tab_node_id);
}
void TabNodePool::ReassociateTabNode(int tab_node_id,
SessionID::id_type tab_id) {
- // Remove from list of unassociated sync_nodes if present.
- std::set<int>::iterator it = unassociated_nodes_.find(tab_node_id);
- if (it != unassociated_nodes_.end()) {
- unassociated_nodes_.erase(it);
+ DCHECK_GT(tab_node_id, kInvalidTabNodeID);
+ DCHECK_GT(tab_id, kInvalidTabID);
+
+ auto tabid_it = tabid_nodeid_map_.find(tab_id);
+ if (tabid_it != tabid_nodeid_map_.end()) {
+ if (tabid_it->second == tab_node_id) {
+ return; // Already associated properly.
+ } else {
+ // Another node is already associated with this tab. Free it.
+ FreeTab(tab_id);
+ }
+ }
+
+ auto nodeid_it = nodeid_tabid_map_.find(tab_node_id);
+ if (nodeid_it != nodeid_tabid_map_.end()) {
+ // This node was already associated with another tab. Free it.
+ FreeTab(nodeid_it->second);
} else {
- // tab_node_id must be an already associated node.
- DCHECK(nodeid_tabid_map_.find(tab_node_id) != nodeid_tabid_map_.end());
+ // This is a new tab node. Add it before association.
+ AddTabNode(tab_node_id);
}
- nodeid_tabid_map_[tab_node_id] = tab_id;
+
+ AssociateTabNode(tab_node_id, tab_id);
}
SessionID::id_type TabNodePool::GetTabIdFromTabNodeId(int tab_node_id) const {
@@ -144,27 +119,33 @@ SessionID::id_type TabNodePool::GetTabIdFromTabNodeId(int tab_node_id) const {
return kInvalidTabID;
}
-void TabNodePool::DeleteUnassociatedTabNodes(
- syncer::SyncChangeList* append_changes) {
- for (std::set<int>::iterator it = unassociated_nodes_.begin();
- it != unassociated_nodes_.end();) {
- FreeTabNodeInternal(*it, append_changes);
- unassociated_nodes_.erase(it++);
+void TabNodePool::CleanupTabNodes(std::set<int>* deleted_node_ids) {
+ // If number of free nodes exceed kFreeNodesHighWatermark,
+ // delete sync nodes till number reaches kFreeNodesLowWatermark.
+ // Note: This logic is to mitigate temporary disassociation issues with old
+ // clients: http://crbug.com/259918. Newer versions do not need this.
+ if (free_nodes_pool_.size() > kFreeNodesHighWatermark) {
+ for (std::set<int>::iterator free_it = free_nodes_pool_.begin();
+ free_it != free_nodes_pool_.end();) {
+ deleted_node_ids->insert(*free_it);
+ free_nodes_pool_.erase(free_it++);
+ if (free_nodes_pool_.size() <= kFreeNodesLowWatermark) {
+ return;
+ }
+ }
}
- DCHECK(unassociated_nodes_.empty());
}
// Clear tab pool.
void TabNodePool::Clear() {
- unassociated_nodes_.clear();
free_nodes_pool_.clear();
nodeid_tabid_map_.clear();
+ tabid_nodeid_map_.clear();
max_used_tab_node_id_ = kInvalidTabNodeID;
}
size_t TabNodePool::Capacity() const {
- return nodeid_tabid_map_.size() + unassociated_nodes_.size() +
- free_nodes_pool_.size();
+ return nodeid_tabid_map_.size() + free_nodes_pool_.size();
}
bool TabNodePool::Empty() const {
@@ -175,8 +156,4 @@ bool TabNodePool::Full() {
return nodeid_tabid_map_.empty();
}
-void TabNodePool::SetMachineTag(const std::string& machine_tag) {
- machine_tag_ = machine_tag;
-}
-
} // namespace sync_sessions
« no previous file with comments | « components/sync_sessions/tab_node_pool.h ('k') | components/sync_sessions/tab_node_pool_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698