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

Do not perform fallback for non-characters

In r492524 we resumed performing per-character fallback for unassigned
characters. This was intended to avoid problems upgrading the emoji data
file. When updating the emoji data before updating ICU new emoji may
have appeared in the unassigned range, which we would erroneously
exclude from fallback in that case.

However, allowing the full range of unassigned characters does not make
sense either and triggered a memory regression in our benchmark when the
amazon.in page triggers fallback for the non printable U+FFFE sentinel
character.

To solve this, in addition to private use area codepoints we exclude 66
Unicode non-characters, defined in
http://www.unicode.org/faq/private_use.html#nonchar4 They are part of
the unassigned range, but have special meanings and are explicitly
intended as non-characters. We must not try to look for glyphs in fonts
for these.

Bug: 862352
Change-Id: I50b22a4acb0d148595c5ae4f99a57d59ee8c41dd
Reviewed-on: https://chromium-review.googlesource.com/1143277Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576499}
parent e1fe4245
...@@ -286,7 +286,14 @@ scoped_refptr<SimpleFontData> FontCache::FallbackFontForCharacter( ...@@ -286,7 +286,14 @@ scoped_refptr<SimpleFontData> FontCache::FallbackFontForCharacter(
UChar32 lookup_char, UChar32 lookup_char,
const SimpleFontData* font_data_to_substitute, const SimpleFontData* font_data_to_substitute,
FontFallbackPriority fallback_priority) { FontFallbackPriority fallback_priority) {
if (Character::IsPrivateUse(lookup_char)) // In addition to PUA, do not perform fallback for non-characters either. Some
// of these are sentinel characters to detect encodings and do appear on
// websites. More details on
// http://www.unicode.org/faq/private_use.html#nonchar1 - See also
// crbug.com/862352 where performing fallback for U+FFFE causes a memory
// regression.
if (Character::IsPrivateUse(lookup_char) ||
Character::IsNonCharacter(lookup_char))
return nullptr; return nullptr;
return PlatformFallbackFontForCharacter( return PlatformFallbackFontForCharacter(
description, lookup_char, font_data_to_substitute, fallback_priority); description, lookup_char, font_data_to_substitute, fallback_priority);
......
...@@ -299,4 +299,8 @@ bool Character::IsPrivateUse(UChar32 character) { ...@@ -299,4 +299,8 @@ bool Character::IsPrivateUse(UChar32 character) {
return WTF::Unicode::Category(character) & WTF::Unicode::kOther_PrivateUse; return WTF::Unicode::Category(character) & WTF::Unicode::kOther_PrivateUse;
} }
bool Character::IsNonCharacter(UChar32 character) {
return U_IS_UNICODE_NONCHAR(character);
}
} // namespace blink } // namespace blink
...@@ -184,6 +184,7 @@ class PLATFORM_EXPORT Character { ...@@ -184,6 +184,7 @@ class PLATFORM_EXPORT Character {
static bool IsCommonOrInheritedScript(UChar32); static bool IsCommonOrInheritedScript(UChar32);
static bool IsPrivateUse(UChar32); static bool IsPrivateUse(UChar32);
static bool IsNonCharacter(UChar32);
private: private:
static bool IsCJKIdeographOrSymbolSlow(UChar32); static bool IsCJKIdeographOrSymbolSlow(UChar32);
......
...@@ -356,4 +356,30 @@ TEST(CharacterTest, IsBidiControl) { ...@@ -356,4 +356,30 @@ TEST(CharacterTest, IsBidiControl) {
EXPECT_FALSE(Character::IsBidiControl(0x05D0)); EXPECT_FALSE(Character::IsBidiControl(0x05D0));
} }
TEST(CharacterTest, IsNonCharacter) {
// See http://www.unicode.org/faq/private_use.html#nonchar4
EXPECT_FALSE(Character::IsNonCharacter(0xFDD0 - 1));
for (UChar32 bmp_noncharacter = 0xFDD0; bmp_noncharacter < 0xFDEF;
++bmp_noncharacter) {
EXPECT_TRUE(Character::IsNonCharacter(bmp_noncharacter));
}
EXPECT_FALSE(Character::IsNonCharacter(0xFDEF + 1));
EXPECT_FALSE(Character::IsNonCharacter(0xFFFE - 1));
EXPECT_TRUE(Character::IsNonCharacter(0xFFFE));
EXPECT_TRUE(Character::IsNonCharacter(0xFFFF));
EXPECT_FALSE(Character::IsNonCharacter(0xFFFF + 1));
for (uint32_t supplementary_plane_prefix = 0x10000;
supplementary_plane_prefix < 0x100000;
supplementary_plane_prefix += 0x10000) {
EXPECT_FALSE(
Character::IsNonCharacter(supplementary_plane_prefix + 0xFFFE - 1));
EXPECT_TRUE(Character::IsNonCharacter(supplementary_plane_prefix + 0xFFFE));
EXPECT_TRUE(Character::IsNonCharacter(supplementary_plane_prefix + 0xFFFF));
EXPECT_FALSE(
Character::IsNonCharacter(supplementary_plane_prefix + 0xFFFF + 1));
}
}
} // 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