Reland "Add GetAllCookiesWithAccessSemantics to CookieManager interface"

This is a reland of ffcd2de7

Problem that caused revert fixed by https://crrev.com/c/1849114

TBR=morlovich@chromium.org,mbarbella@chromium.org

Original change's description:
> Add GetAllCookiesWithAccessSemantics to CookieManager interface
>
> This CL adds a method, GetAllCookiesWithAccessSemantics, to the mojo
> interface CookieManager. This just directly calls
> CookieStore::GetAllCookiesWithAccessSemanticsAsync() on the
> CookieManager's underlying CookieStore.
>
> Bug: 978172
> Change-Id: Ic26298caf12d34ffd4de5bcf81058f6984528683
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1845803
> Commit-Queue: Lily Chen <chlily@chromium.org>
> Reviewed-by: Maks Orlovich <morlovich@chromium.org>
> Reviewed-by: Martin Barbella <mbarbella@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#703916}

Bug: 978172
Change-Id: Id1e92255ad6a15caba08c3e11bd47d27e549a274
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1850315Reviewed-by: Lily Chen <chlily@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704354}

net/cookies/cookie_constants.h

@@ -70,13 +70,10 @@ enum class CookieSameSiteString {
   kMaxValue = kExtended
 };
 
-// What rules to apply when determining when whether access to a particular
-// cookie is allowed.
-// TODO(crbug.com/978172): Machinery to read the content setting and set the
-// appropriate CookieAccessSemantics on the cookie (will be added as a new
-// metadata field of CanonicalCookie).
+// What rules to apply when determining whether access to a particular cookie is
+// allowed.
 enum class CookieAccessSemantics {
-  // Has not been checked yet.
+  // Has not been checked yet or there is no way to check.
   UNKNOWN = -1,
   // Has been checked and the cookie should *not* be subject to legacy access
   // rules.

services/network/cookie_manager.cc

@@ -73,6 +73,11 @@ void CookieManager::GetAllCookies(GetAllCookiesCallback callback) {
   cookie_store_->GetAllCookiesAsync(std::move(callback));
 }
 
+void CookieManager::GetAllCookiesWithAccessSemantics(
+    GetAllCookiesWithAccessSemanticsCallback callback) {
+  cookie_store_->GetAllCookiesWithAccessSemanticsAsync(std::move(callback));
+}
+
 void CookieManager::GetCookieList(const GURL& url,
                                   const net::CookieOptions& cookie_options,
                                   GetCookieListCallback callback) {

services/network/cookie_manager.h

@@ -56,6 +56,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CookieManager
 
   // mojom::CookieManager
   void GetAllCookies(GetAllCookiesCallback callback) override;
+  void GetAllCookiesWithAccessSemantics(
+      GetAllCookiesWithAccessSemanticsCallback callback) override;
   void GetCookieList(const GURL& url,
                      const net::CookieOptions& cookie_options,
                      GetCookieListCallback callback) override;

services/network/cookie_manager_unittest.cc

@@ -23,6 +23,7 @@
 #include "net/cookies/cookie_store_test_callbacks.h"
 #include "net/cookies/cookie_store_test_helpers.h"
 #include "net/cookies/cookie_util.h"
+#include "net/cookies/test_cookie_access_delegate.h"
 #include "services/network/public/mojom/cookie_manager.mojom.h"
 #include "services/network/session_cleanup_cookie_store.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -82,6 +83,24 @@ class SynchronousCookieManager {
     return cookies_out;
   }
 
+  std::vector<net::CanonicalCookie> GetAllCookiesWithAccessSemantics(
+      std::vector<net::CookieAccessSemantics>* access_semantics_list_out) {
+    base::RunLoop run_loop;
+    std::vector<net::CanonicalCookie> cookies_out;
+    cookie_service_->GetAllCookiesWithAccessSemantics(
+        base::BindLambdaForTesting(
+            [&run_loop, &cookies_out, access_semantics_list_out](
+                const std::vector<net::CanonicalCookie>& cookies,
+                const std::vector<net::CookieAccessSemantics>&
+                    access_semantics_list) {
+              cookies_out = cookies;
+              *access_semantics_list_out = access_semantics_list;
+              run_loop.Quit();
+            }));
+    run_loop.Run();
+    return cookies_out;
+  }
+
   std::vector<net::CanonicalCookie> GetCookieList(const GURL& url,
                                                   net::CookieOptions options) {
     base::RunLoop run_loop;
@@ -439,6 +458,75 @@ TEST_F(CookieManagerTest, GetAllCookies) {
   EXPECT_EQ(net::COOKIE_PRIORITY_MEDIUM, cookies[3].Priority());
 }
 
+TEST_F(CookieManagerTest, GetAllCookiesWithAccessSemantics) {
+  auto delegate = std::make_unique<net::TestCookieAccessDelegate>();
+  delegate->SetExpectationForCookieDomain("domain1.test",
+                                          net::CookieAccessSemantics::UNKNOWN);
+  delegate->SetExpectationForCookieDomain("domain2.test",
+                                          net::CookieAccessSemantics::LEGACY);
+  delegate->SetExpectationForCookieDomain(
+      ".domainwithdot.test", net::CookieAccessSemantics::NONLEGACY);
+  cookie_store()->SetCookieAccessDelegate(std::move(delegate));
+
+  // Set some cookies for the test to play with.
+  // TODO(chlily): Because the order of the cookies with respect to the access
+  // semantics entries should match up, for the purposes of this test we need
+  // the cookies to sort in a predictable order. Since the longest path is
+  // first, we can manipulate the path attribute of each cookie set below to get
+  // them in the right order. This will have to change if CookieSorter ever
+  // starts sorting the cookies differently.
+
+  // UNKNOWN
+  EXPECT_TRUE(SetCanonicalCookie(
+      net::CanonicalCookie("A", "B", "domain1.test",
+                           "/this/path/is/the/longest/for/sorting/purposes",
+                           base::Time(), base::Time(), base::Time(),
+                           /*secure=*/false, /*httponly=*/false,
+                           net::CookieSameSite::NO_RESTRICTION,
+                           net::COOKIE_PRIORITY_MEDIUM),
+      "https", true));
+  // LEGACY
+  EXPECT_TRUE(SetCanonicalCookie(
+      net::CanonicalCookie(
+          "C", "D", "domain2.test", "/with/longer/path", base::Time(),
+          base::Time(), base::Time(), /*secure=*/false, /*httponly=*/false,
+          net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_MEDIUM),
+      "https", true));
+  // not set (UNKNOWN)
+  EXPECT_TRUE(SetCanonicalCookie(
+      net::CanonicalCookie(
+          "HttpOnly", "F", "domain3.test", "/with/path", base::Time(),
+          base::Time(), base::Time(), /*secure=*/false,
+          /*httponly=*/true, net::CookieSameSite::NO_RESTRICTION,
+          net::COOKIE_PRIORITY_MEDIUM),
+      "https", true));
+  // NONLEGACY
+  EXPECT_TRUE(SetCanonicalCookie(
+      net::CanonicalCookie(
+          "Secure", "E", ".domainwithdot.test", "/", base::Time(), base::Time(),
+          base::Time(), /*secure=*/true,
+          /*httponly=*/false, net::CookieSameSite::NO_RESTRICTION,
+          net::COOKIE_PRIORITY_MEDIUM),
+      "https", true));
+
+  std::vector<net::CookieAccessSemantics> access_semantics_list;
+  std::vector<net::CanonicalCookie> cookies =
+      service_wrapper()->GetAllCookiesWithAccessSemantics(
+          &access_semantics_list);
+
+  ASSERT_EQ(4u, cookies.size());
+  EXPECT_EQ(cookies.size(), access_semantics_list.size());
+  EXPECT_EQ("domain1.test", cookies[0].Domain());
+  EXPECT_EQ("domain2.test", cookies[1].Domain());
+  EXPECT_EQ("domain3.test", cookies[2].Domain());
+  EXPECT_EQ(".domainwithdot.test", cookies[3].Domain());
+
+  EXPECT_EQ(net::CookieAccessSemantics::UNKNOWN, access_semantics_list[0]);
+  EXPECT_EQ(net::CookieAccessSemantics::LEGACY, access_semantics_list[1]);
+  EXPECT_EQ(net::CookieAccessSemantics::UNKNOWN, access_semantics_list[2]);
+  EXPECT_EQ(net::CookieAccessSemantics::NONLEGACY, access_semantics_list[3]);
+}
+
 TEST_F(CookieManagerTest, GetCookieList) {
   // Set some cookies for the test to play with.
   EXPECT_TRUE(SetCanonicalCookie(

services/network/public/cpp/cookie_manager.typemap

@@ -17,6 +17,7 @@ type_mappings = [
   "network.mojom.CookiePriority=::net::CookiePriority",
   "network.mojom.CookieSameSite=::net::CookieSameSite",
   "network.mojom.CookieSameSiteContext=::net::CookieOptions::SameSiteCookieContext",
+  "network.mojom.CookieAccessSemantics=::net::CookieAccessSemantics",
   "network.mojom.CookieOptions=::net::CookieOptions",
   "network.mojom.CanonicalCookie=::net::CanonicalCookie",
   "network.mojom.CookieInclusionStatusWarningReason=::net::CanonicalCookie::CookieInclusionStatus::WarningReason",

services/network/public/cpp/cookie_manager_mojom_traits.cc

@@ -86,6 +86,43 @@ bool EnumTraits<network::mojom::CookieSameSite, net::CookieSameSite>::FromMojom(
   return false;
 }
 
+network::mojom::CookieAccessSemantics EnumTraits<
+    network::mojom::CookieAccessSemantics,
+    net::CookieAccessSemantics>::ToMojom(net::CookieAccessSemantics input) {
+  switch (input) {
+    case net::CookieAccessSemantics::UNKNOWN:
+      return network::mojom::CookieAccessSemantics::UNKNOWN;
+    case net::CookieAccessSemantics::NONLEGACY:
+      return network::mojom::CookieAccessSemantics::NONLEGACY;
+    case net::CookieAccessSemantics::LEGACY:
+      return network::mojom::CookieAccessSemantics::LEGACY;
+    default:
+      break;
+  }
+  NOTREACHED();
+  return static_cast<network::mojom::CookieAccessSemantics>(input);
+}
+
+bool EnumTraits<network::mojom::CookieAccessSemantics,
+                net::CookieAccessSemantics>::
+    FromMojom(network::mojom::CookieAccessSemantics input,
+              net::CookieAccessSemantics* output) {
+  switch (input) {
+    case network::mojom::CookieAccessSemantics::UNKNOWN:
+      *output = net::CookieAccessSemantics::UNKNOWN;
+      return true;
+    case network::mojom::CookieAccessSemantics::NONLEGACY:
+      *output = net::CookieAccessSemantics::NONLEGACY;
+      return true;
+    case network::mojom::CookieAccessSemantics::LEGACY:
+      *output = net::CookieAccessSemantics::LEGACY;
+      return true;
+    default:
+      break;
+  }
+  return false;
+}
+
 network::mojom::CookieInclusionStatusWarningReason
 EnumTraits<network::mojom::CookieInclusionStatusWarningReason,
            net::CanonicalCookie::CookieInclusionStatus::WarningReason>::

services/network/public/cpp/cookie_manager_mojom_traits.h

@@ -28,6 +28,15 @@ struct EnumTraits<network::mojom::CookieSameSite, net::CookieSameSite> {
                         net::CookieSameSite* output);
 };
 
+template <>
+struct EnumTraits<network::mojom::CookieAccessSemantics,
+                  net::CookieAccessSemantics> {
+  static network::mojom::CookieAccessSemantics ToMojom(
+      net::CookieAccessSemantics input);
+  static bool FromMojom(network::mojom::CookieAccessSemantics input,
+                        net::CookieAccessSemantics* output);
+};
+
 template <>
 struct EnumTraits<network::mojom::CookieInclusionStatusWarningReason,
                   net::CanonicalCookie::CookieInclusionStatus::WarningReason> {

services/network/public/cpp/cookie_manager_mojom_traits_unittest.cc

@@ -117,7 +117,8 @@ TEST(CookieManagerTraitsTest, Roundtrips_CookieWithStatus) {
 TEST(CookieManagerTraitsTest, Roundtrips_CookieSameSite) {
   for (net::CookieSameSite cookie_state :
        {net::CookieSameSite::NO_RESTRICTION, net::CookieSameSite::LAX_MODE,
-        net::CookieSameSite::STRICT_MODE}) {
+        net::CookieSameSite::STRICT_MODE, net::CookieSameSite::EXTENDED_MODE,
+        net::CookieSameSite::UNSPECIFIED}) {
     net::CookieSameSite roundtrip;
     ASSERT_TRUE(SerializeAndDeserializeEnum<mojom::CookieSameSite>(cookie_state,
                                                                    &roundtrip));
@@ -125,6 +126,18 @@ TEST(CookieManagerTraitsTest, Roundtrips_CookieSameSite) {
   }
 }
 
+TEST(CookieManagerTraitsTest, Roundtrips_CookieAccessSemantics) {
+  for (net::CookieAccessSemantics access_semantics :
+       {net::CookieAccessSemantics::UNKNOWN,
+        net::CookieAccessSemantics::NONLEGACY,
+        net::CookieAccessSemantics::LEGACY}) {
+    net::CookieAccessSemantics roundtrip;
+    ASSERT_TRUE(SerializeAndDeserializeEnum<mojom::CookieAccessSemantics>(
+        access_semantics, &roundtrip));
+    EXPECT_EQ(access_semantics, roundtrip);
+  }
+}
+
 TEST(CookieManagerTraitsTest, Roundtrips_CookieSameSiteContext) {
   for (net::CookieOptions::SameSiteCookieContext context_state :
        {net::CookieOptions::SameSiteCookieContext::CROSS_SITE,

services/network/public/mojom/cookie_manager.mojom

@@ -52,7 +52,6 @@ enum CookieSameSite {
   LAX_MODE = 1,
   STRICT_MODE = 2,
   EXTENDED_MODE = 3,
-  // 4 is reserved: LAX_MODE_ALLOW_UNSAFE is not used here.
 };
 
 enum CookieSameSiteContext {
@@ -62,6 +61,15 @@ enum CookieSameSiteContext {
   SAME_SITE_STRICT
 };
 
+// What rules to apply when determining whether access to a particular cookie is
+// allowed.
+// Keep in sync with net/cookies/cookie_constants.h.
+enum CookieAccessSemantics {
+  UNKNOWN = -1,
+  NONLEGACY = 0,
+  LEGACY,
+};
+
 // Keep defaults here in sync with net/cookies/cookie_options.cc.
 struct CookieOptions {
   bool exclude_httponly = true;
@@ -215,6 +223,21 @@ interface CookieManager {
   // on origin.  Should the returned cookies also be sorted by origin?
   GetAllCookies() => (array<CanonicalCookie> cookies);
 
+  // Get all the cookies known to the service.
+  // Returned cookie list is sorted first by path length (longest first)
+  // and second by creation time.
+  // Additionally get a list of the CookieAccessSemantics that applies to each,
+  // if known. The |access_semantics_list| is guaranteed to be the same length
+  // as |cookies|, with each element in |cookies| having the access semantics
+  // which is given by the same index in |access_semantics_list|. If this method
+  // is not implemented in the underlying CookieStore, the returned
+  // |access_semantics_list| will just contain all UNKNOWNs. If it is
+  // supported, the access semantics values will have been determined by
+  // querying the CookieStore's CookieAccessDelegate.
+  GetAllCookiesWithAccessSemantics()
+      => (array<CanonicalCookie> cookies,
+          array<CookieAccessSemantics> access_semantics_list);
+
   // Get all cookies for the specified URL and cookie options.
   // Returned cookie list is sorted first by path length (longest first)
   // and second by creation time. If the |return_excluded_cookies| option is set

services/network/test/test_cookie_manager.h

@@ -27,6 +27,8 @@ class TestCookieManager : public network::mojom::CookieManager {
                           const net::CookieOptions& cookie_options,
                           SetCanonicalCookieCallback callback) override;
   void GetAllCookies(GetAllCookiesCallback callback) override {}
+  void GetAllCookiesWithAccessSemantics(
+      GetAllCookiesWithAccessSemanticsCallback callback) override {}
   void GetCookieList(const GURL& url,
                      const net::CookieOptions& cookie_options,
                      GetCookieListCallback callback) override {}