[iOS] Set CHROME_CONNECTED cookie when the user shows sign-in intent.

Following crrev.com/c/2418576 the CHROME_CONNECTED cookie is added when
navigating to accounts.google.com.

Chrome previously only sent domain names to the
ChromeConnectedHeaderHelper (e.g. google.com) meaning that the URL would
never be accounts.google.com. This patch ensures that we send full URLs
instead of domain names to ChromeConnectedHeaderHelper so that we
can benefit from crrev.com/c/2418576.

Bug: 1125631
Change-Id: Ic020cb90cee8764d6d56c72f3143d058d5d5f4f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2426488
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811164}

components/signin/ios/browser/account_consistency_service.h

@@ -78,10 +78,9 @@ class AccountConsistencyService : public KeyedService,
   // |GetAllCookies| calls on the cookie manager.
   void SetGaiaCookiesIfDeleted();
 
-  // Enqueues a request to set the CHROME_CONNECTED cookie for |domain|.
-  // The cookie is set if it is not already on |domain|.
-  void SetChromeConnectedCookieWithDomains(
-      const std::vector<std::string>& domains);
+  // 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.
+  void SetChromeConnectedCookieWithUrls(const std::vector<const GURL>& urls);
 
   // Notifies the AccountConsistencyService that browsing data has been removed
   // for any time period.
@@ -96,8 +95,8 @@ class AccountConsistencyService : public KeyedService,
   // KeyedService implementation.
   void Shutdown() override;
 
-  // Sets the pending CHROME_CONNECTED cookie for the given |domain|.
-  void SetChromeConnectedCookieWithDomain(const std::string& domain);
+  // Sets the pending CHROME_CONNECTED cookie for the given |url|.
+  void SetChromeConnectedCookieWithUrl(const GURL& url);
 
   // Called when the request to set CHROME_CONNECTED cookie is done.
   void OnChromeConnectedCookieFinished(
@@ -117,11 +116,11 @@ class AccountConsistencyService : public KeyedService,
       const std::string& domain,
       const base::TimeDelta& cookie_refresh_interval);
 
-  // Enqueues a request to set the CHROME_CONNECTED cookie for |domain|.
-  // The cookie is set if it is not already on |domain| or if it is too old
-  // compared to the given |cookie_refresh_interval|.
-  void SetChromeConnectedCookieWithDomains(
-      const std::vector<std::string>& domains,
+  // Enqueues a request to set the CHROME_CONNECTED cookie for the |url|'s
+  // domain. The cookie is set if it is not already on domain or if it is too
+  // old compared to the given |cookie_refresh_interval|.
+  void SetChromeConnectedCookieWithUrls(
+      const std::vector<const GURL>& urls,
       const base::TimeDelta& cookie_refresh_interval);
 
   // Adds CHROME_CONNECTED cookies on all the main Google domains.

components/signin/ios/browser/account_consistency_service.mm

@@ -49,8 +49,15 @@ constexpr base::TimeDelta kDelayThresholdToUpdateChromeConnectedCookie =
 constexpr base::TimeDelta kDelayThresholdToUpdateGaiaCookie =
     base::TimeDelta::FromHours(1);
 
-const char* kGoogleDomain = "google.com";
-const char* kYoutubeDomain = "youtube.com";
+const char* kGoogleUrl = "https://google.com";
+const char* kYoutubeUrl = "https://youtube.com";
+
+// Returns the registered, organization-identifying host, but no subdomains,
+// from the given GURL. Returns an empty string if the GURL is invalid.
+static std::string GetDomainFromUrl(const GURL& url) {
+  return net::registry_controlled_domains::GetDomainAndRegistry(
+      url, net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES);
+}
 
 // WebStatePolicyDecider that monitors the HTTP headers on Gaia responses,
 // reacting on the X-Chrome-Manage-Accounts header and notifying its delegate.
@@ -101,13 +108,13 @@ void AccountConsistencyHandler::ShouldAllowResponse(
   }
 
   GURL url = net::GURLWithNSURL(http_response.URL);
-  // Logged-in user is showing intent to navigate to a Google domain where we
-  // will need to set a CHROME_CONNECTED cookie if it is not already set.
+  // User is showing intent to navigate to a Google-owned domain. Set GAIA and
+  // CHROME_CONNECTED cookies if the user is signed in or if they are not signed
+  // in and navigating to a GAIA sign-on (this is filtered in
+  // ChromeConnectedHelper).
   if (signin::IsUrlEligibleForMirrorCookie(url)) {
-    std::string domain = net::registry_controlled_domains::GetDomainAndRegistry(
-        url, net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES);
-    account_consistency_service_->SetChromeConnectedCookieWithDomains(
-        {domain, kGoogleDomain});
+    account_consistency_service_->SetChromeConnectedCookieWithUrls(
+        {url, GURL(kGoogleUrl)});
     account_consistency_service_->SetGaiaCookiesIfDeleted();
   }
 
@@ -298,22 +305,23 @@ void AccountConsistencyService::OnDeleteCookiesFinished(
   }
 }
 
