Commit 3a8ef4a5 authored by Nohemi Fernandez's avatar Nohemi Fernandez Committed by Chromium LUCI CQ

[iOS] Use signin namespace for utility functions.

As a follow-up to crrev.com/c/2562778, moves all sign-in utilities to
the new signin namespace.

Bug: 1157531
Change-Id: Ic695832a7cd5778e69039b2d3fd1395de3cde50c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593259Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837501}
parent c248711a
...@@ -14,20 +14,20 @@ namespace base { ...@@ -14,20 +14,20 @@ namespace base {
class Version; class Version;
} }
namespace signin {
// Returns true if this user sign-in upgrade should be shown for |browserState|. // Returns true if this user sign-in upgrade should be shown for |browserState|.
bool SigninShouldPresentUserSigninUpgrade(ChromeBrowserState* browserState); bool ShouldPresentUserSigninUpgrade(ChromeBrowserState* browserState);
// Records in user defaults: // Records in user defaults:
// + the Chromium current version. // + the Chromium current version.
// + increases the sign-in promo display count. // + increases the sign-in promo display count.
// + Gaia ids list. // + Gaia ids list.
// Separated out into a discrete function to allow overriding when testing. // Separated out into a discrete function to allow overriding when testing.
void SigninRecordVersionSeen(); void RecordVersionSeen();
// Set the Chromium current version for sign-in. Used for tests only. // Set the Chromium current version for sign-in. Used for tests only.
void SetSigninCurrentVersionForTesting(base::Version* version); void SetCurrentVersionForTesting(base::Version* version);
namespace signin {
// Returns a boolean indicating whether browser sign-in is allowed by policy. // Returns a boolean indicating whether browser sign-in is allowed by policy.
bool IsSigninAllowed(const PrefService* prefs); bool IsSigninAllowed(const PrefService* prefs);
......
...@@ -52,7 +52,9 @@ NSSet* GaiaIdSetWithIdentities(NSArray* identities) { ...@@ -52,7 +52,9 @@ NSSet* GaiaIdSetWithIdentities(NSArray* identities) {
#pragma mark - Public #pragma mark - Public
bool SigninShouldPresentUserSigninUpgrade(ChromeBrowserState* browserState) { namespace signin {
bool ShouldPresentUserSigninUpgrade(ChromeBrowserState* browserState) {
if (tests_hook::DisableSigninRecallPromo()) if (tests_hook::DisableSigninRecallPromo())
return false; return false;
...@@ -110,7 +112,7 @@ bool SigninShouldPresentUserSigninUpgrade(ChromeBrowserState* browserState) { ...@@ -110,7 +112,7 @@ bool SigninShouldPresentUserSigninUpgrade(ChromeBrowserState* browserState) {
![lastKnownGaiaIdSet isEqualToSet:currentGaiaIdSet]; ![lastKnownGaiaIdSet isEqualToSet:currentGaiaIdSet];
} }
void SigninRecordVersionSeen() { void RecordVersionSeen() {
NSUserDefaults* standardDefaults = [NSUserDefaults standardUserDefaults]; NSUserDefaults* standardDefaults = [NSUserDefaults standardUserDefaults];
[standardDefaults [standardDefaults
setObject:base::SysUTF8ToNSString(CurrentVersion().GetString()) setObject:base::SysUTF8ToNSString(CurrentVersion().GetString())
...@@ -128,14 +130,10 @@ void SigninRecordVersionSeen() { ...@@ -128,14 +130,10 @@ void SigninRecordVersionSeen() {
forKey:kSigninPromoViewDisplayCountKey]; forKey:kSigninPromoViewDisplayCountKey];
} }
void SetSigninCurrentVersionForTesting(Version* version) { void SetCurrentVersionForTesting(Version* version) {
g_current_version_for_test = version; g_current_version_for_test = version;
} }
// TODO(crbug.com/1157531): Move the other functions inside the signin namespace
// as well.
namespace signin {
bool IsSigninAllowed(const PrefService* prefs) { bool IsSigninAllowed(const PrefService* prefs) {
// Sign-in is always allowed if the policy handler isn't installed. // Sign-in is always allowed if the policy handler isn't installed.
return !ShouldInstallBrowserSigninPolicyHandler() || return !ShouldInstallBrowserSigninPolicyHandler() ||
......
...@@ -43,10 +43,10 @@ namespace { ...@@ -43,10 +43,10 @@ namespace {
class SigninUtilsTest : public BlockCleanupTest { class SigninUtilsTest : public BlockCleanupTest {
public: public:
SigninUtilsTest() : version_("1.0") { SigninUtilsTest() : version_("1.0") {
SetSigninCurrentVersionForTesting(&version_); signin::SetCurrentVersionForTesting(&version_);
} }
~SigninUtilsTest() override { SetSigninCurrentVersionForTesting(nullptr); } ~SigninUtilsTest() override { signin::SetCurrentVersionForTesting(nullptr); }
void SetUp() override { void SetUp() override {
BlockCleanupTest::SetUp(); BlockCleanupTest::SetUp();
...@@ -92,54 +92,54 @@ class SigninUtilsTest : public BlockCleanupTest { ...@@ -92,54 +92,54 @@ class SigninUtilsTest : public BlockCleanupTest {
// Should show the sign-in upgrade for the first time. // Should show the sign-in upgrade for the first time.
TEST_F(SigninUtilsTest, TestWillDisplay) { TEST_F(SigninUtilsTest, TestWillDisplay) {
EXPECT_TRUE( EXPECT_TRUE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should not show the sign-in upgrade twice on the same version. // Should not show the sign-in upgrade twice on the same version.
TEST_F(SigninUtilsTest, TestWillNotDisplaySameVersion) { TEST_F(SigninUtilsTest, TestWillNotDisplaySameVersion) {
SigninRecordVersionSeen(); signin::RecordVersionSeen();
EXPECT_FALSE( EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should not show the sign-in upgrade twice until two major version after. // Should not show the sign-in upgrade twice until two major version after.
TEST_F(SigninUtilsTest, TestWillNotDisplayOneMinorVersion) { TEST_F(SigninUtilsTest, TestWillNotDisplayOneMinorVersion) {
SigninRecordVersionSeen(); signin::RecordVersionSeen();
// Set the future version to be one minor release ahead. // Set the future version to be one minor release ahead.
Version version_1_1("1.1"); Version version_1_1("1.1");
SetSigninCurrentVersionForTesting(&version_1_1); signin::SetCurrentVersionForTesting(&version_1_1);
EXPECT_FALSE( EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should not show the sign-in upgrade twice until two major version after. // Should not show the sign-in upgrade twice until two major version after.
TEST_F(SigninUtilsTest, TestWillNotDisplayTwoMinorVersions) { TEST_F(SigninUtilsTest, TestWillNotDisplayTwoMinorVersions) {
SigninRecordVersionSeen(); signin::RecordVersionSeen();
// Set the future version to be two minor releases ahead. // Set the future version to be two minor releases ahead.
Version version_1_2("1.2"); Version version_1_2("1.2");
SetSigninCurrentVersionForTesting(&version_1_2); signin::SetCurrentVersionForTesting(&version_1_2);
EXPECT_FALSE( EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should not show the sign-in upgrade twice until two major version after. // Should not show the sign-in upgrade twice until two major version after.
TEST_F(SigninUtilsTest, TestWillNotDisplayOneMajorVersion) { TEST_F(SigninUtilsTest, TestWillNotDisplayOneMajorVersion) {
SigninRecordVersionSeen(); signin::RecordVersionSeen();
// Set the future version to be one major release ahead. // Set the future version to be one major release ahead.
Version version_2_0("2.0"); Version version_2_0("2.0");
SetSigninCurrentVersionForTesting(&version_2_0); signin::SetCurrentVersionForTesting(&version_2_0);
EXPECT_FALSE( EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should show the sign-in upgrade a second time, 2 version after. // Should show the sign-in upgrade a second time, 2 version after.
TEST_F(SigninUtilsTest, TestWillDisplayTwoMajorVersions) { TEST_F(SigninUtilsTest, TestWillDisplayTwoMajorVersions) {
SigninRecordVersionSeen(); signin::RecordVersionSeen();
// Set the future version to be two major releases ahead. // Set the future version to be two major releases ahead.
Version version_3_0("3.0"); Version version_3_0("3.0");
SetSigninCurrentVersionForTesting(&version_3_0); signin::SetCurrentVersionForTesting(&version_3_0);
EXPECT_TRUE( EXPECT_TRUE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Show the sign-in upgrade on version 1.0. // Show the sign-in upgrade on version 1.0.
...@@ -147,14 +147,14 @@ TEST_F(SigninUtilsTest, TestWillDisplayTwoMajorVersions) { ...@@ -147,14 +147,14 @@ TEST_F(SigninUtilsTest, TestWillDisplayTwoMajorVersions) {
// Move to version 5.0. // Move to version 5.0.
// Expected: should not show the sign-in upgrade. // Expected: should not show the sign-in upgrade.
TEST_F(SigninUtilsTest, TestWillShowTwoTimesOnly) { TEST_F(SigninUtilsTest, TestWillShowTwoTimesOnly) {
SigninRecordVersionSeen(); signin::RecordVersionSeen();
Version version_3_0("3.0"); Version version_3_0("3.0");
SetSigninCurrentVersionForTesting(&version_3_0); signin::SetCurrentVersionForTesting(&version_3_0);
SigninRecordVersionSeen(); signin::RecordVersionSeen();
Version version_5_0("5.0"); Version version_5_0("5.0");
SetSigninCurrentVersionForTesting(&version_5_0); signin::SetCurrentVersionForTesting(&version_5_0);
EXPECT_FALSE( EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Show the sign-in upgrade on version 1.0. // Show the sign-in upgrade on version 1.0.
...@@ -163,16 +163,16 @@ TEST_F(SigninUtilsTest, TestWillShowTwoTimesOnly) { ...@@ -163,16 +163,16 @@ TEST_F(SigninUtilsTest, TestWillShowTwoTimesOnly) {
// Add new account. // Add new account.
// Expected: should show the sign-in upgrade. // Expected: should show the sign-in upgrade.
TEST_F(SigninUtilsTest, TestWillShowForNewAccountAdded) { TEST_F(SigninUtilsTest, TestWillShowForNewAccountAdded) {
SigninRecordVersionSeen(); signin::RecordVersionSeen();
Version version_3_0("3.0"); Version version_3_0("3.0");
SetSigninCurrentVersionForTesting(&version_3_0); signin::SetCurrentVersionForTesting(&version_3_0);
SigninRecordVersionSeen(); signin::RecordVersionSeen();
Version version_5_0("5.0"); Version version_5_0("5.0");
SetSigninCurrentVersionForTesting(&version_5_0); signin::SetCurrentVersionForTesting(&version_5_0);
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider() ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
->AddIdentities(@[ @"foo1" ]); ->AddIdentities(@[ @"foo1" ]);
EXPECT_TRUE( EXPECT_TRUE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Add new account. // Add new account.
...@@ -185,12 +185,12 @@ TEST_F(SigninUtilsTest, TestWillNotShowWithAccountRemoved) { ...@@ -185,12 +185,12 @@ TEST_F(SigninUtilsTest, TestWillNotShowWithAccountRemoved) {
NSString* newAccountGaiaId = @"foo1"; NSString* newAccountGaiaId = @"foo1";
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider() ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
->AddIdentities(@[ newAccountGaiaId ]); ->AddIdentities(@[ newAccountGaiaId ]);
SigninRecordVersionSeen(); signin::RecordVersionSeen();
Version version_3_0("3.0"); Version version_3_0("3.0");
SetSigninCurrentVersionForTesting(&version_3_0); signin::SetCurrentVersionForTesting(&version_3_0);
SigninRecordVersionSeen(); signin::RecordVersionSeen();
Version version_5_0("5.0"); Version version_5_0("5.0");
SetSigninCurrentVersionForTesting(&version_5_0); signin::SetCurrentVersionForTesting(&version_5_0);
NSArray* allIdentities = NSArray* allIdentities =
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider() ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
->GetAllIdentities(); ->GetAllIdentities();
...@@ -205,7 +205,7 @@ TEST_F(SigninUtilsTest, TestWillNotShowWithAccountRemoved) { ...@@ -205,7 +205,7 @@ TEST_F(SigninUtilsTest, TestWillNotShowWithAccountRemoved) {
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider() ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
->ForgetIdentity(foo1Identity, nil); ->ForgetIdentity(foo1Identity, nil);
EXPECT_FALSE( EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Show the sign-in upgrade on version 1.0. // Show the sign-in upgrade on version 1.0.
...@@ -214,16 +214,16 @@ TEST_F(SigninUtilsTest, TestWillNotShowWithAccountRemoved) { ...@@ -214,16 +214,16 @@ TEST_F(SigninUtilsTest, TestWillNotShowWithAccountRemoved) {
// Add an account. // Add an account.
// Expected: should not show the sign-in upgrade. // Expected: should not show the sign-in upgrade.
TEST_F(SigninUtilsTest, TestWillNotShowNewAccountUntilTwoVersion) { TEST_F(SigninUtilsTest, TestWillNotShowNewAccountUntilTwoVersion) {
SigninRecordVersionSeen(); signin::RecordVersionSeen();
Version version_3_0("3.0"); Version version_3_0("3.0");
SetSigninCurrentVersionForTesting(&version_3_0); signin::SetCurrentVersionForTesting(&version_3_0);
SigninRecordVersionSeen(); signin::RecordVersionSeen();
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider() ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
->AddIdentities(@[ @"foo1" ]); ->AddIdentities(@[ @"foo1" ]);
Version version_4_0("4.0"); Version version_4_0("4.0");
SetSigninCurrentVersionForTesting(&version_4_0); signin::SetCurrentVersionForTesting(&version_4_0);
EXPECT_FALSE( EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Show the sign-in upgrade on version 1.0. // Show the sign-in upgrade on version 1.0.
...@@ -232,13 +232,13 @@ TEST_F(SigninUtilsTest, TestWillNotShowNewAccountUntilTwoVersion) { ...@@ -232,13 +232,13 @@ TEST_F(SigninUtilsTest, TestWillNotShowNewAccountUntilTwoVersion) {
// Expected: should not show the sign-in upgrade (only display every 2 // Expected: should not show the sign-in upgrade (only display every 2
// versions). // versions).
TEST_F(SigninUtilsTest, TestWillNotShowNewAccountUntilTwoVersionBis) { TEST_F(SigninUtilsTest, TestWillNotShowNewAccountUntilTwoVersionBis) {
SigninRecordVersionSeen(); signin::RecordVersionSeen();
Version version_2_0("2.0"); Version version_2_0("2.0");
SetSigninCurrentVersionForTesting(&version_2_0); signin::SetCurrentVersionForTesting(&version_2_0);
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider() ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
->AddIdentities(@[ @"foo1" ]); ->AddIdentities(@[ @"foo1" ]);
EXPECT_FALSE( EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// Should not show the sign-in upgrade if sign-in is disabled by policy. // Should not show the sign-in upgrade if sign-in is disabled by policy.
...@@ -252,7 +252,7 @@ TEST_F(SigninUtilsTest, TestWillNotShowIfDisabledByPolicy) { ...@@ -252,7 +252,7 @@ TEST_F(SigninUtilsTest, TestWillNotShowIfDisabledByPolicy) {
chrome_browser_state_->GetPrefs()->SetBoolean(prefs::kSigninAllowed, false); chrome_browser_state_->GetPrefs()->SetBoolean(prefs::kSigninAllowed, false);
EXPECT_FALSE( EXPECT_FALSE(
SigninShouldPresentUserSigninUpgrade(chrome_browser_state_.get())); signin::ShouldPresentUserSigninUpgrade(chrome_browser_state_.get()));
} }
// signin::IsSigninAllowed should respect the kSigninAllowed pref. // signin::IsSigninAllowed should respect the kSigninAllowed pref.
......
...@@ -33,7 +33,7 @@ using signin_metrics::RecordSigninUserActionForAccessPoint; ...@@ -33,7 +33,7 @@ using signin_metrics::RecordSigninUserActionForAccessPoint;
LogSigninAccessPointStarted(self.accessPoint, self.promoAction); LogSigninAccessPointStarted(self.accessPoint, self.promoAction);
RecordSigninUserActionForAccessPoint(self.accessPoint, self.promoAction); RecordSigninUserActionForAccessPoint(self.accessPoint, self.promoAction);
} }
SigninRecordVersionSeen(); signin::RecordVersionSeen();
} }
@end @end
...@@ -53,7 +53,7 @@ typedef NS_ENUM(NSUInteger, UserSigninPromoAction) { ...@@ -53,7 +53,7 @@ typedef NS_ENUM(NSUInteger, UserSigninPromoAction) {
// Records in user defaults that the promo has been shown as well as the // Records in user defaults that the promo has been shown as well as the
// number of times it's been displayed. // number of times it's been displayed.
SigninRecordVersionSeen(); signin::RecordVersionSeen();
NSUserDefaults* standardDefaults = [NSUserDefaults standardUserDefaults]; NSUserDefaults* standardDefaults = [NSUserDefaults standardUserDefaults];
int promoSeenCount = int promoSeenCount =
[standardDefaults integerForKey:kDisplayedSSORecallPromoCountKey]; [standardDefaults integerForKey:kDisplayedSSORecallPromoCountKey];
......
...@@ -958,7 +958,7 @@ const char kMultiWindowOpenInNewWindowHistogram[] = ...@@ -958,7 +958,7 @@ const char kMultiWindowOpenInNewWindowHistogram[] =
// Presents the sign-in upgrade promo if is relevant and possible. // Presents the sign-in upgrade promo if is relevant and possible.
// Returns YES if the promo is shown. // Returns YES if the promo is shown.
- (BOOL)presentSigninUpgradePromoIfPossible { - (BOOL)presentSigninUpgradePromoIfPossible {
if (!SigninShouldPresentUserSigninUpgrade( if (!signin::ShouldPresentUserSigninUpgrade(
self.sceneState.appState.mainBrowserState)) self.sceneState.appState.mainBrowserState))
return NO; return NO;
// Don't show promos if first run is shown in any scene. (Note: This flag // Don't show promos if first run is shown in any scene. (Note: This flag
......
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