Sync: fix for the code that checks whether the initial download has completed

The code that detects whether the initial download has completed for
a type had been inadvertently broken by https://codereview.chromium.org/1142423006/

This fix changes Directory::InitialSyncEndedForType back to the
original implementation and adds special handling for implicitly created
root folders (i.e. root folders auto-created locally by the update
handling code). Since the locally created root folders don't get applied
with the rest of downloaded nodes, their base version has to be
explicitly changed from CHANGES_VERSION to some other value at
the end of update application process. That would to indicate that at
least one update has been applied for the type and be consistent with
how the server generated root folders are handled.

BUG=540814

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

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

3 files changed, 54 insertions, 6 deletions

diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index 3b735e9..33cb6d4 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -19,6 +19,7 @@
 #include "sync/syncable/entry.h"
 #include "sync/syncable/entry_kernel.h"
 #include "sync/syncable/in_memory_directory_backing_store.h"
+#include "sync/syncable/model_neutral_mutable_entry.h"
 #include "sync/syncable/on_disk_directory_backing_store.h"
 #include "sync/syncable/scoped_kernel_lock.h"
 #include "sync/syncable/scoped_parent_child_index_updater.h"
@@ -985,9 +986,29 @@ bool Directory::InitialSyncEndedForType(ModelType type) {
 
 bool Directory::InitialSyncEndedForType(
     BaseTransaction* trans, ModelType type) {
-  // True iff the type's root node has been created.
-  syncable::Entry entry(trans, syncable::GET_TYPE_ROOT, type);
-  return entry.good();
+  // True iff the type's root node has been created and changes
+  // for the type have been applied at least once.
+  Entry root(trans, GET_TYPE_ROOT, type);
+  return root.good() && root.GetBaseVersion() != CHANGES_VERSION;
+}
+
+void Directory::MarkInitialSyncEndedForType(BaseWriteTransaction* trans,
+                                            ModelType type) {
+  // If the root folder is downloaded for the server, the root's base version
+  // get updated automatically at the end of update cycle when the update gets
+  // applied. However if this is a type with client generated root, the root
+  // node gets created locally and never goes through the update cycle. In that
+  // case its base version has to be explictly changed from CHANGES_VERSION
+  // at the end of the initial update cycle to mark the type as downloaded.
+  // See Directory::InitialSyncEndedForType
+  DCHECK(IsTypeWithClientGeneratedRoot(type));
+  ModelNeutralMutableEntry root(trans, GET_TYPE_ROOT, type);
+
+  // Some tests don't bother creating type root. Need to check if the root
+  // exists before clearing its base version.
+  if (root.good() && root.GetBaseVersion() == CHANGES_VERSION) {
+    root.PutBaseVersion(0);
+  }
 }
 
 string Directory::store_birthday() const {
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index d71fcf8..7402697 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -300,8 +300,16 @@ class SYNC_EXPORT Directory {
                           ModelType type,
                           const sync_pb::DataTypeContext& context);
 
+  // Returns types for which the initial sync has ended.
   ModelTypeSet InitialSyncEndedTypes();
+
+  // Returns true if the initial sync for |type| has completed.
   bool InitialSyncEndedForType(ModelType type);
+  bool InitialSyncEndedForType(BaseTransaction* trans, ModelType type);
+
+  // Marks the |type| as having its intial sync complete.
+  // This applies only to types with implicitly created root folders.
+  void MarkInitialSyncEndedForType(BaseWriteTransaction* trans, ModelType type);
 
   // (Account) Store birthday is opaque to the client, so we keep it in the
   // format it is in the proto buffer in case we switch to a binary birthday
@@ -620,9 +628,6 @@ class SYNC_EXPORT Directory {
   // detected.
   void OnCatastrophicError();
 
-  // Returns true if the initial sync for |type| has completed.
-  bool InitialSyncEndedForType(BaseTransaction* trans, ModelType type);
-
   // Stops sending events to the delegate and the transaction
   // observer.
   void Close();
diff --git a/sync/syncable/directory_unittest.cc b/sync/syncable/directory_unittest.cc
index 198e534..a9d8058 100644
--- a/sync/syncable/directory_unittest.cc
+++ b/sync/syncable/directory_unittest.cc
@@ -2110,6 +2110,28 @@ TEST_F(SyncableDirectoryTest, SharingOfClientAndServerSpecifics) {
                                            item.GetBaseServerSpecifics()));
 }
 
+// Tests checking and marking a type as having its initial sync completed.
+TEST_F(SyncableDirectoryTest, InitialSyncEndedForType) {
+  // Not completed if there is no root node.
+  EXPECT_FALSE(dir()->InitialSyncEndedForType(PREFERENCES));
+
+  WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
+  // Create the root node.
+  ModelNeutralMutableEntry entry(&trans, syncable::CREATE_NEW_TYPE_ROOT,
+                                 PREFERENCES);
+  DCHECK(entry.good());
+
+  entry.PutServerIsDir(true);
+  entry.PutUniqueServerTag(ModelTypeToRootTag(PREFERENCES));
+
+  // Should still be marked as incomplete.
+  EXPECT_FALSE(dir()->InitialSyncEndedForType(&trans, PREFERENCES));
+
+  // Mark as complete and verify.
+  dir()->MarkInitialSyncEndedForType(&trans, PREFERENCES);
+  EXPECT_TRUE(dir()->InitialSyncEndedForType(&trans, PREFERENCES));
+}
+
 }  // namespace syncable
 
 }  // namespace syncer