diff options
Diffstat (limited to 'chromium-125-debian-bad-font-gc11.patch')
| -rw-r--r-- | chromium-125-debian-bad-font-gc11.patch | 420 |
1 files changed, 420 insertions, 0 deletions
diff --git a/chromium-125-debian-bad-font-gc11.patch b/chromium-125-debian-bad-font-gc11.patch new file mode 100644 index 0000000..da30199 --- /dev/null +++ b/chromium-125-debian-bad-font-gc11.patch @@ -0,0 +1,420 @@ +Revert the following commit: + +commit 2eefeabb12fb7e92f2508116a5ed959c57659be1 +Author: Ian Kilpatrick <ikilpatrick@chromium.org> +Date: Tue Feb 20 17:40:39 2024 +0000 + + [gc] Make HarfBuzzFontData & friends gc'd. + + Previously we had a HbFontCacheEntry which was used to hold onto the + HarfBuzzFontData, and a hb_font_t. + + HarfBuzzFontData is used for holding data specific for various + harfbuzz callbacks, but we can also hold onto the hb_font_t there. + + There should be no user-visible behaviour change. + + Bug: 41490008 + Change-Id: Icaa7ad3b2f75e9807b88014a9a15406cb76eb52e + Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5302175 + Reviewed-by: Dominik Röttsches <drott@chromium.org> + Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> + Cr-Commit-Position: refs/heads/main@{#1262752} + +--- a/third_party/blink/renderer/platform/fonts/font_global_context.cc ++++ b/third_party/blink/renderer/platform/fonts/font_global_context.cc +@@ -8,6 +8,7 @@ + #include "third_party/blink/renderer/platform/fonts/font_cache.h" + #include "third_party/blink/renderer/platform/fonts/font_unique_name_lookup.h" + #include "third_party/blink/renderer/platform/fonts/shaping/harfbuzz_face.h" ++#include "third_party/blink/renderer/platform/fonts/shaping/harfbuzz_font_cache.h" + #include "third_party/blink/renderer/platform/privacy_budget/identifiability_digest_helpers.h" + #include "third_party/blink/renderer/platform/wtf/thread_specific.h" + +@@ -50,6 +51,15 @@ FontUniqueNameLookup* FontGlobalContext: + return Get().font_unique_name_lookup_.get(); + } + ++HarfBuzzFontCache& FontGlobalContext::GetHarfBuzzFontCache() { ++ std::unique_ptr<HarfBuzzFontCache>& global_context_harfbuzz_font_cache = ++ Get().harfbuzz_font_cache_; ++ if (!global_context_harfbuzz_font_cache) { ++ global_context_harfbuzz_font_cache = std::make_unique<HarfBuzzFontCache>(); ++ } ++ return *global_context_harfbuzz_font_cache; ++} ++ + IdentifiableToken FontGlobalContext::GetOrComputeTypefaceDigest( + const FontPlatformData& source) { + SkTypeface* typeface = source.Typeface(); +--- a/third_party/blink/renderer/platform/fonts/font_global_context.h ++++ b/third_party/blink/renderer/platform/fonts/font_global_context.h +@@ -9,7 +9,6 @@ + #include "base/types/pass_key.h" + #include "third_party/blink/public/common/privacy_budget/identifiable_token.h" + #include "third_party/blink/renderer/platform/fonts/font_cache.h" +-#include "third_party/blink/renderer/platform/fonts/shaping/harfbuzz_font_cache.h" + #include "third_party/blink/renderer/platform/platform_export.h" + #include "third_party/blink/renderer/platform/text/layout_locale.h" + #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h" +@@ -34,19 +33,14 @@ class PLATFORM_EXPORT FontGlobalContext + static FontGlobalContext& Get(); + static FontGlobalContext* TryGet(); + +- void Trace(Visitor* visitor) const { +- visitor->Trace(font_cache_); +- visitor->Trace(harfbuzz_font_cache_); +- } ++ void Trace(Visitor* visitor) const { visitor->Trace(font_cache_); } + + FontGlobalContext(const FontGlobalContext&) = delete; + FontGlobalContext& operator=(const FontGlobalContext&) = delete; + + static inline FontCache& GetFontCache() { return Get().font_cache_; } + +- static HarfBuzzFontCache& GetHarfBuzzFontCache() { +- return Get().harfbuzz_font_cache_; +- } ++ static HarfBuzzFontCache& GetHarfBuzzFontCache(); + + static FontUniqueNameLookup* GetFontUniqueNameLookup(); + +@@ -62,7 +56,7 @@ class PLATFORM_EXPORT FontGlobalContext + + private: + FontCache font_cache_; +- HarfBuzzFontCache harfbuzz_font_cache_; ++ std::unique_ptr<HarfBuzzFontCache> harfbuzz_font_cache_; + std::unique_ptr<FontUniqueNameLookup> font_unique_name_lookup_; + base::HashingLRUCache<SkTypefaceID, IdentifiableToken> typeface_digest_cache_; + base::HashingLRUCache<SkTypefaceID, IdentifiableToken> +--- a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_face.cc ++++ b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_face.cc +@@ -65,14 +65,20 @@ + + HarfBuzzFace::HarfBuzzFace(const FontPlatformData* platform_data, + uint64_t unique_id) +- : platform_data_(platform_data), +- harfbuzz_font_data_(FontGlobalContext::GetHarfBuzzFontCache().GetOrCreate( +- unique_id, +- platform_data)) {} ++ : platform_data_(platform_data), unique_id_(unique_id) { ++ HbFontCacheEntry* const cache_entry = ++ FontGlobalContext::GetHarfBuzzFontCache().RefOrNew(unique_id_, ++ platform_data); ++ unscaled_font_ = cache_entry->HbFont(); ++ harfbuzz_font_data_ = cache_entry->HbFontData(); ++} ++ ++HarfBuzzFace::~HarfBuzzFace() { ++ FontGlobalContext::GetHarfBuzzFontCache().Remove(unique_id_); ++} + + void HarfBuzzFace::Trace(Visitor* visitor) const { + visitor->Trace(platform_data_); +- visitor->Trace(harfbuzz_font_data_); + } + + bool HarfBuzzFace::ignore_variation_selectors_ = false; +@@ -234,17 +240,14 @@ bool HarfBuzzFace::HasSpaceInLigaturesOr + + hb::unique_ptr<hb_set_t> glyphs(hb_set_create()); + +- hb_font_t* unscaled_font = harfbuzz_font_data_->unscaled_font_.get(); +- + // Check whether computing is needed and compute for gpos/gsub. + if (features & kKerning && + harfbuzz_font_data_->space_in_gpos_ == + HarfBuzzFontData::SpaceGlyphInOpenTypeTables::kUnknown) { +- if (space == kInvalidCodepoint && !GetSpaceGlyph(unscaled_font, space)) { ++ if (space == kInvalidCodepoint && !GetSpaceGlyph(unscaled_font_, space)) + return false; +- } + // Compute for gpos. +- hb_face_t* face = hb_font_get_face(unscaled_font); ++ hb_face_t* face = hb_font_get_face(unscaled_font_); + DCHECK(face); + harfbuzz_font_data_->space_in_gpos_ = + hb_ot_layout_has_positioning(face) && +@@ -258,11 +261,10 @@ bool HarfBuzzFace::HasSpaceInLigaturesOr + if (features & kLigatures && + harfbuzz_font_data_->space_in_gsub_ == + HarfBuzzFontData::SpaceGlyphInOpenTypeTables::kUnknown) { +- if (space == kInvalidCodepoint && !GetSpaceGlyph(unscaled_font, space)) { ++ if (space == kInvalidCodepoint && !GetSpaceGlyph(unscaled_font_, space)) + return false; +- } + // Compute for gpos. +- hb_face_t* face = hb_font_get_face(unscaled_font); ++ hb_face_t* face = hb_font_get_face(unscaled_font_); + DCHECK(face); + harfbuzz_font_data_->space_in_gsub_ = + hb_ot_layout_has_substitution(face) && +@@ -280,14 +282,14 @@ bool HarfBuzzFace::HasSpaceInLigaturesOr + } + + unsigned HarfBuzzFace::UnitsPerEmFromHeadTable() { +- hb_face_t* face = hb_font_get_face(harfbuzz_font_data_->unscaled_font_.get()); ++ hb_face_t* face = hb_font_get_face(unscaled_font_); + return hb_face_get_upem(face); + } + + Glyph HarfBuzzFace::HbGlyphForCharacter(UChar32 character) { + hb_codepoint_t glyph = 0; +- HarfBuzzGetNominalGlyph(harfbuzz_font_data_->unscaled_font_.get(), +- harfbuzz_font_data_, character, &glyph, nullptr); ++ HarfBuzzGetNominalGlyph(unscaled_font_, harfbuzz_font_data_, character, ++ &glyph, nullptr); + return glyph; + } + +@@ -329,7 +331,7 @@ hb_codepoint_t HarfBuzzFace::HarfBuzzGet + UChar32 variation_selector) { + DCHECK(RuntimeEnabledFeatures::FontVariationSequencesEnabled()); + hb_codepoint_t glyph = 0; +- HarfBuzzGetGlyph(harfbuzz_font_data_->unscaled_font_.get(), ++ HarfBuzzGetGlyph(unscaled_font_, + harfbuzz_font_data_, character, variation_selector, &glyph, + nullptr); + return glyph; +@@ -444,10 +446,9 @@ static hb::unique_ptr<hb_face_t> CreateF + return face; + } + +-namespace { +- +-HarfBuzzFontData* CreateHarfBuzzFontData(hb_face_t* face, +- SkTypeface* typeface) { ++static scoped_refptr<HbFontCacheEntry> CreateHbFontCacheEntry( ++ hb_face_t* face, ++ SkTypeface* typeface) { + hb::unique_ptr<hb_font_t> ot_font(hb_font_create(face)); + hb_ot_font_set_funcs(ot_font.get()); + +@@ -466,26 +467,25 @@ HarfBuzzFontData* CreateHarfBuzzFontData + // Creating a sub font means that non-available functions + // are found from the parent. + hb_font_t* const unscaled_font = hb_font_create_sub_font(ot_font.get()); +- HarfBuzzFontData* data = +- MakeGarbageCollected<HarfBuzzFontData>(unscaled_font); ++ scoped_refptr<HbFontCacheEntry> cache_entry = ++ HbFontCacheEntry::Create(unscaled_font); + hb_font_set_funcs(unscaled_font, +- HarfBuzzSkiaFontFuncs::Get().GetFunctions(typeface), data, +- nullptr); +- return data; ++ HarfBuzzSkiaFontFuncs::Get().GetFunctions(typeface), ++ cache_entry->HbFontData(), nullptr); ++ return cache_entry; + } + +-} // namespace +- +-HarfBuzzFontData* HarfBuzzFontCache::GetOrCreate( ++HbFontCacheEntry* HarfBuzzFontCache::RefOrNew( + uint64_t unique_id, + const FontPlatformData* platform_data) { + const auto& result = font_map_.insert(unique_id, nullptr); + if (result.is_new_entry) { + hb::unique_ptr<hb_face_t> face = CreateFace(platform_data); + result.stored_value->value = +- CreateHarfBuzzFontData(face.get(), platform_data->Typeface()); ++ CreateHbFontCacheEntry(face.get(), platform_data->Typeface()); + } +- return result.stored_value->value.Get(); ++ result.stored_value->value->AddRef(); ++ return result.stored_value->value.get(); + } + + static_assert( +@@ -516,18 +516,17 @@ hb_font_t* HarfBuzzFace::GetScaledFont(s + vertical_layout); + + int scale = SkiaScalarToHarfBuzzPosition(platform_data_->size()); +- hb_font_t* unscaled_font = harfbuzz_font_data_->unscaled_font_.get(); +- hb_font_set_scale(unscaled_font, scale, scale); ++ hb_font_set_scale(unscaled_font_, scale, scale); + // See contended discussion in https://github.com/harfbuzz/harfbuzz/pull/1484 + // Setting ptem here is critical for HarfBuzz to know where to lookup spacing + // offset in the AAT trak table, the unit pt in ptem here means "CoreText" + // points. After discussion on the pull request and with Apple developers, the + // meaning of HarfBuzz' hb_font_set_ptem API was changed to expect the + // equivalent of CSS pixels here. +- hb_font_set_ptem(unscaled_font, specified_size > 0 ? specified_size +- : platform_data_->size()); ++ hb_font_set_ptem(unscaled_font_, specified_size > 0 ? specified_size ++ : platform_data_->size()); + +- return unscaled_font; ++ return unscaled_font_; + } + + hb_font_t* HarfBuzzFace::GetScaledFont() const { +--- a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_face.h ++++ b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_face.h +@@ -55,6 +55,7 @@ class HarfBuzzFace final : public Garbag + HarfBuzzFace(const FontPlatformData* platform_data, uint64_t); + HarfBuzzFace(const HarfBuzzFace&) = delete; + HarfBuzzFace& operator=(const HarfBuzzFace&) = delete; ++ ~HarfBuzzFace(); + + void Trace(Visitor*) const; + +@@ -106,7 +106,11 @@ + void PrepareHarfBuzzFontData(); + + Member<const FontPlatformData> platform_data_; +- Member<HarfBuzzFontData> harfbuzz_font_data_; ++ const uint64_t unique_id_; ++ // TODO(crbug.com/1489080): When briefly given MiraclePtr protection, ++ // these members were both found dangling. ++ hb_font_t* unscaled_font_; ++ HarfBuzzFontData* harfbuzz_font_data_; + static bool ignore_variation_selectors_; + }; + +--- a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_font_cache.cc ++++ b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_font_cache.cc +@@ -8,8 +8,38 @@ + + namespace blink { + +-void HarfBuzzFontCache::Trace(Visitor* visitor) const { +- visitor->Trace(font_map_); ++HbFontCacheEntry::HbFontCacheEntry(hb_font_t* font) ++ : hb_font_(hb::unique_ptr<hb_font_t>(font)), ++ hb_font_data_(std::make_unique<HarfBuzzFontData>()) {} ++ ++HbFontCacheEntry::~HbFontCacheEntry() = default; ++ ++scoped_refptr<HbFontCacheEntry> HbFontCacheEntry::Create(hb_font_t* hb_font) { ++ DCHECK(hb_font); ++ return base::AdoptRef(new HbFontCacheEntry(hb_font)); ++} ++ ++HarfBuzzFontCache::HarfBuzzFontCache() = default; ++HarfBuzzFontCache::~HarfBuzzFontCache() = default; ++ ++// See "harfbuzz_face.cc" for |HarfBuzzFontCache::GetOrCreateFontData()| ++// implementation. ++ ++void HarfBuzzFontCache::Remove(uint64_t unique_id) { ++ auto it = font_map_.find(unique_id); ++ // TODO(https://crbug.com/1417160): In tests such as FontObjectThreadedTest ++ // that test taking down FontGlobalContext an object may not be found due to ++ // existing issues with refcounting of font objects at thread destruction ++ // time. ++ if (it == font_map_.end()) { ++ return; ++ } ++ DCHECK(!it.Get()->value->HasOneRef()); ++ it.Get()->value->Release(); ++ if (!it.Get()->value->HasOneRef()) { ++ return; ++ } ++ font_map_.erase(it); + } + + } // namespace blink +--- a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_font_cache.h ++++ b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_font_cache.h +@@ -6,9 +6,12 @@ + #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_FONTS_SHAPING_HARFBUZZ_FONT_CACHE_H_ + + #include "third_party/blink/renderer/platform/fonts/font_metrics.h" +-#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/fonts/unicode_range_set.h" ++ ++#include <hb.h> ++#include <hb-cplusplus.hh> ++ ++#include <memory> + + namespace blink { + +@@ -22,21 +25,39 @@ struct HarfBuzzFontData; + // FIXME, crbug.com/609099: We should fix the FontCache to only keep one + // FontPlatformData object independent of size, then consider using this here. + +-class HarfBuzzFontCache final { +- DISALLOW_NEW(); ++class HbFontCacheEntry : public RefCounted<HbFontCacheEntry> { ++ USING_FAST_MALLOC(HbFontCacheEntry); ++ ++ public: ++ static scoped_refptr<HbFontCacheEntry> Create(hb_font_t* hb_font); ++ ++ hb_font_t* HbFont() { return hb_font_.get(); } ++ HarfBuzzFontData* HbFontData() { return hb_font_data_.get(); } ++ ++ ~HbFontCacheEntry(); + ++ private: ++ explicit HbFontCacheEntry(hb_font_t* font); ++ ++ hb::unique_ptr<hb_font_t> hb_font_; ++ std::unique_ptr<HarfBuzzFontData> hb_font_data_; ++}; ++ ++class HarfBuzzFontCache final { + public: +- void Trace(Visitor* visitor) const; +- // See "harfbuzz_face.cc" for |HarfBuzzFontCache::GetOrCreateFontData()| +- // implementation. +- HarfBuzzFontData* GetOrCreate(uint64_t unique_id, +- const FontPlatformData* platform_data); ++ HarfBuzzFontCache(); ++ ~HarfBuzzFontCache(); ++ ++ HbFontCacheEntry* RefOrNew(uint64_t unique_id, ++ const FontPlatformData* platform_data); ++ void Remove(uint64_t unique_id); + + private: +- HeapHashMap<uint64_t, +- WeakMember<HarfBuzzFontData>, +- IntWithZeroKeyHashTraits<uint64_t>> +- font_map_; ++ using HbFontDataMap = HashMap<uint64_t, ++ scoped_refptr<HbFontCacheEntry>, ++ IntWithZeroKeyHashTraits<uint64_t>>; ++ ++ HbFontDataMap font_map_; + }; + + } // namespace blink +--- a/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_font_data.h ++++ b/third_party/blink/renderer/platform/fonts/shaping/harfbuzz_font_data.h +@@ -22,18 +22,15 @@ const unsigned kInvalidFallbackMetricsVa + // The HarfBuzzFontData struct carries user-pointer data for + // |hb_font_t| callback functions/operations. It contains metrics and OpenType + // layout information related to a font scaled to a particular size. +-struct HarfBuzzFontData final : public GarbageCollected<HarfBuzzFontData> { ++struct HarfBuzzFontData final { ++ USING_FAST_MALLOC(HarfBuzzFontData); ++ + public: +- explicit HarfBuzzFontData(hb_font_t* unscaled_font) +- : unscaled_font_(hb::unique_ptr<hb_font_t>(unscaled_font)), +- vertical_data_(nullptr), +- range_set_(nullptr) {} ++ HarfBuzzFontData() : vertical_data_(nullptr), range_set_(nullptr) {} + + HarfBuzzFontData(const HarfBuzzFontData&) = delete; + HarfBuzzFontData& operator=(const HarfBuzzFontData&) = delete; + +- void Trace(Visitor*) const {} +- + // The vertical origin and vertical advance functions in HarfBuzzFace require + // the ascent and height metrics as fallback in case no specific vertical + // layout information is found from the font. +@@ -81,7 +78,6 @@ struct HarfBuzzFontData final : public G + return vertical_data_; + } + +- const hb::unique_ptr<hb_font_t> unscaled_font_; + SkFont font_; + + // Capture these scaled fallback metrics from FontPlatformData so that a |