Skip to content

Commit

Permalink
{M103 PICK} [Track postMessage usage] (3) Add 3P info retention to Bl…
Browse files Browse the repository at this point in the history
…inkStorageKey and mojom

This CL spreads the 3P info retention from the prior CL through the rest
of the StorageKey stack. This preserves the information that would be
set if third party storage partitioning was enabled for measurement
purposes. Our prior change did not propagate the information deep
enough and we weren't able to detect 3P usage of post message.
**This will be picked into M103**

This CL is a part of a series:
(1) Consolidate existing metrics
(2) Track cross-origin calls
(3) Add 3P support to BlinkStorageKey and mojom

(cherry picked from commit aefcdae)

Bug: 1159586
Change-Id: Id8ea924c7da2b80ce2bf1eff6000dda505935a7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3653884
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Steven Bingler <bingler@chromium.org>
Reviewed-by: Steven Bingler <bingler@chromium.org>
Auto-Submit: Ari Chivukula <arichiv@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1006541}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3660963
Commit-Queue: Ari Chivukula <arichiv@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#273}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
arichiv authored and Chromium LUCI CQ committed May 26, 2022
1 parent d16e496 commit 1aada6b
Show file tree
Hide file tree
Showing 9 changed files with 257 additions and 123 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

#include "third_party/blink/public/common/storage_key/storage_key_mojom_traits.h"