-void AccountConsistencyService::SetChromeConnectedCookieWithDomains(
-    const std::vector<std::string>& domains) {
-  SetChromeConnectedCookieWithDomains(
-      domains, kDelayThresholdToUpdateChromeConnectedCookie);
+void AccountConsistencyService::SetChromeConnectedCookieWithUrls(
+    const std::vector<const GURL>& urls) {
+  SetChromeConnectedCookieWithUrls(
+      urls, kDelayThresholdToUpdateChromeConnectedCookie);
 }
 
-void AccountConsistencyService::SetChromeConnectedCookieWithDomains(
-    const std::vector<std::string>& domains,
+void AccountConsistencyService::SetChromeConnectedCookieWithUrls(
+    const std::vector<const GURL>& urls,
     const base::TimeDelta& cookie_refresh_interval) {
-  for (const std::string& domain : domains) {
+  for (const GURL& url : urls) {
+    const std::string domain = GetDomainFromUrl(url);
     if (!ShouldSetChromeConnectedCookieToDomain(domain,
                                                 cookie_refresh_interval)) {
       continue;
     }
     last_cookie_update_map_[domain] = base::Time::Now();
-    SetChromeConnectedCookieWithDomain(domain);
+    SetChromeConnectedCookieWithUrl(url);
   }
 }
 
@@ -339,9 +347,9 @@ void AccountConsistencyService::Shutdown() {
   web_state_handlers_.clear();
 }
 
-void AccountConsistencyService::SetChromeConnectedCookieWithDomain(
-    const std::string& domain) {
-  const GURL url("https://" + domain);
+void AccountConsistencyService::SetChromeConnectedCookieWithUrl(
+    const GURL& url) {
+  const std::string domain = GetDomainFromUrl(url);
   std::string cookie_value = signin::BuildMirrorRequestCookieIfPossible(
       url, identity_manager_->GetPrimaryAccountInfo().gaia,
       signin::AccountConsistencyMethod::kMirror, cookie_settings_.get(),
@@ -355,7 +363,7 @@ void AccountConsistencyService::SetChromeConnectedCookieWithDomain(
       net::CanonicalCookie::CreateSanitizedCookie(
           url,
           /*name=*/kChromeConnectedCookieName, cookie_value,
-          /*domain=*/url.host(),
+          /*domain=*/domain,
           /*path=*/std::string(),
           /*creation_time=*/base::Time::Now(),
           // Create expiration date of Now+2y to roughly follow the SAPISID
@@ -398,7 +406,7 @@ void AccountConsistencyService::AddChromeConnectedCookies() {
   DCHECK(!browser_state_->IsOffTheRecord());
   // These cookie requests are preventive and not a strong signal (unlike
   // navigation to a domain). Don't force update the old cookies in this case.
-  SetChromeConnectedCookieWithDomains({kGoogleDomain, kYoutubeDomain},
+  SetChromeConnectedCookieWithUrls({GURL(kGoogleUrl), GURL(kYoutubeUrl)},
                                    base::TimeDelta::Max());
 }
 

components/signin/ios/browser/account_consistency_service_unittest.mm

@@ -78,8 +78,7 @@ bool ContainsCookie(const std::vector<net::CanonicalCookie>& cookies,
                     const std::string& name,
                     const std::string& domain) {
   for (const auto& cookie : cookies) {
-    if (cookie.Name() ==
-        AccountConsistencyService::kChromeConnectedCookieName) {
+    if (cookie.Name() == name) {
       if (domain.empty() || cookie.Domain() == domain)
         return true;
     }
@@ -289,8 +288,8 @@ class AccountConsistencyServiceTest : public PlatformTest {
   // Simulates setting the CHROME_CONNECTED cookie for the Google domain at the
   // designated time interval. Returns the time at which the cookie was updated.
   void SimulateSetChromeConnectedCookieForGoogleDomain() {
-    account_consistency_service_->SetChromeConnectedCookieWithDomains(
-        {kGoogleDomain});
+    account_consistency_service_->SetChromeConnectedCookieWithUrls(
+        {GURL("https://google.com")});
     WaitUntilAllCookieRequestsAreApplied();
   }
 
@@ -624,8 +623,9 @@ TEST_F(AccountConsistencyServiceTest, DeleteChromeConnectedCookiesAfterSet) {
   // |kGoogleDomain| or |kYouTubeDomain| otherwise they will not be reset since
   // it is before the update time. Add multiple URLs to test for race conditions
   // with remove call.
-  account_consistency_service_->SetChromeConnectedCookieWithDomains(
-      {"google.ca", "google.fr", kCountryGoogleDomain});
+  account_consistency_service_->SetChromeConnectedCookieWithUrls(
+      {GURL("https://google.ca"), GURL("https://google.fr"),
+       GURL("https://google.de")});
   SimulateGaiaCookieManagerServiceLogout();
 
   WaitUntilAllCookieRequestsAreApplied();
@@ -641,12 +641,44 @@ TEST_F(AccountConsistencyServiceTest, SetChromeConnectedCookiesAfterDelete) {
   // |kGoogleDomain| or |kYouTubeDomain| otherwise they will not be reset since
   // it is before the update time. Add multiple URLs to test for race conditions
   // with remove call.
-  account_consistency_service_->SetChromeConnectedCookieWithDomains(
-      {"google.ca", "google.fr", kCountryGoogleDomain});
+  account_consistency_service_->SetChromeConnectedCookieWithUrls(
+      {GURL("https://google.ca"), GURL("https://google.fr"),
+       GURL("https://google.de")});
   SimulateGaiaCookieManagerServiceLogout();
-  account_consistency_service_->SetChromeConnectedCookieWithDomains(
-      {"google.ca"});
+  account_consistency_service_->SetChromeConnectedCookieWithUrls(
+      {GURL("https://google.ca")});
 
   WaitUntilAllCookieRequestsAreApplied();
   CheckDomainHasChromeConnectedCookie("google.ca");
 }
+
+// Ensures that CHROME_CONNECTED cookies are only set on GAIA urls when the user
+// is signed out for |kMobileIdentityConsistency| experiment.
+TEST_F(AccountConsistencyServiceTest,
+       SetMiceChromeConnectedCookiesSignedOutUser) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(signin::kMobileIdentityConsistency);
+
+  id delegate =
+      [OCMockObject mockForProtocol:@protocol(ManageAccountsDelegate)];
+  NSDictionary* headers = [NSDictionary dictionary];
+  NSHTTPURLResponse* responseGoogle = [[NSHTTPURLResponse alloc]
+       initWithURL:[NSURL URLWithString:@"https://google.com/"]
+        statusCode:200
+       HTTPVersion:@"HTTP/1.1"
+      headerFields:headers];
+  account_consistency_service_->SetWebStateHandler(&web_state_, delegate);
+
+  EXPECT_TRUE(web_state_.ShouldAllowResponse(responseGoogle,
+                                             /* for_main_frame = */ true));
+  CheckNoChromeConnectedCookies();
+
+  NSHTTPURLResponse* responseGaia = [[NSHTTPURLResponse alloc]
+       initWithURL:[NSURL URLWithString:@"https://accounts.google.com/"]
+        statusCode:200
+       HTTPVersion:@"HTTP/1.1"
+      headerFields:headers];
+  EXPECT_TRUE(web_state_.ShouldAllowResponse(responseGaia,
+                                             /* for_main_frame = */ true));
+  CheckDomainHasChromeConnectedCookie("google.com");
+}