Sync protocol: Remove deprecated fields and replace by "reserved"

Some deprecated fields were instead commented out, or prefixed with
"deprecated_". Better to properly remove them, and make sure the tags
aren't accidentally reused.

Bug: 950904
Change-Id: Ibe7515c1658adc2076f775281b5b928bc16687d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1820802Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699301}

components/sync/engine_impl/model_type_worker_unittest.cc

@@ -1305,7 +1305,6 @@ TEST_F(ModelTypeWorkerTest, CommitOnly) {
   const SyncEntity entity =
       server()->GetNthCommitMessage(0).commit().entries(0);
 
-  EXPECT_EQ(0, entity.attachment_id_size());
   EXPECT_FALSE(entity.has_ctime());
   EXPECT_FALSE(entity.has_deleted());
   EXPECT_FALSE(entity.has_folder());

components/sync/protocol/client_debug_info.proto

@@ -28,24 +28,15 @@ message TypeHint {
   optional bool has_valid_hint = 2;
 }
 
-// A dummy message definition for deprecated fields. Never add any fields to
-// this message.
-message DeprecatedMessage {}
-
 // The additional info here is from the StatusController. They get sent when
 // the event SYNC_CYCLE_COMPLETED  is sent.
 message SyncCycleCompletedEventInfo {
-  // optional bool syncer_stuck = 1; // Was always false, now obsolete.
-
-  // The client has never set these values correctly.  It set
-  // num_blocking_conflicts to the total number of conflicts detected and set
-  // num_non_blocking_conflicts to the number of blocking (aka. simple)
-  // conflicts.
-  //
-  // These counters have been deprecated to avoid further confusion.  The newer
-  // counters provide more detail and are less buggy.
-  optional int32 num_blocking_conflicts = 2 [deprecated = true];
-  optional int32 num_non_blocking_conflicts = 3 [deprecated = true];
+  reserved 1;
+  reserved "syncer_stuck";
+  reserved 2;
+  reserved "num_blocking_conflicts";
+  reserved 3;
+  reserved "num_non_blocking_conflicts";
 
   // These new conflict counters replace the ones above.
   optional int32 num_encryption_conflicts = 4;
@@ -61,8 +52,9 @@ message SyncCycleCompletedEventInfo {
   // contains the |notifications_enabled| flag.
   optional GetUpdatesCallerInfo caller_info = 10;
 
-  // |source_info| was unused and marked deprecated in M67.
-  repeated DeprecatedMessage source_info = 11 [deprecated = true];
+  // Deprecated in M67.
+  reserved 11;
+  reserved "source_info";
 
   optional SyncEnums.GetUpdatesOrigin get_updates_origin = 12;
 }

components/sync/protocol/sync.proto

@@ -311,10 +311,8 @@ message SyncEntity {
   // include |position_in_parent| instead.
   optional string insert_after_item_id = 16;
 
-  // Arbitrary key/value pairs associated with this item.
-  // Present in both GetUpdatesResponse and CommitMessage.
-  // Deprecated.
-  // optional ExtendedAttributes extended_attributes = 17;
+  reserved 17;
+  reserved "extended_attributes";
 
   // If true, indicates that this item has been (or should be) deleted.
   // Present in both GetUpdatesResponse and CommitMessage.
@@ -401,7 +399,8 @@ message SyncEntity {
 
   // This used to be a list of sync attachment IDs, but it was never launched
   // and the code has been removed as of M66.
-  repeated DeprecatedMessage attachment_id = 26 [deprecated = true];
+  reserved 26;
+  reserved "attachment_id";
 };
 
 // This message contains diagnostic information used to correlate
@@ -565,11 +564,8 @@ message GarbageCollectionDirective {
   // based on last modified time.
   optional int32 age_watermark_in_days = 3;
 
-  // This field specifies the max number of items that the client should keep
-  // for a specific datatype.  If the number of items exceeds this limit, the
-  // client should purge the extra sync entities based on the LRU rule.
-  // Deprecated in M76.
-  optional int32 deprecated_max_number_of_items = 4 [deprecated = true];
+  reserved 4;
+  reserved "max_number_of_items";
 }
 
 message DataTypeProgressMarker {
@@ -782,7 +778,7 @@ message ClientToServerMessage {
   reserved 6;
   reserved "authenticate";
 
-  optional DeprecatedMessage deprecated_field_9 = 9 [deprecated = true];
+  reserved 9;
 
   optional string store_birthday = 7;  // Opaque store ID; if it changes, duck!
   // The client sets this if it detects a sync issue. The server will tell it
@@ -858,9 +854,8 @@ message CommitResponse {
     // should be filled if our parent was assigned a new ID.
     optional string parent_id_string = 4;
 
-    // This used to be set to the same as the position_in_parent value in
-    // the SyncEntity message in GetUpdatesResponse, but it's not set anymore.
-    optional int64 deprecated_position_in_parent = 5 [deprecated = true];
+    reserved 5;
+    reserved "position_in_parent";
 
     // The item's current version.
     optional int64 version = 6;
@@ -896,8 +891,8 @@ message GetUpdatesResponse {
   // instead.
   optional int64 new_timestamp = 2;
 
-  // DEPRECATED FIELD - server does not set this anymore.
-  optional int64 deprecated_newest_timestamp = 3;
+  reserved 3;
+  reserved "newest_timestamp";
 
   // Approximate count of changes remaining - use this for UI feedback.
   // If present and zero, this estimate is firm: the server has no changes
@@ -982,7 +977,7 @@ message ClientToServerResponse {
 
   optional ClientCommand client_command = 7;
   optional ProfilingData profiling_data = 8;
-  optional DeprecatedMessage deprecated_field_9 = 9 [deprecated = true];
+  reserved 9;
   optional GetUpdatesMetadataResponse stream_metadata = 10;
   // If GetUpdatesStreamingResponse is contained in the ClientToServerResponse,
   // none of the other fields (error_code and etc) will be set.