https://bugs.webkit.org/show_bug.cgi?id=155009
Reviewed by Simon Fraser.
Source/WebCore:
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).
LayoutTests:
* fast/text/font-face-set-document-multiple-failure-expected.txt: Added.
* fast/text/font-face-set-document-multiple-failure.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@197804
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2016-03-08 Myles C. Maxfield <mmaxfield@apple.com>
+
+ [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 <ryanhaddad@apple.com>
Skip fast/events/prevent-default-prevents-interaction-with-scrollbars.html on ios-simulator
--- /dev/null
+PASS globalX.code is globalX.NETWORK_ERR
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
--- /dev/null
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<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="../../resources/js-test-post.js"></script>
+</body>
+</html>
\ No newline at end of file
+2016-03-08 Myles C. Maxfield <mmaxfield@apple.com>
+
+ [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 <commit-queue@webkit.org>
Unreviewed, rolling out r197793 and r197799.
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) {
{
}
-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))
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;
}
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;
}
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;
}
m_ranges.append(UnicodeRange(range.from(), range.to()));
}
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
m_variantSettings.historicalLigatures = ligatures.historicalLigatures;
m_variantSettings.contextualAlternates = ligatures.contextualAlternates;
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
return false;
m_variantSettings.position = downcast<CSSPrimitiveValue>(variantPosition);
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
return false;
m_variantSettings.caps = downcast<CSSPrimitiveValue>(variantCaps);
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
m_variantSettings.numericOrdinal = numeric.ordinal;
m_variantSettings.numericSlashedZero = numeric.slashedZero;
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
return false;
m_variantSettings.alternates = downcast<CSSPrimitiveValue>(variantAlternates);
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
m_variantSettings.eastAsianWidth = eastAsian.width;
m_variantSettings.eastAsianRuby = eastAsian.ruby;
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
m_featureSettings.insert(FontFeature(feature.tag(), feature.value()));
}
- notifyClientsOfFontPropertyChange();
+ iterateClients(m_clients, [&](Client& client) {
+ client.fontPropertyChanged(*this);
+ });
return true;
}
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;
}
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()
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
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) {
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();
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;
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>&&);
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;
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&);
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;
+ }
}
}
public:
Vector<RefPtr<FontFace>> faces;
Promise promise;
+ bool hasReachedTerminalState { false };
};
FontFaceSet(Document&, const Vector<RefPtr<FontFace>>&);
Ref<FontFaceSet> Document::fonts()
{
+ updateStyleIfNeeded();
return fontSelector().fontFaceSet();
}
git://git.webkit.org / WebKit-https.git / commitdiff