Add CookieAccessSemantics to cookie change notifications

[Merge to M79]

This change adds a CookieAccessSemantics field to the cookie change
notification interfaces, such that the access semantics of a cookie at
the time of the reported change can also be included in the change
notification. The CookieAccessSemantics is used to determine whether a
cookie should be treated according to "legacy" rules, based on a
policy setting. It affects whether the cookie may be included on a given
request, and some cookie change consumers care about this because they
check whether the cookie change should be observed based on whether it
would have been included on a request.

This is accomplished by bundling the changed cookie, the
CookieAccessSemantics, and CookieChangeCause into a new struct,
CookieChangeInfo, and passing a CookieChangeInfo along to notification
subscribers.

This change should not produce any behavior change for consumers who
don't care about the CookieAccessSemantics, which is most of them, for
which this will just pass them an extra parameter that they don't need.
The places where behavior changes are in the CookieStore API
(CookieChangeSubscription::ShouldObserveChangeTo), the
RestrictedCookieManager::Listener (OnCookieChange), and
CookieMonsterChangeDispatcher::Subscription (DispatchChange), where
now the call to CanonicalCookie::IncludeForRequestURL is able to pass
the CookieAccessSemantics in order to properly compute whether the
cookie change should be observed.

TBR=khorimoto@chromium.org

(cherry picked from commit 19cced4)

