[Sync] Remove the obsolete Sync.Entities.PositioningScheme histogram

This patch removes Sync.Entities.PositioningScheme histogram from code
and marks it as obsolete.

Bug: 1089540.
Change-Id: I43155064094bbcea207fcdc3ea9fc558b18a7990
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2235833Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Rushan Suleymanov <rushans@google.com>
Cr-Commit-Position: refs/heads/master@{#776876}

components/sync/engine_impl/bookmark_update_preprocessing.cc

@@ -24,18 +24,6 @@ namespace syncer {
 
 namespace {
 
-// Enumeration of possible values for the positioning schemes used in Sync
-// entities. Used in UMA metrics. Do not re-order or delete these entries; they
-// are used in a UMA histogram. Please edit SyncPositioningScheme in enums.xml
-// if a value is added.
-enum class SyncPositioningScheme {
-  kUniquePosition = 0,
-  kPositionInParent = 1,
-  kInsertAfterItemId = 2,
-  kMissing = 3,
-  kMaxValue = kMissing
-};
-
 // Used in metric "Sync.BookmarkGUIDSource2". These values are persisted to
 // logs. Entries should not be renumbered and numeric values should never be
 // reused.
@@ -121,12 +109,8 @@ void AdaptUniquePositionForBookmark(const sync_pb::SyncEntity& update_entity,
     return;
   }
 
-  bool has_position_scheme = false;
-  SyncPositioningScheme sync_positioning_scheme;
   if (update_entity.has_unique_position()) {
     data->unique_position = update_entity.unique_position();
-    has_position_scheme = true;
-    sync_positioning_scheme = SyncPositioningScheme::kUniquePosition;
   } else if (update_entity.has_position_in_parent() ||
              update_entity.has_insert_after_item_id()) {
     bool missing_originator_fields = false;
@@ -146,24 +130,14 @@ void AdaptUniquePositionForBookmark(const sync_pb::SyncEntity& update_entity,
       data->unique_position =
           UniquePosition::FromInt64(update_entity.position_in_parent(), suffix)
               .ToProto();
-      has_position_scheme = true;
-      sync_positioning_scheme = SyncPositioningScheme::kPositionInParent;
     } else {
       // If update_entity has insert_after_item_id, use 0 index.
       DCHECK(update_entity.has_insert_after_item_id());
       data->unique_position = UniquePosition::FromInt64(0, suffix).ToProto();
-      has_position_scheme = true;
-      sync_positioning_scheme = SyncPositioningScheme::kInsertAfterItemId;
     }
   } else {
     DLOG(ERROR) << "Missing required position information in update: "
                 << update_entity.id_string();
-    has_position_scheme = true;
-    sync_positioning_scheme = SyncPositioningScheme::kMissing;
-  }
-  if (has_position_scheme) {
-    UMA_HISTOGRAM_ENUMERATION("Sync.Entities.PositioningScheme",
-                              sync_positioning_scheme);
   }
 }
 

components/sync/engine_impl/bookmark_update_preprocessing_unittest.cc

@@ -22,14 +22,6 @@ using testing::Eq;
 using testing::IsEmpty;
 using testing::Ne;
 
-enum class ExpectedSyncPositioningScheme {
-  kUniquePosition = 0,
-  kPositionInParent = 1,
-  kInsertAfterItemId = 2,
-  kMissing = 3,
-  kMaxValue = kMissing
-};
-
 enum class ExpectedBookmarkGuidSource {
   kSpecifics = 0,
   kValidOCII = 1,
@@ -45,18 +37,11 @@ TEST(BookmarkUpdatePreprocessingTest, ShouldPropagateUniquePosition) {
   *entity.mutable_unique_position() =
       UniquePosition::InitialPosition(UniquePosition::RandomSuffix()).ToProto();
 
-  base::HistogramTester histogram_tester;
   EntityData entity_data;
   AdaptUniquePositionForBookmark(entity, &entity_data);
 
   EXPECT_TRUE(
       syncer::UniquePosition::FromProto(entity_data.unique_position).IsValid());
-
-  histogram_tester.ExpectUniqueSample(
-      "Sync.Entities.PositioningScheme",
-      /*sample=*/
-      ExpectedSyncPositioningScheme::kUniquePosition,
-      /*count=*/1);
 }
 
 TEST(BookmarkUpdatePreprocessingTest,
@@ -66,18 +51,11 @@ TEST(BookmarkUpdatePreprocessingTest,
   entity.set_originator_client_item_id("1");
   entity.set_position_in_parent(5);
 
-  base::HistogramTester histogram_tester;
   EntityData entity_data;
   AdaptUniquePositionForBookmark(entity, &entity_data);
 
   EXPECT_TRUE(
       syncer::UniquePosition::FromProto(entity_data.unique_position).IsValid());
-
-  histogram_tester.ExpectUniqueSample(
-      "Sync.Entities.PositioningScheme",
-      /*sample=*/
-      ExpectedSyncPositioningScheme::kPositionInParent,
-      /*count=*/1);
 }
 
 TEST(BookmarkUpdatePreprocessingTest,
@@ -87,18 +65,11 @@ TEST(BookmarkUpdatePreprocessingTest,
   entity.set_originator_client_item_id("1");
   entity.set_insert_after_item_id("ITEM_ID");
 
-  base::HistogramTester histogram_tester;
   EntityData entity_data;
   AdaptUniquePositionForBookmark(entity, &entity_data);
 
   EXPECT_TRUE(
       syncer::UniquePosition::FromProto(entity_data.unique_position).IsValid());
-
-  histogram_tester.ExpectUniqueSample(
-      "Sync.Entities.PositioningScheme",
-      /*sample=*/
-      ExpectedSyncPositioningScheme::kInsertAfterItemId,
-      /*count=*/1);
 }
 
 // Tests that AdaptGuidForBookmark() propagates GUID in specifics if the field

components/sync/engine_impl/model_type_worker_unittest.cc

@@ -47,14 +47,6 @@ const char kValue1[] = "value1";
 const char kValue2[] = "value2";
 const char kValue3[] = "value3";
 
-enum class ExpectedSyncPositioningScheme {
-  UNIQUE_POSITION = 0,
-  POSITION_IN_PARENT = 1,
-  INSERT_AFTER_ITEM_ID = 2,
-  MISSING = 3,
-  kMaxValue = MISSING
-};
-
 EntitySpecifics GenerateSpecifics(const std::string& tag,
                                   const std::string& value) {
   EntitySpecifics specifics;
@@ -1459,7 +1451,6 @@ TEST_F(ModelTypeWorkerTest,
   *entity.mutable_specifics() = GenerateSpecifics(kTag1, kValue1);
 
   UpdateResponseData response_data;
-  base::HistogramTester histogram_tester;
 
   EXPECT_EQ(ModelTypeWorker::SUCCESS,
             ModelTypeWorker::PopulateUpdateResponseData(
@@ -1467,12 +1458,6 @@ TEST_F(ModelTypeWorkerTest,
   const EntityData& data = response_data.entity;
   EXPECT_TRUE(
       syncer::UniquePosition::FromProto(data.unique_position).IsValid());
-
-  histogram_tester.ExpectUniqueSample(
-      "Sync.Entities.PositioningScheme",
-      /*sample=*/
-      ExpectedSyncPositioningScheme::UNIQUE_POSITION,
-      /*count=*/1);
 }
 
 TEST_F(ModelTypeWorkerTest,
@@ -1486,7 +1471,6 @@ TEST_F(ModelTypeWorkerTest,
   *entity.mutable_specifics() = GenerateSpecifics(kTag1, kValue1);
 
   UpdateResponseData response_data;
-  base::HistogramTester histogram_tester;
 
   EXPECT_EQ(ModelTypeWorker::SUCCESS,
             ModelTypeWorker::PopulateUpdateResponseData(
@@ -1494,12 +1478,6 @@ TEST_F(ModelTypeWorkerTest,
   const EntityData& data = response_data.entity;
   EXPECT_TRUE(
       syncer::UniquePosition::FromProto(data.unique_position).IsValid());
-
-  histogram_tester.ExpectUniqueSample(
-      "Sync.Entities.PositioningScheme",
-      /*sample=*/
-      ExpectedSyncPositioningScheme::POSITION_IN_PARENT,
-      /*count=*/1);
 }
 
 TEST_F(ModelTypeWorkerTest,
@@ -1513,7 +1491,6 @@ TEST_F(ModelTypeWorkerTest,
   *entity.mutable_specifics() = GenerateSpecifics(kTag1, kValue1);
 
   UpdateResponseData response_data;
-  base::HistogramTester histogram_tester;
 
   EXPECT_EQ(ModelTypeWorker::SUCCESS,
             ModelTypeWorker::PopulateUpdateResponseData(
@@ -1521,11 +1498,6 @@ TEST_F(ModelTypeWorkerTest,
   const EntityData& data = response_data.entity;
   EXPECT_TRUE(
       syncer::UniquePosition::FromProto(data.unique_position).IsValid());
-  histogram_tester.ExpectUniqueSample(
-      "Sync.Entities.PositioningScheme",
-      /*sample=*/
-      ExpectedSyncPositioningScheme::INSERT_AFTER_ITEM_ID,
-      /*count=*/1);
 }
 
 TEST_F(ModelTypeWorkerTest,
@@ -1541,7 +1513,6 @@ TEST_F(ModelTypeWorkerTest,
   *entity.mutable_specifics() = specifics;
 
   UpdateResponseData response_data;
-  base::HistogramTester histogram_tester;
 
   EXPECT_EQ(ModelTypeWorker::SUCCESS,
             ModelTypeWorker::PopulateUpdateResponseData(
@@ -1549,10 +1520,6 @@ TEST_F(ModelTypeWorkerTest,
   const EntityData& data = response_data.entity;
   EXPECT_FALSE(
       syncer::UniquePosition::FromProto(data.unique_position).IsValid());
-  histogram_tester.ExpectUniqueSample("Sync.Entities.PositioningScheme",
-                                      /*sample=*/
-                                      ExpectedSyncPositioningScheme::MISSING,
-                                      /*count=*/1);
 }
 
 TEST_F(ModelTypeWorkerTest,
@@ -1564,7 +1531,6 @@ TEST_F(ModelTypeWorkerTest,
   *entity.mutable_specifics() = GenerateSpecifics(kTag1, kValue1);
 
   UpdateResponseData response_data;
-  base::HistogramTester histogram_tester;
 
   EXPECT_EQ(
       ModelTypeWorker::SUCCESS,
@@ -1573,8 +1539,6 @@ TEST_F(ModelTypeWorkerTest,
   const EntityData& data = response_data.entity;
   EXPECT_FALSE(
       syncer::UniquePosition::FromProto(data.unique_position).IsValid());
-  histogram_tester.ExpectTotalCount("Sync.Entities.PositioningScheme",
-                                    /*count=*/0);
 }
 
 TEST_F(ModelTypeWorkerTest, ShouldPropagateCommitFailure) {

tools/metrics/histograms/histograms.xml

@@ -168396,6 +168396,9 @@ should be kept until we use this API.  -->
 
 <histogram name="Sync.Entities.PositioningScheme" enum="SyncPositioningScheme"
     expires_after="M85">
+  <obsolete>
+    Removed as of M85.
+  </obsolete>
   <owner>mamir@chromium.org</owner>
   <owner>mastiz@chromium.org</owner>
   <summary>