Commit 4b025813 authored by Nohemi Fernandez's avatar Nohemi Fernandez Committed by Chromium LUCI CQ

[iOS] Track GAIA ADD SESSION requests when user is signed in to Chrome.

This patch adds a metric to track when the user attempts to add an
account to the web when they are already signed in to Chrome. This
situation is possible when all GAIA cookies are removed from the web
during a user session.

Bug: 1154244
Change-Id: I3818d6ad6fcae014d381ccaf67ab9e0a46025ff5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566755
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: default avatarBrian White <bcwhite@chromium.org>
Reviewed-by: default avatarJérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833299}
parent d63b3325
......@@ -137,9 +137,6 @@ class AccountConsistencyService : public KeyedService,
const net::CookieAccessResultList& cookie_list,
const net::CookieAccessResultList& excluded_cookies);
// Records whether Gaia cookies were present on navigation in UMA histogram.
static void LogIOSGaiaCookiesPresentOnNavigation(bool is_present);
// Clears all pending cookie requests and cached domains.
void ResetInternalState();
......
......@@ -67,6 +67,22 @@ static std::string GetDomainFromUrl(const GURL& url) {
url, net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES);
}
// The Gaia cookie state on navigation for a signed-in Chrome user.
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class GaiaCookieStateOnSignedInNavigation {
kGaiaCookiePresentOnNavigation = 0,
kGaiaCookieAbsentOnGoogleAssociatedDomainNavigation = 1,
kGaiaCookieAbsentOnAddSessionNavigation = 2,
kMaxValue = kGaiaCookieAbsentOnAddSessionNavigation
};
// Records the state of Gaia cookies for a navigation in UMA histogram.
void LogIOSGaiaCookiesState(GaiaCookieStateOnSignedInNavigation state) {
base::UmaHistogramEnumeration("Signin.IOSGaiaCookieStateOnSignedInNavigation",
state);
}
// Allows for manual testing by reducing the polling interval for verifying the
// existence of the GAIA cookie.
base::TimeDelta GetDelayThresholdToUpdateGaiaCookie() {
......@@ -94,6 +110,7 @@ class AccountConsistencyHandler : public web::WebStatePolicyDecider,
AccountConsistencyHandler(web::WebState* web_state,
AccountConsistencyService* service,
AccountReconcilor* account_reconcilor,
signin::IdentityManager* identity_manager,
id<ManageAccountsDelegate> delegate);
void WebStateDestroyed(web::WebState* web_state) override;
......@@ -120,6 +137,7 @@ class AccountConsistencyHandler : public web::WebStatePolicyDecider,
bool gaia_cookies_restored_ = false;
AccountConsistencyService* account_consistency_service_; // Weak.
AccountReconcilor* account_reconcilor_; // Weak.
signin::IdentityManager* identity_manager_;
__weak id<ManageAccountsDelegate> delegate_;
base::WeakPtrFactory<AccountConsistencyHandler> weak_ptr_factory_;
};
......@@ -129,10 +147,12 @@ AccountConsistencyHandler::AccountConsistencyHandler(
web::WebState* web_state,
AccountConsistencyService* service,
AccountReconcilor* account_reconcilor,
signin::IdentityManager* identity_manager,
id<ManageAccountsDelegate> delegate)
: web::WebStatePolicyDecider(web_state),
account_consistency_service_(service),
account_reconcilor_(account_reconcilor),
identity_manager_(identity_manager),
delegate_(delegate),
weak_ptr_factory_(this) {
web_state->AddObserver(this);
......@@ -210,6 +230,12 @@ void AccountConsistencyHandler::ShouldAllowResponse(
}
case signin::GAIA_SERVICE_TYPE_SIGNUP:
case signin::GAIA_SERVICE_TYPE_ADDSESSION:
// This situation is only possible if the all cookies have been deleted by
// ITP restrictions and Chrome has not triggered a cookie refresh.
if (identity_manager_->HasPrimaryAccount()) {
LogIOSGaiaCookiesState(GaiaCookieStateOnSignedInNavigation::
kGaiaCookieAbsentOnAddSessionNavigation);
}
if (params.show_consistency_promo) {
show_consistency_promo_ = true;
// Allows the URL response to load before showing the consistency promo.
......@@ -325,7 +351,7 @@ void AccountConsistencyService::SetWebStateHandler(
id<ManageAccountsDelegate> delegate) {
DCHECK_EQ(0u, web_state_handlers_.count(web_state));
web_state_handlers_[web_state].reset(new AccountConsistencyHandler(
web_state, this, account_reconcilor_, delegate));
web_state, this, account_reconcilor_, identity_manager_, delegate));
}
void AccountConsistencyService::RemoveWebStateHandler(
......@@ -363,14 +389,17 @@ void AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted(
const net::CookieAccessResultList& unused_excluded_cookies) {
for (const auto& cookie : cookie_list) {
if (cookie.cookie.Name() == kGaiaCookieName) {
LogIOSGaiaCookiesPresentOnNavigation(true);
LogIOSGaiaCookiesState(
GaiaCookieStateOnSignedInNavigation::kGaiaCookiePresentOnNavigation);
return;
}
}
// The SAPISID cookie may have been deleted previous to this update due to
// ITP restrictions marking Google domains as potential trackers.
LogIOSGaiaCookiesPresentOnNavigation(false);
LogIOSGaiaCookiesState(
GaiaCookieStateOnSignedInNavigation::
kGaiaCookieAbsentOnGoogleAssociatedDomainNavigation);
if (!base::FeatureList::IsEnabled(signin::kRestoreGaiaCookiesIfDeleted)) {
return;
......@@ -383,12 +412,6 @@ void AccountConsistencyService::TriggerGaiaCookieChangeIfDeleted(
}
}
void AccountConsistencyService::LogIOSGaiaCookiesPresentOnNavigation(
bool is_present) {
base::UmaHistogramBoolean("Signin.IOSGaiaCookiePresentOnNavigation",
is_present);
}
void AccountConsistencyService::RemoveAllChromeConnectedCookies(
base::OnceClosure callback) {
DCHECK(!browser_state_->IsOffTheRecord());
......
......@@ -55,9 +55,10 @@ const char* kYoutubeDomain = "youtube.com";
// Google domain where the CHROME_CONNECTED cookie is set/removed.
const char* kCountryGoogleDomain = "google.de";
// Name of the histogram to record whether the GAIA cookie is present.
const char* kGAIACookiePresentHistogram =
"Signin.IOSGaiaCookiePresentOnNavigation";
// Name of the histogram to record the state of the GAIA cookie for the
// navigation.
const char* kGAIACookieOnNavigationHistogram =
"Signin.IOSGaiaCookieStateOnSignedInNavigation";
// Returns a cookie domain that applies for all origins on |host_domain|.
std::string GetCookieDomain(const std::string& host_domain) {
......@@ -735,17 +736,17 @@ TEST_F(AccountConsistencyServiceTest, GAIACookieStatusLoggedProperly) {
cookie_updated = true;
});
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
histogram_tester.ExpectTotalCount(kGAIACookieOnNavigationHistogram, 0);
SimulateUpdateGaiaCookie(std::move(callback));
base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
histogram_tester.ExpectTotalCount(kGAIACookieOnNavigationHistogram, 0);
ASSERT_FALSE(cookie_updated);
SignIn();
SimulateUpdateGaiaCookie(std::move(callback));
base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 1);
histogram_tester.ExpectTotalCount(kGAIACookieOnNavigationHistogram, 1);
ASSERT_FALSE(cookie_updated);
}
......@@ -757,14 +758,14 @@ TEST_F(AccountConsistencyServiceTest, GAIACookieRestoreCallbackFinished) {
signin::kRestoreGaiaCookiesIfDeleted);
base::HistogramTester histogram_tester;
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
histogram_tester.ExpectTotalCount(kGAIACookieOnNavigationHistogram, 0);
__block bool cookie_updated = false;
SimulateUpdateGaiaCookie(base::BindOnce(^{
cookie_updated = true;
}));
base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 0);
histogram_tester.ExpectTotalCount(kGAIACookieOnNavigationHistogram, 0);
ASSERT_FALSE(cookie_updated);
SignIn();
......@@ -772,10 +773,44 @@ TEST_F(AccountConsistencyServiceTest, GAIACookieRestoreCallbackFinished) {
cookie_updated = true;
}));
base::RunLoop().RunUntilIdle();
histogram_tester.ExpectTotalCount(kGAIACookiePresentHistogram, 1);
histogram_tester.ExpectTotalCount(kGAIACookieOnNavigationHistogram, 1);
ASSERT_TRUE(cookie_updated);
}
// Tests that navigating to accounts.google.com without a GAIA cookie is logged
// by the navigation histogram.
TEST_F(AccountConsistencyServiceTest, GAIACookieMissingOnSignin) {
SignIn();
id delegate =
[OCMockObject mockForProtocol:@protocol(ManageAccountsDelegate)];
[[delegate expect] onAddAccount];
NSDictionary* headers =
[NSDictionary dictionaryWithObject:@"action=ADDSESSION"
forKey:@"X-Chrome-Manage-Accounts"];
NSHTTPURLResponse* response = [[NSHTTPURLResponse alloc]
initWithURL:[NSURL URLWithString:@"https://accounts.google.com/"]
statusCode:200
HTTPVersion:@"HTTP/1.1"
headerFields:headers];
EXPECT_CALL(*account_reconcilor_, OnReceivedManageAccountsResponse(
signin::GAIA_SERVICE_TYPE_ADDSESSION))
.Times(2);
SimulateNavigateToURLWithInterruption(response, delegate);
base::HistogramTester histogram_tester;
histogram_tester.ExpectTotalCount(kGAIACookieOnNavigationHistogram, 0);
SimulateExternalSourceRemovesAllGoogleDomainCookies();
[[delegate expect] onAddAccount];
SimulateNavigateToURLWithInterruption(response, delegate);
histogram_tester.ExpectTotalCount(kGAIACookieOnNavigationHistogram, 1);
EXPECT_OCMOCK_VERIFY(delegate);
}
// Ensures that set and remove cookie operations are handled in the order
// they are called resulting in no cookies.
TEST_F(AccountConsistencyServiceTest, DeleteChromeConnectedCookiesAfterSet) {
......
......@@ -32526,6 +32526,12 @@ Called by update_feature_policy_enum.py.-->
</int>
</enum>
<enum name="GaiaCookieStateOnSignedInNavigation">
<int value="0" label="Gaia cookie is present"/>
<int value="1" label="Gaia cookie is missing from Google-associated domain"/>
<int value="2" label="Gaia cookie is missing from Add Session"/>
</enum>
<enum name="GaiaPasswordChangedScreenUserAction">
<summary>Actions which happen on the screen.</summary>
<int value="0" label="Resync user data"/>
......@@ -476,6 +476,21 @@ prefs when the profile is loaded. -->
</summary>
</histogram>
<histogram name="Signin.IOSGaiaCookieStateOnSignedInNavigation"
enum="GaiaCookieStateOnSignedInNavigation" expires_after="2021-08-05">
<owner>fernandex@chromium.org</owner>
<owner>msarda@chromium.org</owner>
<owner>chrome-signin-team@google.com</owner>
<summary>
Records whether the Gaia cookie is present when a user signed in to Chrome
navigates to a Google-owned domain that is eligible for Mirror account
consistency. This metrics tracks the user journeys that interact with
authentication cookies and that could be disrupted if an external force
(e.g. Apple's ITP) were to remove these cookies. This logging is limited to
once every hour due to performance constraints.
</summary>
</histogram>
<histogram name="Signin.IOSLoginMethodAndSyncState"
enum="SigninIOSLoginMethodAndSyncState" expires_after="2021-04-30">
<owner>jebel@chromium.org</owner>
......
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