Skip to content

Commit

Permalink
HashTraits<scoped_refptr<const SecurityOrigin>>
Browse files Browse the repository at this point in the history
This makes the original SecurityOriginHashTraits the default
HashTraits for scoped_refptr<const SecurityOrigin>. The definition is
moved into security_origin.h to prevent errors where
security_origin_hash.h is not included.

This fixes an issue of
blink.mojom.URLLoaderFactoryBundle.isolated_world_factories which
previously used the generic HashTraits for scoped_refptr for
scoped_refptr<const SecurityOrigin>. The issue didn't seem to cause
any real problem because the HashMap mapping seemed not used.
Nevertheless, this CL can prevent future issues like this.

Bug: 1374475
Change-Id: Ifbbe523e3aa14b1224131bdb97a15514161e3118
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4183217
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1095955}
  • Loading branch information
wangxianzhu authored and Chromium LUCI CQ committed Jan 24, 2023
1 parent 7f4305c commit c4c3649
Show file tree
Hide file tree
Showing 12 changed files with 71 additions and 117 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "third_party/blink/renderer/platform/heap/persistent.h"
#include "third_party/blink/renderer/platform/weborigin/scheme_registry.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin_hash.h"
#include "third_party/blink/renderer/platform/wtf/hash_functions.h"
#include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
#include "third_party/blink/renderer/platform/wtf/text/string_hash.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_map.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/heap/member.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin_hash.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"

