Commit 70452749 authored by Nohemi Fernandez's avatar Nohemi Fernandez Committed by Commit Bot

[iOS] Add experiment feature flag kRestoreGaiaCookiesIfDeleted.

Bug: 1131027
Change-Id: I02a1824f34d25c29d4a67f66b362514d63963236
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424163
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: default avatarRohit Rao <rohitrao@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809746}
parent 77c87112
...@@ -3907,6 +3907,11 @@ ...@@ -3907,6 +3907,11 @@
"owners": [ "bokan" ], "owners": [ "bokan" ],
"expiry_milestone": 80 "expiry_milestone": 80
}, },
{
"name": "restore-gaia-cookies-if-deleted",
"owners": [ "fernandex", "chrome-signin-team" ],
"expiry_milestone": 96
},
{ {
"name": "restrict-gamepad-access", "name": "restrict-gamepad-access",
"owners": [ "//device/gamepad/OWNERS", "jameshollyer@chromium.org" ], "owners": [ "//device/gamepad/OWNERS", "jameshollyer@chromium.org" ],
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include <string> #include <string>
#include "base/callback.h" #include "base/callback.h"
#include "base/feature_list.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/time/time.h" #include "base/time/time.h"
...@@ -33,9 +32,6 @@ class WebStatePolicyDecider; ...@@ -33,9 +32,6 @@ class WebStatePolicyDecider;
class AccountReconcilor; class AccountReconcilor;
class PrefService; class PrefService;
// Feature flag controlling whether to restore GAIA cookies if they are deleted.
extern const base::Feature kRestoreGAIACookiesIfDeleted;
// Handles actions necessary for keeping the list of Google accounts available // Handles actions necessary for keeping the list of Google accounts available
// on the web and those available on the iOS device from first-party Google apps // on the web and those available on the iOS device from first-party Google apps
// consistent. This includes setting the Account Consistency cookie, // consistent. This includes setting the Account Consistency cookie,
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "components/prefs/scoped_user_pref_update.h" #include "components/prefs/scoped_user_pref_update.h"
#include "components/signin/core/browser/account_reconcilor.h" #include "components/signin/core/browser/account_reconcilor.h"
#include "components/signin/core/browser/signin_header_helper.h" #include "components/signin/core/browser/signin_header_helper.h"
#include "components/signin/ios/browser/features.h"
#include "components/signin/public/base/account_consistency_method.h" #include "components/signin/public/base/account_consistency_method.h"
#include "components/signin/public/identity_manager/accounts_cookie_mutator.h" #include "components/signin/public/identity_manager/accounts_cookie_mutator.h"
#include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/gaia_urls.h"
...@@ -78,9 +79,6 @@ class AccountConsistencyHandler : public web::WebStatePolicyDecider { ...@@ -78,9 +79,6 @@ class AccountConsistencyHandler : public web::WebStatePolicyDecider {
}; };
} // namespace } // namespace
const base::Feature kRestoreGAIACookiesIfDeleted{
"RestoreGAIACookiesIfDeleted", base::FEATURE_DISABLED_BY_DEFAULT};
AccountConsistencyHandler::AccountConsistencyHandler( AccountConsistencyHandler::AccountConsistencyHandler(
web::WebState* web_state, web::WebState* web_state,
AccountConsistencyService* service, AccountConsistencyService* service,
...@@ -222,10 +220,11 @@ void AccountConsistencyService::RemoveWebStateHandler( ...@@ -222,10 +220,11 @@ void AccountConsistencyService::RemoveWebStateHandler(
void AccountConsistencyService::SetGaiaCookiesIfDeleted() { void AccountConsistencyService::SetGaiaCookiesIfDeleted() {
// We currently enforce a time threshold to update the Gaia cookie // We currently enforce a time threshold to update the Gaia cookie
// to prevent calling the expensive call to the cookie manager's // for signed-in users to prevent calling the expensive method
// |GetAllCookies|. // |GetAllCookies| in the cookie manager.
if (base::Time::Now() - last_gaia_cookie_verification_time_ < if (base::Time::Now() - last_gaia_cookie_verification_time_ <
kDelayThresholdToUpdateGaiaCookie) { kDelayThresholdToUpdateGaiaCookie ||
!identity_manager_->HasPrimaryAccount()) {
return; return;
} }
network::mojom::CookieManager* cookie_manager = network::mojom::CookieManager* cookie_manager =
...@@ -253,7 +252,7 @@ void AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted( ...@@ -253,7 +252,7 @@ void AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted(
// ITP restrictions marking Google domains as potential trackers. // ITP restrictions marking Google domains as potential trackers.
LogIOSGaiaCookiesPresentOnNavigation(false); LogIOSGaiaCookiesPresentOnNavigation(false);
if (!base::FeatureList::IsEnabled(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.
......
...@@ -577,6 +577,7 @@ TEST_F(AccountConsistencyServiceTest, SetChromeConnectedCookieAtUpdateTime) { ...@@ -577,6 +577,7 @@ TEST_F(AccountConsistencyServiceTest, SetChromeConnectedCookieAtUpdateTime) {
} }
TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateNotUpdateTime) { TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateNotUpdateTime) {
SignIn();
SimulateUpdateGaiaCookie(); SimulateUpdateGaiaCookie();
// Advance clock, but stay within the one-hour Gaia update time. // Advance clock, but stay within the one-hour Gaia update time.
...@@ -588,6 +589,7 @@ TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateNotUpdateTime) { ...@@ -588,6 +589,7 @@ TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateNotUpdateTime) {
} }
TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateAtUpdateTime) { TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateAtUpdateTime) {
SignIn();
SimulateUpdateGaiaCookie(); SimulateUpdateGaiaCookie();
// Advance clock past one-hour Gaia update time. // Advance clock past one-hour Gaia update time.
...@@ -602,12 +604,14 @@ TEST_F(AccountConsistencyServiceTest, SetGaiaCookieUpdateAtUpdateTime) { ...@@ -602,12 +604,14 @@ 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;
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(kRestoreGAIACookiesIfDeleted);
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0); histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
SimulateUpdateGaiaCookie(); SimulateUpdateGaiaCookie();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
SignIn();
SimulateUpdateGaiaCookie();
base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 1); histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 1);
} }
......
...@@ -13,4 +13,7 @@ bool ForceStartupSigninPromo() { ...@@ -13,4 +13,7 @@ bool ForceStartupSigninPromo() {
return base::FeatureList::IsEnabled(kForceStartupSigninPromo); return base::FeatureList::IsEnabled(kForceStartupSigninPromo);
} }
const base::Feature kRestoreGaiaCookiesIfDeleted{
"RestoreGAIACookiesIfDeleted", base::FEATURE_DISABLED_BY_DEFAULT};
} // namespace signin } // namespace signin
...@@ -15,6 +15,9 @@ extern const base::Feature kForceStartupSigninPromo; ...@@ -15,6 +15,9 @@ extern const base::Feature kForceStartupSigninPromo;
// Returns true if the startup sign-in promo should be displayed at boot. // Returns true if the startup sign-in promo should be displayed at boot.
bool ForceStartupSigninPromo(); bool ForceStartupSigninPromo();
// Feature controlling whether to restore GAIA cookies if they are deleted.
extern const base::Feature kRestoreGaiaCookiesIfDeleted;
} // namespace signin } // namespace signin
#endif // COMPONENTS_SIGNIN_IOS_BROWSER_FEATURES_H_ #endif // COMPONENTS_SIGNIN_IOS_BROWSER_FEATURES_H_
...@@ -694,6 +694,11 @@ const flags_ui::FeatureEntry kFeatureEntries[] = { ...@@ -694,6 +694,11 @@ const flags_ui::FeatureEntry kFeatureEntries[] = {
flag_descriptions::kAutofillUseRendererIDsDescription, flags_ui::kOsIos, flag_descriptions::kAutofillUseRendererIDsDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE( FEATURE_VALUE_TYPE(
autofill::features::kAutofillUseUniqueRendererIDsOnIOS)}, autofill::features::kAutofillUseUniqueRendererIDsOnIOS)},
{"restore-gaia-cookies-if-deleted",
flag_descriptions::kRestoreGaiaCookiesIfDeletedName,
flag_descriptions::kRestoreGaiaCookiesIfDeletedDescription,
flags_ui::kOsIos,
FEATURE_VALUE_TYPE(signin::kRestoreGaiaCookiesIfDeleted)},
}; };
bool SkipConditionalFeatureEntry(const flags_ui::FeatureEntry& entry) { bool SkipConditionalFeatureEntry(const flags_ui::FeatureEntry& entry) {
......
...@@ -424,6 +424,12 @@ const char kReloadSadTabDescription[] = ...@@ -424,6 +424,12 @@ const char kReloadSadTabDescription[] =
"When enabled, the first time the renderer crashes, the page is reloaded " "When enabled, the first time the renderer crashes, the page is reloaded "
"instead of showing the SadTab"; "instead of showing the SadTab";
const char kRestoreGaiaCookiesIfDeletedName[] =
"Restore GAIA cookies if deleted";
const char kRestoreGaiaCookiesIfDeletedDescription[] =
"When enabled, will restore GAIA cookies for signed-in Chrome users if "
"they are deleted.";
const char kSafeBrowsingAvailableName[] = "Make Safe Browsing available"; const char kSafeBrowsingAvailableName[] = "Make Safe Browsing available";
const char kSafeBrowsingAvailableDescription[] = const char kSafeBrowsingAvailableDescription[] =
"When enabled, navigation URLs are compared to Safe Browsing blocklists, " "When enabled, navigation URLs are compared to Safe Browsing blocklists, "
......
...@@ -473,6 +473,9 @@ extern const char kWebPageTextAccessibilityDescription[]; ...@@ -473,6 +473,9 @@ extern const char kWebPageTextAccessibilityDescription[];
extern const char kWellKnownChangePasswordName[]; extern const char kWellKnownChangePasswordName[];
extern const char kWellKnownChangePasswordDescription[]; extern const char kWellKnownChangePasswordDescription[];
extern const char kRestoreGaiaCookiesIfDeletedName[];
extern const char kRestoreGaiaCookiesIfDeletedDescription[];
// Please add names and descriptions above in alphabetical order. // Please add names and descriptions above in alphabetical order.
} // namespace flag_descriptions } // namespace flag_descriptions
......
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