#include "base/test/scoped_feature_list.h"
#include "base/unguessable_token.h"
#include "mojo/public/cpp/test_support/test_utils.h"
#include "net/base/schemeful_site.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/common/storage_key/storage_key.h"
#include "third_party/blink/public/mojom/storage_key/ancestor_chain_bit.mojom.h"
#include "url/gurl.h"
Expand All @@ -21,42 +23,50 @@ namespace blink {
namespace {

TEST(StorageKeyMojomTraitsTest, SerializeAndDeserialize) {
StorageKey test_keys[] = {
StorageKey(url::Origin::Create(GURL("https://example.com"))),
StorageKey(url::Origin::Create(GURL("http://example.com"))),
StorageKey(url::Origin::Create(GURL("https://example.test"))),
StorageKey(url::Origin::Create(GURL("https://sub.example.com"))),
StorageKey(url::Origin::Create(GURL("http://sub2.example.com"))),
StorageKey(url::Origin::Create(GURL("https://example.com")),
url::Origin::Create(GURL("https://example.com"))),
StorageKey(url::Origin::Create(GURL("http://example.com")),
url::Origin::Create(GURL("https://example2.com"))),
StorageKey(url::Origin::Create(GURL("https://example.test")),
url::Origin::Create(GURL("https://example.com"))),
StorageKey(url::Origin::Create(GURL("https://sub.example.com")),
url::Origin::Create(GURL("https://example2.com"))),
StorageKey(url::Origin::Create(GURL("http://sub2.example.com")),
url::Origin::Create(GURL("https://example.com"))),
StorageKey(url::Origin()),
StorageKey::CreateWithNonce(
url::Origin::Create(GURL("https://.example.com")),
base::UnguessableToken::Create()),
StorageKey::CreateWithNonce(url::Origin(),
base::UnguessableToken::Create()),
StorageKey::CreateWithOptionalNonce(
url::Origin::Create(GURL("http://sub2.example.com")),
net::SchemefulSite(url::Origin::Create(GURL("https://example.com"))),
nullptr, blink::mojom::AncestorChainBit::kCrossSite)};
for (const bool toggle : {false, true}) {
base::test::ScopedFeatureList scope_feature_list;
scope_feature_list.InitWithFeatureState(
features::kThirdPartyStoragePartitioning, toggle);
StorageKey test_keys[] = {
StorageKey(url::Origin::Create(GURL("https://example.com"))),
StorageKey(url::Origin::Create(GURL("http://example.com"))),
StorageKey(url::Origin::Create(GURL("https://example.test"))),
StorageKey(url::Origin::Create(GURL("https://sub.example.com"))),
StorageKey(url::Origin::Create(GURL("http://sub2.example.com"))),
StorageKey(url::Origin::Create(GURL("https://example.com")),
url::Origin::Create(GURL("https://example.com"))),
StorageKey(url::Origin::Create(GURL("http://example.com")),
url::Origin::Create(GURL("https://example2.com"))),
StorageKey(url::Origin::Create(GURL("https://example.test")),
url::Origin::Create(GURL("https://example.com"))),
StorageKey(url::Origin::Create(GURL("https://sub.example.com")),
url::Origin::Create(GURL("https://example2.com"))),
StorageKey(url::Origin::Create(GURL("http://sub2.example.com")),
url::Origin::Create(GURL("https://example.com"))),
StorageKey(url::Origin()),
StorageKey::CreateWithNonce(
url::Origin::Create(GURL("https://.example.com")),
base::UnguessableToken::Create()),
StorageKey::CreateWithNonce(url::Origin(),
base::UnguessableToken::Create()),
StorageKey::CreateWithOptionalNonce(
url::Origin::Create(GURL("http://sub2.example.com")),
net::SchemefulSite(
url::Origin::Create(GURL("https://example.com"))),
nullptr, blink::mojom::AncestorChainBit::kCrossSite)};

for (auto& original : test_keys) {
StorageKey copied;
EXPECT_TRUE(mojo::test::SerializeAndDeserialize<mojom::StorageKey>(original,
copied));
EXPECT_EQ(original, copied);
// The top_level_site doesn't factor into comparisons unless
// features::kThirdPartyStoragePartitioning is enabled. Since we just want
// to see if the field is correct or not let's check it here.
EXPECT_EQ(original.top_level_site(), copied.top_level_site());
for (auto& original : test_keys) {
StorageKey copied;
EXPECT_TRUE(mojo::test::SerializeAndDeserialize<mojom::StorageKey>(
original, copied));
EXPECT_EQ(original, copied);

// Ensure the comparison works if `kThirdPartyStoragePartitioning` is
// force enabled. This verifies `top_level_site_` and
// `ancestor_chain_bit_`.
EXPECT_EQ(original.CopyWithForceEnabledThirdPartyStoragePartitioning(),
copied.CopyWithForceEnabledThirdPartyStoragePartitioning());
}
}
}

Expand Down
9 changes: 8 additions & 1 deletion third_party/blink/common/storage_key/storage_key_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -677,16 +677,23 @@ TEST(StorageKeyTest, CopyWithForceEnabledThirdPartyStoragePartitioning) {
scope_feature_list.InitWithFeatureState(
features::kThirdPartyStoragePartitioning, toggle);

const StorageKey storage_key(kOrigin, kOtherOrigin);
const StorageKey storage_key = StorageKey::CreateWithOptionalNonce(
kOrigin, net::SchemefulSite(kOtherOrigin), nullptr,
blink::mojom::AncestorChainBit::kCrossSite);
EXPECT_EQ(storage_key.IsThirdPartyContext(), toggle);
EXPECT_EQ(storage_key.top_level_site(),
net::SchemefulSite(toggle ? kOtherOrigin : kOrigin));
EXPECT_EQ(storage_key.ancestor_chain_bit(),
toggle ? blink::mojom::AncestorChainBit::kCrossSite
: blink::mojom::AncestorChainBit::kSameSite);

const StorageKey storage_key_with_3psp =
storage_key.CopyWithForceEnabledThirdPartyStoragePartitioning();
EXPECT_TRUE(storage_key_with_3psp.IsThirdPartyContext());
EXPECT_EQ(storage_key_with_3psp.top_level_site(),
net::SchemefulSite(kOtherOrigin));
EXPECT_EQ(storage_key_with_3psp.ancestor_chain_bit(),
blink::mojom::AncestorChainBit::kCrossSite);
}
}

Expand Down
15 changes: 6 additions & 9 deletions third_party/blink/public/common/storage_key/storage_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,8 @@ class BLINK_COMMON_EXPORT StorageKey {

// Returns a copy of what this storage key would have been if
// `kThirdPartyStoragePartitioning` were enabled. This is a convenience
// function for callsites that benefit from future functionality that
// should be removed when storage partitioning is fully launched.
// TODO(crbug.com/1159586): Add support in BlinkStorageKey if needed.
// function for callsites that benefit from future functionality.
// TODO(crbug.com/1159586): Remove when no longer needed.
StorageKey CopyWithForceEnabledThirdPartyStoragePartitioning() const {
StorageKey storage_key = *this;
storage_key.top_level_site_ =
Expand Down Expand Up @@ -233,9 +232,8 @@ class BLINK_COMMON_EXPORT StorageKey {

// Stores the value `top_level_site_` would have had if
// `kThirdPartyStoragePartitioning` were enabled. This isn't used in
// serialization or comparison, and this information is lost if you convert
// to a BlinkStorageKey or send it via mojom.
// TODO(crbug.com/1159586): Add support in BlinkStorageKey if needed.
// serialization or comparison.
// TODO(crbug.com/1159586): Remove when no longer needed.
net::SchemefulSite top_level_site_if_third_party_enabled_;

// An optional nonce, forcing a partitioned storage from anything else. Used
Expand All @@ -251,9 +249,8 @@ class BLINK_COMMON_EXPORT StorageKey {

// Stores the value `ancestor_chain_bit_` would have had if
// `kThirdPartyStoragePartitioning` were enabled. This isn't used in
// serialization or comparison, and this information is lost if you convert
// to a BlinkStorageKey or send it via mojom.
// TODO(crbug.com/1159586): Add support in BlinkStorageKey if needed.
// serialization or comparison.
// TODO(crbug.com/1159586): Remove when no longer needed.
blink::mojom::AncestorChainBit ancestor_chain_bit_if_third_party_enabled_{
blink::mojom::AncestorChainBit::kSameSite};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,14 @@ class BLINK_COMMON_EXPORT
return key.origin();
}

static const net::SchemefulSite& top_level_site(
const blink::StorageKey& key) {
return key.top_level_site();
// TODO(crbug.com/1159586): Return by reference when internal copy is removed.
static const net::SchemefulSite top_level_site(const blink::StorageKey& key) {
// We use `CopyWithForceEnabledThirdPartyStoragePartitioning` to ensure the
// partitioned values are preserved. The constructor on the other side will
// properly restore `top_level_site_` as derived from origin_ if
// `kThirdPartyStoragePartitioning` is disabled.
return key.CopyWithForceEnabledThirdPartyStoragePartitioning()
.top_level_site();
}

static const absl::optional<base::UnguessableToken>& nonce(
Expand All @@ -46,7 +51,12 @@ class BLINK_COMMON_EXPORT

static blink::mojom::AncestorChainBit ancestor_chain_bit(
const blink::StorageKey& key) {
return key.ancestor_chain_bit();
// We use `CopyWithForceEnabledThirdPartyStoragePartitioning` to ensure the
// partitioned values are preserved. The constructor on the other side will
// properly restore `ancestor_chain_bit_` to be `kSameSite` if
// `kThirdPartyStoragePartitioning` is disabled.
return key.CopyWithForceEnabledThirdPartyStoragePartitioning()
.ancestor_chain_bit();
}

static bool Read(blink::mojom::StorageKeyDataView data,
Expand Down
28 changes: 22 additions & 6 deletions third_party/blink/renderer/platform/storage/blink_storage_key.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,13 @@ BlinkStorageKey::BlinkStorageKey(
blink::StorageKey::IsThirdPartyStoragePartitioningEnabled()
? top_level_site
: BlinkSchemefulSite(origin)),
top_level_site_if_third_party_enabled_(top_level_site),
nonce_(nonce ? absl::make_optional(*nonce) : absl::nullopt),
ancestor_chain_bit_(
blink::StorageKey::IsThirdPartyStoragePartitioningEnabled()
? ancestor_chain_bit
: mojom::blink::AncestorChainBit::kSameSite) {
: mojom::blink::AncestorChainBit::kSameSite),
ancestor_chain_bit_if_third_party_enabled_(ancestor_chain_bit) {
DCHECK(origin_);
}

Expand All @@ -87,15 +89,29 @@ BlinkStorageKey BlinkStorageKey::CreateFromStringForTesting(
BlinkStorageKey::BlinkStorageKey(const StorageKey& storage_key)
: BlinkStorageKey(
SecurityOrigin::CreateFromUrlOrigin(storage_key.origin()),
BlinkSchemefulSite(storage_key.top_level_site()),
BlinkSchemefulSite(
storage_key.CopyWithForceEnabledThirdPartyStoragePartitioning()
.top_level_site()),
storage_key.nonce() ? &storage_key.nonce().value() : nullptr,
storage_key.nonce() ? mojom::blink::AncestorChainBit::kSameSite
: storage_key.ancestor_chain_bit()) {}
storage_key.nonce()
? mojom::blink::AncestorChainBit::kSameSite
: storage_key.CopyWithForceEnabledThirdPartyStoragePartitioning()
.ancestor_chain_bit()) {
// We use `CopyWithForceEnabledThirdPartyStoragePartitioning` to preserve the
// partitioned values. The constructor on the other side restores the default
// values if `kThirdPartyStoragePartitioning` is disabled.
}

BlinkStorageKey::operator StorageKey() const {
// We use `top_level_site_if_third_party_enabled_` and
// `ancestor_chain_bit_if_third_party_enabled_` to preserve the partitioned
// values. The constructor on the other side restores the default values if
// `kThirdPartyStoragePartitioning` is disabled.
return StorageKey::CreateWithOptionalNonce(
origin_->ToUrlOrigin(), static_cast<net::SchemefulSite>(top_level_site_),
base::OptionalOrNullptr(nonce_), ancestor_chain_bit_);
origin_->ToUrlOrigin(),
static_cast<net::SchemefulSite>(top_level_site_if_third_party_enabled_),
base::OptionalOrNullptr(nonce_),
ancestor_chain_bit_if_third_party_enabled_);
}

String BlinkStorageKey::ToDebugString() const {
Expand Down
27 changes: 26 additions & 1 deletion third_party/blink/renderer/platform/storage/blink_storage_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,39 @@ class PLATFORM_EXPORT BlinkStorageKey {

String ToDebugString() const;

// Returns a copy of what this storage key would have been if
// `kThirdPartyStoragePartitioning` were enabled. This is a convenience
// function for callsites that benefit from future functionality.
// TODO(crbug.com/1159586): Remove when no longer needed.
BlinkStorageKey CopyWithForceEnabledThirdPartyStoragePartitioning() const {
BlinkStorageKey storage_key = *this;
storage_key.top_level_site_ =
storage_key.top_level_site_if_third_party_enabled_;
storage_key.ancestor_chain_bit_ =
storage_key.ancestor_chain_bit_if_third_party_enabled_;
return storage_key;
}

private:
BlinkStorageKey(scoped_refptr<const SecurityOrigin> origin,
const base::UnguessableToken* nonce);

scoped_refptr<const SecurityOrigin> origin_;
BlinkSchemefulSite top_level_site_;
// Stores the value `top_level_site_` would have had if
// `kThirdPartyStoragePartitioning` were enabled. This isn't used in
// serialization or comparison.
// TODO(crbug.com/1159586): Remove when no longer needed.
BlinkSchemefulSite top_level_site_if_third_party_enabled_;
absl::optional<base::UnguessableToken> nonce_;
mojom::blink::AncestorChainBit ancestor_chain_bit_;
mojom::blink::AncestorChainBit ancestor_chain_bit_{
mojom::blink::AncestorChainBit::kSameSite};
// Stores the value `ancestor_chain_bit_` would have had if
// `kThirdPartyStoragePartitioning` were enabled. This isn't used in
// serialization or comparison.
// TODO(crbug.com/1159586): Remove when no longer needed.
mojom::blink::AncestorChainBit ancestor_chain_bit_if_third_party_enabled_{
mojom::blink::AncestorChainBit::kSameSite};
};

PLATFORM_EXPORT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,15 @@ struct PLATFORM_EXPORT
return input.GetSecurityOrigin();
}

static const blink::BlinkSchemefulSite& top_level_site(
// TODO(crbug.com/1159586): Return by reference when internal copy is removed.
static const blink::BlinkSchemefulSite top_level_site(
const blink::BlinkStorageKey& input) {
return input.GetTopLevelSite();
// We use `CopyWithForceEnabledThirdPartyStoragePartitioning` to ensure the
// partitioned values are preserved. The constructor on the other side will
// properly restore `top_level_site_` as derived from origin_ if
// `kThirdPartyStoragePartitioning` is disabled.
return input.CopyWithForceEnabledThirdPartyStoragePartitioning()
.GetTopLevelSite();
}

static const absl::optional<base::UnguessableToken>& nonce(
Expand All @@ -43,7 +49,12 @@ struct PLATFORM_EXPORT

static blink::mojom::blink::AncestorChainBit ancestor_chain_bit(
const blink::BlinkStorageKey& input) {
return input.GetAncestorChainBit();
// We use `CopyWithForceEnabledThirdPartyStoragePartitioning` to ensure the
// partitioned values are preserved. The constructor on the other side will
// properly restore `ancestor_chain_bit_` to be `kSameSite` if
// `kThirdPartyStoragePartitioning` is disabled.
return input.CopyWithForceEnabledThirdPartyStoragePartitioning()
.GetAncestorChainBit();
}

static bool Read(blink::mojom::StorageKeyDataView data,
Expand Down

0 comments on commit 1aada6b

Please sign in to comment.