- Revision
- 201799
- Author
- mmaxfi...@apple.com
- Date
- 2016-06-08 01:00:22 -0700 (Wed, 08 Jun 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 (201798 => 201799)
--- trunk/Source/WebCore/ChangeLog 2016-06-08 07:37:08 UTC (rev 201798)
+++ trunk/Source/WebCore/ChangeLog 2016-06-08 08:00:22 UTC (rev 201799)
@@ -1,3 +1,51 @@
+2016-06-08 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ 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-06-08 Adam Bergkvist <adam.bergkv...@ericsson.com>
WebRTC: Imlement MediaEndpointPeerConnection::setLocalDescription()
Modified: trunk/Source/WebCore/css/CSSFontFaceSet.cpp (201798 => 201799)
--- trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2016-06-08 07:37:08 UTC (rev 201798)
+++ trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2016-06-08 08:00:22 UTC (rev 201799)
@@ -244,6 +244,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 (201798 => 201799)
--- trunk/Source/WebCore/css/CSSFontSelector.cpp 2016-06-08 07:37:08 UTC (rev 201798)
+++ trunk/Source/WebCore/css/CSSFontSelector.cpp 2016-06-08 08:00:22 UTC (rev 201799)
@@ -71,10 +71,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);
@@ -102,8 +98,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);
@@ -235,6 +256,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;
@@ -268,7 +291,6 @@
m_document = nullptr;
- // FIXME: This object should outlive the Document.
m_cssFontFaceSet->clear();
m_clients.clear();
}
Modified: trunk/Source/WebCore/css/CSSFontSelector.h (201798 => 201799)
--- trunk/Source/WebCore/css/CSSFontSelector.h 2016-06-08 07:37:08 UTC (rev 201798)
+++ trunk/Source/WebCore/css/CSSFontSelector.h 2016-06-08 08:00:22 UTC (rev 201799)
@@ -65,6 +65,8 @@
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/SourceSizeList.cpp (201798 => 201799)
--- trunk/Source/WebCore/css/SourceSizeList.cpp 2016-06-08 07:37:08 UTC (rev 201798)
+++ trunk/Source/WebCore/css/SourceSizeList.cpp 2016-06-08 08:00:22 UTC (rev 201799)
@@ -47,6 +47,20 @@
CSSPrimitiveValue& primitiveValue = downcast<CSSPrimitiveValue>(value);
if (!primitiveValue.isLength())
return defaultLength(style, renderer);
+
+ // Because we evaluate "sizes" at parse time (before style has been resolved), the font metrics used for these specific units
+ // are not available. The font selector's internal consistency isn't guaranteed just yet, so we can just temporarily clear
+ // the pointer to it for the duration of the unit evaluation. This is acceptible because the style always comes from the
+ // RenderView, which has its font information hardcoded in resolveForDocument() to be -webkit-standard, whose operations
+ // don't require a font selector.
+ if (!primitiveValue.isCalculated() && (primitiveValue.primitiveType() == CSSPrimitiveValue::CSS_EXS || primitiveValue.primitiveType() == CSSPrimitiveValue::CSS_CHS)) {
+ RefPtr<FontSelector> fontSelector = style.fontCascade().fontSelector();
+ style.fontCascade().update(nullptr);
+ float result = primitiveValue.computeLength<float>(conversionData);
+ style.fontCascade().update(fontSelector.get());
+ return result;
+ }
+
return primitiveValue.computeLength<float>(conversionData);
}
if (is<CSSCalcValue>(value))
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (201798 => 201799)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2016-06-08 07:37:08 UTC (rev 201798)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2016-06-08 08:00:22 UTC (rev 201799)
@@ -296,6 +296,9 @@
void StyleResolver::appendAuthorStyleSheets(const Vector<RefPtr<CSSStyleSheet>>& styleSheets)
{
m_ruleSets.appendAuthorStyleSheets(styleSheets, &m_mediaQueryEvaluator, m_inspectorCSSOMWrappers, this);
+
+ document().fontSelector().buildCompleted();
+
if (auto renderView = document().renderView())
renderView->style().fontCascade().update(&document().fontSelector());
Modified: trunk/Source/WebCore/dom/Document.cpp (201798 => 201799)
--- trunk/Source/WebCore/dom/Document.cpp 2016-06-08 07:37:08 UTC (rev 201798)
+++ trunk/Source/WebCore/dom/Document.cpp 2016-06-08 08:00:22 UTC (rev 201799)
@@ -529,6 +529,7 @@
, m_didDispatchViewportPropertiesChanged(false)
#endif
, m_templateDocumentHost(nullptr)
+ , m_fontSelector(CSSFontSelector::create(*this))
#if ENABLE(WEB_REPLAY)
, m_inputCursor(EmptyInputCursor::create())
#endif
@@ -563,6 +564,8 @@
initSecurityContext();
initDNSPrefetch();
+ m_fontSelector->registerForInvalidationCallbacks(*this);
+
for (auto& nodeListAndCollectionCount : m_nodeListAndCollectionCounts)
nodeListAndCollectionCount = 0;
}
@@ -637,6 +640,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).
@@ -2208,26 +2213,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 (201798 => 201799)
--- trunk/Source/WebCore/dom/Document.h 2016-06-08 07:37:08 UTC (rev 201798)
+++ trunk/Source/WebCore/dom/Document.h 2016-06-08 08:00:22 UTC (rev 201799)
@@ -504,7 +504,7 @@
}
StyleResolver& userAgentShadowTreeStyleResolver();
- CSSFontSelector& fontSelector();
+ CSSFontSelector& fontSelector() { return m_fontSelector; }
void notifyRemovePendingSheetIfNeeded();
@@ -1736,7 +1736,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 (201798 => 201799)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-06-08 07:37:08 UTC (rev 201798)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2016-06-08 08:00:22 UTC (rev 201799)
@@ -492,8 +492,10 @@
auto& renderView = *m_document.renderView();
Element* documentElement = m_document.documentElement();
- if (!documentElement)
+ if (!documentElement) {
+ m_document.ensureStyleResolver();
return nullptr;
+ }
if (change != Force && !documentElement->childNeedsStyleRecalc() && !documentElement->needsStyleRecalc())
return nullptr;
