[User Consent] Migrate to consent protos.

Bug: 865902
Change-Id: Iaa2d643eb4cf679511573a6816849db925d346f5
Reviewed-on: https://chromium-review.googlesource.com/1206811
Commit-Queue: Markus Heintz <markusheintz@chromium.org>
Reviewed-by: vitaliii <vitaliii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589173}

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

@@ -44,7 +44,7 @@ class UserConsentEqualityChecker : public SingleClientStatusChangeChecker {
       : SingleClientStatusChangeChecker(service), fake_server_(fake_server) {
     for (const UserConsentSpecifics& specifics : expected_specifics) {
       expected_specifics_.insert(std::pair<int64_t, UserConsentSpecifics>(
-          specifics.confirmation_grd_id(), specifics));
+          specifics.consent_case(), specifics));
     }
   }
 
@@ -65,8 +65,7 @@ class UserConsentEqualityChecker : public SingleClientStatusChangeChecker {
     // modify |expected_specifics_|.
     for (const SyncEntity& entity : entities) {
       UserConsentSpecifics server_specifics = entity.specifics().user_consent();
-      auto iter =
-          expected_specifics_.find(server_specifics.confirmation_grd_id());
+      auto iter = expected_specifics_.find(server_specifics.consent_case());
       EXPECT_TRUE(expected_specifics_.end() != iter);
       if (expected_specifics_.end() == iter) {
         return false;
@@ -84,6 +83,9 @@ class UserConsentEqualityChecker : public SingleClientStatusChangeChecker {
 
  private:
   FakeServer* fake_server_;
+  // TODO(markusheintz): User a string with the serialized proto instead of an
+  // int. The requires creating better expectations with a proper creation
+  // time.
   std::multimap<int64_t, UserConsentSpecifics> expected_specifics_;
 
   DISALLOW_COPY_AND_ASSIGN(UserConsentEqualityChecker);
@@ -122,7 +124,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientUserConsentsSyncTest,
   consent_auditor::ConsentAuditor* consent_service =
       ConsentAuditorFactory::GetForProfile(GetProfile(0));
   UserConsentSpecifics specifics;
-  specifics.set_confirmation_grd_id(1);
+  specifics.mutable_sync_consent()->set_confirmation_grd_id(1);
   specifics.set_account_id(GetAccountId());
 
   SyncConsent sync_consent;
@@ -139,7 +141,7 @@ IN_PROC_BROWSER_TEST_F(
   SetSyncUserConsentSeparateTypeFeature(true);
 
   UserConsentSpecifics specifics;
-  specifics.set_confirmation_grd_id(1);
+  specifics.mutable_sync_consent()->set_confirmation_grd_id(1);
   // Account id may be compared to the synced account, thus, we need them to
   // match.
   specifics.set_account_id(GetAccountId());
@@ -193,7 +195,9 @@ IN_PROC_BROWSER_TEST_F(SingleClientUserConsentsSyncTest,
       ->RecordSyncConsent(GetAccountId(), sync_consent);
 
   UserConsentSpecifics specifics;
-  specifics.set_confirmation_grd_id(1);
+  SyncConsent* expected_sync_consent = specifics.mutable_sync_consent();
+  expected_sync_consent->set_confirmation_grd_id(1);
+  expected_sync_consent->set_status(UserConsentTypes::GIVEN);
   // Account id may be compared to the synced account, thus, we need them to
   // match.
   specifics.set_account_id(GetAccountId());

components/consent_auditor/consent_auditor_impl.cc

@@ -69,26 +69,6 @@ UserConsentTypes::ConsentStatus StatusToProtoEnum(
   return UserConsentTypes::CONSENT_STATUS_UNSPECIFIED;
 }
 
-UserConsentSpecifics::Feature FeatureToUserConsentProtoEnum(
-    consent_auditor::Feature feature) {
-  switch (feature) {
-    case consent_auditor::Feature::CHROME_SYNC:
-      return UserConsentSpecifics::CHROME_SYNC;
-    case consent_auditor::Feature::PLAY_STORE:
-      return UserConsentSpecifics::PLAY_STORE;
-    case consent_auditor::Feature::BACKUP_AND_RESTORE:
-      return UserConsentSpecifics::BACKUP_AND_RESTORE;
-    case consent_auditor::Feature::GOOGLE_LOCATION_SERVICE:
-      return UserConsentSpecifics::GOOGLE_LOCATION_SERVICE;
-    case consent_auditor::Feature::CHROME_UNIFIED_CONSENT:
-      return UserConsentSpecifics::CHROME_UNIFIED_CONSENT;
-    case consent_auditor::Feature::ASSISTANT_ACTIVITY_CONTROL:
-      return UserConsentSpecifics::FEATURE_UNSPECIFIED;
-  }
-  NOTREACHED();
-  return UserConsentSpecifics::FEATURE_UNSPECIFIED;
-}
-
 ConsentStatus ConvertConsentStatus(
     UserConsentTypes::ConsentStatus consent_status) {
   DCHECK_NE(consent_status,
@@ -102,6 +82,20 @@ ConsentStatus ConvertConsentStatus(
   return ConsentStatus::NOT_GIVEN;
 }
 
+std::unique_ptr<sync_pb::UserConsentSpecifics> CreateUserConsentSpecifics(
+    const std::string& account_id,
+    const std::string& locale,
+    base::Clock* clock) {
+  std::unique_ptr<sync_pb::UserConsentSpecifics> specifics =
+      std::make_unique<sync_pb::UserConsentSpecifics>();
+  specifics->set_account_id(account_id);
+  specifics->set_client_consent_time_usec(
+      clock->Now().since_origin().InMicroseconds());
+  specifics->set_locale(locale);
+
+  return specifics;
+}
+
 }  // namespace
 
 ConsentAuditorImpl::ConsentAuditorImpl(
@@ -162,19 +156,12 @@ void ConsentAuditorImpl::RecordGaiaConsent(
       break;
   }
 
-  if (IsSeparateConsentTypeEnabled()) {
-    // TODO(msramek): Pass in the actual account id.
-    std::unique_ptr<sync_pb::UserConsentSpecifics> specifics =
-        ConstructUserConsentSpecifics(account_id, feature, description_grd_ids,
-                                      confirmation_grd_id, status);
-    consent_sync_bridge_->RecordConsent(std::move(specifics));
-  } else {
+  DCHECK(!IsSeparateConsentTypeEnabled());
   // TODO(msramek): Pass in the actual account id.
   std::unique_ptr<sync_pb::UserEventSpecifics> specifics =
       ConstructUserEventSpecifics(account_id, feature, description_grd_ids,
                                   confirmation_grd_id, status);
   user_event_service_->RecordUserEvent(std::move(specifics));
-  }
 }
 
 std::unique_ptr<sync_pb::UserEventSpecifics>
@@ -200,43 +187,31 @@ ConsentAuditorImpl::ConstructUserEventSpecifics(
   return specifics;
 }
 
-std::unique_ptr<sync_pb::UserConsentSpecifics>
-ConsentAuditorImpl::ConstructUserConsentSpecifics(
-    const std::string& account_id,
-    Feature feature,
-    const std::vector<int>& description_grd_ids,
-    int confirmation_grd_id,
-    ConsentStatus status) {
-  DCHECK(IsSeparateConsentTypeEnabled());
-
-  auto specifics = std::make_unique<sync_pb::UserConsentSpecifics>();
-  specifics->set_client_consent_time_usec(
-      clock_->Now().since_origin().InMicroseconds());
-  specifics->set_account_id(account_id);
-  specifics->set_feature(FeatureToUserConsentProtoEnum(feature));
-  for (int id : description_grd_ids) {
-    specifics->add_description_grd_ids(id);
-  }
-  specifics->set_confirmation_grd_id(confirmation_grd_id);
-  specifics->set_locale(app_locale_);
-  specifics->set_status(StatusToProtoEnum(status));
-  return specifics;
-}
-
 void ConsentAuditorImpl::RecordArcPlayConsent(
     const std::string& account_id,
     const ArcPlayTermsOfServiceConsent& consent) {
+  if (IsSeparateConsentTypeEnabled()) {
+    std::unique_ptr<sync_pb::UserConsentSpecifics> specifics =
+        CreateUserConsentSpecifics(account_id, app_locale_, clock_);
+
+    sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent* arc_play_consent =
+        specifics->mutable_arc_play_terms_of_service_consent();
+    arc_play_consent->CopyFrom(consent);
+    consent_sync_bridge_->RecordConsent(std::move(specifics));
+  } else {
     std::vector<int> description_grd_ids;
-  if (consent.consent_flow() == ArcPlayTermsOfServiceConsent::SETTING_CHANGE) {
+    if (consent.consent_flow() ==
+        ArcPlayTermsOfServiceConsent::SETTING_CHANGE) {
       for (int grd_id : consent.description_grd_ids()) {
         description_grd_ids.push_back(grd_id);
       }
     } else {
-    description_grd_ids.push_back(consent.play_terms_of_service_text_length());
+      description_grd_ids.push_back(
+          consent.play_terms_of_service_text_length());
 
-    // TODO(markusheintz): The code below is a copy from the ARC code base. This
-    // will go away when the consent proto is set on the user consent specifics
-    // proto.
+      // TODO(markusheintz): The code below is a copy from the ARC code base.
+      // This will go away when the consent proto is set on the user consent
+      // specifics proto.
       const std::string& hash_str = consent.play_terms_of_service_hash();
       DCHECK_EQ(base::kSHA1Length, hash_str.size());
       const uint8_t* hash = reinterpret_cast<const uint8_t*>(hash_str.data());
@@ -250,50 +225,93 @@ void ConsentAuditorImpl::RecordArcPlayConsent(
     RecordGaiaConsent(account_id, Feature::PLAY_STORE, description_grd_ids,
                       consent.confirmation_grd_id(),
                       ConvertConsentStatus(consent.status()));
+  }
 }
 
 void ConsentAuditorImpl::RecordArcGoogleLocationServiceConsent(
     const std::string& account_id,
     const UserConsentTypes::ArcGoogleLocationServiceConsent& consent) {
+  if (IsSeparateConsentTypeEnabled()) {
+    std::unique_ptr<sync_pb::UserConsentSpecifics> specifics =
+        CreateUserConsentSpecifics(account_id, app_locale_, clock_);
+
+    sync_pb::UserConsentTypes::ArcGoogleLocationServiceConsent*
+        arc_google_location_service_consent =
+            specifics->mutable_arc_location_service_consent();
+    arc_google_location_service_consent->CopyFrom(consent);
+    consent_sync_bridge_->RecordConsent(std::move(specifics));
+  } else {
     std::vector<int> description_grd_ids(consent.description_grd_ids().begin(),
                                          consent.description_grd_ids().end());
 
     RecordGaiaConsent(account_id, Feature::GOOGLE_LOCATION_SERVICE,
                       description_grd_ids, consent.confirmation_grd_id(),
                       ConvertConsentStatus(consent.status()));
+  }
 }
 
 void ConsentAuditorImpl::RecordArcBackupAndRestoreConsent(
     const std::string& account_id,
     const UserConsentTypes::ArcBackupAndRestoreConsent& consent) {
+  if (IsSeparateConsentTypeEnabled()) {
+    std::unique_ptr<sync_pb::UserConsentSpecifics> specifics =
+        CreateUserConsentSpecifics(account_id, app_locale_, clock_);
+
+    sync_pb::UserConsentTypes::ArcBackupAndRestoreConsent*
+        arc_backup_and_restore_consent =
+            specifics->mutable_arc_backup_and_restore_consent();
+    arc_backup_and_restore_consent->CopyFrom(consent);
+    consent_sync_bridge_->RecordConsent(std::move(specifics));
+  } else {
     std::vector<int> description_grd_ids(consent.description_grd_ids().begin(),
                                          consent.description_grd_ids().end());
 
     RecordGaiaConsent(account_id, Feature::BACKUP_AND_RESTORE,
                       description_grd_ids, consent.confirmation_grd_id(),
                       ConvertConsentStatus(consent.status()));
+  }
 }
 
 void ConsentAuditorImpl::RecordSyncConsent(
     const std::string& account_id,
     const UserConsentTypes::SyncConsent& consent) {
+  if (IsSeparateConsentTypeEnabled()) {
+    std::unique_ptr<sync_pb::UserConsentSpecifics> specifics =
+        CreateUserConsentSpecifics(account_id, app_locale_, clock_);
+
+    sync_pb::UserConsentTypes::SyncConsent* sync_consent =
+        specifics->mutable_sync_consent();
+    sync_consent->CopyFrom(consent);
+    consent_sync_bridge_->RecordConsent(std::move(specifics));
+  } else {
     std::vector<int> description_grd_ids(consent.description_grd_ids().begin(),
                                          consent.description_grd_ids().end());
 
     RecordGaiaConsent(account_id, Feature::CHROME_SYNC, description_grd_ids,
                       consent.confirmation_grd_id(),
                       ConvertConsentStatus(consent.status()));
+  }
 }
 
 void ConsentAuditorImpl::RecordUnifiedConsent(
     const std::string& account_id,
     const sync_pb::UserConsentTypes::UnifiedConsent& consent) {
+  if (IsSeparateConsentTypeEnabled()) {
+    std::unique_ptr<sync_pb::UserConsentSpecifics> specifics =
+        CreateUserConsentSpecifics(account_id, app_locale_, clock_);
+
+    sync_pb::UserConsentTypes::UnifiedConsent* unified_consent =
+        specifics->mutable_unified_consent();
+    unified_consent->CopyFrom(consent);
+    consent_sync_bridge_->RecordConsent(std::move(specifics));
+  } else {
     std::vector<int> description_grd_ids(consent.description_grd_ids().begin(),
                                          consent.description_grd_ids().end());
 
     RecordGaiaConsent(account_id, Feature::CHROME_UNIFIED_CONSENT,
                       description_grd_ids, consent.confirmation_grd_id(),
                       ConvertConsentStatus(consent.status()));
+  }
 }
 
 void ConsentAuditorImpl::RecordAssistantActivityControlConsent(
@@ -303,11 +321,7 @@ void ConsentAuditorImpl::RecordAssistantActivityControlConsent(
   // needed.
   if (IsSeparateConsentTypeEnabled()) {
     std::unique_ptr<sync_pb::UserConsentSpecifics> specifics =
-        std::make_unique<sync_pb::UserConsentSpecifics>();
-    specifics->set_account_id(account_id);
-    specifics->set_client_consent_time_usec(
-        clock_->Now().since_origin().InMicroseconds());
-    specifics->set_locale(app_locale_);
+        CreateUserConsentSpecifics(account_id, app_locale_, clock_);
     sync_pb::UserConsentTypes::AssistantActivityControlConsent*
         assistant_consent =
             specifics->mutable_assistant_activity_control_consent();

components/consent_auditor/consent_auditor_impl_unittest.cc

@@ -360,13 +360,18 @@ TEST_F(ConsentAuditorImplTest, RecordGaiaConsentAsUserConsent) {
   EXPECT_EQ(now.since_origin().InMicroseconds(),
             consent.client_consent_time_usec());
   EXPECT_EQ(kAccountId, consent.account_id());
-  EXPECT_EQ(UserConsentSpecifics::CHROME_SYNC, consent.feature());
-  EXPECT_EQ(3, consent.description_grd_ids_size());
-  EXPECT_EQ(kDescriptionMessageIds[0], consent.description_grd_ids(0));
-  EXPECT_EQ(kDescriptionMessageIds[1], consent.description_grd_ids(1));
-  EXPECT_EQ(kDescriptionMessageIds[2], consent.description_grd_ids(2));
-  EXPECT_EQ(kConfirmationMessageId, consent.confirmation_grd_id());
   EXPECT_EQ(kCurrentAppLocale, consent.locale());
+
+  EXPECT_TRUE(consent.has_sync_consent());
+  const SyncConsent& actual_sync_consent = consent.sync_consent();
+  EXPECT_EQ(3, actual_sync_consent.description_grd_ids_size());
+  EXPECT_EQ(kDescriptionMessageIds[0],
+            actual_sync_consent.description_grd_ids(0));
+  EXPECT_EQ(kDescriptionMessageIds[1],
+            actual_sync_consent.description_grd_ids(1));
+  EXPECT_EQ(kDescriptionMessageIds[2],
+            actual_sync_consent.description_grd_ids(2));
+  EXPECT_EQ(kConfirmationMessageId, actual_sync_consent.confirmation_grd_id());
 }
 
 TEST_F(ConsentAuditorImplTest, RecordArcPlayConsentRevocation) {
@@ -405,14 +410,22 @@ TEST_F(ConsentAuditorImplTest, RecordArcPlayConsentRevocation) {
   UserConsentSpecifics consent = consents[0];
 
   EXPECT_EQ(kAccountId, consent.account_id());
-  EXPECT_EQ(UserConsentTypes::NOT_GIVEN, consent.status());
-  EXPECT_EQ(UserConsentSpecifics::PLAY_STORE, consent.feature());
-  EXPECT_EQ(3, consent.description_grd_ids_size());
-  EXPECT_EQ(kDescriptionMessageIds[0], consent.description_grd_ids(0));
-  EXPECT_EQ(kDescriptionMessageIds[1], consent.description_grd_ids(1));
-  EXPECT_EQ(kDescriptionMessageIds[2], consent.description_grd_ids(2));
-  EXPECT_EQ(kConfirmationMessageId, consent.confirmation_grd_id());
   EXPECT_EQ(kCurrentAppLocale, consent.locale());
+
+  EXPECT_TRUE(consent.has_arc_play_terms_of_service_consent());
+  const ArcPlayTermsOfServiceConsent& actual_play_consent =
+      consent.arc_play_terms_of_service_consent();
+  EXPECT_EQ(UserConsentTypes::NOT_GIVEN, actual_play_consent.status());
+  EXPECT_EQ(ArcPlayTermsOfServiceConsent::SETTING_CHANGE,
+            actual_play_consent.consent_flow());
+  EXPECT_EQ(3, actual_play_consent.description_grd_ids_size());
+  EXPECT_EQ(kDescriptionMessageIds[0],
+            actual_play_consent.description_grd_ids(0));
+  EXPECT_EQ(kDescriptionMessageIds[1],
+            actual_play_consent.description_grd_ids(1));
+  EXPECT_EQ(kDescriptionMessageIds[2],
+            actual_play_consent.description_grd_ids(2));
+  EXPECT_EQ(kConfirmationMessageId, actual_play_consent.confirmation_grd_id());
 }
 
 TEST_F(ConsentAuditorImplTest, RecordArcPlayConsent) {
@@ -456,18 +469,20 @@ TEST_F(ConsentAuditorImplTest, RecordArcPlayConsent) {
   UserConsentSpecifics consent = consents[0];
 
   EXPECT_EQ(kAccountId, consent.account_id());
-  EXPECT_EQ(UserConsentSpecifics::PLAY_STORE, consent.feature());
+  EXPECT_EQ(kCurrentAppLocale, consent.locale());
 
-  EXPECT_EQ(6, consent.description_grd_ids_size());
-  EXPECT_EQ(7, consent.description_grd_ids(0));
-  EXPECT_EQ(static_cast<int>(0x2fd4e1c6), consent.description_grd_ids(1));
-  EXPECT_EQ(static_cast<int>(0x7a2d28fc), consent.description_grd_ids(2));
-  EXPECT_EQ(static_cast<int>(0xed849ee1), consent.description_grd_ids(3));
-  EXPECT_EQ(static_cast<int>(0xbb76e739), consent.description_grd_ids(4));
-  EXPECT_EQ(static_cast<int>(0x1b93eb12), consent.description_grd_ids(5));
+  EXPECT_TRUE(consent.has_arc_play_terms_of_service_consent());
+  const ArcPlayTermsOfServiceConsent& actual_play_consent =
+      consent.arc_play_terms_of_service_consent();
 
-  EXPECT_EQ(kConfirmationMessageId, consent.confirmation_grd_id());
-  EXPECT_EQ(kCurrentAppLocale, consent.locale());
+  EXPECT_EQ(7, actual_play_consent.play_terms_of_service_text_length());
+  EXPECT_EQ(std::string(play_tos_hash, base::kSHA1Length),
+            actual_play_consent.play_terms_of_service_hash());
+
+  EXPECT_EQ(kConfirmationMessageId, actual_play_consent.confirmation_grd_id());
+  EXPECT_EQ(ArcPlayTermsOfServiceConsent::SETUP,
+            actual_play_consent.consent_flow());
+  EXPECT_EQ(UserConsentTypes::GIVEN, actual_play_consent.status());
 }
 
 TEST_F(ConsentAuditorImplTest, ShouldReturnNoSyncDelegateWhenNoBridge) {