Skip to content

Commit 2669277

Browse files
committed
Decouple font creation from font loading
https://bugs.webkit.org/show_bug.cgi?id=153414 Reviewed by Darin Adler. Previously, CSSFontFaceSource never triggered a font download until that font was actually used. This means that the function which triggers the download also has the goal of returning a font to use. However, the CSS Font Loading JavaScript API requires being able to trigger a font download without this extra font creation overhead. In addition, this patch adds an explicit (and enforced) state transition diagram. The diagram looks like this: => Success // Pending => Loading \\ => Failure Therefore, the API for CSSFontFaceSource has changed to expose the concept of these new states. This means that its user (CSSSegmentedFontFaceSource) has been updated to handle each possible state that its constituent CSSFontFaceSources may be in. No new tests because there is no behavior change. * css/CSSFontFace.cpp: (WebCore::CSSFontFace::allSourcesFailed): Renamed to make the name clearer. (WebCore::CSSFontFace::addedToSegmentedFontFace): Use references instead of pointers. (WebCore::CSSFontFace::removedFromSegmentedFontFace): Ditto. (WebCore::CSSFontFace::adoptSource): Renamed to make the name clearer. (WebCore::CSSFontFace::fontLoaded): Use references instead of pointers. Also, remove old dead code. (WebCore::CSSFontFace::font): Adapt to the new API of CSSFontFaceSource. (WebCore::CSSFontFace::isValid): Deleted. (WebCore::CSSFontFace::addSource): Deleted. (WebCore::CSSFontFace::notifyFontLoader): Deleted. Old dead code. (WebCore::CSSFontFace::notifyLoadingDone): Deleted. Old dead code. * css/CSSFontFace.h: (WebCore::CSSFontFace::create): Remove old dead code. (WebCore::CSSFontFace::CSSFontFace): Use references instead of pointers. (WebCore::CSSFontFace::loadState): Deleted. Remove old dead code. * css/CSSFontFaceSource.cpp: (WebCore::CSSFontFaceSource::setStatus): Enforce state transitions. (WebCore::CSSFontFaceSource::CSSFontFaceSource): Explicitly handle new state transitions. (WebCore::CSSFontFaceSource::fontLoaded): Update for new states. (WebCore::CSSFontFaceSource::load): Pulled out code from font(). (WebCore::CSSFontFaceSource::font): Moved code into load(). (WebCore::CSSFontFaceSource::isValid): Deleted. (WebCore::CSSFontFaceSource::isDecodeError): Deleted. (WebCore::CSSFontFaceSource::ensureFontData): Deleted. * css/CSSFontFaceSource.h: Much cleaner API. * css/CSSFontSelector.cpp: (WebCore::createFontFace): Migrate to references instead of pointers. This requires a little reorganization. (WebCore::registerLocalFontFacesForFamily): Update to new CSSFontFaceSource API. (WebCore::CSSFontSelector::addFontFaceRule): Ditto. (WebCore::CSSFontSelector::getFontFace): Ditto. * css/CSSSegmentedFontFace.cpp: (WebCore::CSSSegmentedFontFace::CSSSegmentedFontFace): Migrate to references instead of pointers. (WebCore::CSSSegmentedFontFace::~CSSSegmentedFontFace): Ditto. (WebCore::CSSSegmentedFontFace::fontLoaded): Remove old dead code. (WebCore::CSSSegmentedFontFace::appendFontFace): Cleanup. (WebCore::CSSSegmentedFontFace::fontRanges): Adopt to new API. (WebCore::CSSSegmentedFontFace::pruneTable): Deleted. (WebCore::CSSSegmentedFontFace::isLoading): Deleted. Old dead code. (WebCore::CSSSegmentedFontFace::checkFont): Deleted. Ditto. (WebCore::CSSSegmentedFontFace::loadFont): Deleted. Ditto. * css/CSSSegmentedFontFace.h: (WebCore::CSSSegmentedFontFace::create): Migrate to references instead of pointers. (WebCore::CSSSegmentedFontFace::fontSelector): Ditto. (WebCore::CSSSegmentedFontFace::LoadFontCallback::~LoadFontCallback): Deleted. * loader/cache/CachedFont.cpp: (WebCore::CachedFont::didAddClient): Migrate to references instead of pointers. (WebCore::CachedFont::checkNotify): Ditto. * loader/cache/CachedFontClient.h: (WebCore::CachedFontClient::fontLoaded): Ditto. Canonical link: https://commits.webkit.org/172140@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196322 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent e2cb24d commit 2669277

