Implement ConsentRecording using UserEventService

Create a UserConsent object and pass it to the UserEventService.
UserConsent objects are whitelisted to not require history sync
permission as they don't contain history data. They are also
whitelisted to be sent for users who have a passphrase.

Bug: 781765
Change-Id: I13800d9b3a28d3acf44052011adcaa3d7ca172fa
Reviewed-on: https://chromium-review.googlesource.com/785912Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521671}

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

@@ -23,7 +23,7 @@ using sync_pb::CommitResponse;
 
 namespace {
 
-UserEventSpecifics CreateEvent(int microseconds) {
+UserEventSpecifics CreateTestEvent(int microseconds) {
   UserEventSpecifics specifics;
   specifics.set_event_time_usec(microseconds);
   specifics.set_navigation_id(microseconds);
@@ -31,6 +31,13 @@ UserEventSpecifics CreateEvent(int microseconds) {
   return specifics;
 }
 
+UserEventSpecifics CreateUserConsent(int microseconds) {
+  UserEventSpecifics specifics;
+  specifics.set_event_time_usec(microseconds);
+  specifics.mutable_user_consent();
+  return specifics;
+}
+
 CommitResponse::ResponseType BounceType(
     CommitResponse::ResponseType type,
     const syncer::LoopbackServerEntity& entity) {
@@ -163,6 +170,12 @@ class SingleClientUserEventsSyncTest : public SyncTest {
     DisableVerifier();
   }
 
+  bool ExpectUserEvents(std::vector<UserEventSpecifics> expected_specifics) {
+    return UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
+                                    expected_specifics)
+        .Wait();
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(SingleClientUserEventsSyncTest);
 };
@@ -174,16 +187,15 @@ IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, Sanity) {
       GetFakeServer()->GetSyncEntitiesByModelType(syncer::USER_EVENTS).size());
   syncer::UserEventService* event_service =
       browser_sync::UserEventServiceFactory::GetForProfile(GetProfile(0));
-  const UserEventSpecifics specifics = CreateEvent(0);
+  const UserEventSpecifics specifics = CreateTestEvent(0);
   event_service->RecordUserEvent(specifics);
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(), {specifics})
-      .Wait();
+  EXPECT_TRUE(ExpectUserEvents({specifics}));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, RetrySequential) {
   ASSERT_TRUE(SetupSync());
-  const UserEventSpecifics specifics1 = CreateEvent(1);
-  const UserEventSpecifics specifics2 = CreateEvent(2);
+  const UserEventSpecifics specifics1 = CreateTestEvent(1);
+  const UserEventSpecifics specifics2 = CreateTestEvent(2);
   syncer::UserEventService* event_service =
       browser_sync::UserEventServiceFactory::GetForProfile(GetProfile(0));
 
@@ -193,29 +205,24 @@ IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, RetrySequential) {
 
   // This will block until we hit a TRANSIENT_ERROR, at which point we will
   // regain control and can switch back to SUCCESS.
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(), {specifics1})
-      .Wait();
+  EXPECT_TRUE(ExpectUserEvents({specifics1}));
   GetFakeServer()->OverrideResponseType(
       base::Bind(&BounceType, CommitResponse::SUCCESS));
   // Because the fake server records commits even on failure, we are able to
   // verify that the commit for this event reached the server twice.
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
-                           {specifics1, specifics1})
-      .Wait();
+  EXPECT_TRUE(ExpectUserEvents({specifics1, specifics1}));
 
   // Only record |specifics2| after |specifics1| was successful to avoid race
   // conditions.
   event_service->RecordUserEvent(specifics2);
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
-                           {specifics1, specifics1, specifics2})
-      .Wait();
+  EXPECT_TRUE(ExpectUserEvents({specifics1, specifics1, specifics2}));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, RetryParallel) {
   ASSERT_TRUE(SetupSync());
   bool first = true;
-  const UserEventSpecifics specifics1 = CreateEvent(1);
-  const UserEventSpecifics specifics2 = CreateEvent(2);
+  const UserEventSpecifics specifics1 = CreateTestEvent(1);
+  const UserEventSpecifics specifics2 = CreateTestEvent(2);
   UserEventSpecifics retry_specifics;
 
   syncer::UserEventService* event_service =
@@ -229,36 +236,35 @@ IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, RetryParallel) {
 
   event_service->RecordUserEvent(specifics2);
   event_service->RecordUserEvent(specifics1);
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
-                           {specifics1, specifics2})
-      .Wait();
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
-                           {specifics1, specifics2, retry_specifics})
-      .Wait();
+  EXPECT_TRUE(ExpectUserEvents({specifics1, specifics2}));
+  EXPECT_TRUE(ExpectUserEvents({specifics1, specifics2, retry_specifics}));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, NoHistory) {
-  const UserEventSpecifics specifics1 = CreateEvent(1);
-  const UserEventSpecifics specifics2 = CreateEvent(2);
-  const UserEventSpecifics specifics3 = CreateEvent(3);
+  const UserEventSpecifics testEvent1 = CreateTestEvent(1);
+  const UserEventSpecifics testEvent2 = CreateTestEvent(2);
+  const UserEventSpecifics testEvent3 = CreateTestEvent(3);
+  const UserEventSpecifics consent1 = CreateUserConsent(4);
+  const UserEventSpecifics consent2 = CreateUserConsent(5);
+
   ASSERT_TRUE(SetupSync());
   syncer::UserEventService* event_service =
       browser_sync::UserEventServiceFactory::GetForProfile(GetProfile(0));
 
-  event_service->RecordUserEvent(specifics1);
+  event_service->RecordUserEvent(testEvent1);
+  event_service->RecordUserEvent(consent1);
   ASSERT_TRUE(GetClient(0)->DisableSyncForDatatype(syncer::TYPED_URLS));
-  event_service->RecordUserEvent(specifics2);
+  event_service->RecordUserEvent(testEvent2);
+  event_service->RecordUserEvent(consent2);
   ASSERT_TRUE(GetClient(0)->EnableSyncForDatatype(syncer::TYPED_URLS));
-  event_service->RecordUserEvent(specifics3);
+  event_service->RecordUserEvent(testEvent3);
 
-  // No |specifics2| because it was recorded while history was disabled.
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
-                           {specifics1, specifics3})
-      .Wait();
+  // No |testEvent2| because it was recorded while history was disabled.
+  EXPECT_TRUE(ExpectUserEvents({testEvent1, consent1, consent2, testEvent3}));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, NoSessions) {
-  const UserEventSpecifics specifics = CreateEvent(1);
+  const UserEventSpecifics specifics = CreateTestEvent(1);
   ASSERT_TRUE(SetupSync());
   ASSERT_TRUE(GetClient(0)->DisableSyncForDatatype(syncer::PROXY_TABS));
   syncer::UserEventService* event_service =
@@ -267,32 +273,33 @@ IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, NoSessions) {
   event_service->RecordUserEvent(specifics);
 
   // PROXY_TABS shouldn't affect us in any way.
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(), {specifics})
-      .Wait();
+  EXPECT_TRUE(ExpectUserEvents({specifics}));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, Encryption) {
-  const UserEventSpecifics specifics1 = CreateEvent(1);
-  const UserEventSpecifics specifics2 = CreateEvent(2);
+  const UserEventSpecifics testEvent1 = CreateTestEvent(1);
+  const UserEventSpecifics testEvent2 = CreateTestEvent(2);
+  const UserEventSpecifics consent1 = CreateUserConsent(3);
+  const UserEventSpecifics consent2 = CreateUserConsent(4);
 
   ASSERT_TRUE(SetupSync());
   syncer::UserEventService* event_service =
       browser_sync::UserEventServiceFactory::GetForProfile(GetProfile(0));
-  event_service->RecordUserEvent(specifics1);
+  event_service->RecordUserEvent(testEvent1);
+  event_service->RecordUserEvent(consent1);
   ASSERT_TRUE(EnableEncryption(0));
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(), {specifics1})
-      .Wait();
-  event_service->RecordUserEvent(specifics2);
+  EXPECT_TRUE(ExpectUserEvents({testEvent1, consent1}));
+  event_service->RecordUserEvent(testEvent2);
+  event_service->RecordUserEvent(consent2);
 
