Commit 5b38a905 authored by Shuran Huang's avatar Shuran Huang Committed by Chromium LUCI CQ

Remove full_party_context from CookieOptions.

This reverts part of the code in commit 57f97b28.

The reason is that, based on mmenke@'s comment in crrev/c/2570123,
copying the whole set once per saved cookie for comparison is generally
not worth the overhead, should use an enum class object instead.

Bug: 1136102
Change-Id: I3bcc07a84d64e8ce55fb999567e154fdbfe43443
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2580461Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Reviewed-by: default avatarLily Chen <chlily@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Shuran Huang <shuuran@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836730}
parent 66cfb847
...@@ -64,7 +64,6 @@ CookieOptions CookieOptions::MakeAllInclusive() { ...@@ -64,7 +64,6 @@ CookieOptions CookieOptions::MakeAllInclusive() {
options.set_include_httponly(); options.set_include_httponly();
options.set_same_site_cookie_context(SameSiteCookieContext::MakeInclusive()); options.set_same_site_cookie_context(SameSiteCookieContext::MakeInclusive());
options.set_do_not_update_access_time(); options.set_do_not_update_access_time();
options.set_full_party_context(std::set<SchemefulSite>());
options.set_same_party_cookie_context_type( options.set_same_party_cookie_context_type(
SamePartyCookieContextType::kSameParty); SamePartyCookieContextType::kSameParty);
return options; return options;
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/optional.h" #include "base/optional.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "net/base/net_export.h" #include "net/base/net_export.h"
#include "net/base/schemeful_site.h"
#include "net/cookies/cookie_constants.h" #include "net/cookies/cookie_constants.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -109,7 +108,6 @@ class NET_EXPORT CookieOptions { ...@@ -109,7 +108,6 @@ class NET_EXPORT CookieOptions {
// * |set_{include,exclude}_httponly()| // * |set_{include,exclude}_httponly()|
// * |set_same_site_cookie_context()| // * |set_same_site_cookie_context()|
// * |set_do_not_update_access_time()| // * |set_do_not_update_access_time()|
// * |set_full_party_context()|
// * |set_same_party_cookie_context_type()| // * |set_same_party_cookie_context_type()|
CookieOptions(); CookieOptions();
CookieOptions(const CookieOptions& other); CookieOptions(const CookieOptions& other);
...@@ -141,14 +139,6 @@ class NET_EXPORT CookieOptions { ...@@ -141,14 +139,6 @@ class NET_EXPORT CookieOptions {
void unset_return_excluded_cookies() { return_excluded_cookies_ = false; } void unset_return_excluded_cookies() { return_excluded_cookies_ = false; }
bool return_excluded_cookies() const { return return_excluded_cookies_; } bool return_excluded_cookies() const { return return_excluded_cookies_; }
void set_full_party_context(
const base::Optional<std::set<SchemefulSite>>& full_party_context) {
full_party_context_ = full_party_context;
}
const base::Optional<std::set<SchemefulSite>>& full_party_context() const {
return full_party_context_;
}
// How trusted is the current browser environment when it comes to accessing // How trusted is the current browser environment when it comes to accessing
// SameParty cookies. Default is not trusted, e.g. kCrossParty. // SameParty cookies. Default is not trusted, e.g. kCrossParty.
void set_same_party_cookie_context_type( void set_same_party_cookie_context_type(
...@@ -178,7 +168,6 @@ class NET_EXPORT CookieOptions { ...@@ -178,7 +168,6 @@ class NET_EXPORT CookieOptions {
SameSiteCookieContext same_site_cookie_context_; SameSiteCookieContext same_site_cookie_context_;
bool update_access_time_; bool update_access_time_;
bool return_excluded_cookies_ = false; bool return_excluded_cookies_ = false;
base::Optional<std::set<SchemefulSite>> full_party_context_;
SamePartyCookieContextType same_party_cookie_context_type_ = SamePartyCookieContextType same_party_cookie_context_type_ =
SamePartyCookieContextType::kCrossParty; SamePartyCookieContextType::kCrossParty;
......
...@@ -372,18 +372,6 @@ bool StructTraits<network::mojom::CookieOptionsDataView, net::CookieOptions>:: ...@@ -372,18 +372,6 @@ bool StructTraits<network::mojom::CookieOptionsDataView, net::CookieOptions>::
else else
cookie_options->unset_return_excluded_cookies(); cookie_options->unset_return_excluded_cookies();
base::Optional<std::vector<net::SchemefulSite>> mojo_full_party_context;
if (!mojo_options.ReadFullPartyContext(&mojo_full_party_context))
return false;
base::Optional<std::set<net::SchemefulSite>> full_party_context;
if (mojo_full_party_context.has_value()) {
full_party_context.emplace(mojo_full_party_context->begin(),
mojo_full_party_context->end());
if (mojo_full_party_context->size() != full_party_context->size())
return false;
}
cookie_options->set_full_party_context(full_party_context);
net::CookieOptions::SamePartyCookieContextType same_party_cookie_context_type; net::CookieOptions::SamePartyCookieContextType same_party_cookie_context_type;
if (!mojo_options.ReadSamePartyCookieContextType( if (!mojo_options.ReadSamePartyCookieContextType(
&same_party_cookie_context_type)) &same_party_cookie_context_type))
......
...@@ -13,13 +13,8 @@ ...@@ -13,13 +13,8 @@
#include "net/cookies/cookie_constants.h" #include "net/cookies/cookie_constants.h"
#include "net/cookies/cookie_inclusion_status.h" #include "net/cookies/cookie_inclusion_status.h"
#include "net/cookies/cookie_options.h" #include "net/cookies/cookie_options.h"
#include "services/network/public/cpp/schemeful_site_mojom_traits.h"
#include "services/network/public/mojom/cookie_manager.mojom.h" #include "services/network/public/mojom/cookie_manager.mojom.h"
namespace net {
class SchemefulSite;
} // namespace net
namespace mojo { namespace mojo {
template <> template <>
...@@ -125,11 +120,6 @@ struct StructTraits<network::mojom::CookieOptionsDataView, net::CookieOptions> { ...@@ -125,11 +120,6 @@ struct StructTraits<network::mojom::CookieOptionsDataView, net::CookieOptions> {
return o.return_excluded_cookies(); return o.return_excluded_cookies();
} }
static const base::Optional<std::set<net::SchemefulSite>>& full_party_context(
const net::CookieOptions& o) {
return o.full_party_context();
}
static net::CookieOptions::SamePartyCookieContextType static net::CookieOptions::SamePartyCookieContextType
same_party_cookie_context_type(const net::CookieOptions& o) { same_party_cookie_context_type(const net::CookieOptions& o) {
return o.same_party_cookie_context_type(); return o.same_party_cookie_context_type();
......
...@@ -4,13 +4,11 @@ ...@@ -4,13 +4,11 @@
#include "services/network/public/cpp/cookie_manager_mojom_traits.h" #include "services/network/public/cpp/cookie_manager_mojom_traits.h"
#include <set>
#include <vector> #include <vector>
#include "base/test/gtest_util.h" #include "base/test/gtest_util.h"
#include "mojo/public/cpp/base/time_mojom_traits.h" #include "mojo/public/cpp/base/time_mojom_traits.h"
#include "mojo/public/cpp/test_support/test_utils.h" #include "mojo/public/cpp/test_support/test_utils.h"
#include "net/base/schemeful_site.h"
#include "net/cookies/cookie_constants.h" #include "net/cookies/cookie_constants.h"
#include "services/network/public/cpp/cookie_manager_mojom_traits.h" #include "services/network/public/cpp/cookie_manager_mojom_traits.h"
#include "services/network/public/mojom/cookie_manager.mojom.h" #include "services/network/public/mojom/cookie_manager.mojom.h"
...@@ -326,7 +324,6 @@ TEST(CookieManagerTraitsTest, Roundtrips_CookieOptions) { ...@@ -326,7 +324,6 @@ TEST(CookieManagerTraitsTest, Roundtrips_CookieOptions) {
{ {
net::CookieOptions least_trusted, copy; net::CookieOptions least_trusted, copy;
EXPECT_FALSE(least_trusted.return_excluded_cookies()); EXPECT_FALSE(least_trusted.return_excluded_cookies());
least_trusted.set_return_excluded_cookies(); // differ from default. least_trusted.set_return_excluded_cookies(); // differ from default.
least_trusted.set_full_party_context_size(10u); least_trusted.set_full_party_context_size(10u);
...@@ -345,15 +342,12 @@ TEST(CookieManagerTraitsTest, Roundtrips_CookieOptions) { ...@@ -345,15 +342,12 @@ TEST(CookieManagerTraitsTest, Roundtrips_CookieOptions) {
{ {
net::CookieOptions very_trusted, copy; net::CookieOptions very_trusted, copy;
auto kPartyContext = std::set<net::SchemefulSite>{
net::SchemefulSite(url::Origin::Create(GURL("https://a.test")))};
very_trusted.set_include_httponly(); very_trusted.set_include_httponly();
very_trusted.set_same_site_cookie_context( very_trusted.set_same_site_cookie_context(
net::CookieOptions::SameSiteCookieContext::MakeInclusive()); net::CookieOptions::SameSiteCookieContext::MakeInclusive());
very_trusted.set_full_party_context(kPartyContext);
very_trusted.set_same_party_cookie_context_type( very_trusted.set_same_party_cookie_context_type(
net::CookieOptions::SamePartyCookieContextType::kSameParty); net::CookieOptions::SamePartyCookieContextType::kSameParty);
very_trusted.set_full_party_context_size(kPartyContext.size()); very_trusted.set_full_party_context_size(1u);
EXPECT_TRUE(mojo::test::SerializeAndDeserialize<mojom::CookieOptions>( EXPECT_TRUE(mojo::test::SerializeAndDeserialize<mojom::CookieOptions>(
very_trusted, copy)); very_trusted, copy));
...@@ -361,44 +355,12 @@ TEST(CookieManagerTraitsTest, Roundtrips_CookieOptions) { ...@@ -361,44 +355,12 @@ TEST(CookieManagerTraitsTest, Roundtrips_CookieOptions) {
EXPECT_EQ(net::CookieOptions::SameSiteCookieContext::MakeInclusive(), EXPECT_EQ(net::CookieOptions::SameSiteCookieContext::MakeInclusive(),
copy.same_site_cookie_context()); copy.same_site_cookie_context());
EXPECT_FALSE(copy.return_excluded_cookies()); EXPECT_FALSE(copy.return_excluded_cookies());
EXPECT_EQ(kPartyContext, copy.full_party_context());
EXPECT_EQ(net::CookieOptions::SamePartyCookieContextType::kSameParty, EXPECT_EQ(net::CookieOptions::SamePartyCookieContextType::kSameParty,
copy.same_party_cookie_context_type()); copy.same_party_cookie_context_type());
EXPECT_EQ(1u, copy.full_party_context_size()); EXPECT_EQ(1u, copy.full_party_context_size());
} }
} }
TEST(CookieManagerTraitsTest, Roundtrips_FullPartyContext) {
{
std::vector<std::set<net::SchemefulSite>> kTestCases = {
std::set<net::SchemefulSite>(),
std::set<net::SchemefulSite>{net::SchemefulSite()},
std::set<net::SchemefulSite>{
net::SchemefulSite(url::Origin::Create(GURL("https://a.test")))},
std::set<net::SchemefulSite>{
net::SchemefulSite(url::Origin::Create(GURL("http://a.test"))),
net::SchemefulSite(url::Origin::Create(GURL("http://b.test")))},
};
for (const std::set<net::SchemefulSite>& fpc : kTestCases) {
net::CookieOptions options, copy;
options.set_full_party_context(fpc);
EXPECT_TRUE(mojo::test::SerializeAndDeserialize<mojom::CookieOptions>(
options, copy));
EXPECT_EQ(fpc, copy.full_party_context());
}
}
{
base::Optional<std::set<net::SchemefulSite>> kFullPartyContext =
base::nullopt;
net::CookieOptions options, copy;
options.set_full_party_context(kFullPartyContext);
EXPECT_TRUE(mojo::test::SerializeAndDeserialize<mojom::CookieOptions>(
options, copy));
EXPECT_EQ(kFullPartyContext, copy.full_party_context());
}
}
TEST(CookieManagerTraitsTest, Roundtrips_CookieChangeInfo) { TEST(CookieManagerTraitsTest, Roundtrips_CookieChangeInfo) {
auto original_cookie = net::CanonicalCookie::CreateUnsafeCookieForTesting( auto original_cookie = net::CanonicalCookie::CreateUnsafeCookieForTesting(
"A", "B", "x.y", "/path", base::Time(), base::Time(), base::Time(), "A", "B", "x.y", "/path", base::Time(), base::Time(), base::Time(),
......
...@@ -345,7 +345,6 @@ mojom("cookies_mojom") { ...@@ -345,7 +345,6 @@ mojom("cookies_mojom") {
] ]
public_deps = [ public_deps = [
":mojom_schemeful_site",
"//components/content_settings/core/common:mojo_bindings", "//components/content_settings/core/common:mojo_bindings",
"//mojo/public/mojom/base", "//mojo/public/mojom/base",
"//url/mojom:url_mojom_gurl", "//url/mojom:url_mojom_gurl",
......
...@@ -6,7 +6,6 @@ module network.mojom; ...@@ -6,7 +6,6 @@ module network.mojom;
import "components/content_settings/core/common/content_settings.mojom"; import "components/content_settings/core/common/content_settings.mojom";
import "mojo/public/mojom/base/time.mojom"; import "mojo/public/mojom/base/time.mojom";
import "services/network/public/mojom/schemeful_site.mojom";
import "url/mojom/url.mojom"; import "url/mojom/url.mojom";
// Parameters for constructing a cookie manager. // Parameters for constructing a cookie manager.
...@@ -127,7 +126,6 @@ struct CookieOptions { ...@@ -127,7 +126,6 @@ struct CookieOptions {
CookieSameSiteContext same_site_cookie_context; CookieSameSiteContext same_site_cookie_context;
bool update_access_time = true; bool update_access_time = true;
bool return_excluded_cookies = false; bool return_excluded_cookies = false;
array<SchemefulSite>? full_party_context;
SamePartyCookieContextType same_party_cookie_context_type = kCrossParty; SamePartyCookieContextType same_party_cookie_context_type = kCrossParty;
// The size of the isolation_info.party_context plus the top-frame site for // The size of the isolation_info.party_context plus the top-frame site for
// logging purposes. // logging purposes.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment