Sync: Support nodes with implicit permanent folders: Node Browser and out-of-order loading from DB

sync/syncable/parent_child_index.cc

Index: sync/syncable/parent_child_index.cc
diff --git a/sync/syncable/parent_child_index.cc b/sync/syncable/parent_child_index.cc
index 0cab1d3942eb47b0bc1d1ebb8215633deef4492b..b1b493e36f8c4cc25a4897d9447a2ced1630c452 100644
--- a/sync/syncable/parent_child_index.cc
+++ b/sync/syncable/parent_child_index.cc
@@ -40,76 +40,135 @@ bool ChildComparator::operator()(const EntryKernel* a,
 }
 
 ParentChildIndex::ParentChildIndex() {
+  // Pre-allocate these two vectors to the number of model types.
+  model_type_root_ids_.resize(MODEL_TYPE_COUNT);
+  type_root_child_sets_.resize(MODEL_TYPE_COUNT);
 }
 
 ParentChildIndex::~ParentChildIndex() {
-  STLDeleteContainerPairSecondPointers(
-      parent_children_map_.begin(), parent_children_map_.end());
+  for (int i = 0; i < MODEL_TYPE_COUNT; i++) {
+    // Normally all OrderedChildSet instances in |type_root_child_sets_|
+    // are shared with |parent_children_map_|. Null out shared instances and
+    // ScopedVector will take care of the ones that are not shared (if any).
+    if (!model_type_root_ids_[i].IsNull()) {
+      DCHECK_EQ(type_root_child_sets_[i],
+                parent_children_map_.find(model_type_root_ids_[i])->second);
+      type_root_child_sets_[i] = nullptr;
+    }
+  }
+
+  STLDeleteContainerPairSecondPointers(parent_children_map_.begin(),
+                                       parent_children_map_.end());
 }
 
 bool ParentChildIndex::ShouldInclude(const EntryKernel* entry) {
   // This index excludes deleted items and the root item.  The root
   // item is excluded so that it doesn't show up as a child of itself.
-  return !entry->ref(IS_DEL) && !entry->ref(ID).IsRoot();
+  // The index also excludes items without client data e.g. the items that
+  // don't have client SPECIFICS or PARENT_ID. There is no need to include
+  // server-only entries because they shouldn't show up on the client until
+  // they're applied.
+  return !entry->ref(IS_DEL) && !entry->ref(ID).IsRoot() &&
+         (entry->GetModelType() != UNSPECIFIED ||
+          !entry->ref(PARENT_ID).IsNull());
 }
 
 bool ParentChildIndex::Insert(EntryKernel* entry) {
   DCHECK(ShouldInclude(entry));
 
-  const Id& parent_id = GetParentId(entry);
-  // Store type root ID when inserting a type root entry.
-  if (parent_id.IsRoot()) {
-    ModelType model_type = GetModelType(entry);
-    // TODO(stanisc): there are some unit tests that create entries
-    // at the root and don't bother initializing specifics which
-    // produces TOP_LEVEL_FOLDER type here.
-    if (syncer::IsRealDataType(model_type)) {
-      model_type_root_ids_[model_type] = entry->ref(ID);
+  OrderedChildSet* siblings = nullptr;
+  const Id& parent_id = entry->ref(PARENT_ID);
+  ModelType model_type = entry->GetModelType();
+
+  if (ShouldUseParentId(parent_id, model_type)) {
+    // Hierarchical type, lookup child set in the map.
+    DCHECK(!parent_id.IsNull());
+    ParentChildrenMap::iterator it = parent_children_map_.find(parent_id);
+    if (it != parent_children_map_.end()) {
+      siblings = it->second;
+    } else {
+      siblings = new OrderedChildSet();
+      parent_children_map_.insert(std::make_pair(parent_id, siblings));
     }
+  } else {
+    // Non-hierarchical type, return a pre-defined collection by type.
+    siblings = GetOrCreateModelTypeChildSet(model_type);
   }
 
-  OrderedChildSet* children = NULL;
-  ParentChildrenMap::iterator i = parent_children_map_.find(parent_id);
-  if (i != parent_children_map_.end()) {
-    children = i->second;
-  } else {
-    children = new OrderedChildSet();
-    parent_children_map_.insert(std::make_pair(parent_id, children));
+  // If this is one of type root folder for a non-hierarchical type, associate
+  // its ID with the model type and the type's pre-defined child set with the
+  // type root ID.
+  // TODO(stanisc): crbug/438313: Just TypeSupportsHierarchy condition should
+  // theoretically be sufficient but in practice many tests don't properly
+  // initialize entries so TypeSupportsHierarchy ends up failing. Consider
+  // tweaking TypeSupportsHierarchy and fixing all related test code.
+  if (parent_id.IsRoot() && entry->ref(IS_DIR) &&
+      syncer::IsRealDataType(model_type) &&
+      !TypeSupportsHierarchy(model_type)) {
+    const Id& type_root_id = entry->ref(ID);
+    // Disassociate the type root child set with the previous ID if any.
+    const Id& prev_type_root_id = model_type_root_ids_[model_type];
+    if (!prev_type_root_id.IsNull()) {
+      ParentChildrenMap::iterator it =
+          parent_children_map_.find(prev_type_root_id);
+      if (it != parent_children_map_.end()) {
+        // If the entry exists in the map it must already have the same
+        // model type specific child set. This child set will be re-inserted
+        // in the map under the new ID below so it is safe to simply erase
+        // the entry here.
+        DCHECK_EQ(it->second, GetModelTypeChildSet(model_type));
+        parent_children_map_.erase(it);
+      }
+    }
+    // Store the new type root ID and associate the child set.
+    // Note that the child set isn't owned by the map in this case.
+    model_type_root_ids_[model_type] = type_root_id;
+    parent_children_map_.insert(
+        std::make_pair(type_root_id, GetOrCreateModelTypeChildSet(model_type)));
   }
 
-  return children->insert(entry).second;
+  // Finally, insert the entry in the child set.
+  return siblings->insert(entry).second;
 }
 
 // Like the other containers used to help support the syncable::Directory, this
 // one does not own any EntryKernels.  This function removes references to the
 // given EntryKernel but does not delete it.
 void ParentChildIndex::Remove(EntryKernel* e) {
-  const Id& parent_id = GetParentId(e);
-
-  ParentChildrenMap::iterator parent = parent_children_map_.find(parent_id);
-  DCHECK(parent != parent_children_map_.end());
+  OrderedChildSet* siblings = nullptr;
+  ModelType model_type = e->GetModelType();
+  const Id& parent_id = e->ref(PARENT_ID);
+  bool should_erase = false;
+  ParentChildrenMap::iterator it;
+
+  if (ShouldUseParentId(parent_id, model_type)) {
+    // Hierarchical type, lookup child set in the map.
+    DCHECK(!parent_id.IsNull());
+    it = parent_children_map_.find(parent_id);
+    DCHECK(it != parent_children_map_.end());
+    siblings = it->second;
+    should_erase = true;
+  } else {
+    // Non-hierarchical type, return a pre-defined child set by type.
+    siblings = type_root_child_sets_[model_type];
+  }
 
-  OrderedChildSet* children = parent->second;
-  OrderedChildSet::iterator j = children->find(e);
-  DCHECK(j != children->end());
+  OrderedChildSet::iterator j = siblings->find(e);
+  DCHECK(j != siblings->end());
 
-  children->erase(j);
-  if (children->empty()) {
-    delete children;
-    parent_children_map_.erase(parent);
+  // Erase the entry from the child set.
+  siblings->erase(j);
+  // If the set is now empty and isn't shareable with |type_root_child_sets_|,
+  // erase it from the map.
+  if (siblings->empty() && should_erase) {
+    delete siblings;
+    parent_children_map_.erase(it);
   }
 }
 
 bool ParentChildIndex::Contains(EntryKernel *e) const {
-  const Id& parent_id = GetParentId(e);
-  ParentChildrenMap::const_iterator parent =
-      parent_children_map_.find(parent_id);
-  if (parent == parent_children_map_.end()) {
-    return false;
-  }
-  const OrderedChildSet* children = parent->second;
-  DCHECK(children && !children->empty());
-  return children->count(e) > 0;
+  const OrderedChildSet* siblings = GetChildSet(e);
+  return siblings && siblings->count(e) > 0;
 }
 
 const OrderedChildSet* ParentChildIndex::GetChildren(const Id& id) const {
@@ -120,9 +179,12 @@ const OrderedChildSet* ParentChildIndex::GetChildren(const Id& id) const {
     return nullptr;
   }
 
-  // A successful lookup implies at least some children exist.
-  DCHECK(!parent->second->empty());
-  return parent->second;
+  OrderedChildSet* children = parent->second;
+  // The expectation is that the function returns nullptr instead of an empty
+  // child set
+  if (children && children->empty())
+    children = nullptr;
+  return children;
 }
 
 const OrderedChildSet* ParentChildIndex::GetChildren(EntryKernel* e) const {
@@ -130,32 +192,52 @@ const OrderedChildSet* ParentChildIndex::GetChildren(EntryKernel* e) const {
 }
 
 const OrderedChildSet* ParentChildIndex::GetSiblings(EntryKernel* e) const {
+  // This implies the entry is in the index.
   DCHECK(Contains(e));
-  const OrderedChildSet* siblings = GetChildren(GetParentId(e));
+  const OrderedChildSet* siblings = GetChildSet(e);
   DCHECK(siblings && !siblings->empty());
   return siblings;
 }
 
-const Id& ParentChildIndex::GetParentId(EntryKernel* e) const {
-  const Id& parent_id = e->ref(PARENT_ID);
-  if (!parent_id.IsNull()) {
-    return parent_id;
-  }
-  return GetModelTypeRootId(GetModelType(e));
+/* static */
+bool ParentChildIndex::ShouldUseParentId(const Id& parent_id,
+                                         ModelType model_type) {
+  // For compatibility with legacy unit tests, in addition to hierarchical
+  // entries, this returns true any entries directly under root and for entries
+  // of UNSPECIFIED model type.
+  return parent_id.IsRoot() || TypeSupportsHierarchy(model_type) ||
+         !syncer::IsRealDataType(model_type);
 }
 
-ModelType ParentChildIndex::GetModelType(EntryKernel* e) {
-  // TODO(stanisc): is there a more effective way to find out model type?
+const OrderedChildSet* ParentChildIndex::GetChildSet(EntryKernel* e) const {
   ModelType model_type = e->GetModelType();
-  if (!syncer::IsRealDataType(model_type)) {
-    model_type = e->GetServerModelType();
+
+  const Id& parent_id = e->ref(PARENT_ID);
+  if (ShouldUseParentId(parent_id, model_type)) {
+    // Hierarchical type, lookup child set in the map.
+    ParentChildrenMap::const_iterator it = parent_children_map_.find(parent_id);
+    if (it == parent_children_map_.end())
+      return nullptr;
+    return it->second;
   }
-  return model_type;
+
+  // Non-hierarchical type, return a collection indexed by type.
+  return GetModelTypeChildSet(model_type);
+}
+
+const OrderedChildSet* ParentChildIndex::GetModelTypeChildSet(
+    ModelType model_type) const {
+  return type_root_child_sets_[model_type];
+}
+
+OrderedChildSet* ParentChildIndex::GetOrCreateModelTypeChildSet(
+    ModelType model_type) {
+  if (!type_root_child_sets_[model_type])
+    type_root_child_sets_[model_type] = new OrderedChildSet();
+  return type_root_child_sets_[model_type];
 }
 
 const Id& ParentChildIndex::GetModelTypeRootId(ModelType model_type) const {
-  // TODO(stanisc): Review whether this approach is reliable enough.
-  // Should this method simply enumerate children of root node ("r") instead?
   return model_type_root_ids_[model_type];
 }