Sync cleanup: Delete TrialRecorder class

It was used to record field trials as Sync user events, but this feature
was abandoned a while ago.

Bug: 934762, 934333
Change-Id: I1606d5052bda03b1db64a2c084f05fe21c5cc9af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1557020Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648638}

chrome/browser/sync/test/integration/single_client_user_events_sync_test.cc

@@ -304,19 +304,4 @@ IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, Encryption) {
   EXPECT_TRUE(ExpectUserEvents({test_event1}));
 }
 
-IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, FieldTrial) {
-  const std::string trial_name = "TrialName";
-  const std::string group_name = "GroupName";
-
-  ASSERT_TRUE(SetupSync());
-  variations::AssociateGoogleVariationID(variations::CHROME_SYNC_EVENT_LOGGER,
-                                         trial_name, group_name, 123);
-  base::FieldTrialList::CreateFieldTrial(trial_name, group_name);
-  base::FieldTrialList::FindFullName(trial_name);
-
-  UserEventCaseChecker(GetSyncService(0), GetFakeServer(),
-                       {UserEventSpecifics::kFieldTrialEvent})
-      .Wait();
-}
-
 }  // namespace

components/sync/BUILD.gn

@@ -563,8 +563,6 @@ jumbo_static_library("sync") {
     "user_events/fake_user_event_service.h",
     "user_events/no_op_user_event_service.cc",
     "user_events/no_op_user_event_service.h",
-    "user_events/trial_recorder.cc",
-    "user_events/trial_recorder.h",
     "user_events/user_event_service.h",
     "user_events/user_event_service_impl.cc",
     "user_events/user_event_service_impl.h",
@@ -961,7 +959,6 @@ source_set("unit_tests") {
     "syncable/syncable_enum_conversions_unittest.cc",
     "syncable/syncable_id_unittest.cc",
     "syncable/syncable_unittest.cc",
-    "user_events/trial_recorder_unittest.cc",
     "user_events/user_event_service_impl_unittest.cc",
     "user_events/user_event_sync_bridge_unittest.cc",
   ]

components/sync/driver/sync_driver_switches.cc

@@ -97,10 +97,6 @@ const base::Feature kSyncSupportSecondaryAccount{
 const base::Feature kSyncUserEvents{"SyncUserEvents",
                                     base::FEATURE_ENABLED_BY_DEFAULT};
 
-// Gates emission of FieldTrial events.
-const base::Feature kSyncUserFieldTrialEvents{"SyncUserFieldTrialEvents",
-                                              base::FEATURE_ENABLED_BY_DEFAULT};
-
 // Gates registration for user language detection events.
 const base::Feature kSyncUserLanguageDetectionEvents{
     "SyncUserLanguageDetectionEvents", base::FEATURE_DISABLED_BY_DEFAULT};

components/sync/driver/sync_driver_switches.h

@@ -45,7 +45,6 @@ extern const base::Feature kSyncPseudoUSSThemes;
 extern const base::Feature kSyncSendTabToSelf;
 extern const base::Feature kSyncSupportSecondaryAccount;
 extern const base::Feature kSyncUserEvents;
-extern const base::Feature kSyncUserFieldTrialEvents;
 extern const base::Feature kSyncUserLanguageDetectionEvents;
 extern const base::Feature kSyncUserTranslationEvents;
 extern const base::Feature kSyncUSSBookmarks;

components/sync/user_events/trial_recorder.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "components/sync/user_events/trial_recorder.h"
-
-#include <memory>
-#include <set>
-#include <utility>
-
-#include "base/bind.h"
-#include "base/bind_helpers.h"
-#include "base/feature_list.h"
-#include "base/metrics/field_trial.h"
-#include "base/metrics/field_trial_params.h"
-#include "base/stl_util.h"
-#include "base/time/time.h"
-#include "components/sync/driver/sync_driver_switches.h"
-#include "components/variations/active_field_trials.h"
-
-using sync_pb::UserEventSpecifics;
-
-namespace syncer {
-
-namespace {
-
-// A FieldTrial is recorded periodically, using this delay. Although upon chance
-// an event is immediately recorded and we reset to using this delay for the
-// next time.
-base::TimeDelta GetFieldTrialDelay() {
-  return base::TimeDelta::FromSeconds(base::GetFieldTrialParamByFeatureAsInt(
-      switches::kSyncUserFieldTrialEvents, "field_trial_delay_seconds",
-      base::TimeDelta::FromDays(1).InSeconds()));
-}
-
-}  // namespace
-
-TrialRecorder::TrialRecorder(UserEventService* user_event_service)
-    : user_event_service_(user_event_service),
-      variations_(variations::CHROME_SYNC_EVENT_LOGGER,
-                  base::BindRepeating(&TrialRecorder::OnNewVariationId,
-                                      base::Unretained(this))) {
-  DCHECK(user_event_service_);
-  RecordFieldTrials();
-}
-
-TrialRecorder::~TrialRecorder() {}
-
-void TrialRecorder::RecordFieldTrials() {
-  if (!base::FeatureList::IsEnabled(switches::kSyncUserFieldTrialEvents)) {
-    return;
-  }
-
-  std::set<variations::VariationID> ids = variations_.GetIds();
-  if (!ids.empty()) {
-    auto specifics = std::make_unique<UserEventSpecifics>();
-    specifics->set_event_time_usec(
-        (base::Time::Now() - base::Time()).InMicroseconds());
-
-    for (variations::VariationID id : ids) {
-      DCHECK_NE(variations::EMPTY_ID, id);
-      specifics->mutable_field_trial_event()->add_variation_ids(id);
-    }
-    user_event_service_->RecordUserEvent(std::move(specifics));
-  }
-
-  field_trial_timer_.Start(
-      FROM_HERE, GetFieldTrialDelay(),
-      base::BindRepeating(&TrialRecorder::RecordFieldTrials,
-                          base::Unretained(this)));
-}
-
-void TrialRecorder::OnNewVariationId(variations::VariationID id) {
-  RecordFieldTrials();
-}
-
-}  // namespace syncer

components/sync/user_events/trial_recorder.h

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef COMPONENTS_SYNC_USER_EVENTS_TRIAL_RECORDER_H_
-#define COMPONENTS_SYNC_USER_EVENTS_TRIAL_RECORDER_H_
-
-#include "base/macros.h"
-#include "base/timer/timer.h"
-#include "components/sync/protocol/user_event_specifics.pb.h"
-#include "components/sync/user_events/user_event_service.h"
-#include "components/variations/variations_associated_data.h"
-#include "components/variations/variations_id_collection.h"
-
-namespace syncer {
-
-// Watches finalization of trails and records FieldTrial events through its
-// UserEventService on construction, on change, and every so often.
-class TrialRecorder {
- public:
-  explicit TrialRecorder(UserEventService* user_event_service);
-  ~TrialRecorder();
-
- private:
-  // Construct and record a field trial event if applicable.
-  void RecordFieldTrials();
-
-  // Simply drops the |id| param and calls RecordFieldTrials().
-  void OnNewVariationId(variations::VariationID id);
-
-  // Non-owning pointer to interface of how events are actually recorded.
-  UserEventService* user_event_service_;
-
-  // Tracks all the variation ids that we we care about.
-  variations::VariationsIdCollection variations_;
-
-  // Timer used to record a field trial event every given interval.
-  base::OneShotTimer field_trial_timer_;
-
-  DISALLOW_COPY_AND_ASSIGN(TrialRecorder);
-};
-
-}  // namespace syncer
-
-#endif  // COMPONENTS_SYNC_USER_EVENTS_TRIAL_RECORDER_H_

components/sync/user_events/trial_recorder_unittest.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "components/sync/user_events/trial_recorder.h"
-
-#include <memory>
-#include <set>
-#include <string>
-
-#include "base/metrics/field_trial.h"
-#include "base/run_loop.h"
-#include "base/test/scoped_feature_list.h"
-#include "base/test/scoped_task_environment.h"
-#include "components/sync/driver/sync_driver_switches.h"
-#include "components/sync/protocol/sync.pb.h"
-#include "components/sync/user_events/fake_user_event_service.h"
-#include "components/variations/variations_associated_data.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-using sync_pb::UserEventSpecifics;
-
-namespace syncer {
-
-namespace {
-
-const char kTrial1[] = "TrialNameOne";
-const char kTrial2[] = "TrialNameTwo";
-const char kGroup[] = "GroupName";
-const variations::VariationID kVariation1 = 111;
-const variations::VariationID kVariation2 = 222;
-
-void VerifyEvent(std::set<variations::VariationID> expected_variations,
-                 const UserEventSpecifics& actual) {
-  ASSERT_EQ(
-      expected_variations.size(),
-      static_cast<size_t>(actual.field_trial_event().variation_ids_size()));
-  for (int actaul_variation : actual.field_trial_event().variation_ids()) {
-    auto iter = expected_variations.find(actaul_variation);
-    if (iter == expected_variations.end()) {
-      FAIL() << actaul_variation;
-    } else {
-      // Remove to make sure the event doesn't contain duplicates.
-      expected_variations.erase(iter);
-    }
-  }
-}
-
-void SetupAndFinalizeTrial(const std::string& trial_name,
-                           variations::VariationID id) {
-  variations::AssociateGoogleVariationID(variations::CHROME_SYNC_EVENT_LOGGER,
-                                         trial_name, kGroup, id);
-  base::FieldTrialList::CreateFieldTrial(trial_name, kGroup);
-  base::FieldTrialList::FindFullName(trial_name);
-  base::RunLoop().RunUntilIdle();
-}
-
-class TrialRecorderTest : public testing::Test {
- public:
-  TrialRecorderTest() : field_trial_list_(nullptr) {}
-
-  ~TrialRecorderTest() override { variations::testing::ClearAllVariationIDs(); }
-
-  FakeUserEventService* service() { return &service_; }
-  TrialRecorder* recorder() { return recorder_.get(); }
-
-  void VerifyLastEvent(std::set<variations::VariationID> expected_variations) {
-    ASSERT_LE(1u, service()->GetRecordedUserEvents().size());
-    VerifyEvent(expected_variations,
-                *service()->GetRecordedUserEvents().rbegin());
-  }
-
-  void InitRecorder() {
-    recorder_ = std::make_unique<TrialRecorder>(&service_);
-  }
-
- private:
-  base::test::ScopedTaskEnvironment task_environment_;
-  base::FieldTrialList field_trial_list_;
-  FakeUserEventService service_;
-  std::unique_ptr<TrialRecorder> recorder_;
-};
-
-TEST_F(TrialRecorderTest, FinalizedBeforeInit) {
-  SetupAndFinalizeTrial(kTrial1, kVariation1);
-  SetupAndFinalizeTrial(kTrial2, kVariation2);
-  // Should check on initialization to see if there are any trails to record.
-  InitRecorder();
-  VerifyLastEvent({kVariation1, kVariation2});
-}
-
-TEST_F(TrialRecorderTest, FinalizedAfterInit) {
-  InitRecorder();
-  SetupAndFinalizeTrial(kTrial1, kVariation1);
-  SetupAndFinalizeTrial(kTrial2, kVariation2);
-  VerifyLastEvent({kVariation1, kVariation2});
-}
-
-TEST_F(TrialRecorderTest, FinalizedMix) {
-  SetupAndFinalizeTrial(kTrial1, kVariation1);
-  InitRecorder();
-  VerifyLastEvent({kVariation1});
-
-  SetupAndFinalizeTrial(kTrial2, kVariation2);
-  VerifyLastEvent({kVariation1, kVariation2});
-}
-
-TEST_F(TrialRecorderTest, WrongVariationKey) {
-  InitRecorder();
-  variations::AssociateGoogleVariationID(variations::GOOGLE_WEB_PROPERTIES,
-                                         kTrial1, kGroup, kVariation1);
-  base::FieldTrialList::CreateFieldTrial(kTrial1, kGroup);
-  base::FieldTrialList::FindFullName(kTrial1);
-  base::RunLoop().RunUntilIdle();
-
-  EXPECT_EQ(0u, service()->GetRecordedUserEvents().size());
-}
-
-TEST_F(TrialRecorderTest, NoVariation) {
-  InitRecorder();
-  base::FieldTrialList::CreateFieldTrial(kTrial1, kGroup);
-  base::FieldTrialList::FindFullName(kTrial1);
-  base::RunLoop().RunUntilIdle();
-
-  EXPECT_EQ(0u, service()->GetRecordedUserEvents().size());
-}
-
-TEST_F(TrialRecorderTest, FieldTrialFeatureDisabled) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndDisableFeature(
-      switches::kSyncUserFieldTrialEvents);
-  InitRecorder();
-  SetupAndFinalizeTrial(kTrial1, kVariation1);
-
-  EXPECT_EQ(0u, service()->GetRecordedUserEvents().size());
-}
-
-TEST_F(TrialRecorderTest, FieldTrialTimer) {
-  SetupAndFinalizeTrial(kTrial2, kVariation2);
-
-  {
-    // Start with 0 delay, which should mean we post immediately to record.
-    // Need to be not call any methods that might invoke RunUntilIdle() while we
-    // have a delay of 0, like SetupAndFinalizeTrial().
-    base::test::ScopedFeatureList scoped_feature_list;
-    scoped_feature_list.InitAndEnableFeatureWithParameters(
-        switches::kSyncUserFieldTrialEvents,
-        {{"field_trial_delay_seconds", "0"}});
-    InitRecorder();
-    EXPECT_EQ(1u, service()->GetRecordedUserEvents().size());
-  }
-
-  // Now that |scoped_feature_list| is gone, we should reset to default,
-  // otherwise our RunUntilIdle() would infinitively loop with a delay of 0.
-  base::RunLoop().RunUntilIdle();
-  // Should have picked up exactly one more event.
-  EXPECT_EQ(2u, service()->GetRecordedUserEvents().size());
-}
-
-}  // namespace
-
-}  // namespace syncer

components/sync/user_events/user_event_service_impl.cc

@@ -66,8 +66,7 @@ UserEventServiceImpl::UserEventServiceImpl(
     std::unique_ptr<UserEventSyncBridge> bridge)
     : sync_service_(sync_service),
       bridge_(std::move(bridge)),
-      session_id_(base::RandUint64()),
-      trial_recorder_(this) {
+      session_id_(base::RandUint64()) {
   DCHECK(bridge_);
   DCHECK(sync_service_);
 }

components/sync/user_events/user_event_service_impl.h

@@ -12,7 +12,6 @@
 #include "base/memory/weak_ptr.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/sync/protocol/user_event_specifics.pb.h"
-#include "components/sync/user_events/trial_recorder.h"
 #include "components/sync/user_events/user_event_service.h"
 
 namespace syncer {
@@ -57,9 +56,6 @@ class UserEventServiceImpl : public UserEventService {
   // which events came from the same session.
   uint64_t session_id_;
 
-  // Tracks and records field trails when appropriate.
-  TrialRecorder trial_recorder_;
-
   DISALLOW_COPY_AND_ASSIGN(UserEventServiceImpl);
 };
 

components/sync/user_events/user_event_service_impl_unittest.cc

@@ -7,7 +7,6 @@
 #include <utility>
 #include <vector>
 
-#include "base/metrics/field_trial.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_task_environment.h"
 #include "components/sync/base/model_type.h"
@@ -57,13 +56,6 @@ std::unique_ptr<UserEventSpecifics> WithNav(
   return specifics;
 }
 
-MATCHER_P(HasFieldTrialVariationIds, expected_variation_id, "") {
-  const UserEventSpecifics& specifics = arg->specifics.user_event();
-  return specifics.field_trial_event().variation_ids_size() == 1 &&
-         specifics.field_trial_event().variation_ids(0) ==
-             expected_variation_id;
-}
-
 class TestGlobalIdMapper : public GlobalIdMapper {
   void AddGlobalIdChangeObserver(GlobalIdChange callback) override {}
   int64_t GetLatestGlobalId(int64_t global_id) override { return global_id; }
@@ -71,7 +63,7 @@ class TestGlobalIdMapper : public GlobalIdMapper {
 
 class UserEventServiceImplTest : public testing::Test {
  protected:
-  UserEventServiceImplTest() : field_trial_list_(nullptr) {
+  UserEventServiceImplTest() {
     sync_service_.SetPreferredDataTypes(
         {HISTORY_DELETE_DIRECTIVES, USER_EVENTS});
     ON_CALL(mock_processor_, IsTrackingMetadata())
@@ -91,7 +83,6 @@ class UserEventServiceImplTest : public testing::Test {
 
  private:
   base::test::ScopedTaskEnvironment task_environment_;
-  base::FieldTrialList field_trial_list_;
   syncer::TestSyncService sync_service_;
   testing::NiceMock<MockModelTypeChangeProcessor> mock_processor_;
   TestGlobalIdMapper mapper_;
@@ -205,16 +196,6 @@ TEST_F(UserEventServiceImplTest, SessionIdIsDifferent) {
   EXPECT_NE(put_session_ids[0], put_session_ids[1]);
 }
 
-TEST_F(UserEventServiceImplTest, FieldTrial) {
-  variations::AssociateGoogleVariationID(variations::CHROME_SYNC_EVENT_LOGGER,
-                                         "trial", "group", 123);
-  base::FieldTrialList::CreateFieldTrial("trial", "group");
-  base::FieldTrialList::FindFullName("trial");
-
-  EXPECT_CALL(*mock_processor(), Put(_, HasFieldTrialVariationIds(123u), _));
-  UserEventServiceImpl service(sync_service(), MakeBridge());
-}
-
 TEST_F(UserEventServiceImplTest, ShouldNotRecordWhenEventsDatatypeIsDisabled) {
   sync_service()->SetPreferredDataTypes({HISTORY_DELETE_DIRECTIVES});
   UserEventServiceImpl service(sync_service(), MakeBridge());