10 files changed

Lines changed: 265 additions & 334 deletions

Source/WebCore/ChangeLog

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,80 @@
1+
2016-02-09 Myles C. Maxfield <[email protected]>
2+
3+
Decouple font creation from font loading
4+
https://bugs.webkit.org/show_bug.cgi?id=153414
5+
6+
Reviewed by Darin Adler.
7+
8+
Previously, CSSFontFaceSource never triggered a font download until that font was actually used. This means
9+
that the function which triggers the download also has the goal of returning a font to use. However,
10+
the CSS Font Loading JavaScript API requires being able to trigger a font download without this extra font
11+
creation overhead.
12+
13+
In addition, this patch adds an explicit (and enforced) state transition diagram. The diagram looks like
14+
this:
15+
=> Success
16+
//
17+
Pending => Loading
18+
\\
19+
=> Failure
20+
21+
Therefore, the API for CSSFontFaceSource has changed to expose the concept of these new states. This means
22+
that its user (CSSSegmentedFontFaceSource) has been updated to handle each possible state that its constituent
23+
CSSFontFaceSources may be in.
24+
25+
No new tests because there is no behavior change.
26+
27+
* css/CSSFontFace.cpp:
28+
(WebCore::CSSFontFace::allSourcesFailed): Renamed to make the name clearer.
29+
(WebCore::CSSFontFace::addedToSegmentedFontFace): Use references instead of pointers.
30+
(WebCore::CSSFontFace::removedFromSegmentedFontFace): Ditto.
31+
(WebCore::CSSFontFace::adoptSource): Renamed to make the name clearer.
32+
(WebCore::CSSFontFace::fontLoaded): Use references instead of pointers. Also, remove old dead code.
33+
(WebCore::CSSFontFace::font): Adapt to the new API of CSSFontFaceSource.
34+
(WebCore::CSSFontFace::isValid): Deleted.
35+
(WebCore::CSSFontFace::addSource): Deleted.
36+
(WebCore::CSSFontFace::notifyFontLoader): Deleted. Old dead code.
37+
(WebCore::CSSFontFace::notifyLoadingDone): Deleted. Old dead code.
38+
* css/CSSFontFace.h:
39+
(WebCore::CSSFontFace::create): Remove old dead code.
40+
(WebCore::CSSFontFace::CSSFontFace): Use references instead of pointers.
41+
(WebCore::CSSFontFace::loadState): Deleted. Remove old dead code.
42+
* css/CSSFontFaceSource.cpp:
43+
(WebCore::CSSFontFaceSource::setStatus): Enforce state transitions.
44+
(WebCore::CSSFontFaceSource::CSSFontFaceSource): Explicitly handle new state transitions.
45+
(WebCore::CSSFontFaceSource::fontLoaded): Update for new states.
46+
(WebCore::CSSFontFaceSource::load): Pulled out code from font().
47+
(WebCore::CSSFontFaceSource::font): Moved code into load().
48+
(WebCore::CSSFontFaceSource::isValid): Deleted.
49+
(WebCore::CSSFontFaceSource::isDecodeError): Deleted.
50+
(WebCore::CSSFontFaceSource::ensureFontData): Deleted.
51+
* css/CSSFontFaceSource.h: Much cleaner API.
52+
* css/CSSFontSelector.cpp:
53+
(WebCore::createFontFace): Migrate to references instead of pointers. This requires a little
54+
reorganization.
55+
(WebCore::registerLocalFontFacesForFamily): Update to new CSSFontFaceSource API.
56+
(WebCore::CSSFontSelector::addFontFaceRule): Ditto.
57+
(WebCore::CSSFontSelector::getFontFace): Ditto.
58+
* css/CSSSegmentedFontFace.cpp:
59+
(WebCore::CSSSegmentedFontFace::CSSSegmentedFontFace): Migrate to references instead of pointers.
60+
(WebCore::CSSSegmentedFontFace::~CSSSegmentedFontFace): Ditto.
61+
(WebCore::CSSSegmentedFontFace::fontLoaded): Remove old dead code.
62+
(WebCore::CSSSegmentedFontFace::appendFontFace): Cleanup.
63+
(WebCore::CSSSegmentedFontFace::fontRanges): Adopt to new API.
64+
(WebCore::CSSSegmentedFontFace::pruneTable): Deleted.
65+
(WebCore::CSSSegmentedFontFace::isLoading): Deleted. Old dead code.
66+
(WebCore::CSSSegmentedFontFace::checkFont): Deleted. Ditto.
67+
(WebCore::CSSSegmentedFontFace::loadFont): Deleted. Ditto.
68+
* css/CSSSegmentedFontFace.h:
69+
(WebCore::CSSSegmentedFontFace::create): Migrate to references instead of pointers.
70+
(WebCore::CSSSegmentedFontFace::fontSelector): Ditto.
71+
(WebCore::CSSSegmentedFontFace::LoadFontCallback::~LoadFontCallback): Deleted.
72+
* loader/cache/CachedFont.cpp:
73+
(WebCore::CachedFont::didAddClient): Migrate to references instead of pointers.
74+
(WebCore::CachedFont::checkNotify): Ditto.
75+
* loader/cache/CachedFontClient.h:
76+
(WebCore::CachedFontClient::fontLoaded): Ditto.
77+
178
2016-02-09 Brady Eidson <[email protected]>
279

