New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 865902 link

Starred by 1 user
Status: Started
Owner:
█ markusheintz@chromium.org
Last visit 15 days ago
Cc:
msramek@chromium.org
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Update the Consent Auditor API to support other Consents

Project Member Reported by markusheintz@chromium.org, Jul 20 
The original API of the ConsentAuditor was designed to record the Sync Consent.

Now that multiple consents are recorded through the ConsentAuditor the API needs to be updated in order to support this properly.

This is done in multiple steps:

1) Prework if requried
2) Add RecordsABCConsent methods to the API that internally use the RecordsGaiaConsent method
3) Replace all calls to RecordGaiaConsent with calls to the proper RecordABCConsent method and make the RecordGaiaConsent private.
4) Replace the internal calls to RecoardGaiaConsent by creating UserConsentSpecifics and setting the consent protos there. (this means migrating to the new per consent protos)

Comment 1 by markusheintz@chromium.org, Jul 20

Cc: msramek@chromium.org
Pre-work was done in this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1124849

Comment 2 by markusheintz@chromium.org, Jul 20

Labels: -Pri-3 Pri-2
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 20 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bf8040e07f764effdaab95449818630960e9320d

commit bf8040e07f764effdaab95449818630960e9320d
Author: Markus Heintz <markusheintz@chromium.org>
Date: Fri Jul 20 13:52:59 2018

[User Consent] Add new public ConsentAuditor API and use it for recording the SyncConsent.

The ConsentAuditor is recording all UserConsents in Chrome. But originally the API was only designed in order to record the Sync consent. This CL updates the API. This makes it easier to record consent s properly and will allow us clean up code.
In a followup CL I will migrate the remaining clients to use the new API.
And in another CL I will update the sync internal code too to use the new protos.

BUG: 865902

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I34ffd4b02746b49b39ec88f5676c071256d9b33a
Reviewed-on: https://chromium-review.googlesource.com/1122628
Commit-Queue: Markus Heintz <markusheintz@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: vitaliii <vitaliii@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576847}
[modify] https://crrev.com/bf8040e07f764effdaab95449818630960e9320d/chrome/browser/android/consent_auditor/consent_auditor_bridge.cc
[modify] https://crrev.com/bf8040e07f764effdaab95449818630960e9320d/chrome/browser/ui/webui/signin/sync_confirmation_handler.cc
[modify] https://crrev.com/bf8040e07f764effdaab95449818630960e9320d/components/consent_auditor/consent_auditor.h
[modify] https://crrev.com/bf8040e07f764effdaab95449818630960e9320d/components/consent_auditor/consent_auditor_impl.cc
[modify] https://crrev.com/bf8040e07f764effdaab95449818630960e9320d/components/consent_auditor/consent_auditor_impl.h
[modify] https://crrev.com/bf8040e07f764effdaab95449818630960e9320d/components/consent_auditor/consent_auditor_impl_unittest.cc
[modify] https://crrev.com/bf8040e07f764effdaab95449818630960e9320d/components/consent_auditor/fake_consent_auditor.cc
[modify] https://crrev.com/bf8040e07f764effdaab95449818630960e9320d/components/consent_auditor/fake_consent_auditor.h
[modify] https://crrev.com/bf8040e07f764effdaab95449818630960e9320d/ios/chrome/browser/ui/authentication/chrome_signin_view_controller.mm
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 2 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c86e2ec5f36323129e1010879e9f3006e5ed81ed

commit c86e2ec5f36323129e1010879e9f3006e5ed81ed
Author: Markus Heintz <markusheintz@chromium.org>
Date: Thu Aug 02 08:27:10 2018

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}
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/chrome/browser/chromeos/arc/arc_play_store_enabled_preference_handler.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/chrome/browser/chromeos/arc/arc_play_store_enabled_preference_handler_unittest.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/chrome/browser/chromeos/arc/arc_support_host.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/chrome/browser/chromeos/arc/arc_support_host.h
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/chrome/browser/chromeos/arc/arc_support_host_unittest.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/chrome/browser/sync/test/integration/single_client_user_consents_sync_test.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/chrome/browser/ui/webui/chromeos/login/arc_terms_of_service_screen_handler.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/components/consent_auditor/BUILD.gn
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/components/consent_auditor/consent_auditor.h
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/components/consent_auditor/consent_auditor_impl.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/components/consent_auditor/consent_auditor_impl.h
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/components/consent_auditor/consent_auditor_impl_unittest.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/components/consent_auditor/fake_consent_auditor.cc
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/components/consent_auditor/fake_consent_auditor.h
[modify] https://crrev.com/c86e2ec5f36323129e1010879e9f3006e5ed81ed/components/sync/protocol/user_consent_types.proto

Comment 6 by mastiz@chromium.org, Nov 13

Labels: -Type-Bug Type-Feature

Comment 7 by vitaliii@chromium.org, Nov 13

Cc: -vitaliii@chromium.org
►

Sign in to add a comment