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

Add match type to FontCacheKey information

Fixes situations in which a locally unique font name match masks a
result for a family name match.

This cannot be tested before src: local() matching is enabled by
switching the flag. A layout test covering this situation will be added
when the flag is enabled.

Bug: 921029
Change-Id: I4123cb70aa65cb59e4a00c3a58e312ef6fa24b64
Reviewed-on: https://chromium-review.googlesource.com/c/1407069
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622167}
parent c6598a20
...@@ -58,7 +58,9 @@ scoped_refptr<SimpleFontData> CSSFontFaceSource::GetFontData( ...@@ -58,7 +58,9 @@ scoped_refptr<SimpleFontData> CSSFontFaceSource::GetFontData(
return CreateFontData(font_description, font_selection_capabilities); return CreateFontData(font_description, font_selection_capabilities);
} }
FontCacheKey key = font_description.CacheKey(FontFaceCreationParams()); bool is_unique_match = false;
FontCacheKey key =
font_description.CacheKey(FontFaceCreationParams(), is_unique_match);
// Get or create the font data. Take care to avoid dangling references into // Get or create the font data. Take care to avoid dangling references into
// font_data_table_, because it is modified below during pruning. // font_data_table_, because it is modified below during pruning.
......
...@@ -42,7 +42,9 @@ unsigned SimulateHashCalculation(float size) { ...@@ -42,7 +42,9 @@ unsigned SimulateHashCalculation(float size) {
FontDescription font_description; FontDescription font_description;
font_description.SetSizeAdjust(size); font_description.SetSizeAdjust(size);
font_description.SetAdjustedSize(size); font_description.SetAdjustedSize(size);
return font_description.CacheKey(FontFaceCreationParams()).GetHash(); bool is_unique_match = false;
return font_description.CacheKey(FontFaceCreationParams(), is_unique_match)
.GetHash();
} }
} }
...@@ -50,9 +52,9 @@ TEST(CSSFontFaceSourceTest, HashCollision) { ...@@ -50,9 +52,9 @@ TEST(CSSFontFaceSourceTest, HashCollision) {
DummyFontFaceSource font_face_source; DummyFontFaceSource font_face_source;
// Even if the hash value collide, fontface cache should return different // Even if the hash value collide, fontface cache should return different
// value for different fonts. // value for different fonts.
EXPECT_EQ(SimulateHashCalculation(527), SimulateHashCalculation(3099)); EXPECT_EQ(SimulateHashCalculation(6009), SimulateHashCalculation(8634));
EXPECT_NE(font_face_source.GetFontDataForSize(527), EXPECT_NE(font_face_source.GetFontDataForSize(6009),
font_face_source.GetFontDataForSize(3099)); font_face_source.GetFontDataForSize(8634));
} }
// Exercises the size font_data_table_ assertions in CSSFontFaceSource. // Exercises the size font_data_table_ assertions in CSSFontFaceSource.
......
...@@ -98,8 +98,9 @@ scoped_refptr<FontData> CSSSegmentedFontFace::GetFontData( ...@@ -98,8 +98,9 @@ scoped_refptr<FontData> CSSSegmentedFontFace::GetFontData(
const FontSelectionRequest& font_selection_request = const FontSelectionRequest& font_selection_request =
font_description.GetFontSelectionRequest(); font_description.GetFontSelectionRequest();
FontCacheKey key = font_description.CacheKey(FontFaceCreationParams(), bool is_unique_match = false;
font_selection_request); FontCacheKey key = font_description.CacheKey(
FontFaceCreationParams(), is_unique_match, font_selection_request);
scoped_refptr<SegmentedFontData>& font_data = scoped_refptr<SegmentedFontData>& font_data =
font_data_table_.insert(key, nullptr).stored_value->value; font_data_table_.insert(key, nullptr).stored_value->value;
......
...@@ -118,7 +118,10 @@ FontPlatformData* FontCache::GetFontPlatformData( ...@@ -118,7 +118,10 @@ FontPlatformData* FontCache::GetFontPlatformData(
float size = font_description.EffectiveFontSize(); float size = font_description.EffectiveFontSize();
unsigned rounded_size = size * FontCacheKey::PrecisionMultiplier(); unsigned rounded_size = size * FontCacheKey::PrecisionMultiplier();
FontCacheKey key = font_description.CacheKey(creation_params); bool is_unique_match =
alternate_font_name == AlternateFontName::kLocalUniqueFace;
FontCacheKey key =
font_description.CacheKey(creation_params, is_unique_match);
// Remove the font size from the cache key, and handle the font size // Remove the font size from the cache key, and handle the font size
// separately in the inner HashMap. So that different size of FontPlatformData // separately in the inner HashMap. So that different size of FontPlatformData
......
...@@ -53,17 +53,20 @@ struct FontCacheKey { ...@@ -53,17 +53,20 @@ struct FontCacheKey {
: creation_params_(), : creation_params_(),
font_size_(0), font_size_(0),
options_(0), options_(0),
device_scale_factor_(0) {} device_scale_factor_(0),
is_unique_match_(false) {}
FontCacheKey(FontFaceCreationParams creation_params, FontCacheKey(FontFaceCreationParams creation_params,
float font_size, float font_size,
unsigned options, unsigned options,
float device_scale_factor, float device_scale_factor,
scoped_refptr<FontVariationSettings> variation_settings) scoped_refptr<FontVariationSettings> variation_settings,
bool is_unique_match)
: creation_params_(creation_params), : creation_params_(creation_params),
font_size_(font_size * kFontSizePrecisionMultiplier), font_size_(font_size * kFontSizePrecisionMultiplier),
options_(options), options_(options),
device_scale_factor_(device_scale_factor), device_scale_factor_(device_scale_factor),
variation_settings_(std::move(variation_settings)) {} variation_settings_(std::move(variation_settings)),
is_unique_match_(is_unique_match) {}
FontCacheKey(WTF::HashTableDeletedValueType) FontCacheKey(WTF::HashTableDeletedValueType)
: font_size_(HashTableDeletedSize()) {} : font_size_(HashTableDeletedSize()) {}
...@@ -71,10 +74,13 @@ struct FontCacheKey { ...@@ -71,10 +74,13 @@ struct FontCacheKey {
unsigned GetHash() const { unsigned GetHash() const {
// Convert from float with 3 digit precision before hashing. // Convert from float with 3 digit precision before hashing.
unsigned device_scale_factor_hash = device_scale_factor_ * 1000; unsigned device_scale_factor_hash = device_scale_factor_ * 1000;
unsigned hash_codes[5] = { unsigned hash_codes[6] = {
creation_params_.GetHash(), font_size_, options_, creation_params_.GetHash(),
font_size_,
options_,
device_scale_factor_hash, device_scale_factor_hash,
variation_settings_ ? variation_settings_->GetHash() : 0}; variation_settings_ ? variation_settings_->GetHash() : 0,
is_unique_match_};
return StringHasher::HashMemory<sizeof(hash_codes)>(hash_codes); return StringHasher::HashMemory<sizeof(hash_codes)>(hash_codes);
} }
...@@ -82,7 +88,8 @@ struct FontCacheKey { ...@@ -82,7 +88,8 @@ struct FontCacheKey {
return creation_params_ == other.creation_params_ && return creation_params_ == other.creation_params_ &&
font_size_ == other.font_size_ && options_ == other.options_ && font_size_ == other.font_size_ && options_ == other.options_ &&
device_scale_factor_ == other.device_scale_factor_ && device_scale_factor_ == other.device_scale_factor_ &&
variation_settings_ == other.variation_settings_; variation_settings_ == other.variation_settings_ &&
is_unique_match_ == other.is_unique_match_;
} }
bool IsHashTableDeletedValue() const { bool IsHashTableDeletedValue() const {
...@@ -105,6 +112,7 @@ struct FontCacheKey { ...@@ -105,6 +112,7 @@ struct FontCacheKey {
// device_scale_factor_ to be a part of computing the cache key. // device_scale_factor_ to be a part of computing the cache key.
float device_scale_factor_; float device_scale_factor_;
scoped_refptr<FontVariationSettings> variation_settings_; scoped_refptr<FontVariationSettings> variation_settings_;
bool is_unique_match_;
}; };
struct FontCacheKeyHash { struct FontCacheKeyHash {
......
...@@ -214,6 +214,7 @@ float FontDescription::EffectiveFontSize() const { ...@@ -214,6 +214,7 @@ float FontDescription::EffectiveFontSize() const {
FontCacheKey FontDescription::CacheKey( FontCacheKey FontDescription::CacheKey(
const FontFaceCreationParams& creation_params, const FontFaceCreationParams& creation_params,
bool is_unique_match,
const FontSelectionRequest& font_selection_request) const { const FontSelectionRequest& font_selection_request) const {
unsigned options = unsigned options =
static_cast<unsigned>(fields_.synthetic_italic_) << 6 | // bit 7 static_cast<unsigned>(fields_.synthetic_italic_) << 6 | // bit 7
...@@ -229,7 +230,8 @@ FontCacheKey FontDescription::CacheKey( ...@@ -229,7 +230,8 @@ FontCacheKey FontDescription::CacheKey(
#endif #endif
FontCacheKey cache_key(creation_params, EffectiveFontSize(), FontCacheKey cache_key(creation_params, EffectiveFontSize(),
options | font_selection_request_.GetHash() << 8, options | font_selection_request_.GetHash() << 8,
device_scale_factor_for_key, variation_settings_); device_scale_factor_for_key, variation_settings_,
is_unique_match);
return cache_key; return cache_key;
} }
......
...@@ -252,6 +252,7 @@ class PLATFORM_EXPORT FontDescription { ...@@ -252,6 +252,7 @@ class PLATFORM_EXPORT FontDescription {
const; // Returns either the computedSize or the computedPixelSize const; // Returns either the computedSize or the computedPixelSize
FontCacheKey CacheKey( FontCacheKey CacheKey(
const FontFaceCreationParams&, const FontFaceCreationParams&,
bool is_unique_match,
const FontSelectionRequest& = FontSelectionRequest()) const; const FontSelectionRequest& = FontSelectionRequest()) const;
void SetFamily(const FontFamily& family) { family_list_ = family; } void SetFamily(const FontFamily& family) { family_list_ = family; }
......
...@@ -202,7 +202,8 @@ FallbackListCompositeKey FontFallbackList::CompositeKey( ...@@ -202,7 +202,8 @@ FallbackListCompositeKey FontFallbackList::CompositeKey(
platform_data); platform_data);
} }
if (result) { if (result) {
key.Add(font_description.CacheKey(params)); bool is_unique_match = false;
key.Add(font_description.CacheKey(params, is_unique_match));
if (!result->IsSegmented() && !result->IsCustomFont()) if (!result->IsSegmented() && !result->IsCustomFont())
FontCache::GetFontCache()->ReleaseFontData(ToSimpleFontData(result)); FontCache::GetFontCache()->ReleaseFontData(ToSimpleFontData(result));
} }
......
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