Bug: 978172
Change-Id: I712972391ff0adcc4b94481bba156dc0e7b759a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854649
Reviewed-by: Lily Chen <chlily@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Martin Barbella <mbarbella@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#707116}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872783
Cr-Commit-Position: refs/branch-heads/3945@{#27}
Cr-Branched-From: e4635ff-refs/heads/master@{#706915}

android_webview/browser/network_service/aw_proxying_restricted_cookie_manager.cc

@@ -30,11 +30,10 @@ class AwProxyingRestrictedCookieManagerListener
         aw_restricted_cookie_manager_(aw_restricted_cookie_manager),
         client_listener_(std::move(client_listener)) {}
 
-  void OnCookieChange(const net::CanonicalCookie& cookie,
-                      network::mojom::CookieChangeCause cause) override {
+  void OnCookieChange(const net::CookieChangeInfo& change) override {
     if (aw_restricted_cookie_manager_ &&
         aw_restricted_cookie_manager_->AllowCookies(url_, site_for_cookies_))
-      client_listener_->OnCookieChange(cookie, cause);
+      client_listener_->OnCookieChange(change);
   }
 
  private:

chrome/browser/chromeos/android_sms/android_sms_pairing_state_tracker_impl.cc

@@ -75,10 +75,9 @@ void AndroidSmsPairingStateTrackerImpl::OnCookiesRetrieved(
 }
 
 void AndroidSmsPairingStateTrackerImpl::OnCookieChange(
-    const net::CanonicalCookie& cookie,
-    network::mojom::CookieChangeCause cause) {
-  DCHECK_EQ(kMessagesPairStateCookieName, cookie.Name());
-  DCHECK(cookie.IsDomainMatch(GetPairingUrl().host()));
+    const net::CookieChangeInfo& change) {
+  DCHECK_EQ(kMessagesPairStateCookieName, change.cookie.Name());
+  DCHECK(change.cookie.IsDomainMatch(GetPairingUrl().host()));
 
   // NOTE: cookie.Value() cannot be trusted in this callback. The cookie may
   // have expired or been removed and the Value() does not get updated. It's

chrome/browser/chromeos/android_sms/android_sms_pairing_state_tracker_impl.h

@@ -36,8 +36,7 @@ class AndroidSmsPairingStateTrackerImpl
 
  private:
   // network::mojom::CookieChangeListener:
-  void OnCookieChange(const net::CanonicalCookie& cookie,
-                      network::mojom::CookieChangeCause cause) override;
+  void OnCookieChange(const net::CookieChangeInfo& change) override;
 
   // AndroidSmsAppManager::Observer:
   void OnInstalledAppUrlChanged() override;

chrome/browser/extensions/api/cookies/cookies_api.cc

@@ -95,9 +95,8 @@ CookiesEventRouter::CookieChangeListener::CookieChangeListener(
 CookiesEventRouter::CookieChangeListener::~CookieChangeListener() = default;
 
 void CookiesEventRouter::CookieChangeListener::OnCookieChange(
-    const net::CanonicalCookie& canonical_cookie,
-    network::mojom::CookieChangeCause cause) {
-  router_->OnCookieChange(otr_, canonical_cookie, cause);
+    const net::CookieChangeInfo& change) {
+  router_->OnCookieChange(otr_, change);
 }
 
 CookiesEventRouter::CookiesEventRouter(content::BrowserContext* context)
@@ -110,50 +109,48 @@ CookiesEventRouter::~CookiesEventRouter() {
   BrowserList::RemoveObserver(this);
 }
 
-void CookiesEventRouter::OnCookieChange(
-    bool otr,
-    const net::CanonicalCookie& canonical_cookie,
-    network::mojom::CookieChangeCause cause) {
+void CookiesEventRouter::OnCookieChange(bool otr,
+                                        const net::CookieChangeInfo& change) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   std::unique_ptr<base::ListValue> args(new base::ListValue());
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
   dict->SetBoolean(cookies_api_constants::kRemovedKey,
-                   cause != network::mojom::CookieChangeCause::INSERTED);
+                   change.cause != net::CookieChangeCause::INSERTED);
 
   Profile* profile =
       otr ? profile_->GetOffTheRecordProfile() : profile_->GetOriginalProfile();
   api::cookies::Cookie cookie = cookies_helpers::CreateCookie(
-      canonical_cookie, cookies_helpers::GetStoreIdFromProfile(profile));
+      change.cookie, cookies_helpers::GetStoreIdFromProfile(profile));
   dict->Set(cookies_api_constants::kCookieKey, cookie.ToValue());
 
   // Map the internal cause to an external string.
   std::string cause_dict_entry;
-  switch (cause) {
+  switch (change.cause) {
     // Report an inserted cookie as an "explicit" change cause. All other causes
     // only make sense for deletions.
-    case network::mojom::CookieChangeCause::INSERTED:
-    case network::mojom::CookieChangeCause::EXPLICIT:
+    case net::CookieChangeCause::INSERTED:
+    case net::CookieChangeCause::EXPLICIT:
       cause_dict_entry = cookies_api_constants::kExplicitChangeCause;
       break;
 
-    case network::mojom::CookieChangeCause::OVERWRITE:
+    case net::CookieChangeCause::OVERWRITE:
       cause_dict_entry = cookies_api_constants::kOverwriteChangeCause;
       break;
 
-    case network::mojom::CookieChangeCause::EXPIRED:
+    case net::CookieChangeCause::EXPIRED:
       cause_dict_entry = cookies_api_constants::kExpiredChangeCause;
       break;
 
-    case network::mojom::CookieChangeCause::EVICTED:
+    case net::CookieChangeCause::EVICTED:
       cause_dict_entry = cookies_api_constants::kEvictedChangeCause;
       break;
 
-    case network::mojom::CookieChangeCause::EXPIRED_OVERWRITE:
+    case net::CookieChangeCause::EXPIRED_OVERWRITE:
       cause_dict_entry = cookies_api_constants::kExpiredOverwriteChangeCause;
       break;
 
-    case network::mojom::CookieChangeCause::UNKNOWN_DELETION:
+    case net::CookieChangeCause::UNKNOWN_DELETION:
       NOTREACHED();
   }
   dict->SetString(cookies_api_constants::kCauseKey, cause_dict_entry);
@@ -162,7 +159,7 @@ void CookiesEventRouter::OnCookieChange(
 
   DispatchEvent(profile, events::COOKIES_ON_CHANGED,
                 api::cookies::OnChanged::kEventName, std::move(args),
-                cookies_helpers::GetURLFromCanonicalCookie(canonical_cookie));
+                cookies_helpers::GetURLFromCanonicalCookie(change.cookie));
 }
 
 void CookiesEventRouter::OnBrowserAdded(Browser* browser) {

chrome/browser/extensions/api/cookies/cookies_api.h

@@ -22,6 +22,7 @@
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "net/cookies/canonical_cookie.h"
+#include "net/cookies/cookie_change_dispatcher.h"
 #include "services/network/public/mojom/cookie_manager.mojom.h"
 #include "url/gurl.h"
 
@@ -49,8 +50,7 @@ class CookiesEventRouter : public BrowserListObserver {
     ~CookieChangeListener() override;
 
     // network::mojom::CookieChangeListener:
-    void OnCookieChange(const net::CanonicalCookie& canonical_cookie,
-                        network::mojom::CookieChangeCause cause) override;
+    void OnCookieChange(const net::CookieChangeInfo& change) override;
 
    private:
     CookiesEventRouter* router_;
@@ -65,9 +65,7 @@ class CookiesEventRouter : public BrowserListObserver {
       Profile* profile);
   void OnConnectionError(
       mojo::Receiver<network::mojom::CookieChangeListener>* receiver);
-  void OnCookieChange(bool otr,
-                      const net::CanonicalCookie& canonical_cookie,
-                      network::mojom::CookieChangeCause cause);
+  void OnCookieChange(bool otr, const net::CookieChangeInfo& change);
 
   // This method dispatches events to the extension message service.
   void DispatchEvent(content::BrowserContext* context,

chrome/browser/signin/consistency_cookie_browsertest.cc

@@ -67,9 +67,8 @@ class TestConsistencyCookieManager
   }
 
   // CookieChangeListener:
-  void OnCookieChange(const net::CanonicalCookie& cookie,
-                      network::mojom::CookieChangeCause cause) override {
-    if (cookie.Name() != kConsistencyCookieName)
+  void OnCookieChange(const net::CookieChangeInfo& change) override {
+    if (change.cookie.Name() != kConsistencyCookieName)
       return;
     if (!run_loop_quit_closure_.is_null())
       std::move(run_loop_quit_closure_).Run();

components/signin/internal/identity_manager/gaia_cookie_manager_service.cc

@@ -32,6 +32,7 @@
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/cookies/cookie_change_dispatcher.h"
+#include "net/cookies/cookie_constants.h"
 #include "net/http/http_status_code.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "services/network/public/cpp/resource_request.h"
@@ -580,7 +581,9 @@ void GaiaCookieManagerService::ForceOnCookieChangeProcessing() {
           base::Time(), base::Time(), base::Time(), true /* secure */,
           false /* httponly */, net::CookieSameSite::NO_RESTRICTION,
           net::COOKIE_PRIORITY_DEFAULT));
-  OnCookieChange(*cookie, network::mojom::CookieChangeCause::UNKNOWN_DELETION);
+  OnCookieChange(
+      net::CookieChangeInfo(*cookie, net::CookieAccessSemantics::UNKNOWN,
+                            net::CookieChangeCause::UNKNOWN_DELETION));
 }
 
 void GaiaCookieManagerService::LogOutAllAccounts(gaia::GaiaSource source) {
@@ -658,13 +661,13 @@ void GaiaCookieManagerService::MarkListAccountsStale() {
 }
 
 void GaiaCookieManagerService::OnCookieChange(
-    const net::CanonicalCookie& cookie,
-    network::mojom::CookieChangeCause cause) {
-  DCHECK_EQ(kGaiaCookieName, cookie.Name());
-  DCHECK(cookie.IsDomainMatch(GaiaUrls::GetInstance()->google_url().host()));
+    const net::CookieChangeInfo& change) {
+  DCHECK_EQ(kGaiaCookieName, change.cookie.Name());
+  DCHECK(change.cookie.IsDomainMatch(
+      GaiaUrls::GetInstance()->google_url().host()));
   list_accounts_stale_ = true;
 
-  if (cause == network::mojom::CookieChangeCause::EXPLICIT) {
+  if (change.cause == net::CookieChangeCause::EXPLICIT) {
     DCHECK(net::CookieChangeCauseIsDeletion(net::CookieChangeCause::EXPLICIT));
     if (gaia_cookie_deleted_by_user_action_callback_) {
       gaia_cookie_deleted_by_user_action_callback_.Run();

components/signin/internal/identity_manager/gaia_cookie_manager_service.h

@@ -26,6 +26,7 @@
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "net/base/backoff_entry.h"
+#include "net/cookies/cookie_change_dispatcher.h"
 #include "services/network/public/mojom/cookie_manager.mojom.h"
 
 class GaiaAuthFetcher;
@@ -312,8 +313,7 @@ class GaiaCookieManagerService : public GaiaAuthConsumer,
   // Overridden from network::mojom::CookieChangeListner. If the cookie relates
   // to a GAIA APISID cookie, then we call ListAccounts and fire
   // OnGaiaAccountsInCookieUpdated.
-  void OnCookieChange(const net::CanonicalCookie& cookie,
-                      network::mojom::CookieChangeCause cause) override;
+  void OnCookieChange(const net::CookieChangeInfo& change) override;
   void OnCookieListenerConnectionError();
 
   // Overridden from GaiaAuthConsumer.

components/signin/public/identity_manager/identity_manager_unittest.cc

@@ -41,6 +41,8 @@
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "google_apis/gaia/core_account_id.h"
 #include "google_apis/gaia/google_service_auth_error.h"
+#include "net/cookies/cookie_change_dispatcher.h"
+#include "net/cookies/cookie_constants.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/network/test/test_cookie_manager.h"
 #include "services/network/test/test_url_loader_factory.h"
@@ -383,8 +385,9 @@ class IdentityManagerTest : public testing::Test {
   void SimulateCookieDeletedByUser(
       network::mojom::CookieChangeListener* listener,
       const net::CanonicalCookie& cookie) {
-    listener->OnCookieChange(cookie,
-                             network::mojom::CookieChangeCause::EXPLICIT);
+    listener->OnCookieChange(
+        net::CookieChangeInfo(cookie, net::CookieAccessSemantics::UNKNOWN,
+                              net::CookieChangeCause::EXPLICIT));
   }
 
   void SimulateOAuthMultiloginFinished(GaiaCookieManagerService* manager,
@@ -2061,7 +2064,8 @@ TEST_F(IdentityManagerTest, OnNetworkInitialized) {
       base::Time(), /*secure=*/true, false, net::CookieSameSite::NO_RESTRICTION,
       net::COOKIE_PRIORITY_DEFAULT);
   test_cookie_manager_ptr->DispatchCookieChange(
-      cookie, network::mojom::CookieChangeCause::EXPLICIT);
+      net::CookieChangeInfo(cookie, net::CookieAccessSemantics::UNKNOWN,
+                            net::CookieChangeCause::EXPLICIT));
   run_loop.Run();
 }
 

content/browser/cookie_store/cookie_change_subscription.cc

@@ -158,7 +158,8 @@ void CookieChangeSubscription::Serialize(
 }
 
 bool CookieChangeSubscription::ShouldObserveChangeTo(
-    const net::CanonicalCookie& cookie) const {
+    const net::CanonicalCookie& cookie,
+    net::CookieAccessSemantics access_semantics) const {
   switch (match_type_) {
     case ::network::mojom::CookieMatchType::EQUALS:
       if (cookie.Name() != name_)
@@ -174,7 +175,8 @@ bool CookieChangeSubscription::ShouldObserveChangeTo(
   net_options.set_same_site_cookie_context(
       net::CookieOptions::SameSiteCookieContext::SAME_SITE_STRICT);
 
-  return cookie.IncludeForRequestURL(url_, net_options).IsInclude();
+  return cookie.IncludeForRequestURL(url_, net_options, access_semantics)
+      .IsInclude();
 }
 
 }  // namespace content

content/browser/cookie_store/cookie_change_subscription.h

@@ -98,7 +98,8 @@ class CookieChangeSubscription
       blink::mojom::CookieChangeSubscription* mojo_subscription) const;
 
   // True if the subscription covers a change to the given cookie.
-  bool ShouldObserveChangeTo(const net::CanonicalCookie& cookie) const;
+  bool ShouldObserveChangeTo(const net::CanonicalCookie& cookie,
+                             net::CookieAccessSemantics access_semantics) const;
 
  private:
   const GURL url_;

content/browser/cookie_store/cookie_store_manager.cc

@@ -444,16 +444,14 @@ void CookieStoreManager::OnStorageWiped() {
   subscriptions_by_registration_.clear();
 }
 
-void CookieStoreManager::OnCookieChange(
-    const net::CanonicalCookie& cookie,
-    ::network::mojom::CookieChangeCause cause) {
+void CookieStoreManager::OnCookieChange(const net::CookieChangeInfo& change) {
   // Waiting for on-disk subscriptions to be loaded ensures that changes are
   // delivered to all service workers that subscribed to them in previous
   // browser sessions. Without waiting, workers might miss cookie changes.
   if (!done_loading_subscriptions_) {
     subscriptions_loaded_callbacks_.emplace_back(
         base::BindOnce(&CookieStoreManager::OnCookieChange,
-                       weak_factory_.GetWeakPtr(), cookie, cause));
+                       weak_factory_.GetWeakPtr(), change));
     return;
   }
 
@@ -464,7 +462,7 @@ void CookieStoreManager::OnCookieChange(
   //               net::CookieMonsterChangeDispatcher::DomainKey. Extract that
   //               implementation into net/cookies.cookie_util.h and call it.
   std::string url_key = net::registry_controlled_domains::GetDomainAndRegistry(
-      cookie.Domain(),
+      change.cookie.Domain(),
       net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
   auto it = subscriptions_by_url_key_.find(url_key);
   if (it == subscriptions_by_url_key_.end())
@@ -475,7 +473,8 @@ void CookieStoreManager::OnCookieChange(
            subscriptions.head();
        node != subscriptions.end(); node = node->next()) {
     const CookieChangeSubscription* subscription = node->value();
-    if (subscription->ShouldObserveChangeTo(cookie)) {
+    if (subscription->ShouldObserveChangeTo(change.cookie,
+                                            change.access_semantics)) {
       interested_registration_ids.insert(
           subscription->service_worker_registration_id());
     }
@@ -487,8 +486,7 @@ void CookieStoreManager::OnCookieChange(
         registration_id,
         base::BindOnce(
             [](base::WeakPtr<CookieStoreManager> manager,
-               const net::CanonicalCookie& cookie,
-               ::network::mojom::CookieChangeCause cause,
+               const net::CookieChangeInfo& change,
                blink::ServiceWorkerStatusCode find_status,
                scoped_refptr<ServiceWorkerRegistration> registration) {
               if (find_status != blink::ServiceWorkerStatusCode::kOk)
@@ -497,43 +495,40 @@ void CookieStoreManager::OnCookieChange(
               DCHECK(registration);
               if (!manager)
                 return;
-              manager->DispatchChangeEvent(std::move(registration), cookie,
-                                           cause);
+              manager->DispatchChangeEvent(std::move(registration), change);
             },
-            weak_factory_.GetWeakPtr(), cookie, cause));
+            weak_factory_.GetWeakPtr(), change));
   }
 }
 
 void CookieStoreManager::DispatchChangeEvent(
     scoped_refptr<ServiceWorkerRegistration> registration,
-    const net::CanonicalCookie& cookie,
-    ::network::mojom::CookieChangeCause cause) {
+    const net::CookieChangeInfo& change) {
   scoped_refptr<ServiceWorkerVersion> active_version =
       registration->active_version();
   if (active_version->running_status() != EmbeddedWorkerStatus::RUNNING) {
     active_version->RunAfterStartWorker(
         ServiceWorkerMetrics::EventType::COOKIE_CHANGE,
         base::BindOnce(&CookieStoreManager::DidStartWorkerForChangeEvent,
                        weak_factory_.GetWeakPtr(), std::move(registration),
-                       cookie, cause));
+                       change));
     return;
   }
 
   int request_id = active_version->StartRequest(
       ServiceWorkerMetrics::EventType::COOKIE_CHANGE, base::DoNothing());
 
   active_version->endpoint()->DispatchCookieChangeEvent(
-      cookie, cause, active_version->CreateSimpleEventCallback(request_id));
+      change, active_version->CreateSimpleEventCallback(request_id));
 }
 
 void CookieStoreManager::DidStartWorkerForChangeEvent(
     scoped_refptr<ServiceWorkerRegistration> registration,
-    const net::CanonicalCookie& cookie,
-    ::network::mojom::CookieChangeCause cause,
+    const net::CookieChangeInfo& change,
     blink::ServiceWorkerStatusCode start_worker_status) {
   if (start_worker_status != blink::ServiceWorkerStatusCode::kOk)
     return;
-  DispatchChangeEvent(std::move(registration), cookie, cause);
+  DispatchChangeEvent(std::move(registration), change);
 }
 
 }  // namespace content