Commit 0d4a2724 authored by Dominik Röttsches's avatar Dominik Röttsches Committed by Commit Bot

Avoid unbounded growth of font_data_table in CSSFontFaceSource

Animating or using an ever increasing number of instances of variable
fonts can make the font_data_table_ in CSSFontFaceSource grow without
bounds. Set a limit, and remove least recently used entries from the
table. In my experiments this reduces the tab memory leakage for the URL
from issue 778352 to a third and should help keep the tab running a lot
longer.

Bug: 778352
Change-Id: I9a812afdffb318be977def0b2c0301321603ab15
Reviewed-on: https://chromium-review.googlesource.com/824172
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523793}
parent add81c80
...@@ -31,6 +31,17 @@ ...@@ -31,6 +31,17 @@
#include "platform/fonts/FontFaceCreationParams.h" #include "platform/fonts/FontFaceCreationParams.h"
#include "platform/fonts/SimpleFontData.h" #include "platform/fonts/SimpleFontData.h"
namespace {
// An excessive amount of SimpleFontData objects is generated from
// CSSFontFaceSource if a lot of varying FontDescriptions point to a web
// font. These FontDescriptions can vary in size, font-feature-settings or
// font-variation settings. Well known cases are animations of font-variation
// settings, compare crbug.com/778352. For a start, let's reduce this number to
// 1024, which is still a large number and should have enough steps for font
// animations from the same font face source, but avoids unbounded growth.
const size_t kMaxCachedFontData = 1024;
} // namespace
namespace blink { namespace blink {
CSSFontFaceSource::~CSSFontFaceSource() = default; CSSFontFaceSource::~CSSFontFaceSource() = default;
...@@ -53,11 +64,28 @@ scoped_refptr<SimpleFontData> CSSFontFaceSource::GetFontData( ...@@ -53,11 +64,28 @@ scoped_refptr<SimpleFontData> CSSFontFaceSource::GetFontData(
font_data_table_.insert(key, nullptr).stored_value->value; font_data_table_.insert(key, nullptr).stored_value->value;
if (!font_data) if (!font_data)
font_data = CreateFontData(font_description, font_selection_capabilities); font_data = CreateFontData(font_description, font_selection_capabilities);
font_cache_key_age.PrependOrMoveToFirst(key);
PruneOldestIfNeeded();
DCHECK_LE(font_data_table_.size(), kMaxCachedFontData);
// No release, because fontData is a reference to a RefPtr that is held in the // No release, because fontData is a reference to a RefPtr that is held in the
// font_data_table_. // font_data_table_.
return font_data; return font_data;
} }
void CSSFontFaceSource::PruneOldestIfNeeded() {
if (font_cache_key_age.size() > kMaxCachedFontData) {
DCHECK_EQ(font_cache_key_age.size() - 1, kMaxCachedFontData);
FontCacheKey& key = font_cache_key_age.back();
font_cache_key_age.pop_back();
auto font_data_entry = font_data_table_.Take(key);
DCHECK_EQ(font_cache_key_age.size(), kMaxCachedFontData);
if (font_data_entry && font_data_entry->GetCustomFontData())
font_data_entry->GetCustomFontData()->ClearFontFaceSource();
}
}
void CSSFontFaceSource::PruneTable() { void CSSFontFaceSource::PruneTable() {
if (font_data_table_.IsEmpty()) if (font_data_table_.IsEmpty())
return; return;
...@@ -67,6 +95,7 @@ void CSSFontFaceSource::PruneTable() { ...@@ -67,6 +95,7 @@ void CSSFontFaceSource::PruneTable() {
if (font_data && font_data->GetCustomFontData()) if (font_data && font_data->GetCustomFontData())
font_data->GetCustomFontData()->ClearFontFaceSource(); font_data->GetCustomFontData()->ClearFontFaceSource();
} }
font_cache_key_age.clear();
font_data_table_.clear(); font_data_table_.clear();
} }
......
...@@ -33,6 +33,7 @@ ...@@ -33,6 +33,7 @@
#include "platform/heap/Handle.h" #include "platform/heap/Handle.h"
#include "platform/wtf/Allocator.h" #include "platform/wtf/Allocator.h"
#include "platform/wtf/HashMap.h" #include "platform/wtf/HashMap.h"
#include "platform/wtf/LinkedHashSet.h"
namespace blink { namespace blink {
...@@ -71,12 +72,16 @@ class CORE_EXPORT CSSFontFaceSource ...@@ -71,12 +72,16 @@ class CORE_EXPORT CSSFontFaceSource
void PruneTable(); void PruneTable();
private: private:
void PruneOldestIfNeeded();
using FontDataTable = HashMap<FontCacheKey, using FontDataTable = HashMap<FontCacheKey,
scoped_refptr<SimpleFontData>, scoped_refptr<SimpleFontData>,
FontCacheKeyHash, FontCacheKeyHash,
FontCacheKeyTraits>; FontCacheKeyTraits>;
using FontCacheKeyAgeList =
LinkedHashSet<FontCacheKey, FontCacheKeyHash, FontCacheKeyTraits>;
FontDataTable font_data_table_; FontDataTable font_data_table_;
FontCacheKeyAgeList font_cache_key_age;
DISALLOW_COPY_AND_ASSIGN(CSSFontFaceSource); DISALLOW_COPY_AND_ASSIGN(CSSFontFaceSource);
}; };
......
...@@ -56,4 +56,27 @@ TEST(CSSFontFaceSourceTest, HashCollision) { ...@@ -56,4 +56,27 @@ TEST(CSSFontFaceSourceTest, HashCollision) {
font_face_source.GetFontDataForSize(4925)); font_face_source.GetFontDataForSize(4925));
} }
// Exercises the size font_data_table_ assertions in CSSFontFaceSource.
TEST(CSSFontFaceSourceTest, UnboundedGrowth) {
DummyFontFaceSource font_face_source;
FontDescription font_description_variable;
FontSelectionCapabilities normal_capabilities(
{NormalWidthValue(), NormalWidthValue()},
{NormalSlopeValue(), NormalSlopeValue()},
{NormalWeightValue(), NormalWeightValue()});
// Roughly 3000 font variants.
for (float wght = 700; wght < 705; wght += 1 / 6.f) {
for (float wdth = 100; wdth < 125; wdth += 1 / 4.f) {
scoped_refptr<FontVariationSettings> variation_settings =
FontVariationSettings::Create();
variation_settings->Append(FontVariationAxis("wght", wght));
variation_settings->Append(FontVariationAxis("wdth", wdth));
font_description_variable.SetVariationSettings(variation_settings);
font_face_source.GetFontData(font_description_variable,
normal_capabilities);
}
}
}
} // namespace blink } // namespace blink
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment