Move FontFaceCache object onto the heap.

This is in preparation for being able to share FontFaceCache between
CSSFontSelectors which share the same @font-face rules via the same set
of StyleSheetContents.

Bug: 336876
Change-Id: I65b0aec9077b54465d74f48a66230679a854346d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2562380
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832524}

third_party/blink/renderer/core/css/css_font_selector.cc

@@ -55,8 +55,9 @@ CSSFontSelector::CSSFontSelector(const TreeScope& tree_scope)
   DCHECK(tree_scope.GetDocument().GetFrame());
   FontCache::GetFontCache()->AddClient(this);
   if (tree_scope.RootNode().IsDocumentNode()) {
+    font_face_cache_ = MakeGarbageCollected<FontFaceCache>();
     FontFaceSetDocument::From(tree_scope.GetDocument())
-        ->AddFontFacesToFontFaceCache(&font_face_cache_);
+        ->AddFontFacesToFontFaceCache(font_face_cache_);
   }
 }
 
@@ -75,7 +76,7 @@ void CSSFontSelector::UnregisterForInvalidationCallbacks(
 
 void CSSFontSelector::DispatchInvalidationCallbacks(
     FontInvalidationReason reason) {
-  font_face_cache_.IncrementVersion();
+  font_face_cache_->IncrementVersion();
 
   HeapVector<Member<FontSelectorClient>> clients;
   CopyToVector(clients_, clients);
@@ -99,7 +100,7 @@ scoped_refptr<FontData> CSSFontSelector::GetFontData(
     const AtomicString& family_name) {
   Document& document = GetTreeScope()->GetDocument();
   if (CSSSegmentedFontFace* face =
-          font_face_cache_.Get(font_description, family_name)) {
+          font_face_cache_->Get(font_description, family_name)) {
     document.GetFontMatchingMetrics()->ReportWebFontFamily(family_name);
     return face->GetFontData(font_description);
   }
@@ -130,7 +131,7 @@ scoped_refptr<FontData> CSSFontSelector::GetFontData(
 void CSSFontSelector::WillUseFontData(const FontDescription& font_description,
                                       const AtomicString& family,
                                       const String& text) {
-  CSSSegmentedFontFace* face = font_face_cache_.Get(font_description, family);
+  CSSSegmentedFontFace* face = font_face_cache_->Get(font_description, family);
   if (face)
     face->WillUseFontData(font_description, text);
 }
@@ -138,7 +139,7 @@ void CSSFontSelector::WillUseFontData(const FontDescription& font_description,
 void CSSFontSelector::WillUseRange(const FontDescription& font_description,
                                    const AtomicString& family,
                                    const FontDataForRangeSet& range_set) {
-  CSSSegmentedFontFace* face = font_face_cache_.Get(font_description, family);
+  CSSSegmentedFontFace* face = font_face_cache_->Get(font_description, family);
   if (face)
     face->WillUseRange(font_description, range_set);
 }

third_party/blink/renderer/core/css/css_font_selector.h

@@ -45,7 +45,7 @@ class CORE_EXPORT CSSFontSelector : public FontSelector {
   explicit CSSFontSelector(const TreeScope&);
   ~CSSFontSelector() override;
 
-  unsigned Version() const override { return font_face_cache_.Version(); }
+  unsigned Version() const override { return font_face_cache_->Version(); }
 
   void ReportNotDefGlyph() const override;
 
@@ -102,7 +102,7 @@ class CORE_EXPORT CSSFontSelector : public FontSelector {
   ExecutionContext* GetExecutionContext() const override {
     return tree_scope_ ? GetDocument().GetExecutionContext() : nullptr;
   }
-  FontFaceCache* GetFontFaceCache() override { return &font_face_cache_; }
+  FontFaceCache* GetFontFaceCache() override { return font_face_cache_; }
 
   const GenericFontFamilySettings& GetGenericFontFamilySettings() const {
     return generic_font_family_settings_;
@@ -125,9 +125,7 @@ class CORE_EXPORT CSSFontSelector : public FontSelector {
   // currently leak because ComputedStyle and its data are not on the heap.
   // See crbug.com/383860 for details.
   WeakMember<const TreeScope> tree_scope_;
-  // TODO(futhark): Make this a Member which can be shared between scopes
-  // sharing the same set of @font-faces.
-  FontFaceCache font_face_cache_;
+  Member<FontFaceCache> font_face_cache_;
   HeapHashSet<WeakMember<FontSelectorClient>> clients_;
   GenericFontFamilySettings generic_font_family_settings_;
 };

third_party/blink/renderer/core/css/font_face_cache.h

@@ -42,9 +42,7 @@ namespace blink {
 
 class FontDescription;
 
-class CORE_EXPORT FontFaceCache final {
-  DISALLOW_NEW();
-
+class CORE_EXPORT FontFaceCache final : public GarbageCollected<FontFaceCache> {
  public:
   FontFaceCache();
 

third_party/blink/renderer/core/css/font_face_cache_test.cc

@@ -41,9 +41,7 @@ class FontFaceCacheTest : public PageTestBase {
                                             FontSelectionValue style,
                                             FontSelectionValue weight);
 
-  FontFaceCache cache_;
-
-  void Trace(Visitor*) const;
+  Persistent<FontFaceCache> cache_;
 
  protected:
   const AtomicString kFontNameForTesting{"Arial"};
@@ -51,11 +49,12 @@ class FontFaceCacheTest : public PageTestBase {
 
 void FontFaceCacheTest::SetUp() {
   PageTestBase::SetUp();
+  cache_ = MakeGarbageCollected<FontFaceCache>();
   ClearCache();
 }
 
 void FontFaceCacheTest::ClearCache() {
-  cache_.ClearAll();
+  cache_->ClearAll();
 }
 
 void FontFaceCacheTest::AppendTestFaceForCapabilities(const CSSValue& stretch,
@@ -83,7 +82,7 @@ void FontFaceCacheTest::AppendTestFaceForCapabilities(const CSSValue& stretch,
       MakeGarbageCollected<StyleRuleFontFace>(font_face_descriptor);
   FontFace* font_face = FontFace::Create(&GetDocument(), style_rule_font_face);
   CHECK(font_face);
-  cache_.Add(style_rule_font_face, font_face);
+  cache_->Add(style_rule_font_face, font_face);
 }
 
 void FontFaceCacheTest::AppendTestFaceForCapabilities(
@@ -125,7 +124,7 @@ TEST_F(FontFaceCacheTest, Instantiate) {
                                 *weight_value);
   AppendTestFaceForCapabilities(*stretch_value_condensed, *style_value,
                                 *weight_value);
-  ASSERT_EQ(cache_.GetNumSegmentedFacesForTesting(), 2ul);
+  ASSERT_EQ(cache_->GetNumSegmentedFacesForTesting(), 2ul);
 }
 
 TEST_F(FontFaceCacheTest, SimpleWidthMatch) {
@@ -141,12 +140,12 @@ TEST_F(FontFaceCacheTest, SimpleWidthMatch) {
                                 *weight_value);
   AppendTestFaceForCapabilities(*stretch_value_condensed, *style_value,
                                 *weight_value);
-  ASSERT_EQ(cache_.GetNumSegmentedFacesForTesting(), 2ul);
+  ASSERT_EQ(cache_->GetNumSegmentedFacesForTesting(), 2ul);
 
   const FontDescription& description_condensed = FontDescriptionForRequest(
       CondensedWidthValue(), NormalSlopeValue(), NormalWeightValue());
   CSSSegmentedFontFace* result =
-      cache_.Get(description_condensed, kFontNameForTesting);
+      cache_->Get(description_condensed, kFontNameForTesting);
   ASSERT_TRUE(result);
 
   FontSelectionCapabilities result_capabilities =
@@ -172,12 +171,12 @@ TEST_F(FontFaceCacheTest, SimpleWeightMatch) {
       CSSNumericLiteralValue::Create(100, CSSPrimitiveValue::UnitType::kNumber);
   AppendTestFaceForCapabilities(*stretch_value, *style_value,
                                 *weight_value_thin);
-  ASSERT_EQ(cache_.GetNumSegmentedFacesForTesting(), 2ul);
+  ASSERT_EQ(cache_->GetNumSegmentedFacesForTesting(), 2ul);
 
   const FontDescription& description_bold = FontDescriptionForRequest(
       NormalWidthValue(), NormalSlopeValue(), BoldWeightValue());
   CSSSegmentedFontFace* result =
-      cache_.Get(description_bold, kFontNameForTesting);
+      cache_->Get(description_bold, kFontNameForTesting);
   ASSERT_TRUE(result);
   FontSelectionCapabilities result_capabilities =
       result->GetFontSelectionCapabilities();
@@ -272,7 +271,7 @@ TEST_F(FontFaceCacheTest, DISABLED_MatchCombinations) {
         }
         for (FontDescription& test_description : test_descriptions) {
           CSSSegmentedFontFace* result =
-              cache_.Get(test_description, kFontNameForTesting);
+              cache_->Get(test_description, kFontNameForTesting);
           ASSERT_TRUE(result);
           FontSelectionCapabilities result_capabilities =
               result->GetFontSelectionCapabilities();
@@ -315,12 +314,12 @@ TEST_F(FontFaceCacheTest, WidthRangeMatching) {
   AppendTestFaceForCapabilities(*stretch_value, *style_value,
                                 *second_weight_list);
 
-  ASSERT_EQ(cache_.GetNumSegmentedFacesForTesting(), 2ul);
+  ASSERT_EQ(cache_->GetNumSegmentedFacesForTesting(), 2ul);
 
   const FontDescription& description_bold = FontDescriptionForRequest(
       NormalWidthValue(), NormalSlopeValue(), BoldWeightValue());
   CSSSegmentedFontFace* result =
-      cache_.Get(description_bold, kFontNameForTesting);
+      cache_->Get(description_bold, kFontNameForTesting);
   ASSERT_TRUE(result);
   FontSelectionCapabilities result_capabilities =
       result->GetFontSelectionCapabilities();
@@ -365,14 +364,14 @@ TEST_F(FontFaceCacheTest, WidthRangeMatchingBetween400500) {
                                 *(weight_values_lower[0]),
                                 *(weight_values_upper[0]));
 
-  ASSERT_EQ(cache_.GetNumSegmentedFacesForTesting(), 1ul);
+  ASSERT_EQ(cache_->GetNumSegmentedFacesForTesting(), 1ul);
 
   FontSelectionValue test_weight(450);
 
   const FontDescription& description_expanded = FontDescriptionForRequest(
       NormalWidthValue(), NormalSlopeValue(), test_weight);
   CSSSegmentedFontFace* result =
-      cache_.Get(description_expanded, kFontNameForTesting);
+      cache_->Get(description_expanded, kFontNameForTesting);
   ASSERT_TRUE(result);
   ASSERT_EQ(result->GetFontSelectionCapabilities().weight.minimum,
             FontSelectionValue(600));
@@ -380,9 +379,9 @@ TEST_F(FontFaceCacheTest, WidthRangeMatchingBetween400500) {
   AppendTestFaceForCapabilities(*stretch_value, *style_value,
                                 *(weight_values_lower[1]),
                                 *(weight_values_upper[1]));
-  ASSERT_EQ(cache_.GetNumSegmentedFacesForTesting(), 2ul);
+  ASSERT_EQ(cache_->GetNumSegmentedFacesForTesting(), 2ul);
 
-  result = cache_.Get(description_expanded, kFontNameForTesting);
+  result = cache_->Get(description_expanded, kFontNameForTesting);
   ASSERT_TRUE(result);
   ASSERT_EQ(result->GetFontSelectionCapabilities().weight.minimum,
             FontSelectionValue(415));
@@ -390,9 +389,9 @@ TEST_F(FontFaceCacheTest, WidthRangeMatchingBetween400500) {
   AppendTestFaceForCapabilities(*stretch_value, *style_value,
                                 *(weight_values_lower[2]),
                                 *(weight_values_upper[2]));
-  ASSERT_EQ(cache_.GetNumSegmentedFacesForTesting(), 3ul);
+  ASSERT_EQ(cache_->GetNumSegmentedFacesForTesting(), 3ul);
 
-  result = cache_.Get(description_expanded, kFontNameForTesting);
+  result = cache_->Get(description_expanded, kFontNameForTesting);
   ASSERT_TRUE(result);
   ASSERT_EQ(result->GetFontSelectionCapabilities().weight.minimum,
             FontSelectionValue(475));
@@ -424,12 +423,12 @@ TEST_F(FontFaceCacheTest, StretchRangeMatching) {
   AppendTestFaceForCapabilities(*second_stretch_list, *style_value,
                                 *weight_value);
 
-  ASSERT_EQ(cache_.GetNumSegmentedFacesForTesting(), 2ul);
+  ASSERT_EQ(cache_->GetNumSegmentedFacesForTesting(), 2ul);
 
   const FontDescription& description_expanded = FontDescriptionForRequest(
       FontSelectionValue(105), NormalSlopeValue(), NormalWeightValue());
   CSSSegmentedFontFace* result =
-      cache_.Get(description_expanded, kFontNameForTesting);
+      cache_->Get(description_expanded, kFontNameForTesting);
   ASSERT_TRUE(result);
   FontSelectionCapabilities result_capabilities =
       result->GetFontSelectionCapabilities();
@@ -477,12 +476,12 @@ TEST_F(FontFaceCacheTest, ObliqueRangeMatching) {
   AppendTestFaceForCapabilities(*stretch_value, *oblique_value_second,
                                 *weight_value);
 
-  ASSERT_EQ(cache_.GetNumSegmentedFacesForTesting(), 2ul);
+  ASSERT_EQ(cache_->GetNumSegmentedFacesForTesting(), 2ul);
 
   const FontDescription& description_italic = FontDescriptionForRequest(
       NormalWidthValue(), ItalicSlopeValue(), NormalWeightValue());
   CSSSegmentedFontFace* result =
-      cache_.Get(description_italic, kFontNameForTesting);
+      cache_->Get(description_italic, kFontNameForTesting);
   ASSERT_TRUE(result);
   FontSelectionCapabilities result_capabilities =
       result->GetFontSelectionCapabilities();
@@ -495,8 +494,4 @@ TEST_F(FontFaceCacheTest, ObliqueRangeMatching) {
       FontSelectionRange({FontSelectionValue(30), FontSelectionValue(35)}));
 }
 
-void FontFaceCacheTest::Trace(Visitor* visitor) const {
-  visitor->Trace(cache_);
-}
-
 }  // namespace blink

third_party/blink/renderer/core/css/offscreen_font_selector.cc

@@ -21,7 +21,7 @@
 namespace blink {
 
 OffscreenFontSelector::OffscreenFontSelector(WorkerGlobalScope* worker)
-    : worker_(worker) {
+    : font_face_cache_(MakeGarbageCollected<FontFaceCache>()), worker_(worker) {
   DCHECK(worker);
   FontCache::GetFontCache()->AddClient(this);
 }
@@ -43,7 +43,7 @@ scoped_refptr<FontData> OffscreenFontSelector::GetFontData(
     const FontDescription& font_description,
     const AtomicString& family_name) {
   if (CSSSegmentedFontFace* face =
-          font_face_cache_.Get(font_description, family_name)) {
+          font_face_cache_->Get(font_description, family_name)) {
     worker_->GetFontMatchingMetrics()->ReportWebFontFamily(family_name);
     return face->GetFontData(font_description);
   }
@@ -74,7 +74,7 @@ void OffscreenFontSelector::WillUseFontData(
     const FontDescription& font_description,
     const AtomicString& family,
     const String& text) {
-  CSSSegmentedFontFace* face = font_face_cache_.Get(font_description, family);
+  CSSSegmentedFontFace* face = font_face_cache_->Get(font_description, family);
   if (face)
     face->WillUseFontData(font_description, text);
 }
@@ -83,7 +83,7 @@ void OffscreenFontSelector::WillUseRange(
     const FontDescription& font_description,
     const AtomicString& family,
     const FontDataForRangeSet& range_set) {
-  CSSSegmentedFontFace* face = font_face_cache_.Get(font_description, family);
+  CSSSegmentedFontFace* face = font_face_cache_->Get(font_description, family);
   if (face)
     face->WillUseRange(font_description, range_set);
 }
@@ -166,7 +166,7 @@ void OffscreenFontSelector::ReportLastResortFallbackFontLookup(
 }
 
 void OffscreenFontSelector::FontCacheInvalidated() {
-  font_face_cache_.IncrementVersion();
+  font_face_cache_->IncrementVersion();
 }
 
 void OffscreenFontSelector::FontFaceInvalidated(FontInvalidationReason) {

third_party/blink/renderer/core/css/offscreen_font_selector.h

@@ -81,7 +81,7 @@ class CORE_EXPORT OffscreenFontSelector : public FontSelector {
 
   void UpdateGenericFontFamilySettings(const GenericFontFamilySettings&);
 
-  FontFaceCache* GetFontFaceCache() override { return &font_face_cache_; }
+  FontFaceCache* GetFontFaceCache() override { return font_face_cache_; }
 
   bool IsPlatformFamilyMatchAvailable(
       const FontDescription&,
@@ -99,7 +99,7 @@ class CORE_EXPORT OffscreenFontSelector : public FontSelector {
  private:
   GenericFontFamilySettings generic_font_family_settings_;
 
-  FontFaceCache font_face_cache_;
+  Member<FontFaceCache> font_face_cache_;
 
   Member<WorkerGlobalScope> worker_;
 };