sync: Support non-blocking initial sync in proto

Adds support for non-blocking initial sync to the sync protocol.  This
allows some types to request an initial sync without having the
scheduler enter configure mode.  The protocol changes are necessary to
keep the server informed that the request is due to an initial sync
request.

The remainder of the changes in this CL were required to plumb
information about the cause of the sync cycle through the SyncScheduler.
This CL also includes some new tests.

At the moment, there are no types in use that support this feature.
This CL will not affect behavior in any way.

BUG=351005

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284159 0039d316-1c4b-4281-b951-d872f2087c98

sync/engine/get_updates_processor.h

@@ -93,6 +93,7 @@ class SYNC_EXPORT_PRIVATE GetUpdatesProcessor {
 
   FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, BookmarkNudge);
   FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, NotifyMany);
+  FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, InitialSyncRequest);
   FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, ConfigureTest);
   FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, PollTest);
   FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, RetryTest);

sync/engine/get_updates_processor_unittest.cc

@@ -187,6 +187,44 @@ TEST_F(GetUpdatesProcessorTest, NotifyMany) {
   }
 }
 
+// Basic test to ensure initial sync requests are expressed in the request.
+TEST_F(GetUpdatesProcessorTest, InitialSyncRequest) {
+  sessions::NudgeTracker nudge_tracker;
+  nudge_tracker.RecordInitialSyncRequired(AUTOFILL);
+  nudge_tracker.RecordInitialSyncRequired(PREFERENCES);
+
+  ModelTypeSet initial_sync_types = ModelTypeSet(AUTOFILL, PREFERENCES);
+
+  sync_pb::ClientToServerMessage message;
+  NormalGetUpdatesDelegate normal_delegate(nudge_tracker);
+  scoped_ptr<GetUpdatesProcessor> processor(
+      BuildGetUpdatesProcessor(normal_delegate));
+  processor->PrepareGetUpdates(enabled_types(), &message);
+
+  const sync_pb::GetUpdatesMessage& gu_msg = message.get_updates();
+  EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::DATATYPE_REFRESH,
+            gu_msg.caller_info().source());
+  EXPECT_EQ(sync_pb::SyncEnums::GU_TRIGGER, gu_msg.get_updates_origin());
+  for (int i = 0; i < gu_msg.from_progress_marker_size(); ++i) {
+    syncer::ModelType type = GetModelTypeFromSpecificsFieldNumber(
+        gu_msg.from_progress_marker(i).data_type_id());
+
+    const sync_pb::DataTypeProgressMarker& progress_marker =
+        gu_msg.from_progress_marker(i);
+    const sync_pb::GetUpdateTriggers& gu_trigger =
+        progress_marker.get_update_triggers();
+
+    // We perform some basic tests of GU trigger and source fields here.  The
+    // more complicated scenarios are tested by the NudgeTracker tests.
+    if (initial_sync_types.Has(type)) {
+      EXPECT_TRUE(gu_trigger.initial_sync_in_progress());
+    } else {
+      EXPECT_TRUE(gu_trigger.has_initial_sync_in_progress());
+      EXPECT_FALSE(gu_trigger.initial_sync_in_progress());
+    }
+  }
+}
+
 TEST_F(GetUpdatesProcessorTest, ConfigureTest) {
   sync_pb::ClientToServerMessage message;
   ConfigureGetUpdatesDelegate configure_delegate(

sync/engine/sync_scheduler.h

@@ -119,6 +119,14 @@ class SYNC_EXPORT_PRIVATE SyncScheduler
       scoped_ptr<InvalidationInterface> invalidation,
       const tracked_objects::Location& nudge_location) = 0;
 
+  // Requests a non-blocking initial sync request for the specified type.
+  //
+  // Many types can only complete initial sync while the scheduler is in
+  // configure mode, but a few of them are able to perform their initial sync
+  // while the scheduler is in normal mode.  This non-blocking initial sync
+  // can be requested through this function.
+  virtual void ScheduleInitialSyncNudge(syncer::ModelType model_type) = 0;
+
   // Change status of notifications in the SyncSessionContext.
   virtual void SetNotificationsEnabled(bool notifications_enabled) = 0;
 

sync/engine/sync_scheduler_impl.cc

@@ -398,6 +398,15 @@ void SyncSchedulerImpl::ScheduleInvalidationNudge(
   ScheduleNudgeImpl(desired_delay, nudge_location);
 }
 
+void SyncSchedulerImpl::ScheduleInitialSyncNudge(syncer::ModelType model_type) {
+  DCHECK(CalledOnValidThread());
+
+  SDVLOG(2) << "Scheduling non-blocking initial sync for "
+            << ModelTypeToString(model_type);
+  nudge_tracker_.RecordInitialSyncRequired(model_type);
+  ScheduleNudgeImpl(TimeDelta::FromSeconds(0), FROM_HERE);
+}
+
 // TODO(zea): Consider adding separate throttling/backoff for datatype
 // refresh requests.
 void SyncSchedulerImpl::ScheduleNudgeImpl(

sync/engine/sync_scheduler_impl.h

@@ -68,6 +68,7 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl
       syncer::ModelType type,
       scoped_ptr<InvalidationInterface> invalidation,
       const tracked_objects::Location& nudge_location) OVERRIDE;
+  virtual void ScheduleInitialSyncNudge(syncer::ModelType model_type) OVERRIDE;
   virtual void SetNotificationsEnabled(bool notifications_enabled) OVERRIDE;
 
   virtual base::TimeDelta GetSessionsCommitDelay() const OVERRIDE;

sync/internal_api/sync_manager_impl.cc

@@ -901,9 +901,8 @@ void SyncManagerImpl::RequestNudgeForDataTypes(
 }
 
 void SyncManagerImpl::NudgeForInitialDownload(syncer::ModelType type) {
-  // TODO(rlarocque): Initial downloads should have a separate nudge type.
   DCHECK(thread_checker_.CalledOnValidThread());
-  RefreshTypes(ModelTypeSet(type));
+  scheduler_->ScheduleInitialSyncNudge(type);
 }
 
 void SyncManagerImpl::NudgeForCommit(syncer::ModelType type) {

sync/protocol/sync.proto

@@ -477,6 +477,17 @@ message GetUpdateTriggers {
   // This flag is set if the invalidation server reports that it may have
   // dropped some invalidations at some point.  Introduced in M33.
   optional bool server_dropped_hints = 6;
+
+  // This flag is set if this GetUpdate request is due at least in part due
+  // to the fact that this type has not finished initial sync yet, and the
+  // client would like to initialize itself with the server data.
+  //
+  // Only some types support performing an initial sync as part of a normal
+  // GetUpdate request.  Many types must be in configure mode when fetching
+  // initial sync data.
+  //
+  // Introduced in M38.
+  optional bool initial_sync_in_progress = 7;
 }
 
 message GarbageCollectionDirective {

sync/sessions/data_type_tracker.cc

@@ -14,7 +14,8 @@ namespace sessions {
 DataTypeTracker::DataTypeTracker()
     : local_nudge_count_(0),
       local_refresh_request_count_(0),
-      payload_buffer_size_(NudgeTracker::kDefaultMaxPayloadsPerType) {
+      payload_buffer_size_(NudgeTracker::kDefaultMaxPayloadsPerType),
+      initial_sync_required_(false) {
 }
 
 DataTypeTracker::~DataTypeTracker() { }
@@ -86,6 +87,10 @@ void DataTypeTracker::RecordRemoteInvalidation(
   }
 }
 
+void DataTypeTracker::RecordInitialSyncRequired() {
+  initial_sync_required_ = true;
+}
+
 void DataTypeTracker::RecordSuccessfulSyncCycle() {
   // If we were throttled, then we would have been excluded from this cycle's
   // GetUpdates and Commit actions.  Our state remains unchanged.
@@ -111,6 +116,8 @@ void DataTypeTracker::RecordSuccessfulSyncCycle() {
     last_dropped_invalidation_->Acknowledge();
     last_dropped_invalidation_.reset();
   }
+
+  initial_sync_required_ = false;
 }
 
 // This limit will take effect on all future invalidations received.
@@ -124,7 +131,8 @@ bool DataTypeTracker::IsSyncRequired() const {
 
 bool DataTypeTracker::IsGetUpdatesRequired() const {
   return !IsThrottled() &&
-      (HasRefreshRequestPending() || HasPendingInvalidation());
+         (HasRefreshRequestPending() || HasPendingInvalidation() ||
+          IsInitialSyncRequired());
 }
 
 bool DataTypeTracker::HasLocalChangePending() const {
@@ -139,6 +147,10 @@ bool DataTypeTracker::HasPendingInvalidation() const {
   return !pending_invalidations_.empty() || last_dropped_invalidation_;
 }
 
+bool DataTypeTracker::IsInitialSyncRequired() const {
+  return initial_sync_required_;
+}
+
 void DataTypeTracker::SetLegacyNotificationHint(
     sync_pb::DataTypeProgressMarker* progress) const {
   DCHECK(!IsThrottled())
@@ -178,6 +190,7 @@ void DataTypeTracker::FillGetUpdatesTriggersMessage(
   msg->set_client_dropped_hints(last_dropped_invalidation_);
   msg->set_local_modification_nudges(local_nudge_count_);
   msg->set_datatype_refresh_nudges(local_refresh_request_count_);
+  msg->set_initial_sync_in_progress(initial_sync_required_);
 }
 
 bool DataTypeTracker::IsThrottled() const {

sync/sessions/data_type_tracker.h

@@ -39,6 +39,9 @@ class DataTypeTracker {
   // Tracks that we received invalidation notifications for this type.
   void RecordRemoteInvalidation(scoped_ptr<InvalidationInterface> incoming);
 
+  // Takes note that initial sync is pending for this type.
+  void RecordInitialSyncRequired();
+
   // Records that a sync cycle has been performed successfully.
   // Generally, this means that all local changes have been committed and all
   // remote changes have been downloaded, so we can clear any flags related to
@@ -67,6 +70,9 @@ class DataTypeTracker {
   // Returns true if an explicit refresh request is still outstanding.
   bool HasRefreshRequestPending() const;
 
+  // Returns true if this type is requesting an initial sync.
+  bool IsInitialSyncRequired() const;
+
   // Fills in the legacy invalidaiton payload information fields.
   void SetLegacyNotificationHint(
       sync_pb::DataTypeProgressMarker* progress) const;
@@ -108,6 +114,10 @@ class DataTypeTracker {
 
   size_t payload_buffer_size_;
 
+  // Set to true if this type is ready for, but has not yet completed initial
+  // sync.
+  bool initial_sync_required_;
+
   // If !unthrottle_time_.is_null(), this type is throttled and may not download
   // or commit data until the specified time.
   base::TimeTicks unthrottle_time_;

sync/sessions/nudge_tracker.cc

@@ -106,6 +106,12 @@ void NudgeTracker::RecordRemoteInvalidation(
   tracker_it->second->RecordRemoteInvalidation(invalidation.Pass());
 }
 
+void NudgeTracker::RecordInitialSyncRequired(syncer::ModelType type) {
+  TypeTrackerMap::iterator tracker_it = type_trackers_.find(type);
+  DCHECK(tracker_it != type_trackers_.end());
+  tracker_it->second->RecordInitialSyncRequired();
+}
+
 void NudgeTracker::OnInvalidationsEnabled() {
   invalidations_enabled_ = true;
 }
@@ -228,6 +234,7 @@ sync_pb::GetUpdatesCallerInfo::GetUpdatesSource NudgeTracker::GetLegacySource()
   bool has_invalidation_pending = false;
   bool has_refresh_request_pending = false;
   bool has_commit_pending = false;
+  bool is_initial_sync_required = false;
   bool has_retry = IsRetryRequired();
 
   for (TypeTrackerMap::const_iterator it = type_trackers_.begin();
@@ -242,12 +249,19 @@ sync_pb::GetUpdatesCallerInfo::GetUpdatesSource NudgeTracker::GetLegacySource()
     if (!tracker.IsThrottled() && tracker.HasLocalChangePending()) {
       has_commit_pending = true;
     }
+    if (!tracker.IsThrottled() && tracker.IsInitialSyncRequired()) {
+      is_initial_sync_required = true;
+    }
   }
 
   if (has_invalidation_pending) {
     return sync_pb::GetUpdatesCallerInfo::NOTIFICATION;
   } else if (has_refresh_request_pending) {
     return sync_pb::GetUpdatesCallerInfo::DATATYPE_REFRESH;
+  } else if (is_initial_sync_required) {
+    // Not quite accurate, but good enough for our purposes.  This setting of
+    // SOURCE is just a backward-compatibility hack anyway.
+    return sync_pb::GetUpdatesCallerInfo::DATATYPE_REFRESH;
   } else if (has_commit_pending) {
     return sync_pb::GetUpdatesCallerInfo::LOCAL;
   } else if (has_retry) {

sync/sessions/nudge_tracker.h

@@ -62,6 +62,9 @@ class SYNC_EXPORT_PRIVATE NudgeTracker {
   void RecordRemoteInvalidation(syncer::ModelType type,
                                 scoped_ptr<InvalidationInterface> invalidation);
 
+  // Take note that an initial sync is pending for this type.
+  void RecordInitialSyncRequired(syncer::ModelType type);
+
   // These functions should be called to keep this class informed of the status
   // of the connection to the invalidations server.
   void OnInvalidationsEnabled();

sync/sessions/nudge_tracker_unittest.cc

@@ -137,6 +137,27 @@ TEST_F(NudgeTrackerTest, SourcePriorities) {
             nudge_tracker_.GetLegacySource());
 }
 
+TEST_F(NudgeTrackerTest, SourcePriority_InitialSyncRequest) {
+  nudge_tracker_.RecordInitialSyncRequired(BOOKMARKS);
+
+  // For lack of a better source, we describe an initial sync request as having
+  // source DATATYPE_REFRESH.
+  EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::DATATYPE_REFRESH,
+            nudge_tracker_.GetLegacySource());
+
+  // This should never happen in practice.  But, if it did, we'd want the
+  // initial sync required to keep the source set to DATATYPE_REFRESH.
+  nudge_tracker_.RecordLocalChange(ModelTypeSet(BOOKMARKS));
+  EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::DATATYPE_REFRESH,
+            nudge_tracker_.GetLegacySource());
+
+  // It should be safe to let NOTIFICATIONs override it.
+  nudge_tracker_.RecordRemoteInvalidation(BOOKMARKS,
+                                          BuildInvalidation(1, "hint"));
+  EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::NOTIFICATION,
+            nudge_tracker_.GetLegacySource());
+}
+
 // Verifies the management of invalidation hints and GU trigger fields.
 TEST_F(NudgeTrackerTest, HintCoalescing) {
   // Easy case: record one hint.
@@ -344,6 +365,12 @@ TEST_F(NudgeTrackerTest, WriteRefreshRequestedTypesToProto) {
 TEST_F(NudgeTrackerTest, IsSyncRequired) {
   EXPECT_FALSE(nudge_tracker_.IsSyncRequired());
 
+  // Initial sync request.
+  nudge_tracker_.RecordInitialSyncRequired(BOOKMARKS);
+  EXPECT_TRUE(nudge_tracker_.IsSyncRequired());
+  nudge_tracker_.RecordSuccessfulSyncCycle();
+  EXPECT_FALSE(nudge_tracker_.IsSyncRequired());
+
   // Local changes.
   nudge_tracker_.RecordLocalChange(ModelTypeSet(SESSIONS));
   EXPECT_TRUE(nudge_tracker_.IsSyncRequired());
@@ -368,6 +395,12 @@ TEST_F(NudgeTrackerTest, IsSyncRequired) {
 TEST_F(NudgeTrackerTest, IsGetUpdatesRequired) {
   EXPECT_FALSE(nudge_tracker_.IsGetUpdatesRequired());
 
+  // Initial sync request.
+  nudge_tracker_.RecordInitialSyncRequired(BOOKMARKS);
+  EXPECT_TRUE(nudge_tracker_.IsGetUpdatesRequired());
+  nudge_tracker_.RecordSuccessfulSyncCycle();
+  EXPECT_FALSE(nudge_tracker_.IsGetUpdatesRequired());
+
   // Local changes.
   nudge_tracker_.RecordLocalChange(ModelTypeSet(SESSIONS));
   EXPECT_FALSE(nudge_tracker_.IsGetUpdatesRequired());

sync/test/engine/fake_sync_scheduler.cc

@@ -40,6 +40,9 @@ void FakeSyncScheduler::ScheduleConfiguration(
   params.ready_task.Run();
 }
 
+void FakeSyncScheduler::ScheduleInitialSyncNudge(syncer::ModelType model_type) {
+}
+
 void FakeSyncScheduler::SetNotificationsEnabled(bool notifications_enabled) {
 }
 

sync/test/engine/fake_sync_scheduler.h

@@ -36,6 +36,7 @@ class FakeSyncScheduler : public SyncScheduler {
       const tracked_objects::Location& nudge_location) OVERRIDE;
   virtual void ScheduleConfiguration(
       const ConfigurationParams& params) OVERRIDE;
+  virtual void ScheduleInitialSyncNudge(syncer::ModelType model_type) OVERRIDE;
   virtual void SetNotificationsEnabled(bool notifications_enabled) OVERRIDE;
 
   virtual base::TimeDelta GetSessionsCommitDelay() const OVERRIDE;