Skip to content

Commit

Permalink
Add UMA metric for max SameSite=None cookies per host
Browse files Browse the repository at this point in the history
This metric will be used to get a sense of how many cross-site cookies
sites are using. This should give us more information on how many
partitioned cookies we should allow domains to use per-partition, which
is being discussed in privacycg/CHIPS#48.

Since CHIPS is not shipped and only enabled on a small fraction of clients' machines, this metric only tracks unpartitioned cookies.

Bug: 1351841
Change-Id: I50f4b6f9bfd6e3bef2ca36d5621bf98aa6476ba9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3822813
Reviewed-by: Chris Fredrickson <cfredric@chromium.org>
Commit-Queue: Dylan Cutler <dylancutler@google.com>
Cr-Commit-Position: refs/heads/main@{#1034162}
  • Loading branch information
DCtheTall authored and Chromium LUCI CQ committed Aug 11, 2022
1 parent 7df4a71 commit 4917adb
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 0 deletions.
14 changes: 14 additions & 0 deletions net/cookies/cookie_monster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2265,6 +2265,20 @@ bool CookieMonster::DoRecordPeriodicStats() {
// Can be up to kMaxCookies.
UMA_HISTOGRAM_COUNTS_10000("Cookie.NumKeys", num_keys_);

std::map<std::string, size_t> n_same_site_none_cookies;
for (const auto& [host_key, host_cookie] : cookies_) {
if (!host_cookie || !host_cookie->IsEffectivelySameSiteNone())
continue;
n_same_site_none_cookies[host_key]++;
}
size_t max_n_cookies = 0;
for (const auto& entry : n_same_site_none_cookies) {
max_n_cookies = std::max(max_n_cookies, entry.second);
}
// Can be up to 180 cookies, the max per-domain.
base::UmaHistogramCounts1000("Cookie.MaxSameSiteNoneCookiesPerKey",
max_n_cookies);

// Collect stats for partitioned cookies if they are enabled.
if (base::FeatureList::IsEnabled(features::kPartitionedCookies)) {
base::UmaHistogramCounts1000("Cookie.PartitionCount",
Expand Down
51 changes: 51 additions & 0 deletions net/cookies/cookie_monster_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3469,6 +3469,57 @@ TEST_F(CookieMonsterTest, NumKeysHistogram) {
}
}

TEST_F(CookieMonsterTest, MaxSameSiteNoneCookiesPerKey) {
const char kHistogramName[] = "Cookie.MaxSameSiteNoneCookiesPerKey";

auto store = base::MakeRefCounted<MockPersistentCookieStore>();
auto cm = std::make_unique<CookieMonster>(store.get(), net::NetLog::Get(),
kFirstPartySetsDefault);
ASSERT_EQ(0u, GetAllCookies(cm.get()).size());

{ // Only SameSite cookies should not log a sample.
base::HistogramTester histogram_tester;

ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain1.test"),
"A=1;SameSite=Lax",
CookieOptions::MakeAllInclusive()));
ASSERT_EQ(1u, GetAllCookies(cm.get()).size());
ASSERT_TRUE(cm->DoRecordPeriodicStatsForTesting());
histogram_tester.ExpectUniqueSample(kHistogramName, 0 /* sample */,
1 /* count */);
}

{ // SameSite=None cookie should log a sample.
base::HistogramTester histogram_tester;

ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain1.test"),
"B=2;SameSite=None;Secure",
CookieOptions::MakeAllInclusive()));
ASSERT_EQ(2u, GetAllCookies(cm.get()).size());
ASSERT_TRUE(cm->DoRecordPeriodicStatsForTesting());
histogram_tester.ExpectUniqueSample(kHistogramName, 1 /* sample */,
1 /* count */);
}

{ // Should log the maximum number of SameSite=None cookies.
base::HistogramTester histogram_tester;

ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain2.test"),
"A=1;SameSite=None;Secure",
CookieOptions::MakeAllInclusive()));
ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain2.test"),
"B=2;SameSite=None;Secure",
CookieOptions::MakeAllInclusive()));
ASSERT_TRUE(CreateAndSetCookie(cm.get(), GURL("https://domain3.test"),
"A=1;SameSite=None;Secure",
CookieOptions::MakeAllInclusive()));
ASSERT_EQ(5u, GetAllCookies(cm.get()).size());
ASSERT_TRUE(cm->DoRecordPeriodicStatsForTesting());
histogram_tester.ExpectUniqueSample(kHistogramName, 2 /* sample */,
1 /* count */);
}
}

// Test that localhost URLs can set and get secure cookies, even if
// non-cryptographic.
TEST_F(CookieMonsterTest, SecureCookieLocalhost) {
Expand Down
12 changes: 12 additions & 0 deletions tools/metrics/histograms/metadata/cookie/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,18 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="Cookie.MaxSameSiteNoneCookiesPerKey" units="units"
expires_after="2023-02-01">
<owner>dylancutler@chromium.org</owner>
<owner>src/net/cookies/OWNERS</owner>
<summary>
Maximum number of SameSite=None cookies that belong to a single domain on
the client. This histogram will be used to inform the
per-partition-per-domain limit for partitioned cookies. Recorded every 10
minutes of active browsing time.
</summary>
</histogram>

<histogram name="Cookie.NumDomainPurgedKeys" units="keys"
expires_after="2022-08-28">
<owner>cfredric@chromium.org</owner>
Expand Down

0 comments on commit 4917adb

Please sign in to comment.