From 74138b9febd37eac0fc26b8efb110014a83a52c6 Mon Sep 17 00:00:00 2001 From: Jeremy Roman Date: Wed, 7 Aug 2019 13:26:48 +0000 Subject: [PATCH] WTF: Make LinkedHashSet understand values for which memset initialization would be bad. Includes a unit test which fails before, and uses this to fix FontCacheKeyTraits. Bug: 980025 Change-Id: If41f97444c7fd37b9b95d6dadaf3da5689079e9e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1739948 Reviewed-by: Kentaro Hara Reviewed-by: Yutaka Hirano Commit-Queue: Jeremy Roman Cr-Commit-Position: refs/heads/master@{#684731} --- .../renderer/platform/fonts/font_cache_key.h | 4 ++ .../renderer/platform/wtf/linked_hash_set.h | 10 ++++- .../platform/wtf/list_hash_set_test.cc | 45 +++++++++++++++++-- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/third_party/blink/renderer/platform/fonts/font_cache_key.h b/third_party/blink/renderer/platform/fonts/font_cache_key.h index 0efc8fb909..90063cb2ea 100644 --- a/third_party/blink/renderer/platform/fonts/font_cache_key.h +++ b/third_party/blink/renderer/platform/fonts/font_cache_key.h @@ -133,6 +133,10 @@ struct FontCacheKeyHash { struct FontCacheKeyTraits : WTF::SimpleClassHashTraits { STATIC_ONLY(FontCacheKeyTraits); + + // std::string's empty state need not be zero in all implementations, + // and it is held within FontFaceCreationParams. + static const bool kEmptyValueIsZero = false; }; } // namespace blink diff --git a/third_party/blink/renderer/platform/wtf/linked_hash_set.h b/third_party/blink/renderer/platform/wtf/linked_hash_set.h index b35b6e95f1..77e524c084 100644 --- a/third_party/blink/renderer/platform/wtf/linked_hash_set.h +++ b/third_party/blink/renderer/platform/wtf/linked_hash_set.h @@ -146,6 +146,11 @@ class LinkedHashSetNode : public LinkedHashSetNodeBase { LinkedHashSetNodeBase* next) : LinkedHashSetNodeBase(prev, next), value_(value) {} + LinkedHashSetNode(ValueArg&& value, + LinkedHashSetNodeBase* prev, + LinkedHashSetNodeBase* next) + : LinkedHashSetNodeBase(prev, next), value_(std::move(value)) {} + LinkedHashSetNode(LinkedHashSetNode&& other) : LinkedHashSetNodeBase(std::move(other)), value_(std::move(other.value_)) {} @@ -445,10 +450,13 @@ struct LinkedHashSetTraits // The slot is empty when the next_ field is zero so it's safe to zero // the backing. - static const bool kEmptyValueIsZero = true; + static const bool kEmptyValueIsZero = ValueTraits::kEmptyValueIsZero; static const bool kHasIsEmptyValueFunction = true; static bool IsEmptyValue(const Node& node) { return !node.next_; } + static Node EmptyValue() { + return Node(ValueTraits::EmptyValue(), nullptr, nullptr); + } static const int kDeletedValue = -1; diff --git a/third_party/blink/renderer/platform/wtf/list_hash_set_test.cc b/third_party/blink/renderer/platform/wtf/list_hash_set_test.cc index 4c3f8990b0..cd1be0089b 100644 --- a/third_party/blink/renderer/platform/wtf/list_hash_set_test.cc +++ b/third_party/blink/renderer/platform/wtf/list_hash_set_test.cc @@ -487,6 +487,7 @@ struct Simple { }; struct Complicated { + Complicated() : Complicated(0) {} Complicated(int value) : simple_(value) { objects_constructed_++; } Complicated(const Complicated& other) : simple_(other.simple_) { @@ -495,9 +496,6 @@ struct Complicated { Simple simple_; static int objects_constructed_; - - private: - Complicated() = delete; }; int Complicated::objects_constructed_ = 0; @@ -731,4 +729,45 @@ TYPED_TEST(ListOrLinkedHashSetMoveOnlyTest, MoveOnlyValue) { } // anonymous namespace +// A unit type which objects to its state being initialized wrong. +struct InvalidZeroValue { + InvalidZeroValue() = default; + InvalidZeroValue(WTF::HashTableDeletedValueType) : deleted_(true) {} + ~InvalidZeroValue() { CHECK(ok_); } + bool IsHashTableDeletedValue() const { return deleted_; } + + bool ok_ = true; + bool deleted_ = false; +}; + +template <> +struct HashTraits : SimpleClassHashTraits { + static const bool kEmptyValueIsZero = false; +}; + +template <> +struct DefaultHash { + struct Hash { + static unsigned GetHash(const InvalidZeroValue&) { return 0; } + static bool Equal(const InvalidZeroValue&, const InvalidZeroValue&) { + return true; + } + }; +}; + +template +class ListOrLinkedHashSetInvalidZeroTest : public testing::Test {}; + +using InvalidZeroValueSetTypes = + testing::Types, + ListHashSet, + LinkedHashSet>; +TYPED_TEST_SUITE(ListOrLinkedHashSetInvalidZeroTest, InvalidZeroValueSetTypes); + +TYPED_TEST(ListOrLinkedHashSetInvalidZeroTest, InvalidZeroValue) { + using Set = TypeParam; + Set set; + set.insert(InvalidZeroValue()); +} + } // namespace WTF