Migrate ARC user consent recording to the new Consent Auditor API

TBR=alemate@chromium.org

Bug: 865902
Change-Id: I4ea0613be8db3bb2120d1be9c5137ca81ecd54f4
Reviewed-on: https://chromium-review.googlesource.com/1145318
Commit-Queue: Markus Heintz <markusheintz@chromium.org>
Reviewed-by: Josh Horwich <jhorwich@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Tim Schumann <tschumann@chromium.org>
Reviewed-by: vitaliii <vitaliii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580111}

chrome/browser/chromeos/arc/arc_play_store_enabled_preference_handler.cc

@@ -25,6 +25,8 @@
 #include "components/sync_preferences/pref_service_syncable.h"
 #include "content/public/browser/browser_thread.h"
 
+using sync_pb::UserConsentTypes;
+
 namespace arc {
 
 ArcPlayStoreEnabledPreferenceHandler::ArcPlayStoreEnabledPreferenceHandler(
@@ -125,11 +127,17 @@ void ArcPlayStoreEnabledPreferenceHandler::OnPreferenceChanged() {
       if (signin_manager->IsAuthenticated()) {
         const std::string account_id =
             signin_manager->GetAuthenticatedAccountId();
-        ConsentAuditorFactory::GetForProfile(profile_)->RecordGaiaConsent(
-            account_id, consent_auditor::Feature::PLAY_STORE,
-            {IDS_SETTINGS_ANDROID_APPS_DISABLE_DIALOG_MESSAGE},
-            IDS_SETTINGS_ANDROID_APPS_DISABLE_DIALOG_REMOVE,
-            consent_auditor::ConsentStatus::NOT_GIVEN);
+
+        UserConsentTypes::ArcPlayTermsOfServiceConsent play_consent;
+        play_consent.set_status(UserConsentTypes::NOT_GIVEN);
+        play_consent.set_confirmation_grd_id(
+            IDS_SETTINGS_ANDROID_APPS_DISABLE_DIALOG_REMOVE);
+        play_consent.add_description_grd_ids(
+            IDS_SETTINGS_ANDROID_APPS_DISABLE_DIALOG_MESSAGE);
+        play_consent.set_consent_flow(
+            UserConsentTypes::ArcPlayTermsOfServiceConsent::SETTING_CHANGE);
+        ConsentAuditorFactory::GetForProfile(profile_)->RecordArcPlayConsent(
+            account_id, play_consent);
       }
     }
   }

chrome/browser/chromeos/arc/arc_play_store_enabled_preference_handler_unittest.cc

@@ -31,8 +31,14 @@
 #include "components/user_manager/scoped_user_manager.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "services/identity/public/cpp/identity_manager.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ArcPlayTermsOfServiceConsent =
+    sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent;
+using sync_pb::UserConsentTypes;
+using testing::_;
+
 namespace arc {
 namespace {
 
@@ -168,6 +174,20 @@ TEST_F(ArcPlayStoreEnabledPreferenceHandlerTest, RemoveDataDir_Managed) {
 }
 
 TEST_F(ArcPlayStoreEnabledPreferenceHandlerTest, PrefChangeRevokesConsent) {
+  consent_auditor::FakeConsentAuditor* auditor = consent_auditor();
+
+  ArcPlayTermsOfServiceConsent play_consent;
+  play_consent.set_status(UserConsentTypes::NOT_GIVEN);
+  play_consent.set_confirmation_grd_id(
+      IDS_SETTINGS_ANDROID_APPS_DISABLE_DIALOG_REMOVE);
+  play_consent.add_description_grd_ids(
+      IDS_SETTINGS_ANDROID_APPS_DISABLE_DIALOG_MESSAGE);
+  play_consent.set_consent_flow(
+      UserConsentTypes::ArcPlayTermsOfServiceConsent::SETTING_CHANGE);
+  EXPECT_CALL(*auditor, RecordArcPlayConsent(
+                            GetAuthenticatedAccountId(),
+                            consent_auditor::ArcPlayConsentEq(play_consent)));
+
   ASSERT_FALSE(IsArcPlayStoreEnabledForProfile(profile()));
   arc_session_manager()->SetProfile(profile());
   arc_session_manager()->Initialize();
@@ -179,23 +199,6 @@ TEST_F(ArcPlayStoreEnabledPreferenceHandlerTest, PrefChangeRevokesConsent) {
             arc_session_manager()->state());
 
   SetArcPlayStoreEnabledForProfile(profile(), false);
-
-  // Make sure consent auditing is recording the expected revocation of consent.
-  const std::vector<int> tos_consent = {
-      IDS_SETTINGS_ANDROID_APPS_DISABLE_DIALOG_MESSAGE};
-  const std::vector<std::vector<int>> description_ids = {tos_consent};
-  const std::vector<int> confirmation_ids = {
-      IDS_SETTINGS_ANDROID_APPS_DISABLE_DIALOG_REMOVE};
-  const std::vector<consent_auditor::Feature> features = {
-      consent_auditor::Feature::PLAY_STORE};
-  const std::vector<consent_auditor::ConsentStatus> statuses = {
-      consent_auditor::ConsentStatus::NOT_GIVEN};
-
-  EXPECT_EQ(consent_auditor()->account_id(), GetAuthenticatedAccountId());
-  EXPECT_EQ(consent_auditor()->recorded_confirmation_ids(), confirmation_ids);
-  EXPECT_EQ(consent_auditor()->recorded_id_vectors(), description_ids);
-  EXPECT_EQ(consent_auditor()->recorded_features(), features);
-  EXPECT_EQ(consent_auditor()->recorded_statuses(), statuses);
 }
 
 }  // namespace

chrome/browser/chromeos/arc/arc_support_host.cc

@@ -36,6 +36,8 @@
 #include "ui/chromeos/devicetype_utils.h"
 #include "ui/display/screen.h"
 
+using sync_pb::UserConsentTypes;
+
 namespace {
 constexpr char kAction[] = "action";
 constexpr char kArcManaged[] = "arcManaged";
@@ -178,28 +180,6 @@ std::ostream& operator<<(std::ostream& os, ArcSupportHost::Error error) {
 
 }  // namespace
 
-// static
-std::vector<int> ArcSupportHost::ComputePlayToSConsentIds(
-    const std::string& content) {
-  std::vector<int> result;
-
-  // Record the content length and the SHA1 hash of the content, rather than
-  // wastefully copying the entire content which is dynamically loaded from
-  // Play, rather than included in the Chrome build itself.
-  result.push_back(static_cast<int>(content.length()));
-
-  uint8_t hash[base::kSHA1Length];
-  base::SHA1HashBytes(reinterpret_cast<const uint8_t*>(content.c_str()),
-                      content.size(), hash);
-  for (size_t i = 0; i < base::kSHA1Length; i += 4) {
-    uint32_t acc =
-        hash[i] << 24 | hash[i + 1] << 16 | hash[i + 2] << 8 | hash[i + 3];
-    result.push_back(static_cast<int>(acc));
-  }
-
-  return result;
-}
-
 ArcSupportHost::ArcSupportHost(Profile* profile)
     : profile_(profile),
       request_open_app_callback_(base::Bind(&RequestOpenApp)) {
@@ -670,38 +650,53 @@ void ArcSupportHost::OnMessage(const base::DictionaryValue& message) {
 
     // Record acceptance of ToS if it was shown to the user, otherwise simply
     // record acceptance of an empty ToS.
-    // TODO(jhorwich): Replace this approach when passing |is_managed| boolean
-    // is supported by the underlying consent protos.
-    if (!tos_shown)
-      tos_content.clear();
-    ConsentAuditorFactory::GetForProfile(profile_)->RecordGaiaConsent(
-        account_id, consent_auditor::Feature::PLAY_STORE,
-        ComputePlayToSConsentIds(tos_content),
-        IDS_ARC_OPT_IN_DIALOG_BUTTON_AGREE,
-        accepted ? consent_auditor::ConsentStatus::GIVEN
-                 : consent_auditor::ConsentStatus::NOT_GIVEN);
+    UserConsentTypes::ArcPlayTermsOfServiceConsent play_consent;
+    play_consent.set_status(accepted ? UserConsentTypes::GIVEN
+                                     : UserConsentTypes::NOT_GIVEN);
+    play_consent.set_confirmation_grd_id(IDS_ARC_OPT_IN_DIALOG_BUTTON_AGREE);
+    play_consent.set_consent_flow(
+        UserConsentTypes::ArcPlayTermsOfServiceConsent::SETUP);
+    if (tos_shown) {
+      play_consent.set_play_terms_of_service_text_length(tos_content.length());
+      play_consent.set_play_terms_of_service_hash(
+          base::SHA1HashString(tos_content));
+    }
+    ConsentAuditorFactory::GetForProfile(profile_)->RecordArcPlayConsent(
+        account_id, play_consent);
 
     // If the user - not policy - controls Backup and Restore setting, record
     // whether consent was given.
     if (!is_backup_restore_managed) {
-      ConsentAuditorFactory::GetForProfile(profile_)->RecordGaiaConsent(
-          account_id, consent_auditor::Feature::BACKUP_AND_RESTORE,
-          {IDS_ARC_OPT_IN_DIALOG_BACKUP_RESTORE},
-          IDS_ARC_OPT_IN_DIALOG_BUTTON_AGREE,
-          is_backup_restore_enabled
-              ? consent_auditor::ConsentStatus::GIVEN
-              : consent_auditor::ConsentStatus::NOT_GIVEN);
+      UserConsentTypes::ArcBackupAndRestoreConsent backup_and_restore_consent;
+      backup_and_restore_consent.set_confirmation_grd_id(
+          IDS_ARC_OPT_IN_DIALOG_BUTTON_AGREE);
+      backup_and_restore_consent.add_description_grd_ids(
+          IDS_ARC_OPT_IN_DIALOG_BACKUP_RESTORE);
+      backup_and_restore_consent.set_status(is_backup_restore_enabled
+                                                ? UserConsentTypes::GIVEN
+                                                : UserConsentTypes::NOT_GIVEN);
+
+      ConsentAuditorFactory::GetForProfile(profile_)
+          ->RecordArcBackupAndRestoreConsent(account_id,
+                                             backup_and_restore_consent);
     }
 
     // If the user - not policy - controls Location Services setting, record
     // whether consent was given.
     if (!is_location_service_managed) {
-      ConsentAuditorFactory::GetForProfile(profile_)->RecordGaiaConsent(
-          account_id, consent_auditor::Feature::GOOGLE_LOCATION_SERVICE,
-          {IDS_ARC_OPT_IN_LOCATION_SETTING}, IDS_ARC_OPT_IN_DIALOG_BUTTON_AGREE,
-          is_location_service_enabled
-              ? consent_auditor::ConsentStatus::GIVEN
-              : consent_auditor::ConsentStatus::NOT_GIVEN);
+      UserConsentTypes::ArcGoogleLocationServiceConsent
+          location_service_consent;
+      location_service_consent.set_confirmation_grd_id(
+          IDS_ARC_OPT_IN_DIALOG_BUTTON_AGREE);
+      location_service_consent.add_description_grd_ids(
+          IDS_ARC_OPT_IN_LOCATION_SETTING);
+      location_service_consent.set_status(is_location_service_enabled
+                                              ? UserConsentTypes::GIVEN
+                                              : UserConsentTypes::NOT_GIVEN);
+
+      ConsentAuditorFactory::GetForProfile(profile_)
+          ->RecordArcGoogleLocationServiceConsent(account_id,
+                                                  location_service_consent);
     }
 
     if (accepted) {

chrome/browser/chromeos/arc/arc_support_host.h

@@ -170,12 +170,6 @@ class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
   void SetRequestOpenAppCallbackForTesting(
       const RequestOpenAppCallback& callback);
 
-  // Computes consent IDs based on the content of the Play ToS. Useful as the
-  // Play ToS is not contained within the Chrome binary, but rather fetched
-  // live.
-  // Returns a vector of consent "IDs" based on the Play ToS content.
-  static std::vector<int> ComputePlayToSConsentIds(const std::string& content);
-
  private:
   struct PreferenceCheckboxData {
     PreferenceCheckboxData() : PreferenceCheckboxData(false, false) {}

chrome/browser/chromeos/arc/arc_support_host_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <vector>
 
+#include "base/sha1.h"
 #include "chrome/browser/chromeos/arc/extensions/fake_arc_support.h"
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/consent_auditor/consent_auditor_factory.h"
@@ -183,6 +184,11 @@ TEST_F(ArcSupportHostTest, AuthRetryOnError) {
 }
 
 TEST_F(ArcSupportHostTest, TermsOfServiceAccept) {
+  consent_auditor::FakeConsentAuditor* ca = consent_auditor();
+  EXPECT_CALL(*ca, RecordArcPlayConsent(_, _));
+  EXPECT_CALL(*ca, RecordArcBackupAndRestoreConsent(_, _));
+  EXPECT_CALL(*ca, RecordArcGoogleLocationServiceConsent(_, _));
+
   MockTermsOfServiceDelegate tos_delegate;
   support_host()->SetTermsOfServiceDelegate(&tos_delegate);
 
@@ -270,19 +276,4 @@ TEST_F(ArcSupportHostTest, SendFeedbackOnError) {
   fake_arc_support()->ClickSendFeedbackButton();
 }
 
-TEST_F(ArcSupportHostTest, CalculateToSHashInRightOrder) {
-  std::vector<int> output = ArcSupportHost::ComputePlayToSConsentIds(
-      "The quick brown fox jumps over the lazy dog");
-  // Expect length and 5 ints for the hash.
-  EXPECT_EQ(6, static_cast<int>(output.size()));
-  // Check string length.
-  EXPECT_EQ(43, output[0]);
-  // Verify the hash: 2fd4e1c6 7a2d28fc ed849ee1 bb76e739 1b93eb12.
-  EXPECT_EQ(static_cast<int>(0x2fd4e1c6), output[1]);
-  EXPECT_EQ(static_cast<int>(0x7a2d28fc), output[2]);
-  EXPECT_EQ(static_cast<int>(0xed849ee1), output[3]);
-  EXPECT_EQ(static_cast<int>(0xbb76e739), output[4]);
-  EXPECT_EQ(static_cast<int>(0x1b93eb12), output[5]);
-}
-
 }  // namespace

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

@@ -20,6 +20,8 @@ using consent_auditor::Feature;
 using fake_server::FakeServer;
 using sync_pb::SyncEntity;
 using sync_pb::UserConsentSpecifics;
+using sync_pb::UserConsentTypes;
+using SyncConsent = sync_pb::UserConsentTypes::SyncConsent;
 
 namespace {
 
@@ -68,7 +70,7 @@ class UserConsentEqualityChecker : public SingleClientStatusChangeChecker {
       EXPECT_TRUE(expected_specifics_.end() != iter);
       if (expected_specifics_.end() == iter) {
         return false;
-      };
+      }
       EXPECT_EQ(iter->second.account_id(), server_specifics.account_id());
       expected_specifics_.erase(iter);
     }
@@ -122,9 +124,12 @@ IN_PROC_BROWSER_TEST_F(SingleClientUserConsentsSyncTest,
   UserConsentSpecifics specifics;
   specifics.set_confirmation_grd_id(1);
   specifics.set_account_id(GetAccountId());
-  consent_service->RecordGaiaConsent(
-      GetAccountId(), Feature::CHROME_SYNC, /*description_grd_ids=*/{},
-      /*confirmation_grd_id=*/1, ConsentStatus::GIVEN);
+
+  SyncConsent sync_consent;
+  sync_consent.set_confirmation_grd_id(1);
+  sync_consent.set_status(UserConsentTypes::GIVEN);
+
+  consent_service->RecordSyncConsent(GetAccountId(), sync_consent);
   EXPECT_TRUE(ExpectUserConsents({specifics}));
 }
 
@@ -142,9 +147,11 @@ IN_PROC_BROWSER_TEST_F(
   ASSERT_TRUE(SetupSync());
   consent_auditor::ConsentAuditor* consent_service =
       ConsentAuditorFactory::GetForProfile(GetProfile(0));
-  consent_service->RecordGaiaConsent(
-      GetAccountId(), Feature::CHROME_SYNC, /*description_grd_ids=*/{},
-      /*confirmation_grd_id=*/1, ConsentStatus::GIVEN);
+
+  SyncConsent sync_consent;
+  sync_consent.set_confirmation_grd_id(1);
+  sync_consent.set_status(UserConsentTypes::GIVEN);
+  consent_service->RecordSyncConsent(GetAccountId(), sync_consent);
 
   GetClient(0)->StopSyncService(syncer::SyncService::CLEAR_DATA);
   ASSERT_TRUE(GetClient(0)->StartSyncService());

chrome/browser/ui/webui/chromeos/login/arc_terms_of_service_screen_handler.cc

@@ -6,6 +6,7 @@
 
 #include "base/command_line.h"
 #include "base/i18n/timezone.h"
+#include "base/sha1.h"
 #include "chrome/browser/chromeos/arc/arc_support_host.h"
 #include "chrome/browser/chromeos/arc/arc_util.h"
 #include "chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.h"
@@ -32,6 +33,15 @@
 #include "content/public/browser/web_ui.h"
 #include "ui/base/l10n/l10n_util.h"
 
+using ArcBackupAndRestoreConsent =
+    sync_pb::UserConsentTypes::ArcBackupAndRestoreConsent;
+using ArcGoogleLocationServiceConsent =
+    sync_pb::UserConsentTypes::ArcGoogleLocationServiceConsent;
+using ArcPlayTermsOfServiceConsent =
+    sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent;
+
+using sync_pb::UserConsentTypes;
+
 namespace {
 
 const char kJsScreenPath[] = "login.ArcTermsOfServiceScreen";
@@ -296,32 +306,44 @@ void ArcTermsOfServiceScreenHandler::RecordConsents(
   DCHECK(signin_manager->IsAuthenticated());
   const std::string account_id = signin_manager->GetAuthenticatedAccountId();
 
-  // TODO(jhorwich): Replace this approach when passing |is_managed| boolean is
-  // supported by the underlying consent protos.
-  const std::vector<int> consent_ids = ArcSupportHost::ComputePlayToSConsentIds(
-      record_tos_content ? tos_content : "");
-
-  consent_auditor->RecordGaiaConsent(
-      account_id, consent_auditor::Feature::PLAY_STORE, consent_ids,
-      IDS_ARC_OOBE_TERMS_BUTTON_ACCEPT,
-      tos_accepted ? consent_auditor::ConsentStatus::GIVEN
-                   : consent_auditor::ConsentStatus::NOT_GIVEN);
+  ArcPlayTermsOfServiceConsent play_consent;
+  play_consent.set_status(tos_accepted ? UserConsentTypes::GIVEN
+                                       : UserConsentTypes::NOT_GIVEN);
+  play_consent.set_confirmation_grd_id(IDS_ARC_OOBE_TERMS_BUTTON_ACCEPT);
+  play_consent.set_consent_flow(ArcPlayTermsOfServiceConsent::SETUP);
+  if (record_tos_content) {
+    play_consent.set_play_terms_of_service_text_length(tos_content.length());
+    play_consent.set_play_terms_of_service_hash(
+        base::SHA1HashString(tos_content));
+  }
+  consent_auditor->RecordArcPlayConsent(account_id, play_consent);
 
   if (record_backup_consent) {
-    consent_auditor->RecordGaiaConsent(
-        account_id, consent_auditor::Feature::BACKUP_AND_RESTORE,
-        {IDS_ARC_OPT_IN_DIALOG_BACKUP_RESTORE},
-        IDS_ARC_OOBE_TERMS_BUTTON_ACCEPT,
-        backup_accepted ? consent_auditor::ConsentStatus::GIVEN
-                        : consent_auditor::ConsentStatus::NOT_GIVEN);
+    ArcBackupAndRestoreConsent backup_and_restore_consent;
+    backup_and_restore_consent.set_confirmation_grd_id(
+        IDS_ARC_OOBE_TERMS_BUTTON_ACCEPT);
+    backup_and_restore_consent.add_description_grd_ids(
+        IDS_ARC_OPT_IN_DIALOG_BACKUP_RESTORE);
+    backup_and_restore_consent.set_status(backup_accepted
+                                              ? UserConsentTypes::GIVEN
+                                              : UserConsentTypes::NOT_GIVEN);
+
+    consent_auditor->RecordArcBackupAndRestoreConsent(
+        account_id, backup_and_restore_consent);
   }
 
   if (record_location_consent) {
-    consent_auditor->RecordGaiaConsent(
-        account_id, consent_auditor::Feature::GOOGLE_LOCATION_SERVICE,
-        {IDS_ARC_OPT_IN_LOCATION_SETTING}, IDS_ARC_OOBE_TERMS_BUTTON_ACCEPT,
-        location_accepted ? consent_auditor::ConsentStatus::GIVEN
-                          : consent_auditor::ConsentStatus::NOT_GIVEN);
+    ArcGoogleLocationServiceConsent location_service_consent;
+    location_service_consent.set_confirmation_grd_id(
+        IDS_ARC_OOBE_TERMS_BUTTON_ACCEPT);
+    location_service_consent.add_description_grd_ids(
+        IDS_ARC_OPT_IN_LOCATION_SETTING);
+    location_service_consent.set_status(location_accepted
+                                            ? UserConsentTypes::GIVEN
+                                            : UserConsentTypes::NOT_GIVEN);
+
+    consent_auditor->RecordArcGoogleLocationServiceConsent(
+        account_id, location_service_consent);
   }
 }
 

components/consent_auditor/BUILD.gn

@@ -44,6 +44,7 @@ source_set("test_support") {
   deps = [
     ":consent_auditor",
     "//components/sync",
+    "//testing/gmock",
   ]
 }
 

components/consent_auditor/consent_auditor.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/macros.h"
@@ -47,21 +48,6 @@ class ConsentAuditor : public KeyedService {
   ConsentAuditor();
   ~ConsentAuditor() override;
 
-  // Records a consent for |feature| for the signed-in GAIA account with
-  // the ID |account_id| (as defined in AccountInfo).
-  // Consent text consisted of strings with |consent_grd_ids|, and the UI
-  // element the user clicked had the ID |confirmation_grd_id|.
-  // Whether the consent was GIVEN or NOT_GIVEN is passed as |status|.
-  //
-  // DEPRECATED
-  // TODO(markusheintz): Make this method private once all clients have been
-  // migrated to the new API.
-  virtual void RecordGaiaConsent(const std::string& account_id,
-                                 Feature feature,
-                                 const std::vector<int>& description_grd_ids,
-                                 int confirmation_grd_id,
-                                 ConsentStatus status) = 0;
-
   // Records the ARC Play |consent| for the signed-in GAIA account with the ID
   // |account_id| (as defined in AccountInfo).
   virtual void RecordArcPlayConsent(

components/consent_auditor/consent_auditor_impl.cc

@@ -5,6 +5,7 @@
 #include "components/consent_auditor/consent_auditor_impl.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/metrics/histogram_macros.h"
 #include "base/sha1.h"
@@ -17,6 +18,8 @@
 #include "components/sync/model/model_type_sync_bridge.h"
 #include "components/sync/user_events/user_event_service.h"
 
+using ArcPlayTermsOfServiceConsent =
+    sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent;
 using sync_pb::UserConsentTypes;
 using sync_pb::UserConsentSpecifics;
 using sync_pb::UserEventSpecifics;
@@ -218,8 +221,13 @@ ConsentAuditorImpl::ConstructUserConsentSpecifics(
 
 void ConsentAuditorImpl::RecordArcPlayConsent(
     const std::string& account_id,
-    const UserConsentTypes::ArcPlayTermsOfServiceConsent& consent) {
+    const ArcPlayTermsOfServiceConsent& consent) {
   std::vector<int> description_grd_ids;
+  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());
 
     // TODO(markusheintz): The code below is a copy from the ARC code base. This
@@ -233,6 +241,7 @@ void ConsentAuditorImpl::RecordArcPlayConsent(
           hash[i] << 24 | hash[i + 1] << 16 | hash[i + 2] << 8 | hash[i + 3];
       description_grd_ids.push_back(static_cast<int>(acc));
     }
+  }
 
   RecordGaiaConsent(account_id, Feature::PLAY_STORE, description_grd_ids,
                     consent.confirmation_grd_id(),

components/consent_auditor/consent_auditor_impl.h

@@ -50,12 +50,6 @@ class ConsentAuditorImpl : public ConsentAuditor {
   // Registers the preferences needed by this service.
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
-  // Consent auditor implementation.
-  void RecordGaiaConsent(const std::string& account_id,
-                         Feature feature,
-                         const std::vector<int>& description_grd_ids,
-                         int confirmation_grd_id,
-                         ConsentStatus status) override;
   void RecordArcPlayConsent(
       const std::string& account_id,
       const sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent& consent)
@@ -81,6 +75,17 @@ class ConsentAuditorImpl : public ConsentAuditor {
   GetControllerDelegateOnUIThread() override;
 
  private:
+  // Records a consent for |feature| for the signed-in GAIA account with
+  // the ID |account_id| (as defined in AccountInfo).
+  // Consent text consisted of strings with |consent_grd_ids|, and the UI
+  // element the user clicked had the ID |confirmation_grd_id|.
+  // Whether the consent was GIVEN or NOT_GIVEN is passed as |status|.
+  void RecordGaiaConsent(const std::string& account_id,
+                         Feature feature,
+                         const std::vector<int>& description_grd_ids,
+                         int confirmation_grd_id,
+                         ConsentStatus status);
+
   std::unique_ptr<sync_pb::UserEventSpecifics> ConstructUserEventSpecifics(
       const std::string& account_id,
       Feature feature,

components/consent_auditor/consent_auditor_impl_unittest.cc

@@ -7,8 +7,10 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <utility>
 
 #include "base/memory/weak_ptr.h"
+#include "base/sha1.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/simple_test_clock.h"
 #include "base/time/default_clock.h"
@@ -19,8 +21,12 @@
 #include "components/sync/user_events/fake_user_event_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ArcPlayTermsOfServiceConsent =
+    sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent;
+using SyncConsent = sync_pb::UserConsentTypes::SyncConsent;
 using sync_pb::UserConsentSpecifics;
 using sync_pb::UserEventSpecifics;
+using sync_pb::UserConsentTypes;
 
 namespace consent_auditor {
 
@@ -250,9 +256,8 @@ TEST_F(ConsentAuditorImplTest, RecordingEnabled) {
   SetUserEventService(std::make_unique<syncer::FakeUserEventService>());
   BuildConsentAuditorImpl();
 
-  sync_pb::UserConsentTypes::SyncConsent sync_consent;
-  sync_consent.set_status(sync_pb::UserConsentTypes::ConsentStatus::
-                              UserConsentTypes_ConsentStatus_GIVEN);
+  SyncConsent sync_consent;
+  sync_consent.set_status(UserConsentTypes::GIVEN);
 
   consent_auditor()->RecordSyncConsent(kAccountId, sync_consent);
   auto& events = user_event_service()->GetRecordedUserEvents();
@@ -267,9 +272,8 @@ TEST_F(ConsentAuditorImplTest, RecordingDisabled) {
 
   base::test::ScopedFeatureList scoped_feature_list;
   scoped_feature_list.InitAndDisableFeature(switches::kSyncUserConsentEvents);
-  sync_pb::UserConsentTypes::SyncConsent sync_consent;
-  sync_consent.set_status(sync_pb::UserConsentTypes::ConsentStatus::
-                              UserConsentTypes_ConsentStatus_GIVEN);
+  SyncConsent sync_consent;
+  sync_consent.set_status(UserConsentTypes::GIVEN);
   consent_auditor()->RecordSyncConsent(kAccountId, sync_consent);
   auto& events = user_event_service()->GetRecordedUserEvents();
   EXPECT_EQ(0U, events.size());
@@ -293,9 +297,8 @@ TEST_F(ConsentAuditorImplTest, RecordGaiaConsentAsUserEvent) {
   ASSERT_TRUE(base::Time::FromUTCString("2017-11-14T15:15:38Z", &now));
   test_clock.SetNow(now);
 
-  sync_pb::UserConsentTypes::SyncConsent sync_consent;
-  sync_consent.set_status(sync_pb::UserConsentTypes::ConsentStatus::
-                              UserConsentTypes_ConsentStatus_GIVEN);
+  SyncConsent sync_consent;
+  sync_consent.set_status(UserConsentTypes::GIVEN);
   sync_consent.set_confirmation_grd_id(kConfirmationMessageId);
   for (int id : kDescriptionMessageIds) {
     sync_consent.add_description_grd_ids(id);
@@ -339,9 +342,8 @@ TEST_F(ConsentAuditorImplTest, RecordGaiaConsentAsUserConsent) {
   ASSERT_TRUE(base::Time::FromUTCString("2017-11-14T15:15:38Z", &now));
   test_clock.SetNow(now);
 
-  sync_pb::UserConsentTypes::SyncConsent sync_consent;
-  sync_consent.set_status(sync_pb::UserConsentTypes::ConsentStatus::
-                              UserConsentTypes_ConsentStatus_GIVEN);
+  SyncConsent sync_consent;
+  sync_consent.set_status(UserConsentTypes::GIVEN);
   sync_consent.set_confirmation_grd_id(kConfirmationMessageId);
   for (int id : kDescriptionMessageIds) {
     sync_consent.add_description_grd_ids(id);
@@ -365,6 +367,107 @@ TEST_F(ConsentAuditorImplTest, RecordGaiaConsentAsUserConsent) {
   EXPECT_EQ(kCurrentAppLocale, consent.locale());
 }
 
+TEST_F(ConsentAuditorImplTest, RecordArcPlayConsentRevocation) {
+  SetIsSeparateConsentTypeEnabledFeature(true);
+
+  auto wrapped_fake_bridge = std::make_unique<FakeConsentSyncBridge>();
+  FakeConsentSyncBridge* fake_bridge = wrapped_fake_bridge.get();
+  base::SimpleTestClock test_clock;
+
+  SetConsentSyncBridge(std::move(wrapped_fake_bridge));
+  SetUserEventService(nullptr);
+  SetAppVersion(kCurrentAppVersion);
+  SetAppLocale(kCurrentAppLocale);
+  SetClock(&test_clock);
+  BuildConsentAuditorImpl();
+
+  std::vector<int> kDescriptionMessageIds = {12, 37, 42};
+  int kConfirmationMessageId = 47;
+
+  base::Time now;
+  ASSERT_TRUE(base::Time::FromUTCString("2017-11-14T15:15:38Z", &now));
+  test_clock.SetNow(now);
+
+  ArcPlayTermsOfServiceConsent play_consent;
+  play_consent.set_status(UserConsentTypes::NOT_GIVEN);
+  play_consent.set_confirmation_grd_id(kConfirmationMessageId);
+  for (int id : kDescriptionMessageIds) {
+    play_consent.add_description_grd_ids(id);
+  }
+  play_consent.set_consent_flow(ArcPlayTermsOfServiceConsent::SETTING_CHANGE);
+  consent_auditor()->RecordArcPlayConsent(kAccountId, play_consent);
+
+  std::vector<UserConsentSpecifics> consents =
+      fake_bridge->GetRecordedUserConsents();
+  ASSERT_EQ(1U, consents.size());
+  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());
+}
+
+TEST_F(ConsentAuditorImplTest, RecordArcPlayConsent) {
+  SetIsSeparateConsentTypeEnabledFeature(true);
+
+  auto wrapped_fake_bridge = std::make_unique<FakeConsentSyncBridge>();
+  FakeConsentSyncBridge* fake_bridge = wrapped_fake_bridge.get();
+  base::SimpleTestClock test_clock;
+
+  SetConsentSyncBridge(std::move(wrapped_fake_bridge));
+  SetUserEventService(nullptr);
+  SetAppVersion(kCurrentAppVersion);
+  SetAppLocale(kCurrentAppLocale);
+  SetClock(&test_clock);
+  BuildConsentAuditorImpl();
+
+  int kConfirmationMessageId = 47;
+
+  base::Time now;
+  ASSERT_TRUE(base::Time::FromUTCString("2017-11-14T15:15:38Z", &now));
+  test_clock.SetNow(now);
+
+  ArcPlayTermsOfServiceConsent play_consent;
+  play_consent.set_status(UserConsentTypes::GIVEN);
+  play_consent.set_confirmation_grd_id(kConfirmationMessageId);
+  play_consent.set_consent_flow(ArcPlayTermsOfServiceConsent::SETUP);
+
+  // Verify the hash: 2fd4e1c6 7a2d28fc ed849ee1 bb76e739 1b93eb12.
+  const char play_tos_hash[] = {0x2f, 0xd4, 0xe1, 0xc6, 0x7a, 0x2d, 0x28,
+                                0xfc, 0xed, 0x84, 0x9e, 0xe1, 0xbb, 0x76,
+                                0xe7, 0x39, 0x1b, 0x93, 0xeb, 0x12};
+  play_consent.set_play_terms_of_service_hash(
+      std::string(play_tos_hash, base::kSHA1Length));
+  play_consent.set_play_terms_of_service_text_length(7);
+
+  consent_auditor()->RecordArcPlayConsent(kAccountId, play_consent);
+
+  std::vector<UserConsentSpecifics> consents =
+      fake_bridge->GetRecordedUserConsents();
+  ASSERT_EQ(1U, consents.size());
+  UserConsentSpecifics consent = consents[0];
+
+  EXPECT_EQ(kAccountId, consent.account_id());
+  EXPECT_EQ(UserConsentSpecifics::PLAY_STORE, consent.feature());
+
+  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_EQ(kConfirmationMessageId, consent.confirmation_grd_id());
+  EXPECT_EQ(kCurrentAppLocale, consent.locale());
+}
+
 TEST_F(ConsentAuditorImplTest, ShouldReturnNoSyncDelegateWhenNoBridge) {
   SetIsSeparateConsentTypeEnabledFeature(false);
   SetConsentSyncBridge(nullptr);
@@ -396,33 +499,4 @@ TEST_F(ConsentAuditorImplTest, ShouldReturnSyncDelegateWhenBridgePresent) {
             consent_auditor()->GetControllerDelegateOnUIThread().get());
 }
 
-// Test that RecordSyncConsent and RecordGaiaConsent record an identical user
-// consent proto with the user event service. This test ensures that the two
-// methods don't diverge during the migration to the new dedicated protos for
-// user events.
-TEST_F(ConsentAuditorImplTest, RecordGaiaUserRecordSyncConsentEquivalence) {
-  SetIsSeparateConsentTypeEnabledFeature(false);
-  SetConsentSyncBridge(nullptr);
-  SetUserEventService(std::make_unique<syncer::FakeUserEventService>());
-  BuildConsentAuditorImpl();
-
-  sync_pb::UserConsentTypes::SyncConsent sync_consent;
-  sync_consent.set_status(sync_pb::UserConsentTypes::ConsentStatus::
-                              UserConsentTypes_ConsentStatus_GIVEN);
-  sync_consent.set_confirmation_grd_id(21);
-  sync_consent.add_description_grd_ids(1);
-  sync_consent.add_description_grd_ids(2);
-  sync_consent.add_description_grd_ids(3);
-
-  consent_auditor()->RecordSyncConsent(kAccountId, sync_consent);
-  consent_auditor()->RecordGaiaConsent(
-      kAccountId, consent_auditor::Feature::CHROME_SYNC, {1, 2, 3}, 21,
-      consent_auditor::ConsentStatus::GIVEN);
-
-  auto& events = user_event_service()->GetRecordedUserEvents();
-  EXPECT_EQ(2U, events.size());
-  EXPECT_EQ(events.at(0).user_consent().SerializeAsString(),
-            events.at(1).user_consent().SerializeAsString());
-}
-
 }  // namespace consent_auditor

components/consent_auditor/fake_consent_auditor.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <string>
 #include <utility>
 
 #include "components/consent_auditor/consent_auditor.h"
@@ -42,24 +43,6 @@ void FakeConsentAuditor::RecordSyncConsent(
                     ConvertConsentStatus(consent.status()));
 }
 
-void FakeConsentAuditor::RecordArcPlayConsent(
-    const std::string& account_id,
-    const sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent& consent) {
-  NOTIMPLEMENTED();
-}
-
-void FakeConsentAuditor::RecordArcGoogleLocationServiceConsent(
-    const std::string& account_id,
-    const sync_pb::UserConsentTypes::ArcGoogleLocationServiceConsent& consent) {
-  NOTIMPLEMENTED();
-}
-
-void FakeConsentAuditor::RecordArcBackupAndRestoreConsent(
-    const std::string& account_id,
-    const sync_pb::UserConsentTypes::ArcBackupAndRestoreConsent& consent) {
-  NOTIMPLEMENTED();
-}
-
 void FakeConsentAuditor::RecordUnifiedConsent(
     const std::string& account_id,
     const sync_pb::UserConsentTypes::UnifiedConsent& consent) {

components/consent_auditor/fake_consent_auditor.h

@@ -5,13 +5,17 @@
 #ifndef COMPONENTS_CONSENT_AUDITOR_FAKE_CONSENT_AUDITOR_H_
 #define COMPONENTS_CONSENT_AUDITOR_FAKE_CONSENT_AUDITOR_H_
 
+#include <string>
 #include <vector>
 
 #include "base/macros.h"
 #include "components/consent_auditor/consent_auditor.h"
+#include "testing/gmock/include/gmock/gmock.h"
 
-namespace consent_auditor {
+using ::testing::Matcher;
 
+namespace consent_auditor {
+// TODO(markusheintz): Rename to MockConsentAuditor
 class FakeConsentAuditor : public ConsentAuditor {
  public:
   FakeConsentAuditor();
@@ -21,31 +25,22 @@ class FakeConsentAuditor : public ConsentAuditor {
   void RecordSyncConsent(
       const std::string& account_id,
       const sync_pb::UserConsentTypes::SyncConsent& consent) override;
-
-  void RecordArcPlayConsent(
-      const std::string& account_id,
-      const sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent& consent)
-      override;
-
-  void RecordArcGoogleLocationServiceConsent(
-      const std::string& account_id,
-      const sync_pb::UserConsentTypes::ArcGoogleLocationServiceConsent& consent)
-      override;
-
-  void RecordArcBackupAndRestoreConsent(
-      const std::string& account_id,
-      const sync_pb::UserConsentTypes::ArcBackupAndRestoreConsent& consent)
-      override;
-
+  MOCK_METHOD2(
+      RecordArcPlayConsent,
+      void(const std::string&,
+           const sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent&));
+  MOCK_METHOD2(
+      RecordArcBackupAndRestoreConsent,
+      void(const std::string&,
+           const sync_pb::UserConsentTypes::ArcBackupAndRestoreConsent&));
+  MOCK_METHOD2(
+      RecordArcGoogleLocationServiceConsent,
+      void(const std::string&,
+           const sync_pb::UserConsentTypes::ArcGoogleLocationServiceConsent&));
   void RecordUnifiedConsent(
       const std::string& account_id,
       const sync_pb::UserConsentTypes::UnifiedConsent& consent) override;
 
-  void RecordGaiaConsent(const std::string& account_id,
-                         consent_auditor::Feature feature,
-                         const std::vector<int>& description_grd_ids,
-                         int confirmation_grd_id,
-                         consent_auditor::ConsentStatus status) override;
   void RecordLocalConsent(const std::string& feature,
                           const std::string& description_text,
                           const std::string& confirmation_text) override;
@@ -53,12 +48,24 @@ class FakeConsentAuditor : public ConsentAuditor {
   GetControllerDelegateOnUIThread() override;
 
   // Methods for fake.
+  // TODO(markusheintz): Replace the usage of this methods in all tests.
+  void RecordGaiaConsent(const std::string& account_id,
+                         consent_auditor::Feature feature,
+                         const std::vector<int>& description_grd_ids,
+                         int confirmation_grd_id,
+                         consent_auditor::ConsentStatus status);
+
   const std::string& account_id() const { return account_id_; }
 
   const sync_pb::UserConsentTypes::SyncConsent& recorded_sync_consent() const {
     return recorded_sync_consent_;
   }
 
+  const sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent&
+  recorded_play_consent() const {
+    return recorded_play_consent_;
+  }
+
   const std::vector<std::vector<int>>& recorded_id_vectors() {
     return recorded_id_vectors_;
   }
@@ -73,9 +80,11 @@ class FakeConsentAuditor : public ConsentAuditor {
     return recorded_statuses_;
   }
 
+ private:
   std::string account_id_;
 
   sync_pb::UserConsentTypes::SyncConsent recorded_sync_consent_;
+  sync_pb::UserConsentTypes_ArcPlayTermsOfServiceConsent recorded_play_consent_;
 
   std::vector<std::vector<int>> recorded_id_vectors_;
   std::vector<int> recorded_confirmation_ids_;
@@ -85,6 +94,38 @@ class FakeConsentAuditor : public ConsentAuditor {
   DISALLOW_COPY_AND_ASSIGN(FakeConsentAuditor);
 };
 
+MATCHER_P(ArcPlayConsentEq, expected_consent, "") {
+  const sync_pb::UserConsentTypes::ArcPlayTermsOfServiceConsent&
+      actual_consent = arg;
+
+  if (actual_consent.SerializeAsString() ==
+      expected_consent.SerializeAsString())
+    return true;
+
+  LOG(ERROR) << "ERROR: actual proto does not match the expected proto";
+  return false;
+}
+
+MATCHER_P(ArcGoogleLocationServiceConsentEq, expected_consent, "") {
+  const sync_pb::UserConsentTypes::ArcGoogleLocationServiceConsent&
+      actual_consent = arg;
+
+  if (actual_consent.SerializeAsString() ==
+      expected_consent.SerializeAsString())
+    return true;
+
+  LOG(ERROR) << "ERROR: actual proto does not match the expected proto";
+  return false;
+}
+
+MATCHER_P(ArcBackupAndRestoreConsentEq, expected_consent, "") {
+  if (arg.SerializeAsString() == expected_consent.SerializeAsString())
+    return true;
+
+  LOG(ERROR) << "ERROR: actual proto does not match the expected proto";
+  return false;
+}
+
 }  // namespace consent_auditor
 
 #endif  // COMPONENTS_CONSENT_AUDITOR_FAKE_CONSENT_AUDITOR_H_

components/sync/protocol/user_consent_types.proto

@@ -75,14 +75,32 @@ message UserConsentTypes {
     // The length of the UTF-8 encoded string of the Play Terms of Service
     // text. The length is given in number of bytes.
     optional int32 play_terms_of_service_text_length = 1;
+
     // The SHA1 hash of UTF-8 encoded string of the Play Terms of Service
     // displayed to the user.
     optional bytes play_terms_of_service_hash = 2;
+
+    // GRD Ids of the strings on the consent UI that was shown to the user.
+    repeated int32 description_grd_ids = 5;
+
     // Id of the string of the UI element the user clicked when consenting.
     optional int32 confirmation_grd_id = 3;
+
     // The status of the Play Terms of Service consent. This specifies whether
     // the consent was given or not given/revoked.
     optional ConsentStatus status = 4;
+
+    // Enum describing different user flow during which the ARC Play Terms of
+    // service consent can be recorded.
+    enum ConsentFlow {
+      // The ARC setup flow on a CROS device.
+      SETUP = 1;
+      // The user flow for changing the ARC setting on a CROS device.
+      SETTING_CHANGE = 2;
+    }
+    // The user flow during which the consent was recorded. This is necessary in
+    // order to create the correct UI context and Event for the backend.
+    optional ConsentFlow consent_flow = 6;
   }
 
   // The User Consent for Chrome Sync is determined by the user action on the