Revert "Add GetAllCookiesWithAccessSemantics to CookieManager interface"

This reverts commit ffcd2de7.

Reason for revert: Followup to revert here:
https://chromium-review.googlesource.com/c/chromium/src/+/1849135

Which was reverted due to red ios/slimnav bot

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}

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

Change-Id: I72e9457762658e296a7442384f093391a41c4e57
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 978172
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849138Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704067}

net/cookies/cookie_constants.h

@@ -70,10 +70,13 @@ enum class CookieSameSiteString {
   kMaxValue = kExtended
 };
 
-// What rules to apply when determining whether access to a particular cookie is
-// allowed.
+// 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).
 enum class CookieAccessSemantics {
-  // Has not been checked yet or there is no way to check.
+  // Has not been checked yet.
   UNKNOWN = -1,
   // Has been checked and the cookie should *not* be subject to legacy access
   // rules.

services/network/cookie_manager.cc

@@ -73,11 +73,6 @@ 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,8 +56,6 @@ 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,7 +23,6 @@
 #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"
@@ -83,24 +82,6 @@ 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;
@@ -458,75 +439,6 @@ 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,7 +17,6 @@ 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,43 +86,6 @@ 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,15 +28,6 @@ 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,8 +117,7 @@ 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::EXTENDED_MODE,
-        net::CookieSameSite::UNSPECIFIED}) {
+        net::CookieSameSite::STRICT_MODE}) {
     net::CookieSameSite roundtrip;
     ASSERT_TRUE(SerializeAndDeserializeEnum<mojom::CookieSameSite>(cookie_state,
                                                                    &roundtrip));
@@ -126,18 +125,6 @@ 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,6 +52,7 @@ enum CookieSameSite {
   LAX_MODE = 1,
   STRICT_MODE = 2,
   EXTENDED_MODE = 3,
+  // 4 is reserved: LAX_MODE_ALLOW_UNSAFE is not used here.
 };
 
 enum CookieSameSiteContext {
@@ -61,15 +62,6 @@ 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;
@@ -223,21 +215,6 @@ 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,8 +27,6 @@ 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 {}