Diff
Modified: trunk/LayoutTests/ChangeLog (197803 => 197804)
--- trunk/LayoutTests/ChangeLog 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/LayoutTests/ChangeLog 2016-03-08 22:22:40 UTC (rev 197804)
@@ -1,3 +1,13 @@
+2016-03-08 Myles C. Maxfield <[email protected]>
+
+ [Font Loading] Crash when a single load request causes multiple fonts to fail loading
+ https://bugs.webkit.org/show_bug.cgi?id=155009
+
+ Reviewed by Simon Fraser.
+
+ * fast/text/font-face-set-document-multiple-failure-expected.txt: Added.
+ * fast/text/font-face-set-document-multiple-failure.html: Added.
+
2016-03-08 Ryan Haddad <[email protected]>
Skip fast/events/prevent-default-prevents-interaction-with-scrollbars.html on ios-simulator
Added: trunk/LayoutTests/fast/text/font-face-set-document-multiple-failure-expected.txt (0 => 197804)
--- trunk/LayoutTests/fast/text/font-face-set-document-multiple-failure-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/text/font-face-set-document-multiple-failure-expected.txt 2016-03-08 22:22:40 UTC (rev 197804)
@@ -0,0 +1,5 @@
+PASS globalX.code is globalX.NETWORK_ERR
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/text/font-face-set-document-multiple-failure.html (0 => 197804)
--- trunk/LayoutTests/fast/text/font-face-set-document-multiple-failure.html (rev 0)
+++ trunk/LayoutTests/fast/text/font-face-set-document-multiple-failure.html 2016-03-08 22:22:40 UTC (rev 197804)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<style>
+@font-face {
+ font-family: "WebFont";
+ src: url("garbage");
+}
+@font-face {
+ font-family: "WebFont";
+ src: url("garbage");
+ font-variant: small-caps;
+}
+</style>
+</head>
+<body>
+<script>
+self.jsTestIsAsync = true;
+var globalX;
+document.fonts.load("16px WebFont").then(function() {
+ testFailed("Bogus URLs should not load");
+ finishJSTest();
+}, function(x) {
+ globalX = x;
+ shouldBe("globalX.code", "globalX.NETWORK_ERR");
+ finishJSTest();
+})
+</script>
+<script src=""
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (197803 => 197804)
--- trunk/Source/WebCore/ChangeLog 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/ChangeLog 2016-03-08 22:22:40 UTC (rev 197804)
@@ -1,3 +1,52 @@
+2016-03-08 Myles C. Maxfield <[email protected]>
+
+ [Font Loading] Crash when a single load request causes multiple fonts to fail loading
+ https://bugs.webkit.org/show_bug.cgi?id=155009
+
+ Reviewed by Simon Fraser.
+
+ In _javascript_, the first promise fulfillment/failure wins. However, in C++, any
+ subsequent fulfillments/failures cause a crash.
+
+ Test: fast/text/font-face-set-document-multiple-failure.html
+
+ * css/CSSFontFace.cpp:
+ (WebCore::iterateClients): Notifying a client may cause some other client
+ to be destroyed, thereby modifying the clients set. This function allows
+ for notifying clients in a resilient manner.
+ (WebCore::CSSFontFace::setStyle): Update to use iterateClients().
+ (WebCore::CSSFontFace::setWeight): Ditto.
+ (WebCore::CSSFontFace::setUnicodeRange): Ditto.
+ (WebCore::CSSFontFace::setVariantLigatures): Ditto.
+ (WebCore::CSSFontFace::setVariantPosition): Ditto.
+ (WebCore::CSSFontFace::setVariantCaps): Ditto.
+ (WebCore::CSSFontFace::setVariantNumeric): Ditto.
+ (WebCore::CSSFontFace::setVariantAlternates): Ditto.
+ (WebCore::CSSFontFace::setVariantEastAsian): Ditto.
+ (WebCore::CSSFontFace::setFeatureSettings): Ditto.
+ (WebCore::CSSFontFace::setStatus): Ditto.
+ (WebCore::CSSFontFace::notifyClientsOfFontPropertyChange): Deleted.
+ * css/CSSFontFace.h: Adding a way for clients to make sure they don't register
+ or deregister another client.
+ * css/CSSFontFaceSet.cpp:
+ (WebCore::CSSFontFaceSet::guardAgainstClientRegistrationChanges): Simple
+ ref()/deref() pair.
+ (WebCore::CSSFontFaceSet::stopGuardingAgainstClientRegistrationChanges):
+ * css/CSSFontFaceSet.h:
+ * css/FontFace.cpp: Ditto.
+ (WebCore::FontFace::guardAgainstClientRegistrationChanges):
+ (WebCore::FontFace::stopGuardingAgainstClientRegistrationChanges):
+ * css/FontFace.h:
+ * css/FontFaceSet.cpp:
+ (WebCore::FontFaceSet::faceFinished): Make sure that we only fulfil or reject
+ a promise once.
+ * css/FontFaceSet.h:
+ * dom/Document.cpp:
+ (WebCore::Document::fonts): The CSSFontFaces inside the CSSFontSelector get
+ created during style recalc. We may be in a state where there is a style
+ recalc pending. In order to make sure the _javascript_ API sees the current
+ state of the world, force a style recalc here (but only if one is pending).
+
2016-03-08 Commit Queue <[email protected]>
Unreviewed, rolling out r197793 and r197799.
Modified: trunk/Source/WebCore/css/CSSFontFace.cpp (197803 => 197804)
--- trunk/Source/WebCore/css/CSSFontFace.cpp 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/css/CSSFontFace.cpp 2016-03-08 22:22:40 UTC (rev 197804)
@@ -48,6 +48,17 @@
namespace WebCore {
+template<typename T> void iterateClients(HashSet<CSSFontFace::Client*>& clients, T callback)
+{
+ Vector<Ref<CSSFontFace::Client>> clientsCopy;
+ clientsCopy.reserveInitialCapacity(clients.size());
+ for (auto* client : clients)
+ clientsCopy.uncheckedAppend(*client);
+
+ for (auto* client : clients)
+ callback(*client);
+}
+
void CSSFontFace::appendSources(CSSFontFace& fontFace, CSSValueList& srcList, Document* document, bool isInitiatingElementInUserAgentShadowTree)
{
for (auto& src : srcList) {
@@ -89,15 +100,6 @@
{
}
-void CSSFontFace::notifyClientsOfFontPropertyChange()
-{
- auto clientsCopy = m_clients;
- for (auto* client : clientsCopy) {
- if (m_clients.contains(client))
- client->fontPropertyChanged(*this);
- }
-}
-
bool CSSFontFace::setFamilies(CSSValue& family)
{
if (!is<CSSValueList>(family))
@@ -110,11 +112,9 @@
RefPtr<CSSValueList> oldFamilies = m_families;
m_families = &familyList;
- auto clientsCopy = m_clients;
- for (auto* client : clientsCopy) {
- if (m_clients.contains(client))
- client->fontPropertyChanged(*this, oldFamilies.get());
- }
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this, oldFamilies.get());
+ });
return true;
}
@@ -142,7 +142,9 @@
if (auto mask = calculateStyleMask(style)) {
m_traitsMask = static_cast<FontTraitsMask>((static_cast<unsigned>(m_traitsMask) & (~FontStyleMask)) | mask.value());
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -189,7 +191,9 @@
if (auto mask = calculateWeightMask(weight)) {
m_traitsMask = static_cast<FontTraitsMask>((static_cast<unsigned>(m_traitsMask) & (~FontWeightMask)) | mask.value());
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -209,7 +213,9 @@
m_ranges.append(UnicodeRange(range.from(), range.to()));
}
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -222,7 +228,9 @@
m_variantSettings.historicalLigatures = ligatures.historicalLigatures;
m_variantSettings.contextualAlternates = ligatures.contextualAlternates;
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -233,7 +241,9 @@
return false;
m_variantSettings.position = downcast<CSSPrimitiveValue>(variantPosition);
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -244,7 +254,9 @@
return false;
m_variantSettings.caps = downcast<CSSPrimitiveValue>(variantCaps);
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -258,7 +270,9 @@
m_variantSettings.numericOrdinal = numeric.ordinal;
m_variantSettings.numericSlashedZero = numeric.slashedZero;
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -269,7 +283,9 @@
return false;
m_variantSettings.alternates = downcast<CSSPrimitiveValue>(variantAlternates);
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -281,7 +297,9 @@
m_variantSettings.eastAsianWidth = eastAsian.width;
m_variantSettings.eastAsianRuby = eastAsian.ruby;
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -298,7 +316,9 @@
m_featureSettings.insert(FontFeature(feature.tag(), feature.value()));
}
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
@@ -380,8 +400,9 @@
break;
}
- for (auto* client : m_clients)
- client->fontStateChanged(*this, m_status, newStatus);
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontStateChanged(*this, m_status, newStatus);
+ });
m_status = newStatus;
}
@@ -397,8 +418,9 @@
ASSERT(m_fontSelector);
m_fontSelector->fontLoaded();
- for (auto* client : m_clients)
- client->fontLoaded(*this);
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontLoaded(*this);
+ });
}
size_t CSSFontFace::pump()
Modified: trunk/Source/WebCore/css/CSSFontFace.h (197803 => 197804)
--- trunk/Source/WebCore/css/CSSFontFace.h 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/css/CSSFontFace.h 2016-03-08 22:22:40 UTC (rev 197804)
@@ -108,9 +108,11 @@
class Client {
public:
virtual ~Client() { }
- virtual void fontLoaded(CSSFontFace&) { };
- virtual void fontStateChanged(CSSFontFace&, Status oldState, Status newState) { UNUSED_PARAM(oldState); UNUSED_PARAM(newState); };
- virtual void fontPropertyChanged(CSSFontFace&, CSSValueList* oldFamilies = nullptr) { UNUSED_PARAM(oldFamilies); };
+ virtual void fontLoaded(CSSFontFace&) { }
+ virtual void fontStateChanged(CSSFontFace&, Status oldState, Status newState) { UNUSED_PARAM(oldState); UNUSED_PARAM(newState); }
+ virtual void fontPropertyChanged(CSSFontFace&, CSSValueList* oldFamilies = nullptr) { UNUSED_PARAM(oldFamilies); }
+ virtual void ref() = 0;
+ virtual void deref() = 0;
};
// Pending => Loading => TimedOut
Modified: trunk/Source/WebCore/css/CSSFontFaceSet.cpp (197803 => 197804)
--- trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2016-03-08 22:22:40 UTC (rev 197804)
@@ -398,13 +398,13 @@
return nullptr;
auto& familyFontFaces = iterator->value;
- auto& segmentedFontFaceCache = m_cache.add(family, HashMap<unsigned, std::unique_ptr<CSSSegmentedFontFace>>()).iterator->value;
+ auto& segmentedFontFaceCache = m_cache.add(family, HashMap<unsigned, RefPtr<CSSSegmentedFontFace>>()).iterator->value;
auto& face = segmentedFontFaceCache.add(traitsMask, nullptr).iterator->value;
if (face)
return face.get();
- face = std::make_unique<CSSSegmentedFontFace>();
+ face = CSSSegmentedFontFace::create();
Vector<std::reference_wrapper<CSSFontFace>, 32> candidateFontFaces;
for (int i = familyFontFaces.size() - 1; i >= 0; --i) {
Modified: trunk/Source/WebCore/css/CSSFontFaceSet.h (197803 => 197804)
--- trunk/Source/WebCore/css/CSSFontFaceSet.h 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/css/CSSFontFaceSet.h 2016-03-08 22:22:40 UTC (rev 197804)
@@ -75,6 +75,10 @@
Vector<std::reference_wrapper<CSSFontFace>> matchingFaces(const String& font, const String& text, ExceptionCode&);
+ // CSSFontFace::Client needs to be able to be held in a RefPtr.
+ void ref() override { RefCounted<CSSFontFaceSet>::ref(); }
+ void deref() override { RefCounted<CSSFontFaceSet>::deref(); }
+
private:
CSSFontFaceSet();
@@ -95,7 +99,7 @@
Vector<Ref<CSSFontFace>> m_faces; // We should investigate moving m_faces to FontFaceSet and making it reference FontFaces. This may clean up the font loading design.
HashMap<String, Vector<Ref<CSSFontFace>>, ASCIICaseInsensitiveHash> m_facesLookupTable;
HashMap<String, Vector<Ref<CSSFontFace>>, ASCIICaseInsensitiveHash> m_locallyInstalledFacesLookupTable;
- HashMap<String, HashMap<unsigned, std::unique_ptr<CSSSegmentedFontFace>>, ASCIICaseInsensitiveHash> m_cache;
+ HashMap<String, HashMap<unsigned, RefPtr<CSSSegmentedFontFace>>, ASCIICaseInsensitiveHash> m_cache;
size_t m_facesPartitionIndex { 0 }; // All entries in m_faces before this index are CSS-connected.
Status m_status { Status::Loaded };
HashSet<CSSFontFaceSetClient*> m_clients;
Modified: trunk/Source/WebCore/css/CSSSegmentedFontFace.h (197803 => 197804)
--- trunk/Source/WebCore/css/CSSSegmentedFontFace.h 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/css/CSSSegmentedFontFace.h 2016-03-08 22:22:40 UTC (rev 197804)
@@ -39,10 +39,13 @@
class CSSFontSelector;
class FontDescription;
-class CSSSegmentedFontFace final : public CSSFontFace::Client {
+class CSSSegmentedFontFace final : public RefCounted<CSSSegmentedFontFace>, public CSSFontFace::Client {
WTF_MAKE_FAST_ALLOCATED;
public:
- CSSSegmentedFontFace();
+ static Ref<CSSSegmentedFontFace> create()
+ {
+ return adoptRef(*new CSSSegmentedFontFace());
+ }
~CSSSegmentedFontFace();
void appendFontFace(Ref<CSSFontFace>&&);
@@ -51,7 +54,12 @@
Vector<Ref<CSSFontFace>, 1>& constituentFaces() { return m_fontFaces; }
+ // CSSFontFace::Client needs to be able to be held in a RefPtr.
+ void ref() override { RefCounted<CSSSegmentedFontFace>::ref(); }
+ void deref() override { RefCounted<CSSSegmentedFontFace>::deref(); }
+
private:
+ CSSSegmentedFontFace();
void fontLoaded(CSSFontFace&) override;
HashMap<FontDescriptionKey, FontRanges, FontDescriptionKeyHash, WTF::SimpleClassHashTraits<FontDescriptionKey>> m_cache;
Modified: trunk/Source/WebCore/css/FontFace.h (197803 => 197804)
--- trunk/Source/WebCore/css/FontFace.h 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/css/FontFace.h 2016-03-08 22:22:40 UTC (rev 197804)
@@ -82,6 +82,10 @@
WeakPtr<FontFace> createWeakPtr() const;
+ // CSSFontFace::Client needs to be able to be held in a RefPtr.
+ void ref() override { RefCounted<FontFace>::ref(); }
+ void deref() override { RefCounted<FontFace>::deref(); }
+
private:
FontFace(JSC::ExecState&, CSSFontSelector&);
FontFace(JSC::ExecState&, CSSFontFace&);
Modified: trunk/Source/WebCore/css/FontFaceSet.cpp (197803 => 197804)
--- trunk/Source/WebCore/css/FontFaceSet.cpp 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/css/FontFaceSet.cpp 2016-03-08 22:22:40 UTC (rev 197804)
@@ -236,12 +236,16 @@
for (auto& pendingPromise : iterator->value) {
if (newStatus == CSSFontFace::Status::Success) {
- if (pendingPromise->hasOneRef())
+ if (pendingPromise->hasOneRef() && !pendingPromise->hasReachedTerminalState) {
pendingPromise->promise.resolve(pendingPromise->faces);
+ pendingPromise->hasReachedTerminalState = true;
+ }
} else {
ASSERT(newStatus == CSSFontFace::Status::Failure);
- // The first resolution wins, so we can just reject early now.
- pendingPromise->promise.reject(DOMCoreException::create(ExceptionCodeDescription(NETWORK_ERR)));
+ if (!pendingPromise->hasReachedTerminalState) {
+ pendingPromise->promise.reject(DOMCoreException::create(ExceptionCodeDescription(NETWORK_ERR)));
+ pendingPromise->hasReachedTerminalState = true;
+ }
}
}
Modified: trunk/Source/WebCore/css/FontFaceSet.h (197803 => 197804)
--- trunk/Source/WebCore/css/FontFaceSet.h 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/css/FontFaceSet.h 2016-03-08 22:22:40 UTC (rev 197804)
@@ -101,6 +101,7 @@
public:
Vector<RefPtr<FontFace>> faces;
Promise promise;
+ bool hasReachedTerminalState { false };
};
FontFaceSet(Document&, const Vector<RefPtr<FontFace>>&);
Modified: trunk/Source/WebCore/dom/Document.cpp (197803 => 197804)
--- trunk/Source/WebCore/dom/Document.cpp 2016-03-08 21:58:30 UTC (rev 197803)
+++ trunk/Source/WebCore/dom/Document.cpp 2016-03-08 22:22:40 UTC (rev 197804)
@@ -6706,6 +6706,7 @@
Ref<FontFaceSet> Document::fonts()
{
+ updateStyleIfNeeded();
return fontSelector().fontFaceSet();
}
