Add methods to check status of cookie SameSite features

This adds methods to net::cookie_util to check whether "SameSite by
default cookies" and "Cookies without SameSite must be Secure" are
enabled. This replaces direct checks of the FeatureList and allows
future changes to the criteria for enabling those features.

Bug: 954551, 953306
Change-Id: If2b7f92b464797b77bf01c80f5eb5511a45ef641
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1726796Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682813}

content/browser/devtools/protocol/network_handler.cc

@@ -54,7 +54,6 @@
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/origin_util.h"
-#include "net/base/features.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
@@ -742,10 +741,10 @@ BuildProtocolBlockedCookies(const net::CookieStatusList& net_list) {
   std::unique_ptr<Array<Network::BlockedCookieWithReason>> protocol_list =
       std::make_unique<Array<Network::BlockedCookieWithReason>>();
 
-  bool samesite_by_default_enabled = base::FeatureList::IsEnabled(
-      net::features::kSameSiteByDefaultCookies);
-  bool must_be_secure_enabled = base::FeatureList::IsEnabled(
-      net::features::kCookiesWithoutSameSiteMustBeSecure);
+  bool samesite_by_default_enabled =
+      net::cookie_util::IsSameSiteByDefaultCookiesEnabled();
+  bool must_be_secure_enabled =
+      net::cookie_util::IsCookiesWithoutSameSiteMustBeSecureEnabled();
 
   for (const net::CookieWithStatus& cookie : net_list) {
     // These CookieInclusionStatus values will be passed to us from network

net/cookies/canonical_cookie.cc

@@ -46,13 +46,11 @@
 
 #include <utility>
 
-#include "base/feature_list.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
-#include "net/base/features.h"
 #include "net/base/url_util.h"
 #include "net/cookies/cookie_util.h"
 #include "net/cookies/parsed_cookie.h"
@@ -380,7 +378,7 @@ CookieSameSite CanonicalCookie::GetEffectiveSameSite() const {
   // If a cookie does not have a SameSite attribute, the effective SameSite
   // mode depends on the SameSiteByDefaultCookies setting.
   if (SameSite() == CookieSameSite::UNSPECIFIED) {
-    if (base::FeatureList::IsEnabled(features::kSameSiteByDefaultCookies))
+    if (cookie_util::IsSameSiteByDefaultCookiesEnabled())
       return CookieSameSite::LAX_MODE;
     return CookieSameSite::NO_RESTRICTION;
   }
@@ -437,9 +435,7 @@ CanonicalCookie::CookieInclusionStatus CanonicalCookie::IncludeForRequestURL(
   // ignored. This can apply to cookies which were created before the
   // experimental options were enabled (as non-SameSite, insecure cookies cannot
   // be set while the options are on).
-  if (base::FeatureList::IsEnabled(features::kSameSiteByDefaultCookies) &&
-      base::FeatureList::IsEnabled(
-          features::kCookiesWithoutSameSiteMustBeSecure) &&
+  if (cookie_util::IsCookiesWithoutSameSiteMustBeSecureEnabled() &&
       GetEffectiveSameSite() == CookieSameSite::NO_RESTRICTION && !IsSecure()) {
     return CanonicalCookie::CookieInclusionStatus::
         EXCLUDE_SAMESITE_NONE_INSECURE;

net/cookies/canonical_cookie.h

@@ -32,6 +32,8 @@ class NET_EXPORT CanonicalCookie {
   // the resulting CanonicalCookies should not be relied on to be canonical
   // unless the caller has done appropriate validation and canonicalization
   // themselves.
+  // NOTE: Prefer using CreateSanitizedCookie() over directly using this
+  // constructor.
   CanonicalCookie(const std::string& name,
                   const std::string& value,
                   const std::string& domain,
@@ -58,15 +60,14 @@ class NET_EXPORT CanonicalCookie {
     EXCLUDE_NOT_ON_PATH,
     EXCLUDE_SAMESITE_STRICT,
     EXCLUDE_SAMESITE_LAX,
-    // TODO(crbug.com/953995): Implement EXTENDED_MODE which will use the
-    // following value.
+    // TODO(crbug.com/989171): Replace this with FirstPartyLax and
+    // FirstPartyStrict.
     EXCLUDE_SAMESITE_EXTENDED,
     // The following two are used for the SameSiteByDefaultCookies experiment,
     // where if the SameSite attribute is not specified, it will be treated as
     // SameSite=Lax by default.
     EXCLUDE_SAMESITE_UNSPECIFIED_TREATED_AS_LAX,
     // This is used if SameSite=None is specified, but the cookie is not Secure.
-    // TODO(chlily): Implement the above.
     EXCLUDE_SAMESITE_NONE_INSECURE,
     EXCLUDE_USER_PREFERENCES,
 

net/cookies/cookie_monster.cc

@@ -49,7 +49,6 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/feature_list.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
@@ -63,7 +62,6 @@
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/process_memory_dump.h"
-#include "net/base/features.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/cookies/canonical_cookie.h"
 #include "net/cookies/cookie_monster_change_dispatcher.h"
@@ -1217,9 +1215,7 @@ void CookieMonster::SetCanonicalCookie(std::unique_ptr<CanonicalCookie> cc,
   // If both SameSiteByDefaultCookies and CookiesWithoutSameSiteMustBeSecure
   // are enabled, non-SameSite cookies without the Secure attribute will be
   // rejected.
-  if (base::FeatureList::IsEnabled(features::kSameSiteByDefaultCookies) &&
-      base::FeatureList::IsEnabled(
-          features::kCookiesWithoutSameSiteMustBeSecure) &&
+  if (cookie_util::IsCookiesWithoutSameSiteMustBeSecureEnabled() &&
       cc->GetEffectiveSameSite() == CookieSameSite::NO_RESTRICTION &&
       !cc->IsSecure()) {
     DVLOG(net::cookie_util::kVlogSetCookies)

net/cookies/cookie_util.cc

@@ -9,12 +9,14 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_tokenizer.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
+#include "net/base/features.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/base/url_util.h"
 #include "net/http/http_util.h"
@@ -500,6 +502,16 @@ CanonicalCookie::CookieInclusionStatus CookieWouldBeExcludedDueToSameSite(
   return CanonicalCookie::CookieInclusionStatus::INCLUDE;
 }
 
+bool IsSameSiteByDefaultCookiesEnabled() {
+  return base::FeatureList::IsEnabled(features::kSameSiteByDefaultCookies);
+}
+
+bool IsCookiesWithoutSameSiteMustBeSecureEnabled() {
+  return IsSameSiteByDefaultCookiesEnabled() &&
+         base::FeatureList::IsEnabled(
+             features::kCookiesWithoutSameSiteMustBeSecure);
+}
+
 base::OnceCallback<void(const CookieList&, const CookieStatusList&)>
 IgnoreCookieStatusList(base::OnceCallback<void(const CookieList&)> callback) {
   return base::BindOnce(

net/cookies/cookie_util.h

@@ -156,6 +156,10 @@ NET_EXPORT CanonicalCookie::CookieInclusionStatus
 CookieWouldBeExcludedDueToSameSite(const CanonicalCookie& cookie,
                                    const CookieOptions& options);
 
+// Returns whether the respective SameSite feature is enabled.
+NET_EXPORT bool IsSameSiteByDefaultCookiesEnabled();
+NET_EXPORT bool IsCookiesWithoutSameSiteMustBeSecureEnabled();
+
 // Takes a OnceCallback with only a CookieList and binds it to a callback that
 // also accepts a CookieStatusList, making it compatible with
 // CookieStore::GetCookieListCallback.