Commit 0bb979be authored by Nohemi Fernandez's avatar Nohemi Fernandez Committed by Commit Bot

[iOS] Allow sign-in notifications following a cookie restoration event.

This patch adds the infrastructure necessary to display a sign-in
notification following a Gaia cookie restoration event.

Bug: 1145592
Change-Id: Ibbc704a3043ffc8c896e604fd319281100e3149a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2519457
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarJérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824092}
parent 98f8b496
...@@ -72,11 +72,11 @@ class AccountConsistencyService : public KeyedService, ...@@ -72,11 +72,11 @@ class AccountConsistencyService : public KeyedService,
// Checks for the presence of Gaia cookies and if they have been deleted // Checks for the presence of Gaia cookies and if they have been deleted
// notifies the AccountReconcilor (the class responsible for rebuilding Gaia // notifies the AccountReconcilor (the class responsible for rebuilding Gaia
// cookies if needed). // cookies if needed). Calls callback if Gaia cookies were restored.
// //
// Applies a one hour time restriction in between updates to avoid too many // Applies a one hour time restriction in between updates to avoid too many
// |GetAllCookies| calls on the cookie manager. // |GetAllCookies| calls on the cookie manager.
void SetGaiaCookiesIfDeleted(); void SetGaiaCookiesIfDeleted(base::OnceClosure cookies_restored_callback);
// Enqueues a request to set the CHROME_CONNECTED cookie for the domain of the // Enqueues a request to set the CHROME_CONNECTED cookie for the domain of the
// |url|. The cookie is set if it is not already on the domain. // |url|. The cookie is set if it is not already on the domain.
...@@ -130,8 +130,10 @@ class AccountConsistencyService : public KeyedService, ...@@ -130,8 +130,10 @@ class AccountConsistencyService : public KeyedService,
// Adds CHROME_CONNECTED cookies on all the main Google domains. // Adds CHROME_CONNECTED cookies on all the main Google domains.
void AddChromeConnectedCookies(); void AddChromeConnectedCookies();
// Triggers a Gaia cookie update on the Google domain. // Triggers a Gaia cookie update on the Google domain. Calls
// |cookies_restored_callback| if the Gaia cookies were restored.
void TriggerGaiaCookieChangeIfDeleted( void TriggerGaiaCookieChangeIfDeleted(
base::OnceClosure cookies_restored_callback,
const net::CookieAccessResultList& cookie_list, const net::CookieAccessResultList& cookie_list,
const net::CookieAccessResultList& excluded_cookies); const net::CookieAccessResultList& excluded_cookies);
......
...@@ -159,7 +159,11 @@ void AccountConsistencyHandler::ShouldAllowResponse( ...@@ -159,7 +159,11 @@ void AccountConsistencyHandler::ShouldAllowResponse(
if (google_util::IsGoogleDomainUrl( if (google_util::IsGoogleDomainUrl(
url, google_util::ALLOW_SUBDOMAIN, url, google_util::ALLOW_SUBDOMAIN,
google_util::DISALLOW_NON_STANDARD_PORTS)) { google_util::DISALLOW_NON_STANDARD_PORTS)) {
account_consistency_service_->SetGaiaCookiesIfDeleted(); account_consistency_service_->SetGaiaCookiesIfDeleted(base::BindOnce(
[](id<ManageAccountsDelegate> delegate) {
[delegate onRestoreGaiaCookies];
},
delegate_));
} }
if (!gaia::IsGaiaSignonRealm(url.GetOrigin())) { if (!gaia::IsGaiaSignonRealm(url.GetOrigin())) {
...@@ -301,7 +305,8 @@ void AccountConsistencyService::RemoveWebStateHandler( ...@@ -301,7 +305,8 @@ void AccountConsistencyService::RemoveWebStateHandler(
web_state_handlers_.erase(web_state); web_state_handlers_.erase(web_state);
} }
void AccountConsistencyService::SetGaiaCookiesIfDeleted() { void AccountConsistencyService::SetGaiaCookiesIfDeleted(
base::OnceClosure cookies_restored_callback) {
// We currently enforce a time threshold to update the Gaia cookie // We currently enforce a time threshold to update the Gaia cookie
// for signed-in users to prevent calling the expensive method // for signed-in users to prevent calling the expensive method
// |GetAllCookies| in the cookie manager. // |GetAllCookies| in the cookie manager.
...@@ -317,11 +322,12 @@ void AccountConsistencyService::SetGaiaCookiesIfDeleted() { ...@@ -317,11 +322,12 @@ void AccountConsistencyService::SetGaiaCookiesIfDeleted() {
net::CookieOptions::MakeAllInclusive(), net::CookieOptions::MakeAllInclusive(),
base::BindOnce( base::BindOnce(
&AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted, &AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted,
base::Unretained(this))); base::Unretained(this), std::move(cookies_restored_callback)));
last_gaia_cookie_verification_time_ = base::Time::Now(); last_gaia_cookie_verification_time_ = base::Time::Now();
} }
void AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted( void AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted(
base::OnceClosure cookies_restored_callback,
const net::CookieAccessResultList& cookie_list, const net::CookieAccessResultList& cookie_list,
const net::CookieAccessResultList& unused_excluded_cookies) { const net::CookieAccessResultList& unused_excluded_cookies) {
for (const auto& cookie : cookie_list) { for (const auto& cookie : cookie_list) {
...@@ -338,8 +344,12 @@ void AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted( ...@@ -338,8 +344,12 @@ void AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted(
if (!base::FeatureList::IsEnabled(signin::kRestoreGaiaCookiesIfDeleted)) { if (!base::FeatureList::IsEnabled(signin::kRestoreGaiaCookiesIfDeleted)) {
return; return;
} }
// Re-generate cookie to ensure that the user is properly signed in. // Re-generate cookie to ensure that the user is properly signed in.
identity_manager_->GetAccountsCookieMutator()->ForceTriggerOnCookieChange(); identity_manager_->GetAccountsCookieMutator()->ForceTriggerOnCookieChange();
if (!cookies_restored_callback.is_null()) {
std::move(cookies_restored_callback).Run();
}
} }
void AccountConsistencyService::LogIOSGaiaCookiesPresentOnNavigation( void AccountConsistencyService::LogIOSGaiaCookiesPresentOnNavigation(
......
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/signin/core/browser/account_reconcilor.h" #include "components/signin/core/browser/account_reconcilor.h"
#include "components/signin/core/browser/account_reconcilor_delegate.h" #include "components/signin/core/browser/account_reconcilor_delegate.h"
#include "components/signin/ios/browser/features.h"
#include "components/signin/public/base/list_accounts_test_utils.h" #include "components/signin/public/base/list_accounts_test_utils.h"
#include "components/signin/public/base/test_signin_client.h" #include "components/signin/public/base/test_signin_client.h"
#include "components/signin/public/identity_manager/identity_manager.h" #include "components/signin/public/identity_manager/identity_manager.h"
...@@ -302,8 +303,8 @@ class AccountConsistencyServiceTest : public PlatformTest { ...@@ -302,8 +303,8 @@ class AccountConsistencyServiceTest : public PlatformTest {
// Simulates updating the Gaia cookie on the Google domain at the designated // Simulates updating the Gaia cookie on the Google domain at the designated
// time interval. Returns the time at which the cookie was updated. // time interval. Returns the time at which the cookie was updated.
void SimulateUpdateGaiaCookie() { void SimulateUpdateGaiaCookie(base::OnceClosure callback) {
account_consistency_service_->SetGaiaCookiesIfDeleted(); account_consistency_service_->SetGaiaCookiesIfDeleted(std::move(callback));
} }
void CheckGoogleDomainHasGaiaCookie() { void CheckGoogleDomainHasGaiaCookie() {
...@@ -687,24 +688,24 @@ TEST_F(AccountConsistencyServiceTest, SetChromeConnectedCookieAtUpdateTime) { ...@@ -687,24 +688,24 @@ TEST_F(AccountConsistencyServiceTest, SetChromeConnectedCookieAtUpdateTime) {
TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateNotUpdateTime) { TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateNotUpdateTime) {
SignIn(); SignIn();
SimulateUpdateGaiaCookie(); SimulateUpdateGaiaCookie(base::OnceClosure());
// Advance clock, but stay within the one-hour Gaia update time. // Advance clock, but stay within the one-hour Gaia update time.
const base::Time first_update_time = base::Time::Now(); const base::Time first_update_time = base::Time::Now();
task_environment_.FastForwardBy(base::TimeDelta::FromMinutes(1)); task_environment_.FastForwardBy(base::TimeDelta::FromMinutes(1));
SimulateUpdateGaiaCookie(); SimulateUpdateGaiaCookie(base::OnceClosure());
EXPECT_EQ(first_update_time, GetGaiaLastUpdateTime()); EXPECT_EQ(first_update_time, GetGaiaLastUpdateTime());
} }
TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateAtUpdateTime) { TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateAtUpdateTime) {
SignIn(); SignIn();
SimulateUpdateGaiaCookie(); SimulateUpdateGaiaCookie(base::OnceClosure());
// Advance clock past one-hour Gaia update time. // Advance clock past one-hour Gaia update time.
task_environment_.FastForwardBy(base::TimeDelta::FromHours(2)); task_environment_.FastForwardBy(base::TimeDelta::FromHours(2));
const base::Time second_update_time = base::Time::Now(); const base::Time second_update_time = base::Time::Now();
SimulateUpdateGaiaCookie(); SimulateUpdateGaiaCookie(base::OnceClosure());
EXPECT_EQ(second_update_time, GetGaiaLastUpdateTime()); EXPECT_EQ(second_update_time, GetGaiaLastUpdateTime());
} }
...@@ -713,15 +714,50 @@ TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateAtUpdateTime) { ...@@ -713,15 +714,50 @@ TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateAtUpdateTime) {
// |kRestoreGAIACookiesIfDeleted| experiment is disabled. // |kRestoreGAIACookiesIfDeleted| experiment is disabled.
TEST_F(AccountConsistencyServiceTest, GAIACookieStatusLoggedProperly) { TEST_F(AccountConsistencyServiceTest, GAIACookieStatusLoggedProperly) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
__block bool cookie_updated = false;
base::OnceClosure callback = base::BindOnce(^() {
cookie_updated = true;
});
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0); histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
SimulateUpdateGaiaCookie();
SimulateUpdateGaiaCookie(std::move(callback));
base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
ASSERT_FALSE(cookie_updated);
SignIn();
SimulateUpdateGaiaCookie(std::move(callback));
base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 1);
ASSERT_FALSE(cookie_updated);
}
// Ensures that in the case Gaia cookies are restored the restoration callback
// is completed.
TEST_F(AccountConsistencyServiceTest, GAIACookieRestoreCallbackFinished) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(
signin::kRestoreGaiaCookiesIfDeleted);
base::HistogramTester histogram_tester;
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
__block bool cookie_updated = false;
SimulateUpdateGaiaCookie(base::BindOnce(^{
cookie_updated = true;
}));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0); histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
ASSERT_FALSE(cookie_updated);
SignIn(); SignIn();
SimulateUpdateGaiaCookie(); SimulateUpdateGaiaCookie(base::BindOnce(^{
cookie_updated = true;
}));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 1); histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 1);
ASSERT_TRUE(cookie_updated);
} }
// Ensures that set and remove cookie operations are handled in the order // Ensures that set and remove cookie operations are handled in the order
......
...@@ -9,6 +9,10 @@ class GURL; ...@@ -9,6 +9,10 @@ class GURL;
@protocol ManageAccountsDelegate<NSObject> @protocol ManageAccountsDelegate<NSObject>
// Called when Gaia cookies have been regenerated for a specific user sign-in.
// This occurs when a SAPISID cookie has been deleted by the operating system.
- (void)onRestoreGaiaCookies;
// Called when the user taps on a manage accounts button in a Google web // Called when the user taps on a manage accounts button in a Google web
// property. // property.
- (void)onManageAccounts; - (void)onManageAccounts;
......
...@@ -517,6 +517,10 @@ const base::Feature kPreloadDelayWebStateReset{ ...@@ -517,6 +517,10 @@ const base::Feature kPreloadDelayWebStateReset{
#pragma mark - ManageAccountsDelegate #pragma mark - ManageAccountsDelegate
- (void)onRestoreGaiaCookies {
[self schedulePrerenderCancel];
}
- (void)onManageAccounts { - (void)onManageAccounts {
[self schedulePrerenderCancel]; [self schedulePrerenderCancel];
} }
......
...@@ -4926,6 +4926,13 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -4926,6 +4926,13 @@ NSString* const kBrowserViewControllerSnackbarCategory =
#pragma mark - ManageAccountsDelegate #pragma mark - ManageAccountsDelegate
- (void)onRestoreGaiaCookies {
signin_metrics::LogAccountReconcilorStateOnGaiaResponse(
ios::AccountReconcilorFactory::GetForBrowserState(self.browserState)
->GetState());
[self.dispatcher showSigninAccountNotificationFromViewController:self];
}
- (void)onManageAccounts { - (void)onManageAccounts {
signin_metrics::LogAccountReconcilorStateOnGaiaResponse( signin_metrics::LogAccountReconcilorStateOnGaiaResponse(
ios::AccountReconcilorFactory::GetForBrowserState(self.browserState) ios::AccountReconcilorFactory::GetForBrowserState(self.browserState)
......
...@@ -149,6 +149,10 @@ enum class KeyRetrievalTriggerForUMA; ...@@ -149,6 +149,10 @@ enum class KeyRetrievalTriggerForUMA;
- (void)showConsistencyPromoFromViewController: - (void)showConsistencyPromoFromViewController:
(UIViewController*)baseViewController; (UIViewController*)baseViewController;
// Shows a notification with the signed-in user account.
- (void)showSigninAccountNotificationFromViewController:
(UIViewController*)baseViewController;
// Sets whether the UI is displaying incognito content. // Sets whether the UI is displaying incognito content.
- (void)setIncognitoContentVisible:(BOOL)incognitoContentVisible; - (void)setIncognitoContentVisible:(BOOL)incognitoContentVisible;
......
...@@ -1178,6 +1178,11 @@ const char kMultiWindowOpenInNewWindowHistogram[] = ...@@ -1178,6 +1178,11 @@ const char kMultiWindowOpenInNewWindowHistogram[] =
[self startSigninCoordinatorWithCompletion:nil]; [self startSigninCoordinatorWithCompletion:nil];
} }
- (void)showSigninAccountNotificationFromViewController:
(UIViewController*)baseViewController {
// TODO(crbug.com/1145592): Add toast for sign-in reminder.
}
- (void)setIncognitoContentVisible:(BOOL)incognitoContentVisible { - (void)setIncognitoContentVisible:(BOOL)incognitoContentVisible {
self.sceneState.incognitoContentVisible = incognitoContentVisible; self.sceneState.incognitoContentVisible = incognitoContentVisible;
} }
......
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