Commit 0e8d43a6 authored by Jérôme Lebel's avatar Jérôme Lebel Committed by Commit Bot

Reland "[iOS] Using UnifiedConsentService::SetUnifiedConsentGiven() in the settings"

This is a reland of 72bd9335

Fixing 2 test problems:

 + ChromeSigninViewControllerTest.TestConsentWithOKGOTIT*
UnifiedConsentService needs UnifiedConsentServiceClient, local and user
preference service. To make the unittest working, the
UnifiedConsentService is prevented to be created.

 + GoogleServicesSettingsTestCase.testOpenManageSyncedDataWebPageWhileSignedIn
To open the manage synced data web page, the user should be signed in
without unified consent.

Original change's description:
> [iOS] Using UnifiedConsentService::SetUnifiedConsentGiven() in the settings
>
> This CL enables unified consent given when the user signs in to Chrome.
> This CL also disabled and enables unified consent given when the user
> toggles the corresponding switch on and off in the Sync and other services
> settings screen.
>
> Bug: 827072
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I5384052f4abe117e7816733e45dc4091afe55708
> Reviewed-on: https://chromium-review.googlesource.com/1165151
> Reviewed-by: Thomas Tangl <tangltom@chromium.org>
> Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
> Reviewed-by: Sergio Collazos <sczs@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581873}