-  // Just checking that we don't see specifics2 isn't very convincing yet,
+  // Just checking that we don't see testEvent2 isn't very convincing yet,
   // because it may simply not have reached the server yet. So lets send
   // something else through the system that we can wait on before checking.
   // Tab/SESSIONS data was picked fairly arbitrarily, note that we expect 2
   // entries, one for the window/header and one for the tab.
   sessions_helper::OpenTab(0, GURL("http://www.one.com/"));
-  ServerCountMatchStatusChecker(syncer::SESSIONS, 2);
-  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(), {specifics1})
-      .Wait();
+  EXPECT_TRUE(ServerCountMatchStatusChecker(syncer::SESSIONS, 2).Wait());
+  EXPECT_TRUE(ExpectUserEvents({testEvent1, consent1, consent2}));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, FieldTrial) {

components/consent_auditor/BUILD.gn

@@ -25,6 +25,7 @@ source_set("unit_tests") {
 
   deps = [
     ":consent_auditor",
+    "//base/test:test_support",
     "//components/prefs:test_support",
     "//components/sync",
     "//testing/gtest",

components/consent_auditor/consent_auditor.cc

@@ -12,8 +12,11 @@
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
+#include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/user_events/user_event_service.h"
 
+using UserEventSpecifics = sync_pb::UserEventSpecifics;
+
 namespace consent_auditor {
 
 namespace {
@@ -23,6 +26,18 @@ const char kLocalConsentConfirmationKey[] = "confirmation";
 const char kLocalConsentVersionKey[] = "version";
 const char kLocalConsentLocaleKey[] = "locale";
 
+UserEventSpecifics::UserConsent::ConsentStatus ToProtoEnum(
+    ConsentAuditor::ConsentStatus status) {
+  switch (status) {
+    case ConsentAuditor::ConsentStatus::REVOKED:
+      return UserEventSpecifics::UserConsent::REVOKED;
+    case ConsentAuditor::ConsentStatus::GIVEN:
+      return UserEventSpecifics::UserConsent::GIVEN;
+  }
+  NOTREACHED();
+  return UserEventSpecifics::UserConsent::GIVEN;
+}
+
 }  // namespace
 
 ConsentAuditor::ConsentAuditor(PrefService* pref_service,
@@ -53,8 +68,33 @@ void ConsentAuditor::RecordGaiaConsent(
     const std::vector<int>& consent_grd_ids,
     const std::vector<std::string>& placeholder_replacements,
     ConsentStatus status) {
-  // TODO(crbug.com/781765): Implement consent recording.
-  NOTIMPLEMENTED();
+  if (!base::FeatureList::IsEnabled(switches::kSyncUserConsentEvents))
+    return;
+  std::unique_ptr<sync_pb::UserEventSpecifics> specifics = ConstructUserConsent(
+      feature, consent_grd_ids, placeholder_replacements, status);
+  user_event_service_->RecordUserEvent(std::move(specifics));
+}
+
+std::unique_ptr<sync_pb::UserEventSpecifics>
+ConsentAuditor::ConstructUserConsent(
+    const std::string& feature,
+    const std::vector<int>& consent_grd_ids,
+    const std::vector<std::string>& placeholder_replacements,
+    ConsentStatus status) {
+  auto specifics = base::MakeUnique<sync_pb::UserEventSpecifics>();
+  specifics->set_event_time_usec(
+      base::Time::Now().since_origin().InMicroseconds());
+  auto* consent = specifics->mutable_user_consent();
+  consent->set_feature(feature);
+  for (int id : consent_grd_ids) {
+    consent->add_consent_grd_ids(id);
+  }
+  for (const auto& string : placeholder_replacements) {
+    consent->add_placeholder_replacements(string);
+  }
+  consent->set_locale(app_locale_);
+  consent->set_status(ToProtoEnum(status));
+  return specifics;
 }
 
 void ConsentAuditor::RecordLocalConsent(const std::string& feature,

components/consent_auditor/consent_auditor.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_CONSENT_AUDITOR_CONSENT_AUDITOR_H_
 #define COMPONENTS_CONSENT_AUDITOR_CONSENT_AUDITOR_H_
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -15,6 +16,10 @@ namespace syncer {
 class UserEventService;
 }
 
+namespace sync_pb {
+class UserEventSpecifics;
+}
+
 class PrefService;
 class PrefRegistrySimple;
 
@@ -56,6 +61,12 @@ class ConsentAuditor : public KeyedService {
                           const std::string& confirmation_text);
 
  private:
+  std::unique_ptr<sync_pb::UserEventSpecifics> ConstructUserConsent(
+      const std::string& feature,
+      const std::vector<int>& consent_grd_ids,
+      const std::vector<std::string>& placeholder_replacements,
+      ConsentAuditor::ConsentStatus status);
+
   PrefService* pref_service_;
   syncer::UserEventService* user_event_service_;
   std::string app_version_;

components/consent_auditor/consent_auditor_unittest.cc

@@ -6,11 +6,15 @@
 
 #include <memory>
 
+#include "base/test/scoped_feature_list.h"
 #include "components/consent_auditor/pref_names.h"
 #include "components/prefs/testing_pref_service.h"
+#include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/user_events/fake_user_event_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using sync_pb::UserEventSpecifics;
+
 namespace consent_auditor {
 
 namespace {
@@ -85,6 +89,10 @@ class ConsentAuditorTest : public testing::Test {
 
   PrefService* pref_service() const { return pref_service_.get(); }
 
+  syncer::FakeUserEventService* user_event_service() {
+    return user_event_service_.get();
+  }
+
  private:
   std::unique_ptr<ConsentAuditor> consent_auditor_;
   std::unique_ptr<TestingPrefServiceSimple> pref_service_;
@@ -152,4 +160,45 @@ TEST_F(ConsentAuditorTest, LocalConsentPrefRepresentation) {
   EXPECT_EQ(2u, consents->size());
 }
 
+TEST_F(ConsentAuditorTest, RecordingEnabled) {
+  consent_auditor()->RecordGaiaConsent("feature1", {}, {},
+                                       ConsentAuditor::ConsentStatus::GIVEN);
+  auto& events = user_event_service()->GetRecordedUserEvents();
+  EXPECT_EQ(1U, events.size());
+}
+
+TEST_F(ConsentAuditorTest, RecordingDisabled) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(switches::kSyncUserConsentEvents);
+  consent_auditor()->RecordGaiaConsent("feature1", {}, {},
+                                       ConsentAuditor::ConsentStatus::GIVEN);
+  auto& events = user_event_service()->GetRecordedUserEvents();
+  EXPECT_EQ(0U, events.size());
+}
+
+TEST_F(ConsentAuditorTest, RecordGaiaConsent) {
+  std::vector<int> kMessageIds = {12, 37, 42};
+  std::vector<std::string> kPlaceholders = {"OK.", "user@example.com"};
+  base::Time t1 = base::Time::Now();
+  consent_auditor()->RecordGaiaConsent("feature1", kMessageIds, kPlaceholders,
+                                       ConsentAuditor::ConsentStatus::GIVEN);
+  base::Time t2 = base::Time::Now();
+  auto& events = user_event_service()->GetRecordedUserEvents();
+  EXPECT_EQ(1U, events.size());
+  EXPECT_LE(t1.since_origin().InMicroseconds(), events[0].event_time_usec());
+  EXPECT_GE(t2.since_origin().InMicroseconds(), events[0].event_time_usec());
+  EXPECT_FALSE(events[0].has_navigation_id());
+  EXPECT_TRUE(events[0].has_user_consent());
+  auto& consent = events[0].user_consent();
+  EXPECT_EQ("feature1", consent.feature());
+  EXPECT_EQ(3, consent.consent_grd_ids_size());
+  EXPECT_EQ(12, consent.consent_grd_ids(0));
+  EXPECT_EQ(37, consent.consent_grd_ids(1));
+  EXPECT_EQ(42, consent.consent_grd_ids(2));
+  EXPECT_EQ(2, consent.placeholder_replacements_size());
+  EXPECT_EQ("OK.", consent.placeholder_replacements(0));
+  EXPECT_EQ("user@example.com", consent.placeholder_replacements(1));
+  EXPECT_EQ(kCurrentAppLocale, consent.locale());
+}
+
 }  // namespace consent_auditor

components/sync/driver/sync_driver_switches.cc

@@ -45,6 +45,10 @@ const base::Feature kSyncUserEvents{"SyncUserEvents",
 const base::Feature kSyncUserFieldTrialEvents{"SyncUserFieldTrialEvents",
                                               base::FEATURE_ENABLED_BY_DEFAULT};
 
+// Gates emission of UserConsent events.
+const base::Feature kSyncUserConsentEvents{"SyncUserConsentEvents",
+                                           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

@@ -22,6 +22,7 @@ extern const char kSyncShortNudgeDelayForTest[];
 extern const base::Feature kSyncClearDataOnPassphraseEncryption;
 extern const base::Feature kSyncUserEvents;
 extern const base::Feature kSyncUserFieldTrialEvents;
+extern const base::Feature kSyncUserConsentEvents;
 extern const base::Feature kSyncUserLanguageDetectionEvents;
 extern const base::Feature kSyncUserTranslationEvents;
 extern const base::Feature kSyncUSSAutocomplete;

components/sync/user_events/user_event_service_impl_unittest.cc

@@ -4,6 +4,8 @@
 
 #include "components/sync/user_events/user_event_service_impl.h"
 
+#include <utility>
+
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/test/scoped_feature_list.h"
@@ -46,6 +48,12 @@ std::unique_ptr<UserEventSpecifics> AsTrial(
   return specifics;
 }
 
+std::unique_ptr<UserEventSpecifics> AsConsent(
+    std::unique_ptr<UserEventSpecifics> specifics) {
+  specifics->mutable_user_consent();
+  return specifics;
+}
+
 std::unique_ptr<UserEventSpecifics> WithNav(
     std::unique_ptr<UserEventSpecifics> specifics,
     int64_t navigation_id = 1) {
@@ -136,6 +144,13 @@ TEST_F(UserEventServiceImplTest, ShouldRecordNoHistory) {
   service.RecordUserEvent(WithNav(AsTest(Event())));
   EXPECT_EQ(0u, processor().put_multimap().size());
   service.RecordUserEvent(AsTest(Event()));
+}
+
+TEST_F(UserEventServiceImplTest, ShouldRecordUserConsentNoHistory) {
+  TestSyncService no_history_sync_service(true, false, ModelTypeSet());
+  UserEventServiceImpl service(&no_history_sync_service, MakeBridge());
+  service.RecordUserEvent(AsConsent(Event()));
+  // UserConsent recording doesn't need history sync to be enabled.
   EXPECT_EQ(1u, processor().put_multimap().size());
 }