Commit a240538b authored by Jérôme Lebel's avatar Jérôme Lebel Committed by Chromium LUCI CQ

[iOS] Add kechain reload flag in ChromeIdentityServiceObserverBridge

The goal of this series of patches is to propagate the keychain reload
flag that comes with the identity list update notification from
GCRSSOService.

This flag will be used by AuthenticationService to have a better signal
when the identity list is updated by another Google app (when the flag
is YES), or if the update comes from Chrome user actions.

* crrev.com/i/3513584: Receive the flag from GCRSSOService notification
=>crrev.com/c/2615421: Add support for the keychain reload flag
* crrev.com/i/3470951: Switching FireIdentityListChanged(bool) method
* crrev.com/c/2615538: FireIdentityListChanged() cleanup

Bug: 1097080
Change-Id: I5763ed1127ba1fe1e8e79953bea321803c06953d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2615421
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: default avatarNohemi Fernandez <fernandex@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842624}
parent 022f806e
...@@ -196,7 +196,7 @@ class AuthenticationService : public KeyedService, ...@@ -196,7 +196,7 @@ class AuthenticationService : public KeyedService,
void OnEndBatchOfRefreshTokenStateChanges() override; void OnEndBatchOfRefreshTokenStateChanges() override;
// ChromeIdentityServiceObserver implementation. // ChromeIdentityServiceObserver implementation.
void OnIdentityListChanged() override; void OnIdentityListChanged(bool keychainReload) override;
void OnAccessTokenRefreshFailed(ChromeIdentity* identity, void OnAccessTokenRefreshFailed(ChromeIdentity* identity,
NSDictionary* user_info) override; NSDictionary* user_info) override;
void OnChromeIdentityServiceWillBeDestroyed() override; void OnChromeIdentityServiceWillBeDestroyed() override;
......
...@@ -435,7 +435,7 @@ void AuthenticationService::OnEndBatchOfRefreshTokenStateChanges() { ...@@ -435,7 +435,7 @@ void AuthenticationService::OnEndBatchOfRefreshTokenStateChanges() {
StoreKnownAccountsWhileInForeground(); StoreKnownAccountsWhileInForeground();
} }
void AuthenticationService::OnIdentityListChanged() { void AuthenticationService::OnIdentityListChanged(bool keychainReload) {
// The list of identities may change while in an authorized call. Signing out // The list of identities may change while in an authorized call. Signing out
// the authenticated user at this time may lead to crashes (e.g. // the authenticated user at this time may lead to crashes (e.g.
// http://crbug.com/398431 ). // http://crbug.com/398431 ).
......
...@@ -126,8 +126,8 @@ class AuthenticationServiceTest : public PlatformTest { ...@@ -126,8 +126,8 @@ class AuthenticationServiceTest : public PlatformTest {
authentication_service()->OnAccessTokenRefreshFailed(identity, user_info); authentication_service()->OnAccessTokenRefreshFailed(identity, user_info);
} }
void FireIdentityListChanged() { void FireIdentityListChanged(bool keychainReload) {
authentication_service()->OnIdentityListChanged(); authentication_service()->OnIdentityListChanged(keychainReload);
} }
void SetCachedMDMInfo(ChromeIdentity* identity, NSDictionary* user_info) { void SetCachedMDMInfo(ChromeIdentity* identity, NSDictionary* user_info) {
...@@ -313,7 +313,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_NoChange) { ...@@ -313,7 +313,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_NoChange) {
authentication_service()->SignIn(identity(0)); authentication_service()->SignIn(identity(0));
identity_service()->AddIdentities(@[ @"foo3" ]); identity_service()->AddIdentities(@[ @"foo3" ]);
FireIdentityListChanged(); FireIdentityListChanged(true);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
// If an account is added while the application is in foreground, then the // If an account is added while the application is in foreground, then the
...@@ -337,7 +337,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_ChangedInBackground) { ...@@ -337,7 +337,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_ChangedInBackground) {
authentication_service()->SignIn(identity(0)); authentication_service()->SignIn(identity(0));
identity_service()->AddIdentities(@[ @"foo3" ]); identity_service()->AddIdentities(@[ @"foo3" ]);
FireIdentityListChanged(); FireIdentityListChanged(true);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_FALSE( EXPECT_FALSE(
authentication_service()->HaveAccountsChangedWhileInBackground()); authentication_service()->HaveAccountsChangedWhileInBackground());
...@@ -355,7 +355,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_CalledInBackground) { ...@@ -355,7 +355,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_CalledInBackground) {
authentication_service()->SignIn(identity(0)); authentication_service()->SignIn(identity(0));
identity_service()->AddIdentities(@[ @"foo3" ]); identity_service()->AddIdentities(@[ @"foo3" ]);
FireIdentityListChanged(); FireIdentityListChanged(true);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_FALSE( EXPECT_FALSE(
authentication_service()->HaveAccountsChangedWhileInBackground()); authentication_service()->HaveAccountsChangedWhileInBackground());
...@@ -364,7 +364,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_CalledInBackground) { ...@@ -364,7 +364,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_CalledInBackground) {
// background. // background.
FireApplicationDidEnterBackground(); FireApplicationDidEnterBackground();
identity_service()->AddIdentities(@[ @"foo4" ]); identity_service()->AddIdentities(@[ @"foo4" ]);
FireIdentityListChanged(); FireIdentityListChanged(true);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(authentication_service()->HaveAccountsChangedWhileInBackground()); EXPECT_TRUE(authentication_service()->HaveAccountsChangedWhileInBackground());
...@@ -379,7 +379,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_ResetOntwoBackgrounds) { ...@@ -379,7 +379,7 @@ TEST_F(AuthenticationServiceTest, HaveAccountsChanged_ResetOntwoBackgrounds) {
authentication_service()->SignIn(identity(0)); authentication_service()->SignIn(identity(0));
identity_service()->AddIdentities(@[ @"foo3" ]); identity_service()->AddIdentities(@[ @"foo3" ]);
FireIdentityListChanged(); FireIdentityListChanged(true);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_FALSE( EXPECT_FALSE(
authentication_service()->HaveAccountsChangedWhileInBackground()); authentication_service()->HaveAccountsChangedWhileInBackground());
......
...@@ -31,7 +31,7 @@ class ChromeIdentityServiceObserverBridge ...@@ -31,7 +31,7 @@ class ChromeIdentityServiceObserverBridge
private: private:
// ios::ChromeIdentityService::Observer implementation. // ios::ChromeIdentityService::Observer implementation.
void OnIdentityListChanged() override; void OnIdentityListChanged(bool keychainReload) override;
void OnAccessTokenRefreshFailed(ChromeIdentity* identity, void OnAccessTokenRefreshFailed(ChromeIdentity* identity,
NSDictionary* user_info) override; NSDictionary* user_info) override;
void OnProfileUpdate(ChromeIdentity* identity) override; void OnProfileUpdate(ChromeIdentity* identity) override;
......
...@@ -21,7 +21,8 @@ ChromeIdentityServiceObserverBridge::ChromeIdentityServiceObserverBridge( ...@@ -21,7 +21,8 @@ ChromeIdentityServiceObserverBridge::ChromeIdentityServiceObserverBridge(
ChromeIdentityServiceObserverBridge::~ChromeIdentityServiceObserverBridge() {} ChromeIdentityServiceObserverBridge::~ChromeIdentityServiceObserverBridge() {}
void ChromeIdentityServiceObserverBridge::OnIdentityListChanged() { void ChromeIdentityServiceObserverBridge::OnIdentityListChanged(
bool keychainReload) {
if ([observer_ respondsToSelector:@selector(identityListChanged)]) if ([observer_ respondsToSelector:@selector(identityListChanged)])
[observer_ identityListChanged]; [observer_ identityListChanged];
} }
......
...@@ -97,7 +97,7 @@ class ChromeIdentityServiceObserverBridgeTest : public PlatformTest { ...@@ -97,7 +97,7 @@ class ChromeIdentityServiceObserverBridgeTest : public PlatformTest {
// Tests that |onIdentityListChanged| is forwarded. // Tests that |onIdentityListChanged| is forwarded.
TEST_F(ChromeIdentityServiceObserverBridgeTest, onIdentityListChanged) { TEST_F(ChromeIdentityServiceObserverBridgeTest, onIdentityListChanged) {
ASSERT_FALSE(GetTestObserver().onIdentityListChangedCalled); ASSERT_FALSE(GetTestObserver().onIdentityListChangedCalled);
GetObserverBridge()->OnIdentityListChanged(); GetObserverBridge()->OnIdentityListChanged(false);
EXPECT_TRUE(GetTestObserver().onIdentityListChangedCalled); EXPECT_TRUE(GetTestObserver().onIdentityListChangedCalled);
EXPECT_FALSE(GetTestObserver().onAccessTokenRefreshFailedCalled); EXPECT_FALSE(GetTestObserver().onAccessTokenRefreshFailedCalled);
EXPECT_FALSE(GetTestObserver().onProfileUpdateCalled); EXPECT_FALSE(GetTestObserver().onProfileUpdateCalled);
......
...@@ -79,8 +79,15 @@ class ChromeIdentityService { ...@@ -79,8 +79,15 @@ class ChromeIdentityService {
virtual ~Observer() {} virtual ~Observer() {}
// Handles identity list changed events. // Handles identity list changed events.
// Deprecated, see OnIdentityListChanged(bool).
virtual void OnIdentityListChanged() {} virtual void OnIdentityListChanged() {}
// Handles identity list changed events.
// |keychainReload| is true if the identity list is updated by reloading the
// keychain. This means that a first party Google app had added or removed
// identities.
virtual void OnIdentityListChanged(bool keychainReload);
// Handles access token refresh failed events. // Handles access token refresh failed events.
// |identity| is the the identity for which the access token refresh failed. // |identity| is the the identity for which the access token refresh failed.
// |user_info| is the user info dictionary in the original notification. It // |user_info| is the user info dictionary in the original notification. It
...@@ -263,8 +270,15 @@ class ChromeIdentityService { ...@@ -263,8 +270,15 @@ class ChromeIdentityService {
protected: protected:
// Fires |OnIdentityListChanged| on all observers. // Fires |OnIdentityListChanged| on all observers.
// Deprecated, see FireIdentityListChanged(bool).
void FireIdentityListChanged(); void FireIdentityListChanged();
// Fires |OnIdentityListChanged| on all observers.
// |keychainReload| is true if the identity list is updated by reloading the
// keychain. This means that a first party Google app had added or removed
// identities.
void FireIdentityListChanged(bool keychainReload);
// Fires |OnAccessTokenRefreshFailed| on all observers, with the corresponding // Fires |OnAccessTokenRefreshFailed| on all observers, with the corresponding
// identity and user info. // identity and user info.
void FireAccessTokenRefreshFailed(ChromeIdentity* identity, void FireAccessTokenRefreshFailed(ChromeIdentity* identity,
......
...@@ -15,6 +15,13 @@ ...@@ -15,6 +15,13 @@
namespace ios { namespace ios {
void ChromeIdentityService::Observer::OnIdentityListChanged(
bool keychainReload) {
// Call to OnIdentityListChanged() is temporary, until all classes in
// ios_internal have migrated to this method.
OnIdentityListChanged();
}
ChromeIdentityService::ChromeIdentityService() {} ChromeIdentityService::ChromeIdentityService() {}
ChromeIdentityService::~ChromeIdentityService() { ChromeIdentityService::~ChromeIdentityService() {
...@@ -157,8 +164,12 @@ bool ChromeIdentityService::IsInvalidGrantError(NSDictionary* user_info) { ...@@ -157,8 +164,12 @@ bool ChromeIdentityService::IsInvalidGrantError(NSDictionary* user_info) {
} }
void ChromeIdentityService::FireIdentityListChanged() { void ChromeIdentityService::FireIdentityListChanged() {
FireIdentityListChanged(true);
}
void ChromeIdentityService::FireIdentityListChanged(bool keychainReload) {
for (auto& observer : observer_list_) for (auto& observer : observer_list_)
observer.OnIdentityListChanged(); observer.OnIdentityListChanged(keychainReload);
} }
void ChromeIdentityService::FireAccessTokenRefreshFailed( void ChromeIdentityService::FireAccessTokenRefreshFailed(
......
...@@ -208,7 +208,7 @@ void FakeChromeIdentityService::ForgetIdentity( ...@@ -208,7 +208,7 @@ void FakeChromeIdentityService::ForgetIdentity(
ChromeIdentity* identity, ChromeIdentity* identity,
ForgetIdentityCallback callback) { ForgetIdentityCallback callback) {
[identities_ removeObject:identity]; [identities_ removeObject:identity];
FireIdentityListChanged(); FireIdentityListChanged(false);
if (callback) { if (callback) {
// Forgetting an identity is normally an asynchronous operation (that // Forgetting an identity is normally an asynchronous operation (that
// require some network calls), this is replicated here by dispatching // require some network calls), this is replicated here by dispatching
...@@ -326,7 +326,7 @@ void FakeChromeIdentityService::AddIdentity(ChromeIdentity* identity) { ...@@ -326,7 +326,7 @@ void FakeChromeIdentityService::AddIdentity(ChromeIdentity* identity) {
if (![identities_ containsObject:identity]) { if (![identities_ containsObject:identity]) {
[identities_ addObject:identity]; [identities_ addObject:identity];
} }
FireIdentityListChanged(); FireIdentityListChanged(false);
} }
void FakeChromeIdentityService::SetFakeMDMError(bool fakeMDMError) { void FakeChromeIdentityService::SetFakeMDMError(bool fakeMDMError) {
......
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