380
Modern IDB: IDBOpenDBRequests leak.

Source/WebCore/css/CSSFontFace.cpp

Lines changed: 33 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -37,135 +37,74 @@
3737

3838
namespace WebCore {
3939

40-
bool CSSFontFace::isValid() const
40+
bool CSSFontFace::allSourcesFailed() const
4141
{
42-
size_t size = m_sources.size();
43-
for (size_t i = 0; i < size; i++) {
44-
if (m_sources[i]->isValid())
45-
return true;
42+
for (auto& source : m_sources) {
43+
if (source->status() != CSSFontFaceSource::Status::Failure)
44+
return false;
4645
}
47-
return false;
46+
return true;
4847
}
4948

50-
void CSSFontFace::addedToSegmentedFontFace(CSSSegmentedFontFace* segmentedFontFace)
49+
void CSSFontFace::addedToSegmentedFontFace(CSSSegmentedFontFace& segmentedFontFace)
5150
{
52-
m_segmentedFontFaces.add(segmentedFontFace);
51+
m_segmentedFontFaces.add(&segmentedFontFace);
5352
}
5453

55-
void CSSFontFace::removedFromSegmentedFontFace(CSSSegmentedFontFace* segmentedFontFace)
54+
void CSSFontFace::removedFromSegmentedFontFace(CSSSegmentedFontFace& segmentedFontFace)
5655
{
57-
m_segmentedFontFaces.remove(segmentedFontFace);
56+
m_segmentedFontFaces.remove(&segmentedFontFace);
5857
}
5958

60-
void CSSFontFace::addSource(std::unique_ptr<CSSFontFaceSource> source)
59+
void CSSFontFace::adoptSource(std::unique_ptr<CSSFontFaceSource>&& source)
6160
{
62-
source->setFontFace(this);
6361
m_sources.append(WTFMove(source));
6462
}
6563

66-
void CSSFontFace::fontLoaded(CSSFontFaceSource* source)
64+
void CSSFontFace::fontLoaded(CSSFontFaceSource&)
6765
{
68-
if (source != m_activeSource)
69-
return;
70-
7166
// FIXME: Can we assert that m_segmentedFontFaces is not empty? That may
7267
// require stopping in-progress font loading when the last
7368
// CSSSegmentedFontFace is removed.
7469
if (m_segmentedFontFaces.isEmpty())
7570
return;
7671

77-
// Use one of the CSSSegmentedFontFaces' font selector. They all have
78-
// the same font selector, so it's wasteful to store it in the CSSFontFace.
79-
CSSFontSelector* fontSelector = (*m_segmentedFontFaces.begin())->fontSelector();
80-
fontSelector->fontLoaded();
81-
82-
#if ENABLE(FONT_LOAD_EVENTS)
83-
if (RuntimeEnabledFeatures::sharedFeatures().fontLoadEventsEnabled() && m_loadState == Loading) {
84-
if (source->ensureFontData())
85-
notifyFontLoader(Loaded);
86-
else if (!isValid())
87-
notifyFontLoader(Error);
88-
}
89-
#endif
72+
(*m_segmentedFontFaces.begin())->fontSelector().fontLoaded();
9073

9174
for (auto* face : m_segmentedFontFaces)
92-
face->fontLoaded(this);
93-
94-
#if ENABLE(FONT_LOAD_EVENTS)
95-
if (RuntimeEnabledFeatures::sharedFeatures().fontLoadEventsEnabled())
96-
notifyLoadingDone();
97-
#endif
75+
face->fontLoaded(*this);
9876
}
9977

10078
RefPtr<Font> CSSFontFace::font(const FontDescription& fontDescription, bool syntheticBold, bool syntheticItalic)
10179
{
102-
m_activeSource = 0;
103-
if (!isValid())
104-
return 0;
80+
if (allSourcesFailed())
81+
return nullptr;
10582

10683
ASSERT(!m_segmentedFontFaces.isEmpty());
107-
CSSFontSelector* fontSelector = (*m_segmentedFontFaces.begin())->fontSelector();
108-
109-
#if ENABLE(FONT_LOAD_EVENTS)
110-
if (RuntimeEnabledFeatures::sharedFeatures().fontLoadEventsEnabled() && m_loadState == NotLoaded)
111-
notifyFontLoader(Loading);
112-
#endif
113-
114-
size_t size = m_sources.size();
115-
for (size_t i = 0; i < size; ++i) {
116-
if (RefPtr<Font> result = m_sources[i]->font(fontDescription, syntheticBold, syntheticItalic, fontSelector, m_featureSettings, m_variantSettings)) {
117-
m_activeSource = m_sources[i].get();
118-
#if ENABLE(FONT_LOAD_EVENTS)
119-
if (RuntimeEnabledFeatures::sharedFeatures().fontLoadEventsEnabled() && m_loadState == Loading && m_sources[i]->isLoaded()) {
120-
notifyFontLoader(Loaded);
121-
notifyLoadingDone();
122-
}
123-
#endif
124-
return result.release();
84+
CSSFontSelector& fontSelector = (*m_segmentedFontFaces.begin())->fontSelector();
85+
86+
for (auto& source : m_sources) {
87+
if (source->status() == CSSFontFaceSource::Status::Pending)
88+
source->load(fontSelector);
89+
90+
switch (source->status()) {
91+
case CSSFontFaceSource::Status::Pending:
92+
ASSERT_NOT_REACHED();
93+
break;
94+
case CSSFontFaceSource::Status::Loading:
95+
return Font::create(FontCache::singleton().lastResortFallbackFont(fontDescription)->platformData(), true, true);
96+
case CSSFontFaceSource::Status::Success:
97+
if (RefPtr<Font> result = source->font(fontDescription, syntheticBold, syntheticItalic, m_featureSettings, m_variantSettings))
98+
return WTFMove(result);
99+
break;
100+
case CSSFontFaceSource::Status::Failure:
101+
break;
125102
}
126103
}
127104

128-
#if ENABLE(FONT_LOAD_EVENTS)
129-
if (RuntimeEnabledFeatures::sharedFeatures().fontLoadEventsEnabled() && m_loadState == Loading) {
130-
notifyFontLoader(Error);
131-
notifyLoadingDone();
132-
}
133-
#endif
134105
return nullptr;
135106
}
136107

137-
#if ENABLE(FONT_LOAD_EVENTS)
138-
void CSSFontFace::notifyFontLoader(LoadState newState)
139-
{
140-
m_loadState = newState;
141-
142-
Document* document = (*m_segmentedFontFaces.begin())->fontSelector()->document();
143-
if (!document)
144-
return;
145-
146-
switch (newState) {
147-
case Loading:
148-
document->fonts()->beginFontLoading(m_rule.get());
149-
break;
150-
case Loaded:
151-
document->fonts()->fontLoaded(m_rule.get());
152-
break;
153-
case Error:
154-
document->fonts()->loadError(m_rule.get(), m_activeSource);
155-
break;
156-
default:
157-
break;
158-
}
159-
}
160-
161-
void CSSFontFace::notifyLoadingDone()
162-
{
163-
Document* document = (*m_segmentedFontFaces.begin())->fontSelector()->document();
164-
if (document)
165-
document->fonts()->loadingDone();
166-
}
167-
#endif
168-
169108
#if ENABLE(SVG_FONTS)
170109
bool CSSFontFace::hasSVGFontFaceSource() const
171110
{

Source/WebCore/css/CSSFontFace.h

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class Font;
4545

4646
class CSSFontFace : public RefCounted<CSSFontFace> {
4747
public:
48-
static Ref<CSSFontFace> create(FontTraitsMask traitsMask, RefPtr<CSSFontFaceRule>&& rule, bool isLocalFallback = false) { return adoptRef(*new CSSFontFace(traitsMask, WTFMove(rule), isLocalFallback)); }
48+
static Ref<CSSFontFace> create(FontTraitsMask traitsMask, bool isLocalFallback = false) { return adoptRef(*new CSSFontFace(traitsMask, isLocalFallback)); }
4949

5050
FontTraitsMask traitsMask() const { return m_traitsMask; }
5151

@@ -72,16 +72,16 @@ class CSSFontFace : public RefCounted<CSSFontFace> {
7272
void setVariantEastAsianWidth(FontVariantEastAsianWidth width) { m_variantSettings.eastAsianWidth = width; }
7373
void setVariantEastAsianRuby(FontVariantEastAsianRuby ruby) { m_variantSettings.eastAsianRuby = ruby; }
7474

75-
void addedToSegmentedFontFace(CSSSegmentedFontFace*);
76-
void removedFromSegmentedFontFace(CSSSegmentedFontFace*);
75+
void addedToSegmentedFontFace(CSSSegmentedFontFace&);
76+
void removedFromSegmentedFontFace(CSSSegmentedFontFace&);
7777

78-
bool isValid() const;
78+
bool allSourcesFailed() const;
7979

8080
bool isLocalFallback() const { return m_isLocalFallback; }
8181

82-
void addSource(std::unique_ptr<CSSFontFaceSource>);
82+
void adoptSource(std::unique_ptr<CSSFontFaceSource>&&);
8383

84-
void fontLoaded(CSSFontFaceSource*);
84+
void fontLoaded(CSSFontFaceSource&);
8585

8686
RefPtr<Font> font(const FontDescription&, bool syntheticBold, bool syntheticItalic);
8787

@@ -104,22 +104,11 @@ class CSSFontFace : public RefCounted<CSSFontFace> {
104104
bool hasSVGFontFaceSource() const;
105105
#endif
106106

107-
#if ENABLE(FONT_LOAD_EVENTS)
108-
enum LoadState { NotLoaded, Loading, Loaded, Error };
109-
LoadState loadState() const { return m_loadState; }
110-
#endif
111-
112107
private:
113-
CSSFontFace(FontTraitsMask traitsMask, RefPtr<CSSFontFaceRule>&& rule, bool isLocalFallback)
108+
CSSFontFace(FontTraitsMask traitsMask, bool isLocalFallback)
114109
: m_traitsMask(traitsMask)
115-
, m_activeSource(0)
116110
, m_isLocalFallback(isLocalFallback)
117-
#if ENABLE(FONT_LOAD_EVENTS)
118-
, m_loadState(isLocalFallback ? Loaded : NotLoaded)
119-
, m_rule(rule)
120-
#endif
121111
{
122-
UNUSED_PARAM(rule);
123112
}
124113

125114
FontTraitsMask m_traitsMask;
@@ -128,14 +117,7 @@ class CSSFontFace : public RefCounted<CSSFontFace> {
128117
FontFeatureSettings m_featureSettings;
129118
FontVariantSettings m_variantSettings;
130119
Vector<std::unique_ptr<CSSFontFaceSource>> m_sources;
131-
CSSFontFaceSource* m_activeSource;
132120
bool m_isLocalFallback;
133-
#if ENABLE(FONT_LOAD_EVENTS)
134-
LoadState m_loadState;
135-
RefPtr<CSSFontFaceRule> m_rule;
136-
void notifyFontLoader(LoadState);
137-
void notifyLoadingDone();
138-
#endif
139121
};
140122

141123
}

0 commit comments

Comments
 (0)