Reland r172943 "Make CSSFontFace::willUseFontData() load fonts with unicode-range"

The original patch was reverted in r173234, because the chagne exposed
a hidden bug of SVG fonts (crbug.com/369633). That bug is being addressed
in https://codereview.chromium.org/271633002/ so I'm relanding this patch.

BUG=369633

> Make CSSFontFace::willUseFontData() load fonts with unicode-range
>
> Before this patch CSSFontFace::willUseFontData() loads font faces that
> have no unicode-range. Since font faces with no unicode-range tends to
> be used as fallback font of segmented font family, this behavior leads
> to unnecessary font downloads.
>
> This patch makes willUseFontData() loads the first unloaded font face
> whose unicode-range intersects with given text. That check does not
> need to be 100% precise (false negative is ok), so it only checks the
> first character of the text, for speed.
>
> TEST=fast/css/font-face-unicode-range-overlap-load.html
> BUG=247920, 246492
>
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=172943

Review URL: https://codereview.chromium.org/270813003

git-svn-id: svn://svn.chromium.org/blink/trunk@173699 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 2d805f94
...@@ -3,14 +3,18 @@ Tests that only necessary fonts are loaded when font faces have overlapping unic ...@@ -3,14 +3,18 @@ Tests that only necessary fonts are loaded when font faces have overlapping unic
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS faces.length is 4 PASS faces.length is 6
PASS faces[0].status is "unloaded" PASS faces[0].status is "unloaded"
PASS faces[1].status is "loaded" PASS faces[1].status is "loaded"
PASS faces[2].status is "loaded" PASS faces[2].status is "loaded"
PASS faces[3].status is "unloaded" PASS faces[3].status is "unloaded"
PASS faces[4].status is "unloaded"
PASS faces[5].status is "loaded"
PASS successfullyParsed is true PASS successfullyParsed is true
TEST COMPLETE TEST COMPLETE
I I
J J
K
...@@ -22,6 +22,16 @@ ...@@ -22,6 +22,16 @@
src: url('../../resources/Ahem.woff2'); src: url('../../resources/Ahem.woff2');
unicode-range: U+49; /* 'I' */ unicode-range: U+49; /* 'I' */
} }
@font-face {
font-family: Test3;
src: url('../../resources/Ahem.ttf');
/* no unicode-range */
}
@font-face {
font-family: Test3;
src: url('../../resources/Ahem.otf');
unicode-range: U+00-FF;
}
</style> </style>
<script> <script>
description('Tests that only necessary fonts are loaded when font faces have overlapping unicode ranges.'); description('Tests that only necessary fonts are loaded when font faces have overlapping unicode ranges.');
...@@ -36,11 +46,13 @@ function getDocumentFontFaces() { ...@@ -36,11 +46,13 @@ function getDocumentFontFaces() {
document.fonts.ready().then(function() { document.fonts.ready().then(function() {
faces = getDocumentFontFaces(); faces = getDocumentFontFaces();
shouldBe('faces.length', '4'); shouldBe('faces.length', '6');
shouldBeEqualToString('faces[0].status', 'unloaded'); shouldBeEqualToString('faces[0].status', 'unloaded');
shouldBeEqualToString('faces[1].status', 'loaded'); shouldBeEqualToString('faces[1].status', 'loaded');
shouldBeEqualToString('faces[2].status', 'loaded'); shouldBeEqualToString('faces[2].status', 'loaded');
shouldBeEqualToString('faces[3].status', 'unloaded'); shouldBeEqualToString('faces[3].status', 'unloaded');
shouldBeEqualToString('faces[4].status', 'unloaded');
shouldBeEqualToString('faces[5].status', 'loaded');
finishJSTest(); finishJSTest();
}); });
</script> </script>
...@@ -48,5 +60,6 @@ document.fonts.ready().then(function() { ...@@ -48,5 +60,6 @@ document.fonts.ready().then(function() {
<body> <body>
<p style="font-family: Test1">I</p> <p style="font-family: Test1">I</p>
<p style="font-family: Test2">J</p> <p style="font-family: Test2">J</p>
<p style="font-family: Test3">K</p>
</body> </body>
</html> </html>
...@@ -126,16 +126,13 @@ PassRefPtr<SimpleFontData> CSSFontFace::getFontData(const FontDescription& fontD ...@@ -126,16 +126,13 @@ PassRefPtr<SimpleFontData> CSSFontFace::getFontData(const FontDescription& fontD
return nullptr; return nullptr;
} }
void CSSFontFace::willUseFontData(const FontDescription& fontDescription) bool CSSFontFace::maybeScheduleFontLoad(const FontDescription& fontDescription, UChar32 character)
{ {
// Kicks off font load here only if the @font-face has no unicode-range. if (m_ranges.contains(character)) {
// @font-faces with unicode-range will be loaded when a GlyphPage for the
// font is created.
// FIXME: Pass around the text to render from RenderText, and kick download
// if m_ranges intersects with the text. Make sure this does not cause
// performance regression.
if (m_ranges.isEntireRange())
load(fontDescription); load(fontDescription);
return true;
}
return false;
} }
void CSSFontFace::load(const FontDescription& fontDescription, CSSFontSelector* fontSelector) void CSSFontFace::load(const FontDescription& fontDescription, CSSFontSelector* fontSelector)
...@@ -217,6 +214,14 @@ CSSFontFace::UnicodeRangeSet::UnicodeRangeSet(const Vector<UnicodeRange>& ranges ...@@ -217,6 +214,14 @@ CSSFontFace::UnicodeRangeSet::UnicodeRangeSet(const Vector<UnicodeRange>& ranges
m_ranges.shrink(targetIndex); m_ranges.shrink(targetIndex);
} }
bool CSSFontFace::UnicodeRangeSet::contains(UChar32 c) const
{
if (isEntireRange())
return true;
Vector<UnicodeRange>::const_iterator it = std::lower_bound(m_ranges.begin(), m_ranges.end(), c);
return it != m_ranges.end() && it->contains(c);
}
bool CSSFontFace::UnicodeRangeSet::intersectsWith(const String& text) const bool CSSFontFace::UnicodeRangeSet::intersectsWith(const String& text) const
{ {
if (text.isEmpty()) if (text.isEmpty())
...@@ -230,8 +235,7 @@ bool CSSFontFace::UnicodeRangeSet::intersectsWith(const String& text) const ...@@ -230,8 +235,7 @@ bool CSSFontFace::UnicodeRangeSet::intersectsWith(const String& text) const
while (index < text.length()) { while (index < text.length()) {
UChar32 c = text.characterStartingAt(index); UChar32 c = text.characterStartingAt(index);
index += U16_LENGTH(c); index += U16_LENGTH(c);
Vector<UnicodeRange>::const_iterator it = std::lower_bound(m_ranges.begin(), m_ranges.end(), c); if (contains(c))
if (it != m_ranges.end() && it->contains(c))
return true; return true;
} }
return false; return false;
......
...@@ -97,6 +97,7 @@ public: ...@@ -97,6 +97,7 @@ public:
class UnicodeRangeSet { class UnicodeRangeSet {
public: public:
explicit UnicodeRangeSet(const Vector<UnicodeRange>&); explicit UnicodeRangeSet(const Vector<UnicodeRange>&);
bool contains(UChar32) const;
bool intersectsWith(const String&) const; bool intersectsWith(const String&) const;
bool isEntireRange() const { return m_ranges.isEmpty(); } bool isEntireRange() const { return m_ranges.isEmpty(); }
size_t size() const { return m_ranges.size(); } size_t size() const { return m_ranges.size(); }
...@@ -106,7 +107,7 @@ public: ...@@ -106,7 +107,7 @@ public:
}; };
FontFace::LoadStatus loadStatus() const { return m_fontFace->loadStatus(); } FontFace::LoadStatus loadStatus() const { return m_fontFace->loadStatus(); }
void willUseFontData(const FontDescription&); bool maybeScheduleFontLoad(const FontDescription&, UChar32);
void load(const FontDescription&, CSSFontSelector* = 0); void load(const FontDescription&, CSSFontSelector* = 0);
bool hadBlankText() { return isValid() && m_sources.first()->hadBlankText(); } bool hadBlankText() { return isValid() && m_sources.first()->hadBlankText(); }
......
...@@ -214,11 +214,11 @@ PassRefPtr<FontData> CSSFontSelector::getFontData(const FontDescription& fontDes ...@@ -214,11 +214,11 @@ PassRefPtr<FontData> CSSFontSelector::getFontData(const FontDescription& fontDes
return FontCache::fontCache()->getFontData(fontDescription, settingsFamilyName); return FontCache::fontCache()->getFontData(fontDescription, settingsFamilyName);
} }
void CSSFontSelector::willUseFontData(const FontDescription& fontDescription, const AtomicString& family) void CSSFontSelector::willUseFontData(const FontDescription& fontDescription, const AtomicString& family, UChar32 character)
{ {
CSSSegmentedFontFace* face = m_fontFaceCache.get(fontDescription, family); CSSSegmentedFontFace* face = m_fontFaceCache.get(fontDescription, family);
if (face) if (face)
face->willUseFontData(fontDescription); face->willUseFontData(fontDescription, character);
} }
#if !ENABLE(OILPAN) #if !ENABLE(OILPAN)
......
...@@ -84,7 +84,7 @@ public: ...@@ -84,7 +84,7 @@ public:
virtual unsigned version() const OVERRIDE { return m_fontFaceCache.version(); } virtual unsigned version() const OVERRIDE { return m_fontFaceCache.version(); }
virtual PassRefPtr<FontData> getFontData(const FontDescription&, const AtomicString&) OVERRIDE; virtual PassRefPtr<FontData> getFontData(const FontDescription&, const AtomicString&) OVERRIDE;
virtual void willUseFontData(const FontDescription&, const AtomicString& family) OVERRIDE; virtual void willUseFontData(const FontDescription&, const AtomicString& family, UChar32) OVERRIDE;
#if !ENABLE(OILPAN) #if !ENABLE(OILPAN)
void clearDocument(); void clearDocument();
......
...@@ -184,10 +184,14 @@ bool CSSSegmentedFontFace::isLoaded() const ...@@ -184,10 +184,14 @@ bool CSSSegmentedFontFace::isLoaded() const
return true; return true;
} }
void CSSSegmentedFontFace::willUseFontData(const FontDescription& fontDescription) void CSSSegmentedFontFace::willUseFontData(const FontDescription& fontDescription, UChar32 character)
{ {
for (FontFaceList::iterator it = m_fontFaces.begin(); it != m_fontFaces.end(); ++it) for (FontFaceList::reverse_iterator it = m_fontFaces.rbegin(); it != m_fontFaces.rend(); ++it) {
(*it)->cssFontFace()->willUseFontData(fontDescription); if ((*it)->loadStatus() != FontFace::Unloaded)
break;
if ((*it)->cssFontFace()->maybeScheduleFontLoad(fontDescription, character))
break;
}
} }
bool CSSSegmentedFontFace::checkFont(const String& text) const bool CSSSegmentedFontFace::checkFont(const String& text) const
......
...@@ -66,7 +66,7 @@ public: ...@@ -66,7 +66,7 @@ public:
bool checkFont(const String&) const; bool checkFont(const String&) const;
void match(const String&, WillBeHeapVector<RefPtrWillBeMember<FontFace> >&) const; void match(const String&, WillBeHeapVector<RefPtrWillBeMember<FontFace> >&) const;
void willUseFontData(const FontDescription&); void willUseFontData(const FontDescription&, UChar32);
void trace(Visitor*); void trace(Visitor*);
......
...@@ -205,8 +205,11 @@ void RenderText::styleDidChange(StyleDifference diff, const RenderStyle* oldStyl ...@@ -205,8 +205,11 @@ void RenderText::styleDidChange(StyleDifference diff, const RenderStyle* oldStyl
if (oldTransform != newStyle->textTransform() || oldSecurity != newStyle->textSecurity()) if (oldTransform != newStyle->textTransform() || oldSecurity != newStyle->textSecurity())
transformText(); transformText();
// This is an optimization that kicks off font load before layout.
// In order to make it fast, we only check if the first character of the
// text is included in the unicode ranges of the fonts.
if (!text().containsOnlyWhitespace()) if (!text().containsOnlyWhitespace())
newStyle->font().willUseFontData(); newStyle->font().willUseFontData(text().characterStartingAt(0));
} }
void RenderText::removeAndDestroyTextBoxes() void RenderText::removeAndDestroyTextBoxes()
......
...@@ -245,11 +245,11 @@ CodePath Font::codePath(const TextRun& run) const ...@@ -245,11 +245,11 @@ CodePath Font::codePath(const TextRun& run) const
return Character::characterRangeCodePath(run.characters16(), run.length()); return Character::characterRangeCodePath(run.characters16(), run.length());
} }
void Font::willUseFontData() const void Font::willUseFontData(UChar32 character) const
{ {
const FontFamily& family = fontDescription().family(); const FontFamily& family = fontDescription().family();
if (m_fontFallbackList && m_fontFallbackList->fontSelector() && !family.familyIsEmpty()) if (m_fontFallbackList && m_fontFallbackList->fontSelector() && !family.familyIsEmpty())
m_fontFallbackList->fontSelector()->willUseFontData(fontDescription(), family.family()); m_fontFallbackList->fontSelector()->willUseFontData(fontDescription(), family.family(), character);
} }
static inline bool isInRange(UChar32 character, UChar32 lowerBound, UChar32 upperBound) static inline bool isInRange(UChar32 character, UChar32 lowerBound, UChar32 upperBound)
......
...@@ -164,7 +164,7 @@ public: ...@@ -164,7 +164,7 @@ public:
FontFallbackList* fontList() const { return m_fontFallbackList.get(); } FontFallbackList* fontList() const { return m_fontFallbackList.get(); }
void willUseFontData() const; void willUseFontData(UChar32) const;
static FloatRect pixelSnappedSelectionRect(float fromX, float toX, float y, float height); static FloatRect pixelSnappedSelectionRect(float fromX, float toX, float y, float height);
private: private:
......
...@@ -29,6 +29,7 @@ ...@@ -29,6 +29,7 @@
#include "platform/fonts/FontCacheClient.h" #include "platform/fonts/FontCacheClient.h"
#include "wtf/Forward.h" #include "wtf/Forward.h"
#include "wtf/PassRefPtr.h" #include "wtf/PassRefPtr.h"
#include "wtf/text/AtomicString.h"
namespace WebCore { namespace WebCore {
...@@ -39,7 +40,7 @@ class FontSelector : public FontCacheClient { ...@@ -39,7 +40,7 @@ class FontSelector : public FontCacheClient {
public: public:
virtual ~FontSelector() { } virtual ~FontSelector() { }
virtual PassRefPtr<FontData> getFontData(const FontDescription&, const AtomicString& familyName) = 0; virtual PassRefPtr<FontData> getFontData(const FontDescription&, const AtomicString& familyName) = 0;
virtual void willUseFontData(const FontDescription&, const AtomicString& familyName) = 0; virtual void willUseFontData(const FontDescription&, const AtomicString& familyName, UChar32) = 0;
virtual unsigned version() const = 0; virtual unsigned version() const = 0;
}; };
......
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