namespace blink {
Expand Down Expand Up @@ -77,18 +77,14 @@ class WindowAgentFactory final : public GarbageCollected<WindowAgentFactory> {
WeakMember<WindowAgent> file_url_agent_;

// Use the SecurityOrigin itself as the key for opaque origins.
HeapHashMap<scoped_refptr<const SecurityOrigin>,
WeakMember<WindowAgent>,
SecurityOriginHashTraits>
HeapHashMap<scoped_refptr<const SecurityOrigin>, WeakMember<WindowAgent>>
opaque_origin_agents_;

// Use the SecurityOrigin itself as the key for origin-keyed origins.
// TODO(wjmaclean,domenic): In future when logical cross-origin-isolation
// (COI) is implemented, we should unify it with logical-OAC so that all the
// origin-keyed isolation relies on a single mechanism.
HeapHashMap<scoped_refptr<const SecurityOrigin>,
WeakMember<WindowAgent>,
SecurityOriginHashTraits>
HeapHashMap<scoped_refptr<const SecurityOrigin>, WeakMember<WindowAgent>>
origin_keyed_agent_cluster_agents_;

// Use registerable domain as the key for general tuple origins.
Expand Down
3 changes: 1 addition & 2 deletions third_party/blink/renderer/core/frame/local_frame_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ class CORE_EXPORT LocalFrameClient : public FrameClient {
// Notifies the observers of the origins for which subresource redirect
// optimizations can be preloaded.
virtual void PreloadSubresourceOptimizationsForOrigins(
const WTF::HashSet<scoped_refptr<const SecurityOrigin>,
SecurityOriginHashTraits>& origins) {}
const WTF::HashSet<scoped_refptr<const SecurityOrigin>>& origins) {}

// Transmits the change in the set of watched CSS selectors property that
// match any element on the frame.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -774,8 +774,7 @@ void LocalFrameClientImpl::DidObserveLayoutShift(double score,
}

void LocalFrameClientImpl::PreloadSubresourceOptimizationsForOrigins(
const WTF::HashSet<scoped_refptr<const SecurityOrigin>,
SecurityOriginHashTraits>& origins) {
const WTF::HashSet<scoped_refptr<const SecurityOrigin>>& origins) {
if (WebLocalFrameClient* client = web_frame_->Client()) {
std::vector<WebSecurityOrigin> origins_list;
for (const auto& origin : origins) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ class CORE_EXPORT LocalFrameClientImpl final : public LocalFrameClient {
void DidObserveSoftNavigation(uint32_t count) override;
void DidObserveLayoutShift(double score, bool after_input_or_scroll) override;
void PreloadSubresourceOptimizationsForOrigins(
const WTF::HashSet<scoped_refptr<const SecurityOrigin>,
SecurityOriginHashTraits>& origins) override;
const WTF::HashSet<scoped_refptr<const SecurityOrigin>>& origins)
override;
void SelectorMatchChanged(const Vector<String>& added_selectors,
const Vector<String>& removed_selectors) override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin_hash.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
#include "third_party/blink/renderer/platform/wtf/vector.h"
#include "v8/include/v8-primitive.h"
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/renderer/platform/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1496,7 +1496,6 @@ component("platform") {
"weborigin/scheme_registry.h",
"weborigin/security_origin.cc",
"weborigin/security_origin.h",
"weborigin/security_origin_hash.h",
"weborigin/security_policy.cc",
"weborigin/security_policy.h",
"webrtc/convert_to_webrtc_video_frame_buffer.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "base/memory/scoped_refptr.h"
#include "third_party/blink/renderer/platform/platform_export.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin_hash.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"

Expand Down Expand Up @@ -120,8 +119,7 @@ namespace WTF {
template <>
struct HashTraits<blink::BlinkSchemefulSite>
: OneFieldHashTraits<blink::BlinkSchemefulSite,
&blink::BlinkSchemefulSite::site_as_origin_,
blink::SecurityOriginHashTraits> {};
&blink::BlinkSchemefulSite::site_as_origin_> {};

} // namespace WTF

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "build/build_config.h"
#include "third_party/blink/renderer/platform/storage/blink_storage_key.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin_hash.h"

namespace blink {

Expand All @@ -18,7 +17,7 @@ struct BlinkStorageKeyHashTraits
absl::optional<base::UnguessableToken> nonce = storage_key->GetNonce();
size_t nonce_hash = nonce ? base::UnguessableTokenHash()(*nonce) : 0;
unsigned hash_codes[] = {
SecurityOriginHashTraits::GetHash(storage_key->GetSecurityOrigin()),
WTF::GetHash(storage_key->GetSecurityOrigin()),
WTF::GetHash(storage_key->GetTopLevelSite()),
static_cast<unsigned>(storage_key->GetAncestorChainBit()),
#if ARCH_CPU_32_BITS
Expand Down
59 changes: 58 additions & 1 deletion third_party/blink/renderer/platform/weborigin/security_origin.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/renderer/platform/platform_export.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
#include "third_party/blink/renderer/platform/wtf/hash_traits.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
#include "third_party/blink/renderer/platform/wtf/thread_safe_ref_counted.h"
#include "url/origin.h"
Expand Down Expand Up @@ -382,7 +383,7 @@ class PLATFORM_EXPORT SecurityOrigin : public RefCounted<SecurityOrigin> {
private:
// Various serialisation and test routines that need direct nonce access.
friend struct mojo::UrlOriginAdapter;
friend struct SecurityOriginHashTraits;
friend struct WTF::HashTraits<scoped_refptr<const SecurityOrigin>>;
friend class SecurityOriginTest;

// For calling GetNonceForSerialization().
Expand Down Expand Up @@ -453,4 +454,60 @@ class PLATFORM_EXPORT SecurityOrigin : public RefCounted<SecurityOrigin> {

} // namespace blink

namespace WTF {

// The default HashTraits of SecurityOrigin implements the "same origin"
// equality relation between two origins. As such it ignores the domain that
// might or might not be set on the origin. If you need "same origin-domain"
// equality you'll need to define a custom hash traits type using a different
// hash function.
template <>
struct HashTraits<scoped_refptr<const blink::SecurityOrigin>>
: GenericHashTraits<scoped_refptr<const blink::SecurityOrigin>> {
static unsigned GetHash(const blink::SecurityOrigin* origin) {
const base::UnguessableToken* nonce = origin->GetNonceForSerialization();
size_t nonce_hash = nonce ? base::UnguessableTokenHash()(*nonce) : 0;

unsigned hash_codes[] = {
origin->Protocol().Impl() ? origin->Protocol().Impl()->GetHash() : 0,
origin->Host().Impl() ? origin->Host().Impl()->GetHash() : 0,
origin->Port(),
#if ARCH_CPU_32_BITS
nonce_hash,
#elif ARCH_CPU_64_BITS
static_cast<unsigned>(nonce_hash),
static_cast<unsigned>(nonce_hash >> 32),
#else
#error "Unknown bits"
#endif
};
return StringHasher::HashMemory<sizeof(hash_codes)>(hash_codes);
}
static unsigned GetHash(
const scoped_refptr<const blink::SecurityOrigin>& origin) {
return GetHash(origin.get());
}

static bool Equal(const blink::SecurityOrigin* a,
const blink::SecurityOrigin* b) {
return a->IsSameOriginWith(b);
}
static bool Equal(const blink::SecurityOrigin* a,
const scoped_refptr<const blink::SecurityOrigin>& b) {
return Equal(a, b.get());
}
static bool Equal(const scoped_refptr<const blink::SecurityOrigin>& a,
const blink::SecurityOrigin* b) {
return Equal(a.get(), b);
}
static bool Equal(const scoped_refptr<const blink::SecurityOrigin>& a,
const scoped_refptr<const blink::SecurityOrigin>& b) {
return Equal(a.get(), b.get());
}

static constexpr bool kSafeToCompareToEmptyOrDeleted = false;
};

} // namespace WTF

#endif // THIRD_PARTY_BLINK_RENDERER_PLATFORM_WEBORIGIN_SECURITY_ORIGIN_H_

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
#include "third_party/blink/renderer/platform/testing/runtime_enabled_features_test_helpers.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"
#include "third_party/blink/renderer/platform/weborigin/scheme_registry.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin_hash.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
#include "third_party/blink/renderer/platform/weborigin/security_policy.h"
#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
#include "third_party/blink/renderer/platform/wtf/text/string_operators.h"
Expand Down Expand Up @@ -719,9 +719,9 @@ TEST_F(SecurityOriginTest, OpaqueIsolatedCopy) {
scoped_refptr<const SecurityOrigin> copied = origin->IsolatedCopy();
EXPECT_TRUE(origin->CanAccess(copied.get()));
EXPECT_TRUE(origin->IsSameOriginWith(copied.get()));
EXPECT_EQ(SecurityOriginHashTraits::GetHash(origin),
SecurityOriginHashTraits::GetHash(copied));
EXPECT_TRUE(SecurityOriginHashTraits::Equal(origin, copied));
EXPECT_EQ(WTF::GetHash(origin), WTF::GetHash(copied));
EXPECT_TRUE(
HashTraits<scoped_refptr<const SecurityOrigin>>::Equal(origin, copied));
}

TEST_F(SecurityOriginTest, EdgeCases) {
Expand Down

0 comments on commit c4c3649

Please sign in to comment.