Bug: 827072
Change-Id: Ideb01f7c7a5bd2f5e1b6d927eb590b681283aad7
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1172402Reviewed-by: default avatarThomas Tangl <tangltom@chromium.org>
Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584413}
parent f30a46a9
......@@ -97,7 +97,7 @@ UnifiedConsentService::UnifiedConsentService(
sync_service_->AddObserver(this);
pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
pref_change_registrar_->Init(pref_service);
pref_change_registrar_->Init(pref_service_);
pref_change_registrar_->Add(
prefs::kUnifiedConsentGiven,
base::BindRepeating(
......
......@@ -42,6 +42,7 @@ source_set("authentication") {
"//components/signin/core/browser",
"//components/signin/ios/browser",
"//components/strings",
"//components/unified_consent",
"//google_apis",
"//ios/chrome/app/strings",
"//ios/chrome/browser",
......@@ -164,6 +165,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/ui/colors",
"//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/signin_interaction/public",
"//ios/chrome/browser/unified_consent",
"//ios/chrome/test:test_support",
"//ios/public/provider/chrome/browser/signin:test_support",
"//ios/third_party/material_components_ios",
......
......@@ -22,6 +22,7 @@
#include "components/signin/core/browser/profile_management_switches.h"
#include "components/signin/core/browser/signin_metrics.h"
#include "components/strings/grit/components_strings.h"
#include "components/unified_consent/unified_consent_service.h"
#import "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/chrome_url_constants.h"
#include "ios/chrome/browser/signin/account_tracker_service_factory.h"
......@@ -45,6 +46,7 @@
#import "ios/chrome/browser/ui/uikit_ui_util.h"
#import "ios/chrome/browser/ui/util/label_link_controller.h"
#include "ios/chrome/browser/unified_consent/feature.h"
#include "ios/chrome/browser/unified_consent/unified_consent_service_factory.h"
#include "ios/chrome/common/string_util.h"
#include "ios/chrome/grit/ios_chromium_strings.h"
#include "ios/chrome/grit/ios_strings.h"
......@@ -296,6 +298,15 @@ enum AuthenticationState {
- (void)acceptSignInAndCommitSyncChanges {
DCHECK(_didSignIn);
if (_unifiedConsentEnabled) {
// The consent has to be given as soon as the user is signed in. Even when
// they open the settings through the link.
unified_consent::UnifiedConsentService* unifiedConsentService =
UnifiedConsentServiceFactory::GetForBrowserState(_browserState);
// |unifiedConsentService| may be null in unit tests.
if (unifiedConsentService)
unifiedConsentService->SetUnifiedConsentGiven(true);
}
SyncSetupServiceFactory::GetForBrowserState(_browserState)->CommitChanges();
[self acceptSignInAndShowAccountsSettings:_unifiedConsentCoordinator
.settingsLinkWasTapped];
......
......@@ -24,6 +24,7 @@
#import "ios/chrome/browser/signin/authentication_service_fake.h"
#include "ios/chrome/browser/sync/consent_auditor_factory.h"
#include "ios/chrome/browser/sync/ios_user_event_service_factory.h"
#include "ios/chrome/browser/unified_consent/unified_consent_service_factory.h"
#include "ios/chrome/grit/ios_chromium_strings.h"
#include "ios/chrome/grit/ios_strings.h"
#import "ios/public/provider/chrome/browser/signin/fake_chrome_identity.h"
......@@ -87,6 +88,11 @@ static std::unique_ptr<KeyedService> CreateFakeConsentAuditor(
return std::make_unique<consent_auditor::FakeConsentAuditor>();
}
static std::unique_ptr<KeyedService> CreateFakeUnifiedConsentService(
web::BrowserState* context) {
return nullptr;
}
// These tests verify that Chrome correctly records user's consent to Chrome
// Sync, which is a GDPR requirement. None of those tests should be turned off.
// If one of those tests fails, one of the following methods should be updated
......@@ -118,6 +124,8 @@ class ChromeSigninViewControllerTest
builder.AddTestingFactory(ConsentAuditorFactory::GetInstance(),
&CreateFakeConsentAuditor);
builder.AddTestingFactory(UnifiedConsentServiceFactory::GetInstance(),
&CreateFakeUnifiedConsentService);
context_ = builder.Build();
ios::FakeChromeIdentityService* identity_service =
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider();
......
......@@ -23,6 +23,7 @@
#import "ios/chrome/browser/ui/settings/google_services_settings_view_controller.h"
#import "ios/chrome/browser/ui/settings/sync_encryption_collection_view_controller.h"
#import "ios/chrome/browser/ui/settings/sync_encryption_passphrase_collection_view_controller.h"
#include "ios/chrome/browser/unified_consent/unified_consent_service_factory.h"
#include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
#import "ios/public/provider/chrome/browser/signin/chrome_identity_browser_opener.h"
#include "ios/public/provider/chrome/browser/signin/chrome_identity_service.h"
......@@ -64,10 +65,13 @@
SyncSetupServiceFactory::GetForBrowserState(self.browserState);
browser_sync::ProfileSyncService* syncService =
ProfileSyncServiceFactory::GetForBrowserState(self.browserState);
unified_consent::UnifiedConsentService* unifiedConsentService =
UnifiedConsentServiceFactory::GetForBrowserState(self.browserState);
self.mediator = [[GoogleServicesSettingsMediator alloc]
initWithPrefService:self.browserState->GetPrefs()
syncService:syncService
syncSetupService:syncSetupService];
syncSetupService:syncSetupService
unifiedConsentService:unifiedConsentService];
self.mediator.consumer = viewController;
self.mediator.authService = self.authService;
viewController.modelDelegate = self.mediator;
......
......@@ -80,8 +80,7 @@ using unified_consent::prefs::kUnifiedConsentGiven;
if (!IsUIRefreshPhase1Enabled())
EARL_GREY_TEST_SKIPPED(@"This test is UIRefresh only.");
[SigninEarlGreyUI signinWithIdentity:[SigninEarlGreyUtils fakeIdentity1]];
PrefService* prefService = GetOriginalBrowserState()->GetPrefs();
prefService->SetBoolean(kUnifiedConsentGiven, false);
[self resetUnifiedConsent];
[self openGoogleServicesSettings];
[self assertSyncEverythingSection];
[self assertPersonalizedServicesCollapsed:NO];
......@@ -98,7 +97,8 @@ using unified_consent::prefs::kUnifiedConsentGiven;
EARL_GREY_TEST_SKIPPED(@"This test is UIRefresh only.");
[SigninEarlGreyUI signinWithIdentity:[SigninEarlGreyUtils fakeIdentity1]];
PrefService* prefService = GetOriginalBrowserState()->GetPrefs();
prefService->SetBoolean(kUnifiedConsentGiven, true);
GREYAssert(prefService->GetBoolean(kUnifiedConsentGiven),
@"Unified consent should be given");
[self openGoogleServicesSettings];
[self assertSyncEverythingSection];
[self assertPersonalizedServicesCollapsed:YES];
......@@ -140,8 +140,7 @@ using unified_consent::prefs::kUnifiedConsentGiven;
- (void)testOpenManageSyncedDataWebPage {
if (!IsUIRefreshPhase1Enabled())
EARL_GREY_TEST_SKIPPED(@"This test is UIRefresh only.");
PrefService* prefService = GetOriginalBrowserState()->GetPrefs();
prefService->SetBoolean(kUnifiedConsentGiven, false);
[self resetUnifiedConsent];
[self openGoogleServicesSettings];
[self togglePersonalizedServicesSection];
[[self cellElementInteractionWithTitleID:
......@@ -151,14 +150,13 @@ using unified_consent::prefs::kUnifiedConsentGiven;
assertWithMatcher:grey_notNil()];
}
// Tests the "Manage synced data" cell closes the settings, to open the web
// page.
// Tests the "Manage synced data" cell closes the settings, and opens the web
// page, while the user is signed in without user consent.
- (void)testOpenManageSyncedDataWebPageWhileSignedIn {
if (!IsUIRefreshPhase1Enabled())
EARL_GREY_TEST_SKIPPED(@"This test is UIRefresh only.");
PrefService* prefService = GetOriginalBrowserState()->GetPrefs();
prefService->SetBoolean(kUnifiedConsentGiven, false);
[SigninEarlGreyUI signinWithIdentity:[SigninEarlGreyUtils fakeIdentity1]];
[self resetUnifiedConsent];
[self openGoogleServicesSettings];
[[self cellElementInteractionWithTitleID:
IDS_IOS_GOOGLE_SERVICES_SETTINGS_MANAGED_SYNC_DATA_TEXT
......@@ -169,6 +167,12 @@ using unified_consent::prefs::kUnifiedConsentGiven;
#pragma mark - Helpers
// Resets the unified consent given by the user.
- (void)resetUnifiedConsent {
PrefService* prefService = GetOriginalBrowserState()->GetPrefs();
prefService->SetBoolean(kUnifiedConsentGiven, false);
}
- (void)openGoogleServicesSettings {
[ChromeEarlGreyUI openSettingsMenu];
[ChromeEarlGreyUI tapSettingsMenuButton:GoogleServicesSettingsButton()];
......
......@@ -19,7 +19,10 @@ class SyncSetupService;
namespace browser_sync {
class ProfileSyncService;
};
} // namespace browser_sync
namespace unified_consent {
class UnifiedConsentService;
} // namespace unified_consent
// Mediator for the Google services settings.
@interface GoogleServicesSettingsMediator
......@@ -31,12 +34,14 @@ class ProfileSyncService;
// Authentication service.
@property(nonatomic, assign) AuthenticationService* authService;
// Designated initializer. |prefService|, |syncService| and |syncSetupService|
// should not be null.
// Designated initializer. |prefService|, |syncService|, |syncSetupService| and
// |unifiedConsentService| should not be null.
- (instancetype)initWithPrefService:(PrefService*)prefService
syncService:
(browser_sync::ProfileSyncService*)syncService
syncSetupService:(SyncSetupService*)syncSetupService
unifiedConsentService:
(unified_consent::UnifiedConsentService*)unifiedConsentService
NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;
......
......@@ -11,6 +11,7 @@
#include "components/prefs/pref_change_registrar.h"
#include "components/prefs/pref_service.h"
#include "components/unified_consent/pref_names.h"
#include "components/unified_consent/unified_consent_service.h"
#include "ios/chrome/browser/pref_names.h"
#import "ios/chrome/browser/signin/authentication_service.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h"
......@@ -88,12 +89,13 @@ typedef NS_ENUM(NSInteger, ItemType) {
std::unique_ptr<SyncObserverBridge> _syncObserver;
}
// Unified consent service.
@property(nonatomic, assign)
unified_consent::UnifiedConsentService* unifiedConsentService;
// Returns YES if the user is authenticated.
@property(nonatomic, assign, readonly) BOOL isAuthenticated;
// Returns YES if the user has given his consent to use Google services.
@property(nonatomic, assign, readonly) BOOL isConsentGiven;
// Preference service.
@property(nonatomic, assign, readonly) PrefService* prefService;
// Sync setup service.
@property(nonatomic, assign, readonly) SyncSetupService* syncSetupService;
// Preference value for the "Autocomplete searches and URLs" feature.
......@@ -126,9 +128,9 @@ typedef NS_ENUM(NSInteger, ItemType) {
@implementation GoogleServicesSettingsMediator
@synthesize unifiedConsentService = _unifiedConsentService;
@synthesize consumer = _consumer;
@synthesize authService = _authService;
@synthesize prefService = _prefService;
@synthesize syncSetupService = _syncSetupService;
@synthesize autocompleteSearchPreference = _autocompleteSearchPreference;
@synthesize anonymizedDataCollectionPreference =
......@@ -145,17 +147,20 @@ typedef NS_ENUM(NSInteger, ItemType) {
#pragma mark - Load model
- (instancetype)initWithPrefService:(PrefService*)prefService
syncService:
(browser_sync::ProfileSyncService*)syncService
syncSetupService:(SyncSetupService*)syncSetupService {
- (instancetype)
initWithPrefService:(PrefService*)prefService
syncService:(browser_sync::ProfileSyncService*)syncService
syncSetupService:(SyncSetupService*)syncSetupService
unifiedConsentService:
(unified_consent::UnifiedConsentService*)unifiedConsentService {
self = [super init];
if (self) {
DCHECK(prefService);
DCHECK(syncService);
DCHECK(syncSetupService);
_prefService = prefService;
DCHECK(unifiedConsentService);
_syncSetupService = syncSetupService;
_unifiedConsentService = unifiedConsentService;
_syncObserver.reset(new SyncObserverBridge(self, syncService));
prefObserverBridge_ = std::make_unique<PrefObserverBridge>(self);
prefChangeRegistrar_.Init(prefService);
......@@ -229,7 +234,7 @@ typedef NS_ENUM(NSInteger, ItemType) {
}
- (BOOL)isConsentGiven {
return self.prefService->GetBoolean(kUnifiedConsentGiven);
return self.unifiedConsentService->IsUnifiedConsentGiven();
}
- (CollectionViewItem*)syncEverythingItem {
......@@ -535,7 +540,7 @@ textItemWithItemType:(NSInteger)itemType
return;
// Mark the switch has being animated to avoid being reloaded.
base::AutoReset<BOOL> autoReset(&_syncEverythingSwitchBeingAnimated, YES);
self.prefService->SetBoolean(kUnifiedConsentGiven, value);
self.unifiedConsentService->SetUnifiedConsentGiven(value);
}
- (void)toggleSyncDataSync:(NSInteger)dataTypeInt withValue:(BOOL)value {
......
......@@ -16,6 +16,7 @@ UnifiedConsentServiceClientImpl::UnifiedConsentServiceClientImpl(
: user_pref_service_(user_pref_service),
local_pref_service_(local_pref_service) {
DCHECK(user_pref_service_);
DCHECK(local_pref_service_);
ObserveServicePrefChange(Service::kMetricsReporting,
metrics::prefs::kMetricsReportingEnabled,
local_pref_service);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment