Sync: ParentChildIndex fix for out of order deletion of entries by PurgeEntriesWithTypeIn.

This is an incremental fix for 438313 that addresses the two issues mentioned here:
https://code.google.com/p/chromium/issues/detail?id=438313#c24
This issues are reproducible only with the alpha server with
implicit permanent folders enabled. I added a test case for
one of them - the out of order deletion which results in
deleting the permanent folder for a datatype before some of
the items of the same datatype. That occurs only in one
particular corner case when a failed datatype need to be
purged and re-downloaded.

BUG=438313

Review URL: https://codereview.chromium.org/1139883007

Cr-Commit-Position: refs/heads/master@{#330657}

chrome/browser/sync/glue/typed_url_change_processor.cc

@@ -267,8 +267,6 @@ void TypedUrlChangeProcessor::ApplyChangesFromSyncModel(
       return;
     }
 
-    // Check that the changed node is a child of the typed_urls folder.
-    DCHECK(typed_url_root.GetId() == sync_node.GetParentId());
     DCHECK(syncer::TYPED_URLS == sync_node.GetModelType());
 
     const sync_pb::TypedUrlSpecifics& typed_url(

sync/syncable/parent_child_index.cc

@@ -85,15 +85,6 @@ bool ParentChildIndex::Insert(EntryKernel* entry) {
 // given EntryKernel but does not delete it.
 void ParentChildIndex::Remove(EntryKernel* e) {
   const Id& parent_id = GetParentId(e);
-  // Clear type root ID when removing a type root entry.
-  if (parent_id.IsRoot()) {
-    ModelType model_type = GetModelType(e);
-    // TODO(stanisc): the check is needed to work around some tests.
-    // See TODO above.
-    if (model_type_root_ids_[model_type] == e->ref(ID)) {
-      model_type_root_ids_[model_type] = Id();
-    }
-  }
 
   ParentChildrenMap::iterator parent = parent_children_map_.find(parent_id);
   DCHECK(parent != parent_children_map_.end());
@@ -126,7 +117,7 @@ const OrderedChildSet* ParentChildIndex::GetChildren(const Id& id) const {
 
   ParentChildrenMap::const_iterator parent = parent_children_map_.find(id);
   if (parent == parent_children_map_.end()) {
-    return NULL;
+    return nullptr;
   }
 
   // A successful lookup implies at least some children exist.

sync/syncable/parent_child_index_unittest.cc

@@ -409,9 +409,45 @@ TEST_F(ParentChildIndexTest, NodesWithImplicitParentId) {
   EXPECT_FALSE(index_.Contains(p2));
   children = index_.GetChildren(type_root);
   ASSERT_EQ(nullptr, children);
+}
+
+// Test that the removal isn't sensitive to the order (PurgeEntriesWithTypeIn
+// removes items in arbitrary order).
+TEST_F(ParentChildIndexTest, RemoveOutOfOrder) {
+  // Insert a type root and two items (with implicit parent ID).
+  syncable::Id type_root_id = syncable::Id::CreateFromServerId("type_root");
+  EntryKernel* type_root = MakeTypeRoot(PREFERENCES, type_root_id);
+  index_.Insert(type_root);
+  EntryKernel* p1 = MakeUniqueClientItem(PREFERENCES, 1);
+  EntryKernel* p2 = MakeUniqueClientItem(PREFERENCES, 2);
+  index_.Insert(p1);
+  index_.Insert(p2);
 
+  // Two items expected under the type root.
+  const OrderedChildSet* children = index_.GetChildren(type_root);
+  ASSERT_TRUE(children);
+  EXPECT_EQ(2UL, children->size());
+
+  // Remove all 3 items in arbitrary order.
+  index_.Remove(p2);
   index_.Remove(type_root);
-  EXPECT_EQ(Id(), IndexKnownModelTypeRootId(PREFERENCES));
+  index_.Remove(p1);
+
+  EXPECT_EQ(nullptr, index_.GetChildren(type_root));
+
+  // Add a new root and another two items again.
+  type_root = MakeTypeRoot(PREFERENCES, type_root_id);
+  index_.Insert(type_root);
+
+  index_.Insert(MakeUniqueClientItem(PREFERENCES, 3));
+  index_.Insert(MakeUniqueClientItem(PREFERENCES, 4));
+
+  children = index_.GetChildren(type_root);
+  ASSERT_TRUE(children);
+  // Should have 2 items. If the out of order removal cleared the implicit
+  // parent folder ID prematurely, the collection would have 3 items including
+  // p1.
+  EXPECT_EQ(2UL, children->size());
 }
 
 }  // namespace syncable