- Revision
- 273374
- Author
- [email protected]
- Date
- 2021-02-24 00:42:16 -0800 (Wed, 24 Feb 2021)
Log Message
Nullptr crash in Document::contextDocument() inside FontFaceSet::FontFaceSet
https://bugs.webkit.org/show_bug.cgi?id=221385
Patch by Carlos Garcia Campos <[email protected]> on 2021-02-24
Reviewed by Ryosuke Niwa.
Source/WebCore:
When the document last reference is removed but still has node references, we need to keep the document valid
for CSSFontSelector to be able to create a FontFaceSet if document.fonts is requested. In that case the font
selector is marked as cleared, ongoing font load stopped and clients unegistered, but keeping the document until
it's actually destroyed.
Test: fast/css/fontloader-on-detached-document-crash.html
* css/CSSFontSelector.cpp:
(WebCore::CSSFontSelector::~CSSFontSelector):
(WebCore::CSSFontSelector::clear):
(WebCore::CSSFontSelector::fontRequested):
(WebCore::CSSFontSelector::fallbackFontCount):
(WebCore::CSSFontSelector::fallbackFontAt):
* css/CSSFontSelector.h:
* dom/Document.cpp:
(WebCore::Document::~Document):
(WebCore::Document::removedLastRef):
LayoutTests:
* fast/css/fontloader-on-detached-document-crash-expected.txt: Added.
* fast/css/fontloader-on-detached-document-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (273373 => 273374)
--- trunk/LayoutTests/ChangeLog 2021-02-24 08:05:43 UTC (rev 273373)
+++ trunk/LayoutTests/ChangeLog 2021-02-24 08:42:16 UTC (rev 273374)
@@ -1,3 +1,13 @@
+2021-02-24 Carlos Garcia Campos <[email protected]>
+
+ Nullptr crash in Document::contextDocument() inside FontFaceSet::FontFaceSet
+ https://bugs.webkit.org/show_bug.cgi?id=221385
+
+ Reviewed by Ryosuke Niwa.
+
+ * fast/css/fontloader-on-detached-document-crash-expected.txt: Added.
+ * fast/css/fontloader-on-detached-document-crash.html: Added.
+
2021-02-24 Rob Buis <[email protected]>
Null check ArrayBufferView RefPtr
Added: trunk/LayoutTests/fast/css/fontloader-on-detached-document-crash-expected.txt (0 => 273374)
--- trunk/LayoutTests/fast/css/fontloader-on-detached-document-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/fontloader-on-detached-document-crash-expected.txt 2021-02-24 08:42:16 UTC (rev 273374)
@@ -0,0 +1 @@
+PASS
Added: trunk/LayoutTests/fast/css/fontloader-on-detached-document-crash.html (0 => 273374)
--- trunk/LayoutTests/fast/css/fontloader-on-detached-document-crash.html (rev 0)
+++ trunk/LayoutTests/fast/css/fontloader-on-detached-document-crash.html 2021-02-24 08:42:16 UTC (rev 273374)
@@ -0,0 +1,15 @@
+<script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ let cdata = new Document().createCDATASection('');
+ if (window.GCController) {
+ GCController.collect();
+ } else {
+ for (let i = 0; i < 25; i++) {
+ new ArrayBuffer(2 ** i);
+ }
+ }
+ cdata.splitText(0).ownerDocument.fonts;
+</script>
+PASS
Modified: trunk/Source/WebCore/ChangeLog (273373 => 273374)
--- trunk/Source/WebCore/ChangeLog 2021-02-24 08:05:43 UTC (rev 273373)
+++ trunk/Source/WebCore/ChangeLog 2021-02-24 08:42:16 UTC (rev 273374)
@@ -1,3 +1,28 @@
+2021-02-24 Carlos Garcia Campos <[email protected]>
+
+ Nullptr crash in Document::contextDocument() inside FontFaceSet::FontFaceSet
+ https://bugs.webkit.org/show_bug.cgi?id=221385
+
+ Reviewed by Ryosuke Niwa.
+
+ When the document last reference is removed but still has node references, we need to keep the document valid
+ for CSSFontSelector to be able to create a FontFaceSet if document.fonts is requested. In that case the font
+ selector is marked as cleared, ongoing font load stopped and clients unegistered, but keeping the document until
+ it's actually destroyed.
+
+ Test: fast/css/fontloader-on-detached-document-crash.html
+
+ * css/CSSFontSelector.cpp:
+ (WebCore::CSSFontSelector::~CSSFontSelector):
+ (WebCore::CSSFontSelector::clear):
+ (WebCore::CSSFontSelector::fontRequested):
+ (WebCore::CSSFontSelector::fallbackFontCount):
+ (WebCore::CSSFontSelector::fallbackFontAt):
+ * css/CSSFontSelector.h:
+ * dom/Document.cpp:
+ (WebCore::Document::~Document):
+ (WebCore::Document::removedLastRef):
+
2021-02-23 Frederic Wang <[email protected]>
Nullptr crash in ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline
Modified: trunk/Source/WebCore/css/CSSFontSelector.cpp (273373 => 273374)
--- trunk/Source/WebCore/css/CSSFontSelector.cpp 2021-02-24 08:05:43 UTC (rev 273373)
+++ trunk/Source/WebCore/css/CSSFontSelector.cpp 2021-02-24 08:42:16 UTC (rev 273374)
@@ -82,7 +82,7 @@
{
LOG(Fonts, "CSSFontSelector %p dtor", this);
- clearDocument();
+ stopLoadingAndClearFonts();
FontCache::singleton().removeClient(*this);
}
@@ -337,14 +337,15 @@
return FontRanges { WTFMove(font) };
}
-void CSSFontSelector::clearDocument()
+void CSSFontSelector::stopLoadingAndClearFonts()
{
- if (!m_document) {
+ if (m_isStopped) {
ASSERT(!m_fontLoadingTimer.isActive());
ASSERT(m_fontsToBeginLoading.isEmpty());
return;
}
+ m_isStopped = true;
m_fontLoadingTimer.stop();
CachedResourceLoader& cachedResourceLoader = m_document->cachedResourceLoader();
@@ -354,8 +355,6 @@
}
m_fontsToBeginLoading.clear();
- m_document = nullptr;
-
m_cssFontFaceSet->clear();
m_clients.clear();
}
@@ -362,13 +361,13 @@
void CSSFontSelector::beginLoadingFontSoon(CachedFont& font)
{
- if (!m_document)
+ if (m_isStopped)
return;
m_fontsToBeginLoading.append(&font);
// Increment the request count now, in order to prevent didFinishLoad from being dispatched
// after this font has been requested but before it began loading. Balanced by
- // decrementRequestCount() in fontLoadingTimerFired() and in clearDocument().
+ // decrementRequestCount() in fontLoadingTimerFired() and in stopLoadingAndClearFonts().
m_document->cachedResourceLoader().incrementRequestCount(font);
if (!m_isFontLoadingSuspended && !m_fontLoadingTimer.isActive())
@@ -417,7 +416,7 @@
size_t CSSFontSelector::fallbackFontCount()
{
- if (!m_document)
+ if (m_isStopped)
return 0;
return m_document->settings().fontFallbackPrefersPictographs() ? 1 : 0;
@@ -427,7 +426,7 @@
{
ASSERT_UNUSED(index, !index);
- if (!m_document)
+ if (m_isStopped)
return nullptr;
if (!m_document->settings().fontFallbackPrefersPictographs())
Modified: trunk/Source/WebCore/css/CSSFontSelector.h (273373 => 273374)
--- trunk/Source/WebCore/css/CSSFontSelector.h 2021-02-24 08:05:43 UTC (rev 273373)
+++ trunk/Source/WebCore/css/CSSFontSelector.h 2021-02-24 08:42:16 UTC (rev 273374)
@@ -63,7 +63,7 @@
size_t fallbackFontCount() final;
RefPtr<Font> fallbackFontAt(const FontDescription&, size_t) final;
- void clearDocument();
+ void stopLoadingAndClearFonts();
void emptyCaches();
void buildStarted();
void buildCompleted();
@@ -139,6 +139,7 @@
unsigned m_computingRootStyleFontCount { 0 };
bool m_creatingFont { false };
bool m_buildIsUnderway { false };
+ bool m_isStopped { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/Document.cpp (273373 => 273374)
--- trunk/Source/WebCore/dom/Document.cpp 2021-02-24 08:05:43 UTC (rev 273373)
+++ trunk/Source/WebCore/dom/Document.cpp 2021-02-24 08:42:16 UTC (rev 273374)
@@ -760,8 +760,7 @@
extensionStyleSheets().detachFromDocument();
styleScope().clearResolver(); // We need to destroy CSSFontSelector before destroying m_cachedResourceLoader.
- m_fontSelector->clearDocument();
- m_fontSelector->unregisterForInvalidationCallbacks(*this);
+ m_fontSelector->stopLoadingAndClearFonts();
// 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).
@@ -809,8 +808,7 @@
#endif
m_associatedFormControls.clear();
- m_fontSelector->clearDocument();
- m_fontSelector->unregisterForInvalidationCallbacks(*this);
+ m_fontSelector->stopLoadingAndClearFonts();
detachParser();
