- Revision
- 197434
- Author
- [email protected]
- Date
- 2016-03-01 18:34:50 -0800 (Tue, 01 Mar 2016)
Log Message
Extend CSSFontSelector's lifetime to be longer than the Document's lifetime
https://bugs.webkit.org/show_bug.cgi?id=154101
Reviewed by Darin Adler.
Rather than destroying the Document's CSSFontSelector, instead, the object should
live for the lifetime of the document, and it should instead be asked to clear its
contents.
This is important for the CSS Font Loading API, where the identity of objects the
CSSFontSelector references needs to persist throughout the lifetime of the
Document. This patch represents the first step to implementing this correctly.
The second step is for the CSSFontSelector to perform a diff instead of a
wholesale clear of its contents. Once this is done, font loading objects can
survive through a call to Document::clearStyleResolver().
This patch gives the CSSFontSelector two states: building underway and building not
underway. The state is building underway in between calls to clearStyleResolver()
and when the style resolver gets built back up. Otherwise, the state is building
not underway. Because of this new design, creation of all FontFace objects can be
postponed until a state transition from building underway to building not underway.
A subsequent patch will perform the diff at this point. An ASSERT() makes sure that
we never service a font lookup request while Building.
No new tests because there is no behavior change.
* css/CSSFontFaceSet.cpp:
(WebCore::CSSFontFaceSet::clear):
* css/CSSFontSelector.cpp:
(WebCore::CSSFontSelector::buildStarted):
(WebCore::CSSFontSelector::buildCompleted):
(WebCore::CSSFontSelector::addFontFaceRule):
(WebCore::CSSFontSelector::fontRangesForFamily):
(WebCore::CSSFontSelector::CSSFontSelector): Deleted.
(WebCore::CSSFontSelector::clearDocument): Deleted.
* css/CSSFontSelector.h:
* css/StyleResolver.cpp:
(WebCore::StyleResolver::appendAuthorStyleSheets):
* dom/Document.cpp:
(WebCore::Document::Document):
(WebCore::Document::~Document):
(WebCore::Document::clearStyleResolver):
(WebCore::Document::fontSelector): Deleted.
* dom/Document.h:
(WebCore::Document::fontSelector):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (197433 => 197434)
--- trunk/Source/WebCore/ChangeLog 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/ChangeLog 2016-03-02 02:34:50 UTC (rev 197434)
@@ -1,3 +1,51 @@
+2016-03-01 Myles C. Maxfield <[email protected]>
+
+ Extend CSSFontSelector's lifetime to be longer than the Document's lifetime
+ https://bugs.webkit.org/show_bug.cgi?id=154101
+
+ Reviewed by Darin Adler.
+
+ Rather than destroying the Document's CSSFontSelector, instead, the object should
+ live for the lifetime of the document, and it should instead be asked to clear its
+ contents.
+
+ This is important for the CSS Font Loading API, where the identity of objects the
+ CSSFontSelector references needs to persist throughout the lifetime of the
+ Document. This patch represents the first step to implementing this correctly.
+ The second step is for the CSSFontSelector to perform a diff instead of a
+ wholesale clear of its contents. Once this is done, font loading objects can
+ survive through a call to Document::clearStyleResolver().
+
+ This patch gives the CSSFontSelector two states: building underway and building not
+ underway. The state is building underway in between calls to clearStyleResolver()
+ and when the style resolver gets built back up. Otherwise, the state is building
+ not underway. Because of this new design, creation of all FontFace objects can be
+ postponed until a state transition from building underway to building not underway.
+ A subsequent patch will perform the diff at this point. An ASSERT() makes sure that
+ we never service a font lookup request while Building.
+
+ No new tests because there is no behavior change.
+
+ * css/CSSFontFaceSet.cpp:
+ (WebCore::CSSFontFaceSet::clear):
+ * css/CSSFontSelector.cpp:
+ (WebCore::CSSFontSelector::buildStarted):
+ (WebCore::CSSFontSelector::buildCompleted):
+ (WebCore::CSSFontSelector::addFontFaceRule):
+ (WebCore::CSSFontSelector::fontRangesForFamily):
+ (WebCore::CSSFontSelector::CSSFontSelector): Deleted.
+ (WebCore::CSSFontSelector::clearDocument): Deleted.
+ * css/CSSFontSelector.h:
+ * css/StyleResolver.cpp:
+ (WebCore::StyleResolver::appendAuthorStyleSheets):
+ * dom/Document.cpp:
+ (WebCore::Document::Document):
+ (WebCore::Document::~Document):
+ (WebCore::Document::clearStyleResolver):
+ (WebCore::Document::fontSelector): Deleted.
+ * dom/Document.h:
+ (WebCore::Document::fontSelector):
+
2016-03-01 Alexey Proskuryakov <[email protected]>
Update Xcode project for InstallAPI
Modified: trunk/Source/WebCore/css/CSSFontFaceSet.cpp (197433 => 197434)
--- trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2016-03-02 02:34:50 UTC (rev 197434)
@@ -242,6 +242,8 @@
m_facesLookupTable.clear();
m_locallyInstalledFacesLookupTable.clear();
m_cache.clear();
+ m_facesPartitionIndex = 0;
+ m_status = Status::Loaded;
}
CSSFontFace& CSSFontFaceSet::operator[](size_t i)
Modified: trunk/Source/WebCore/css/CSSFontSelector.cpp (197433 => 197434)
--- trunk/Source/WebCore/css/CSSFontSelector.cpp 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/css/CSSFontSelector.cpp 2016-03-02 02:34:50 UTC (rev 197434)
@@ -70,10 +70,6 @@
, m_uniqueId(++fontSelectorId)
, m_version(0)
{
- // FIXME: An old comment used to say there was no need to hold a reference to m_document
- // because "we are guaranteed to be destroyed before the document". But there does not
- // seem to be any such guarantee.
-
ASSERT(m_document);
FontCache::singleton().addClient(*this);
m_cssFontFaceSet->addClient(*this);
@@ -101,8 +97,33 @@
return !m_cssFontFaceSet->faceCount();
}
+void CSSFontSelector::buildStarted()
+{
+ m_buildIsUnderway = true;
+ ++m_version;
+}
+
+void CSSFontSelector::buildCompleted()
+{
+ if (!m_buildIsUnderway)
+ return;
+
+ m_buildIsUnderway = false;
+
+ m_cssFontFaceSet->clear();
+
+ for (auto& item : m_stagingArea)
+ addFontFaceRule(item.styleRuleFontFace, item.isInitiatingElementInUserAgentShadowTree);
+ m_stagingArea.clear();
+}
+
void CSSFontSelector::addFontFaceRule(StyleRuleFontFace& fontFaceRule, bool isInitiatingElementInUserAgentShadowTree)
{
+ if (m_buildIsUnderway) {
+ m_stagingArea.append({fontFaceRule, isInitiatingElementInUserAgentShadowTree});
+ return;
+ }
+
const StyleProperties& style = fontFaceRule.properties();
RefPtr<CSSValue> fontFamily = style.getPropertyCSSValue(CSSPropertyFontFamily);
RefPtr<CSSValue> fontStyle = style.getPropertyCSSValue(CSSPropertyFontStyle);
@@ -234,6 +255,8 @@
FontRanges CSSFontSelector::fontRangesForFamily(const FontDescription& fontDescription, const AtomicString& familyName)
{
+ ASSERT(!m_buildIsUnderway); // If this ASSERT() fires, it usually means you forgot a document.updateStyleIfNeeded() somewhere.
+
// FIXME: The spec (and Firefox) says user specified generic families (sans-serif etc.) should be resolved before the @font-face lookup too.
bool resolveGenericFamilyFirst = familyName == standardFamily;
@@ -267,7 +290,6 @@
m_document = nullptr;
- // FIXME: This object should outlive the Document.
m_cssFontFaceSet->clear();
m_clients.clear();
}
Modified: trunk/Source/WebCore/css/CSSFontSelector.h (197433 => 197434)
--- trunk/Source/WebCore/css/CSSFontSelector.h 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/css/CSSFontSelector.h 2016-03-02 02:34:50 UTC (rev 197434)
@@ -65,6 +65,8 @@
virtual RefPtr<Font> fallbackFontAt(const FontDescription&, size_t) override;
void clearDocument();
+ void buildStarted();
+ void buildCompleted();
void addFontFaceRule(StyleRuleFontFace&, bool isInitiatingElementInUserAgentShadowTree);
@@ -91,6 +93,12 @@
void beginLoadTimerFired();
+ struct PendingFontFaceRule {
+ StyleRuleFontFace& styleRuleFontFace;
+ bool isInitiatingElementInUserAgentShadowTree;
+ };
+ Vector<PendingFontFaceRule> m_stagingArea;
+
Document* m_document;
RefPtr<FontFaceSet> m_fontFaceSet;
Ref<CSSFontFaceSet> m_cssFontFaceSet;
@@ -102,6 +110,7 @@
unsigned m_uniqueId;
unsigned m_version;
bool m_creatingFont { false };
+ bool m_buildIsUnderway { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/css/CSSStyleSheet.cpp (197433 => 197434)
--- trunk/Source/WebCore/css/CSSStyleSheet.cpp 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/css/CSSStyleSheet.cpp 2016-03-02 02:34:50 UTC (rev 197434)
@@ -423,7 +423,7 @@
}
CSSStyleSheet::RuleMutationScope::RuleMutationScope(CSSRule* rule)
- : m_styleSheet(rule ? rule->parentStyleSheet() : 0)
+ : m_styleSheet(rule ? rule->parentStyleSheet() : nullptr)
, m_mutationType(OtherMutation)
, m_contentsWereClonedForMutation(ContentsWereNotClonedForMutation)
, m_insertedKeyframesRule(nullptr)
Modified: trunk/Source/WebCore/css/SourceSizeList.cpp (197433 => 197434)
--- trunk/Source/WebCore/css/SourceSizeList.cpp 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/css/SourceSizeList.cpp 2016-03-02 02:34:50 UTC (rev 197434)
@@ -70,10 +70,13 @@
{
if (!view)
return 0;
- RenderStyle& style = view->style();
- for (auto& sourceSize : CSSParser(CSSStrictMode).parseSizesAttribute(sizesAttribute)) {
- if (match(WTFMove(sourceSize._expression_), style, frame))
- return computeLength(sourceSize.length.get(), style, view);
+ if (!sizesAttribute.empty()) {
+ view->document().updateStyleIfNeeded();
+ RenderStyle& style = view->style();
+ for (auto& sourceSize : CSSParser(CSSStrictMode).parseSizesAttribute(sizesAttribute)) {
+ if (match(WTFMove(sourceSize._expression_), style, frame))
+ return computeLength(sourceSize.length.get(), style, view);
+ }
}
return defaultLength(style, view);
}
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (197433 => 197434)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2016-03-02 02:34:50 UTC (rev 197434)
@@ -288,6 +288,9 @@
void StyleResolver::appendAuthorStyleSheets(const Vector<RefPtr<CSSStyleSheet>>& styleSheets)
{
m_ruleSets.appendAuthorStyleSheets(styleSheets, m_medium.get(), m_inspectorCSSOMWrappers, this);
+
+ document().fontSelector().buildCompleted();
+
if (auto renderView = document().renderView())
renderView->style().fontCascade().update(&document().fontSelector());
Modified: trunk/Source/WebCore/dom/Document.cpp (197433 => 197434)
--- trunk/Source/WebCore/dom/Document.cpp 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/dom/Document.cpp 2016-03-02 02:34:50 UTC (rev 197434)
@@ -539,6 +539,7 @@
#if ENABLE(TEMPLATE_ELEMENT)
, m_templateDocumentHost(nullptr)
#endif
+ , m_fontSelector(CSSFontSelector::create(*this))
#if ENABLE(WEB_REPLAY)
, m_inputCursor(EmptyInputCursor::create())
#endif
@@ -573,6 +574,8 @@
initSecurityContext();
initDNSPrefetch();
+ m_fontSelector->registerForInvalidationCallbacks(*this);
+
for (auto& nodeListAndCollectionCount : m_nodeListAndCollectionCounts)
nodeListAndCollectionCount = 0;
}
@@ -649,6 +652,8 @@
extensionStyleSheets().detachFromDocument();
clearStyleResolver(); // We need to destroy CSSFontSelector before destroying m_cachedResourceLoader.
+ m_fontSelector->clearDocument();
+ m_fontSelector->unregisterForInvalidationCallbacks(*this);
// It's possible for multiple Documents to end up referencing the same CachedResourceLoader (e.g., SVGImages
// load the initial empty document and the SVGDocument with the same DocumentLoader).
@@ -2189,26 +2194,12 @@
scheduleForcedStyleRecalc();
}
-CSSFontSelector& Document::fontSelector()
-{
- if (!m_fontSelector) {
- m_fontSelector = CSSFontSelector::create(*this);
- m_fontSelector->registerForInvalidationCallbacks(*this);
- }
- return *m_fontSelector;
-}
-
void Document::clearStyleResolver()
{
m_styleResolver = nullptr;
m_userAgentShadowTreeStyleResolver = nullptr;
- // FIXME: It would be better if the FontSelector could survive this operation.
- if (m_fontSelector) {
- m_fontSelector->clearDocument();
- m_fontSelector->unregisterForInvalidationCallbacks(*this);
- m_fontSelector = nullptr;
- }
+ m_fontSelector->buildStarted();
}
void Document::createRenderTree()
Modified: trunk/Source/WebCore/dom/Document.h (197433 => 197434)
--- trunk/Source/WebCore/dom/Document.h 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/dom/Document.h 2016-03-02 02:34:50 UTC (rev 197434)
@@ -503,7 +503,7 @@
}
StyleResolver& userAgentShadowTreeStyleResolver();
- CSSFontSelector& fontSelector();
+ CSSFontSelector& fontSelector() { return m_fontSelector; }
void notifyRemovePendingSheetIfNeeded();
@@ -1742,7 +1742,7 @@
std::unique_ptr<CustomElementDefinitions> m_customElementDefinitions;
#endif
- RefPtr<CSSFontSelector> m_fontSelector;
+ Ref<CSSFontSelector> m_fontSelector;
#if ENABLE(WEB_REPLAY)
RefPtr<JSC::InputCursor> m_inputCursor;
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (197433 => 197434)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-03-02 02:28:12 UTC (rev 197433)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-03-02 02:34:50 UTC (rev 197434)
@@ -963,8 +963,10 @@
auto& renderView = *m_document.renderView();
Element* documentElement = m_document.documentElement();
- if (!documentElement)
+ if (!documentElement) {
+ m_document.ensureStyleResolver();
return;
+ }
if (change != Force && !documentElement->childNeedsStyleRecalc() && !documentElement->needsStyleRecalc())
return;
